Raise on failed individual asset generation to prevent incorrect cache updates (#1130)

Copilot · oscarlevin · web-flow · commit ad9784bb54f5 · 2026-03-22T12:34:16.000-06:00
* Initial plan * 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 * Initial plan * 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 * drop failed assets from cache --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: oscarlevin <6504596+oscarlevin@users.noreply.github.com> Co-authored-by: Oscar Levin <oscar.levin@unco.edu>
diff --git 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/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")
diff --git 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
@@ -0,0 +1,225 @@
+"""
+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,
+        )
