READABILITY REVIEW, week of 2026-09-11 (read-only scan by the repair role on the maintainer's request: "code that adds unnecessary complexity, redundancy or anything else that would make it hard for a human reader to fully follow the functionality"). Findings are candidates for tasks, ordered by how much they cost a reader; nothing was changed by the scan. Pick the ones worth doing; each is one small unit.

# interactor — readability and complexity review, last seven days

## Summary

On trunk the week is almost nothing: two `.peers/*.json` files arrive via a fork-resolve merge, and five metadata CSVs change only from CRLF to LF line endings (the 64-line diff is pure line-ending noise). The real work sits on the two `session/…` branches under `second-question/`: a full rewrite of the video's narration, scene prompts and the locked cold-open render, plus a small dry-run path fix, all inside one 2,451-line, 125 KB Python file. To a reader the week looks like editorial work done by hand-mirroring one text into four places while the tooling that should produce those places stays broken and is "flagged, not fixed" in three separate documents. The single biggest cost is that nothing in the kit is derived from anything else: narration, scene metadata, request plans, checksums and history are all parallel copies a human is told to keep in step.

## Findings

**1. The narration exists in four copies, and the code asks a human to keep them equal.**
`second_question_production.py:VOICEOVER_SCRIPT`, `SCENES[].voiceover`, `docs/CANONICAL_SCRIPT.md` (Full narration), `metadata/source_manifest.json`. The canonical doc says outright "they are three copies of one text, not three sources of truth… mirror the change in both places"; the fourth copy (`source_manifest.json`) is described as "not produced by any command". `VOICEOVER_SCRIPT` and the per-scene strings also differ (the scene strings carry `…` pause marks, the monolithic script does not), so "byte-for-byte identical" only holds for one pair. Smaller shape: `SCENES` is the source; `full_voiceover_script()` joins scene voiceovers; the doc's narration section and the manifest are written by the `metadata` command.

**2. The checked-in metadata deliverables are produced by no code in the repository.**
`second_question_production.py:write_metadata_csvs` writes six lowercase columns (`id,slug,seconds,voiceover,labels,source_keys`); the tracked `metadata/scenes_flat.csv` has nine title-case columns. `provider_profiles_flat.csv`, `ddex_crosswalk_flat.csv`, `rights_logo_notes.csv`, `CHECKSUMS.sha256` and `MANIFEST.md` have no writer at all. A reader who runs `metadata` gets a different, thinner set of files in a different directory (`second_question_build/metadata/`) and is told in `PROGRESS.md`, `MANIFEST.md` and the commit text that this is a known regression. Smaller shape: one writer that emits exactly the tracked files, run into the tracked directory, so "regenerate" is a command rather than a warning.

**3. One file carries five unrelated programs.**
`second_question_production.py` (whole file): ~600 lines of prose data (`SOURCES`, `SCENES`, `MASTER_SORA_STYLE`, `VOICEOVER_SCRIPT`), two HTTP video-provider clients, three PIL still renderers, an ffmpeg assembly pipeline, and an openpyxl/DDEX metadata writer, plus argparse glue. Nobody holds this in their head; the week's edits touched every region of it at once. Smaller shape: data in its own module (or JSON), and providers / render / assemble / metadata as separate files with the CLI on top.

**4. Three locked-scene renderers each re-declare the same inner helpers and palette.**
`render_locked_cold_open`, `render_locked_invention_reveal`, `render_locked_alternatives_network`: each defines its own nested `base()`, `save()`/`save_still()`, most define `arrow()` (with `import math` inside the nested function), and each repeats the six colour tuples. `render_locked_cold_open` additionally writes its own concat schedule and ffmpeg call inline although `render_still_schedule`, which does exactly that, is defined directly beneath it. Smaller shape: module-level `PALETTE`, `grid_base(w,h)`, `arrow()`, and every locked scene ends with `render_still_schedule(...)`.

**5. The Gemini `store=true` rule is enforced in three layers, then documented in three more.**
`CONTEXT_GENERATION_PROFILES["gemini_omni"]["store"]`, `gemini_omni_payload` (forces `store` when delivery is `uri`), and `create_gemini_omni_clip` (retries once "if store is not True" — unreachable given the builder — then retries again with inline delivery). The comments narrate the API's history ("validation now rejects…", "seeing this branch means either an older profile was edited or the server validation changed"). `README.md` then has both a "URI delivery fix" and a "URI delivery hotfix" section, and `docs/OMNI_URI_DELIVERY_DEBUG.md` repeats them. Smaller shape: the payload builder sets `store=true` for URI delivery, one sentence says why, and the retry ladder and the two README sections go.

**6. The same history is narrated in four documents.**
`second-question/PROGRESS.md` (187 lines, three "passes"), `MANIFEST.md` (History section), `docs/CANONICAL_SCRIPT.md` (25 lines of supersession preamble plus a "Revision 2 fixes" section listing the same ten fixes), and the commit messages. A reader cannot tell which document states current behaviour and which is a log; the canonical script's opening paragraph describes what the previous version of itself was. Smaller shape: the timeline holds history; each document states only what is true now.

**7. Provider selection is spelled three ways and mapped by hand in four places.**
`command_generate_clips`, `command_assemble`, and `command_all` (twice, for `gen_args` and `asm_args`) each translate the `--sora`/`--omni` boolean pair into a `provider_key`, with a mutual-exclusion check repeated in two of them; `provider_output_name` then renames `gemini_omni` to `omni` for directories, so one provider is `gemini_omni`, `omni`, and "Gemini Omni Flash" depending on which line you are reading. Smaller shape: `--provider {mock,sora,gemini_omni}` with one key used everywhere.

**8. Wrappers and parameters that have one or zero callers.**
`write_sora_requests` ("compatibility wrapper", no callers), `sora_prompt`/`gemini_omni_prompt` (one-line aliases of `context_visual_prompt`), `find_font` (unused; `pil_font` repeats its candidate list), `_gemini_request_debug_payload` (a JSON round-trip whose comment claims it avoids writing API keys but changes nothing), `_post_gemini_interaction` (wraps one `requests.post`), `gemini_omni_payload(include_binary=…)` (both branches return the same object), `logo_files` in `create_end_card` (built, never read), `tiny_font` in `render_locked_cold_open` (unused). Smaller shape: delete them; each one is a name the reader looks up and finds empty.

**9. Constants that still describe the previous three-minute cut.**
`metadata_defaults` (`"Duration": "PT3M02S"`), the `Delivery_Checklist` row "Final video duration within 2:45-3:20", the `--tts-speed` help "helps keep cut under ~3:20", and the tracked `release_metadata_flat.csv` ("Intended runtime 2:45-3:20") all contradict the week's 274-second render, which `PROGRESS.md` flags as a deliberate change. Smaller shape: derive duration from `sum(s.seconds for s in SCENES) + END_CARD_SECONDS` and drop the hand-typed runtime targets.

**10. Errors are flattened to one line or to a default value.**
`main` catches every `Exception` and prints `ERROR: <msg>` with no traceback for a pipeline this long; `ffprobe_duration` returns `0.0` on a parse failure, which downstream reads as "video is shorter than audio"; `requested_seconds_for` has a bare `except Exception` fallback; `download_url` logs a WARNING and continues for any failure on non-required assets, after which `create_end_card` silently paints "PLACE OFFICIAL LOGO HERE" placeholders. The last is documented (logos are optional overrides); the first three are not. Smaller shape: let exceptions propagate in `main`, and let `ffprobe_duration` raise.

**11. `command_all` builds two identical argument copies.**
`second_question_production.py:command_all`: `gen_args` and `asm_args` are both `Namespace(**vars(args))` with the same `sora`/`omni` assignments derived from `DEFAULT_FULL_AI_PROFILE`. Smaller shape: compute `provider_key` once and pass it to the two command functions as a parameter instead of round-tripping through argparse namespaces.

**12. Dependencies are declared twice on trunk, and the lock covers one of ten.**
`.gitmodules` and `.peers/peers.json` list the same ten repositories with the same URLs and tracks; `.peers/peers.lock.json` resolves only `BeagleBadge`. A reader of the trunk diff must know that the peers file is the Fossil-side mirror of the git submodules (a documented mechanism) and that the other nine entries are unpinned. Smaller shape: generate `peers.json` from `.gitmodules`, or state in the file which is authoritative.

## Recurring patterns

- The same text or state is kept in several places and a comment or doc asks a human to mirror edits, instead of one place being derived from another.
- History is narrated inside code comments and inside the current-state documents (fix, hotfix, "revision 2", "pass three") rather than left to the timeline.
- Fixes are layered on top of the previous fix (a forced value, then a retry, then a second retry; a wrapper kept "for compatibility") rather than replacing it.

## Method

Read: the trunk numstat and full diff (confirmed CSV changes are CRLF-to-LF only), the session-branch numstat, `second_question_production.py` in full at the branch tip plus its code-only diff against trunk, `PROGRESS.md`, `README.md`, `MANIFEST.md`, `request_plans/README_request_plans.md`, `docs/OMNI_URI_DELIVERY_DEBUG.md`, the outline and preamble of `docs/CANONICAL_SCRIPT.md`, the `docs/Why_Interactor.md` diff, `.peers/*.json`, `.gitmodules`, the tracked CSV headers and the head of `source_manifest.json`.
Not read: `badge-demo/` and `scripts/` (untouched this week), the `.xlsx`, the full DDEX XML, JSONL request bodies, and all binary media; nothing was executed or built.
About 25 minutes of reading; all commands were `fossil cat`/`diff`/`timeline`/`ls` against the repository file.