refactor(screenshots): language-aware banner pipeline + review hardening - #21
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".
| from pathlib import Path | ||
|
|
||
| import pytest | ||
| from PIL import Image, ImageDraw, ImageFont |
There was a problem hiding this comment.
This new import is used during test collection, but CI installs only requirements-dev.txt (.github/workflows/blitztext-linux-ci.yml lines 70-73), and that file lists PyQt6, evdev, openai, and pytest without Pillow (requirements-dev.txt lines 4-7). In a fresh checkout the full python -m pytest tests/ job will fail before running these tests with ModuleNotFoundError: No module named 'PIL'; please add Pillow to the dependency list (and the install script as well if the generator is meant to run from that venv).
Useful? React with 👍 / 👎.
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
1 participant
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