chore: backlog quickwins sweep + sk-vista key migration#21
Conversation
Videos >=50MB silently fell back to filename-only linking with no signal to the user. Route the notice through the existing pipeline errors channel so it renders in the report's errors-section.
The header implied SCREENSCRIBE_API_KEY, OPENAI_API_KEY and
LIBRAXIS_API_KEY were equivalent alternatives ("first non-empty
wins"). Describe the actual per-endpoint precedence so it matches
the provider-mismatch warning already below.
v4.4.0 runs on the deprecated Node 20 action runtime, which GitHub now flags with deprecation annotations. Pin to v7.0.0 (node24 runtime), resolved via gh api to its full commit SHA across all 5 usages in ci.yml, stt-contract.yml, publish-pypi.yml and publish-testpypi.yml.
Quickstart assumed a narrated screencast the user doesn't necessarily have. Add a synthetic-clip recipe (FFmpeg testsrc+sine) so the CLI can be exercised end to end without one, with honest copy about the outcome (empirically verified: no speech -> empty-state report, not fabricated findings).
maskable-512.png was byte-identical to icon-512.png (no safe zone), so mask-clipping platforms (e.g. Android adaptive icons) would crop the logo. Regenerate it: icon-512.png scaled to 80% of the 512x512 canvas, centered on a solid #000000 background (theme_color).
LibraxisAI is migrating off the `vista-` header to `sk-vista` (and other `sk-*` per-project) keys. The old `_key_provider` tagged every `sk-` key as OpenAI and hard-blocked "libraxis endpoint + openai key", which would block our own default provider once the migration lands. Move to a three-class key model: - `sk-vista*` / legacy `vista-*` -> `libraxis` (certain) - any other `sk-*` -> `ambiguous` (never auto-OpenAI) - everything else -> unknown (silent, as before) validate() now hard-blocks only the certain mistake: a LibraxisAI key aimed at the openai.com endpoint (that key must never reach OpenAI). An ambiguous `sk-` key on a LibraxisAI endpoint degrades from Exit(1) to a non-blocking mismatch_warnings() warning that points at `screenscribe config setup`; the run continues. Public validate()/configuration_status()/mismatch_warnings() shapes are unchanged. Tests updated to the new hard-block direction and extended with the migration matrix (sk-vista clean, legacy vista- clean, libraxis->openai blocks, sk-proj on openai clean). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the API key validation and warning logic to distinguish between recognized LibraxisAI keys, ambiguous sk- keys, and other custom keys. Specifically, ambiguous sk- keys on LibraxisAI endpoints now trigger non-blocking warnings instead of hard blocks, while recognized LibraxisAI keys aimed at OpenAI endpoints remain blocked. Additionally, a warning is now surfaced when a video exceeds the 50MB embed limit in HTML reports instead of silently falling back to linking by filename. The review feedback correctly identifies a bug in render_html_report_pro where initializing errors = errors or [] rebinds empty lists to a new list object, preventing callers from seeing appended errors. Fixing this initialization will also allow simplifying the corresponding test workaround.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| errors.append( | ||
| { | ||
| "stage": "embed_video", | ||
| "message": ( | ||
| f"Video is {size_mb:.0f}MB, exceeding the 50MB embed limit; " | ||
| "linking by filename instead of embedding inline." | ||
| ), | ||
| } | ||
| ) |
There was a problem hiding this comment.
The errors parameter is initialized as errors = errors or [] on line 467. Because an empty list [] is falsy in Python, passing an empty list causes errors to be rebound to a new list object inside render_html_report_pro. Consequently, any errors appended here (such as the new embed_video warning) will not be visible to the caller if they passed an empty list.
To fix this, please update line 467 to:
if errors is None:
errors = []This ensures that empty lists passed by callers are mutated in place, preserving the reference.
| # Start from a non-empty (truthy) list: render_html_report_pro does | ||
| # `errors = errors or []` internally, which rebinds to a *new* list when | ||
| # given an empty one — an external `[]` would silently stop observing the | ||
| # append. A pre-seeded truthy list keeps the same object identity, so the | ||
| # append inside the render call is visible here too. | ||
| errors: list[dict[str, str]] = [{"stage": "seed", "message": "keep list truthy"}] |
There was a problem hiding this comment.
This workaround is only necessary because of the errors = errors or [] initialization bug in render_html_report_pro (which rebinds empty lists to a new list object). Once that is fixed to if errors is None: errors = [], we can simplify this test to use a clean empty list.
| # Start from a non-empty (truthy) list: render_html_report_pro does | |
| # `errors = errors or []` internally, which rebinds to a *new* list when | |
| # given an empty one — an external `[]` would silently stop observing the | |
| # append. A pre-seeded truthy list keeps the same object identity, so the | |
| # append inside the render call is visible here too. | |
| errors: list[dict[str, str]] = [{"stage": "seed", "message": "keep list truthy"}] | |
| errors: list[dict[str, str]] = [] |
![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: 983c013c5e
ℹ️ 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".
| if endpoint_provider == "openai" and key_provider == "libraxis": | ||
| errors.append( | ||
| f"{label} provider mismatch: an OpenAI API key would be sent to LibraxisAI.\n" | ||
| " No request was sent. Run `screenscribe config setup` and choose OpenAI." | ||
| ) | ||
| elif endpoint_provider == "openai" and key_provider != "openai": | ||
| errors.append( | ||
| f"{label} provider mismatch: the OpenAI endpoint requires an OpenAI API key.\n" | ||
| " No request was sent. Run `screenscribe config setup` and choose OpenAI." | ||
| f"{label} provider mismatch: a LibraxisAI API key would be sent to OpenAI.\n" | ||
| " No request was sent. Run `screenscribe config setup` and choose LibraxisAI." | ||
| ) |
There was a problem hiding this comment.
When users leave the default Libraxis endpoints but set OPENAI_API_KEY for LLM/Vision (optionally alongside LIBRAXIS_API_KEY for STT, or with review --local), the loader still uses that explicitly named OpenAI key for LLM/Vision, but this softened validation now treats it only as an ambiguous sk- shape and returns no blocking error for the Libraxis endpoint. The run therefore proceeds past the warning and sends an OpenAI credential to LibraxisAI, which is the provider-mismatch safety case this gate previously prevented; preserve the OPENAI_API_KEY provenance or keep blocking that explicit source rather than relying only on key shape.
Useful? React with 👍 / 👎.
| # Surface the size-limit fallback through the existing pipeline-errors | ||
| # channel (rendered in the report's "errors-section") instead of | ||
| # silently linking by filename with no explanation to the user. | ||
| errors.append( |
There was a problem hiding this comment.
In the normal save_html_report_pro path, video_path passed here is the basename returned by _prepare_html_video_source, not the source path, so for default review output directories Path(video_path).exists() is false and this new append is skipped. Oversized --embed-video reports therefore still fall back to a filename without the in-report warning this change is meant to add; do the size check before rewriting the path or pass the output-directory path into the renderer.
Useful? React with 👍 / 👎.
1 participant
Summary
Six commits consolidating backlog quickwins plus the provider-key migration:
--embed-videoinput exceeds the 50MB embed limit instead of silently falling back to a by-name reference (c27f1bc).env.example(d5080dd)setup-nodepin to the Node 24 runtime, SHA-pinned and verified against the action'sruns.using(ab72a44)ffmpegsynthetic clip piped throughscreenscribe review; verified empirically end-to-end (expected result: honest "no speech detected" empty-state report) (86cfa27)icon-512.png) (30c89e1)sk-vistakeys and soften the ambiguous mismatch (983c013) — LibraxisAI is migrating fromvista-tosk-vista*/ per-projectsk-*keys._key_providernow uses a three-class model (libraxis / ambiguous / unknown);validate()hard-blocks only the certain mistake (a LibraxisAI key aimed at openai.com); an ambiguoussk-key on a LibraxisAI endpoint degrades to a non-blocking warning. Publicvalidate()/configuration_status()/mismatch_warnings()shapes unchanged.Test plan
make lint— cleanuv run mypy screenscribe— clean (57 files)uv run pytest -q— 1206 passed, 51 skippedsk-vistaclean, legacyvista-clean, libraxis→openai blocks,sk-projon openai clean🤖 Generated with Claude Code