refactor(screenshots): language-aware banner pipeline + review hardening - #21
Merged
Conversation
Rework scripts/_make_screenshots.py into a language-aware asset generator (EN/DE banners composed from real offscreen UI screenshots) and apply the review fixes from code/security/test passes: - fix: the "AI Workflows" settings screenshot captured the wrong tab (hardcoded index 2 = General); tabs are now resolved by i18n key via _tab_index(), independent of tab order and UI language - robustness: _resize_card() now raises a clear FileNotFoundError/OSError for missing or corrupt screenshots instead of an opaque crash - security/hygiene: tray-menu compositing uses tempfile.NamedTemporaryFile with try/finally cleanup instead of predictable names in the output dir - cleanup: drop unused imports (Callable, QPoint, QRect, QColor, QMenu) - document the history-panel animation settle delay Add tests/test_make_screenshots.py: 11 unit tests for the pure helpers (_resize_card, _draw_multiline, _font, _tab_index), including a regression test (EN+DE) for the tab-index bug. Full suite: 289 passed, 18 skipped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZ6vEXewZvitxbJHfwCDw5
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92382251c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The new tests/test_make_screenshots.py imported PIL at module level, but
Pillow is only a hidden, locally-installed dependency of the screenshot
generator — it is not in requirements-dev.txt. CI (Python 3.11/3.12) failed
during collection with ModuleNotFoundError: No module named 'PIL', which
aborted the entire suite.
- declare Pillow in requirements-dev.txt (it is the real, previously
undeclared dependency of scripts/_make_screenshots.py)
- guard the test module with pytest.importorskip("PIL") so a missing
Pillow skips these tests gracefully instead of breaking collection
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZ6vEXewZvitxbJHfwCDw5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reworks
scripts/_make_screenshots.pyinto a language-aware asset generator (EN/DE banners composed from real offscreen UI screenshots) and applies fixes from a parallel code / security / test review of the diff.This is a manually-run developer asset generator (not shipped app code), so severity was calibrated to that context.
Fixes
settings-ai-workflows-*.pngscreenshot captured the wrong tab —"ai-workflows"was hardcoded to index2, which is the General tab (order is0=speech, 1=workflows, 2=general). Tabs are now resolved by i18n key via_tab_index(), independent of tab order and UI language._resize_card()now raises a clearFileNotFoundError/OSErrorfor missing or corrupt screenshots instead of an opaque crash mid-composite.tempfile.NamedTemporaryFile+try/finallycleanup instead of predictable.tmp.pngnames in the output dir.Callable,QPoint,QRect,QColor,QMenu); documented the history-panel settle delay.Tests
Adds
tests/test_make_screenshots.py— 11 unit tests for the pure helpers (_resize_card,_draw_multiline,_font,_tab_index), including an EN+DE regression test for the tab-index bug. The module is loaded viaimportlib(noQApplicationcreated on import, verified), so the tests run ungated.Test plan
pytest tests/test_make_screenshots.py— 11 passedpytest -q— 289 passed, 18 skipped (GUI-gated)python -m py_compile+ AST unused-import check — cleanai-workflowsbanner tile now shows the correct tab🤖 Generated with Claude Code