From a06c06da10075b08c3e4733c23d1bc112e5b069a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:24:32 +0000 Subject: [PATCH 1/5] Initial plan From 6290d7a3c4658bb8299c424c5df83d965a566dbf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:37:37 +0000 Subject: [PATCH 2/5] Add error handling for failed image generation and tests to verify behavior - In generate.py, each individual_* function now logs an error and raises an Exception when core does not produce the expected output file. This ensures the failure propagates up through generate_assets() so the asset type is not added to the successful_assets list and the cache is not incorrectly updated. - Add tests/test_generate.py with 8 tests covering all four individual_* functions, verifying both the failure case (raises) and success case (no raise). Co-authored-by: oscarlevin <6504596+oscarlevin@users.noreply.github.com> Agent-Logs-Url: https://github.com/PreTeXtBook/pretext-cli/sessions/7b12f474-b534-45aa-813c-9a2f75f4d8c3 --- pretext/project/generate.py | 16 +++ tests/test_generate.py | 222 ++++++++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 tests/test_generate.py diff --git a/pretext/project/generate.py b/pretext/project/generate.py index b6b6a3c6..f5bab71c 100644 --- a/pretext/project/generate.py +++ b/pretext/project/generate.py @@ -74,6 +74,10 @@ def individual_prefigure( f"Created prefigure diagram {annotations_output_file}; saving a copy to cache as {annotations_cache_file}" ) shutil.copy2(annotations_output_file, annotations_cache_file) + else: + msg = f"Failed to generate prefigure diagram {Path(pfdiagram).name} in {format} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_prefigure function") @@ -109,6 +113,10 @@ def individual_asymptote( f"Created asymptote diagram {output_file}; saving a copy to cache as {cache_file}" ) shutil.copy2(output_file, cache_file) + else: + msg = f"Failed to generate asymptote diagram {Path(asydiagram).name} in {outformat} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_asymptote function") @@ -146,6 +154,10 @@ def individual_sage( f"Created sageplot diagram {output_file}; saving a copy to cache as {cache_file}" ) shutil.copy2(output_file, cache_file) + else: + msg = f"Failed to generate sageplot diagram {Path(sageplot).name} in {outformat} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_sage function") @@ -191,6 +203,10 @@ def individual_latex_image( f"Created latex-image {output_files[ext]}; saving a copy to cache as {cache_files[ext]}" ) shutil.copy2(output_files[ext], cache_files[ext]) + else: + msg = f"Failed to generate latex-image {Path(latex_image).name} in {ext} format." + log.error(msg) + raise Exception(msg) log.debug("Finished individual_latex function") diff --git a/tests/test_generate.py b/tests/test_generate.py new file mode 100644 index 00000000..ac74c3d8 --- /dev/null +++ b/tests/test_generate.py @@ -0,0 +1,222 @@ +""" +Tests to ensure that failed image generation is not reported as success. + +When a core conversion function fails to produce an output file, the +individual_* functions in pretext/project/generate.py should raise an +exception so that the asset type is not added to the generated asset cache. +""" +from pathlib import Path +from unittest.mock import patch + +import pytest + +from pretext.project import generate + + +def _make_asset_file(tmp_path: Path, name: str = "test_asset.tex") -> Path: + """Create a minimal asset file in tmp_path for use in tests.""" + asset_file = tmp_path / name + asset_file.write_text("some asset content") + return asset_file + + +def test_individual_latex_image_raises_on_missing_output(tmp_path: Path) -> None: + """individual_latex_image should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "image.tex") + cache_dir = tmp_path / "cache" / "latex-image" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + # Patch core so it does NOT create the output file. + with patch("pretext.project.generate.core.individual_latex_image_conversion"): + with pytest.raises(Exception, match="Failed to generate latex-image"): + generate.individual_latex_image( + latex_image=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + method="xelatex", + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_latex_image_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_latex_image should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "image.tex") + cache_dir = tmp_path / "cache" / "latex-image" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + def fake_conversion(latex_image: str, outformat: str, dest_dir: Path, method: str) -> None: + # Simulate a successful conversion by creating the expected output file. + (dest_dir / Path(latex_image).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_latex_image_conversion", + side_effect=fake_conversion, + ): + # Should not raise. + generate.individual_latex_image( + latex_image=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + method="xelatex", + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_asymptote_raises_on_missing_output(tmp_path: Path) -> None: + """individual_asymptote should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "diagram.asy") + cache_dir = tmp_path / "cache" / "asymptote" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + with patch("pretext.project.generate.core.individual_asymptote_conversion"): + with pytest.raises(Exception, match="Failed to generate asymptote diagram"): + generate.individual_asymptote( + asydiagram=str(asset_file), + outformat="svg", + method="server", + asy_cli=[], + asyversion="", + alberta="", + dest_dir=dest_dir, + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_asymptote_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_asymptote should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "diagram.asy") + cache_dir = tmp_path / "cache" / "asymptote" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + def fake_conversion( + asydiagram: str, + outformat: str, + method: str, + asy_cli: list, + asyversion: str, + alberta: str, + dest_dir: Path, + ) -> None: + (dest_dir / Path(asydiagram).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_asymptote_conversion", + side_effect=fake_conversion, + ): + generate.individual_asymptote( + asydiagram=str(asset_file), + outformat="svg", + method="server", + asy_cli=[], + asyversion="", + alberta="", + dest_dir=dest_dir, + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_sage_raises_on_missing_output(tmp_path: Path) -> None: + """individual_sage should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "plot.sage") + cache_dir = tmp_path / "cache" / "sageplot" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + with patch("pretext.project.generate.core.individual_sage_conversion"): + with pytest.raises(Exception, match="Failed to generate sageplot diagram"): + generate.individual_sage( + sageplot=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + sage_executable_cmd=["sage"], + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_sage_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_sage should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "plot.sage") + cache_dir = tmp_path / "cache" / "sageplot" + cache_dir.mkdir(parents=True) + dest_dir = tmp_path / "dest" + dest_dir.mkdir() + + def fake_conversion( + sageplot: str, + outformat: str, + dest_dir: Path, + sage_executable_cmd: list, + ) -> None: + (dest_dir / Path(sageplot).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_sage_conversion", + side_effect=fake_conversion, + ): + generate.individual_sage( + sageplot=str(asset_file), + outformat="svg", + dest_dir=dest_dir, + sage_executable_cmd=["sage"], + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_prefigure_raises_on_missing_output(tmp_path: Path) -> None: + """individual_prefigure should raise when core does not produce the output file.""" + asset_file = _make_asset_file(tmp_path, "figure.xml") + cache_dir = tmp_path / "cache" / "prefigure" + cache_dir.mkdir(parents=True) + tmp_dir = tmp_path / "tmp_work" + tmp_dir.mkdir() + + with patch("pretext.project.generate.core.individual_prefigure_conversion"): + with pytest.raises(Exception, match="Failed to generate prefigure diagram"): + generate.individual_prefigure( + pfdiagram=str(asset_file), + outformat="svg", + tmp_dir=str(tmp_dir), + cache_dir=tmp_path / "cache", + skip_cache=True, + ) + + +def test_individual_prefigure_succeeds_when_output_exists(tmp_path: Path) -> None: + """individual_prefigure should not raise when core produces the output file.""" + asset_file = _make_asset_file(tmp_path, "figure.xml") + cache_dir = tmp_path / "cache" / "prefigure" + cache_dir.mkdir(parents=True) + tmp_dir = tmp_path / "tmp_work" + tmp_dir.mkdir() + + def fake_conversion(pfdiagram: str, outformat: str) -> None: + output_dir = tmp_dir / "output" + output_dir.mkdir(parents=True, exist_ok=True) + (output_dir / Path(pfdiagram).with_suffix(".svg").name).touch() + + with patch( + "pretext.project.generate.core.individual_prefigure_conversion", + side_effect=fake_conversion, + ): + generate.individual_prefigure( + pfdiagram=str(asset_file), + outformat="svg", + tmp_dir=str(tmp_dir), + cache_dir=tmp_path / "cache", + skip_cache=True, + ) From 3862a17a2fbb0dfb3a75a3a32dc26785db831e14 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 15:24:32 +0000 Subject: [PATCH 3/5] Initial plan From 544552e3d4bf787ad8048784d68d525f8c7427b5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 00:23:19 +0000 Subject: [PATCH 4/5] Rebase on main, run black formatting, add CHANGELOG entry - Rebased on origin/main (includes new tests and changes from #1124-#1128) - Ran `poetry run black .` for formatting (reformatted test_generate.py) - Added Fixed entry in CHANGELOG.md under [Unreleased] Co-authored-by: oscarlevin <6504596+oscarlevin@users.noreply.github.com> Agent-Logs-Url: https://github.com/PreTeXtBook/pretext-cli/sessions/72d5b309-fec0-4dad-8af0-1cbe035c2679 --- CHANGELOG.md | 4 ++++ tests/test_generate.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36e47aa6..3cd2be11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ Instructions: Add a subsection under `[Unreleased]` for additions, fixes, change ## [Unreleased] +### Fixed + +- Failed individual asset generation (when core does not produce the expected output file) now raises an error and is no longer silently treated as a success. This prevents incorrectly caching incomplete asset generation results. + ### Changed - The default Asymptote generation method is now `local` (using the local `asy` binary) instead of `server`. This improves reliability since the server method has been subject to breakage whenever the Asymptote web interface is updated. Users who need the server method can still opt in via `asy-method="server"` in `project.ptx`. diff --git a/tests/test_generate.py b/tests/test_generate.py index ac74c3d8..7550ee72 100644 --- a/tests/test_generate.py +++ b/tests/test_generate.py @@ -5,6 +5,7 @@ individual_* functions in pretext/project/generate.py should raise an exception so that the asset type is not added to the generated asset cache. """ + from pathlib import Path from unittest.mock import patch @@ -49,7 +50,9 @@ def test_individual_latex_image_succeeds_when_output_exists(tmp_path: Path) -> N dest_dir = tmp_path / "dest" dest_dir.mkdir() - def fake_conversion(latex_image: str, outformat: str, dest_dir: Path, method: str) -> None: + def fake_conversion( + latex_image: str, outformat: str, dest_dir: Path, method: str + ) -> None: # Simulate a successful conversion by creating the expected output file. (dest_dir / Path(latex_image).with_suffix(".svg").name).touch() From 115416e8c108dadc16b1ed50572f91684cf3af4e Mon Sep 17 00:00:00 2001 From: Oscar Levin Date: Sun, 22 Mar 2026 11:10:16 -0600 Subject: [PATCH 5/5] drop failed assets from cache --- pretext/project/__init__.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pretext/project/__init__.py b/pretext/project/__init__.py index c7c17ba4..e6e93deb 100644 --- a/pretext/project/__init__.py +++ b/pretext/project/__init__.py @@ -1292,10 +1292,20 @@ def generate_assets( log.debug(e, exc_info=True) # After all assets are generated, update the asset cache (but we shouldn't do this if we didn't generate any assets successfully) log.debug(f"Updated these assets successfully: {successful_assets}") - if len(successful_assets) > 0 and not xmlid: + unsuccessful_assets = [ + asset for asset in assets_to_generate if asset not in successful_assets + ] + log.debug(f"These assets were unsuccessful: {unsuccessful_assets}") + if not xmlid: # If the build limited by xmlid, then we don't know that all assets of that type were build correctly, so we only do this if not generating a subset. + # First create a list of unsuccessful assets to log: for asset_type in successful_assets: saved_asset_table[asset_type] = source_asset_table[asset_type] + for asset_type in unsuccessful_assets: + log.debug( + f"Asset type {asset_type} was requested to be generated, but was not generated successfully. This asset type will be regenerated on the next build, even if the source files have not changed, since we can't be sure that the existing generated assets are up to date." + ) + saved_asset_table.pop(asset_type, None) # Save the asset table to disk: self.save_asset_table(saved_asset_table) log.info("Finished generating assets.\n")