Skip to content

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

Merged
oscarlevin merged 5 commits intomainfrom
copilot/add-tests-for-image-generation-errors
Mar 22, 2026
Merged

Raise on failed individual asset generation to prevent incorrect cache updates#1130
oscarlevin merged 5 commits intomainfrom
copilot/add-tests-for-image-generation-errors

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 20, 2026

  • Fix generate.py to log an error and raise an exception when individual asset generation fails (output file not created)
    • individual_prefigure: raise error when output file doesn't exist after conversion
    • individual_asymptote: raise error when output file doesn't exist after conversion
    • individual_sage: raise error when output file doesn't exist after conversion
    • individual_latex_image: raise error when output file doesn't exist after conversion
  • Add tests/test_generate.py to verify failed image generation is not reported as success
    • Test that each individual generator raises when output file is not created
    • Test that each individual generator succeeds (no raise) when output file is created
  • Rebased on main
  • Ran poetry run black . for formatting
  • Added CHANGELOG.md entry under [Unreleased] > Fixed
  • CodeQL: 0 alerts
Original prompt

This section details on the original issue you should resolve

<issue_title>Add tests to ensure failed image generation is not reported as success</issue_title>
<issue_description>If an asset is not generated by core, there should be an error or warning coming from core. That needs to be picked up by the cli so that the asset type is not added to the generated asset cache list.</issue_description>

Comments on the Issue (you are @copilot in this section)


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI changed the title [WIP] Add tests to ensure failed image generation is not reported as success Raise on failed individual asset generation to prevent incorrect cache updates Mar 20, 2026
Copilot AI requested a review from oscarlevin March 20, 2026 15:38
Copy link
Copy Markdown
Member

@oscarlevin oscarlevin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot , rebase on main and try this again. Make sure you format at follow the new instructions.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 22, 2026

@copilot , rebase on main and try this again. Make sure you format at follow the new instructions.

Done in commit 2322456. Rebased on main, ran poetry run black . (which reformatted tests/test_generate.py), and added a CHANGELOG.md entry under [Unreleased] > Fixed per the new copilot instructions.

Copilot AI requested a review from oscarlevin March 22, 2026 00:24
@oscarlevin oscarlevin marked this pull request as ready for review March 22, 2026 00:38
Copy link
Copy Markdown
Member

@oscarlevin oscarlevin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot , why are all the other commits here part of this pr. Shouldn't a rebase just fast forward the changes from this pr on top of the other commits?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 22, 2026

@copilot , why are all the other commits here part of this pr. Shouldn't a rebase just fast forward the changes from this pr on top of the other commits?

You're right — the PR history is messy. The original clone was shallow (grafted at eb5bb3a). When I ran git rebase origin/main, git couldn't determine the proper common ancestor and copied those intermediate main commits into the branch history instead of fast-forwarding through them.

The actual file diff vs main is correct (only generate.py, tests/test_generate.py, and CHANGELOG.md are changed), but the commit list shows those duplicate commits.

Fixing the commit history requires a force-push of a clean branch, which I'm unable to do with the tools available. If you want a clean history, you could run:

git fetch origin copilot/add-tests-for-image-generation-errors
git checkout copilot/add-tests-for-image-generation-errors
git rebase --onto main eb5bb3a HEAD --empty=drop
# skip any conflicting "from-main" commits with: git rebase --skip
git push --force origin copilot/add-tests-for-image-generation-errors

Copilot AI requested a review from oscarlevin March 22, 2026 00:50
Copilot AI and others added 4 commits March 22, 2026 09:47
…havior

- 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
- 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
@oscarlevin oscarlevin force-pushed the copilot/add-tests-for-image-generation-errors branch from 2322456 to 544552e Compare March 22, 2026 15:54
@oscarlevin oscarlevin merged commit ad9784b into main Mar 22, 2026
6 checks passed
@oscarlevin oscarlevin deleted the copilot/add-tests-for-image-generation-errors branch March 22, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tests to ensure failed image generation is not reported as success

2 participants