fix(916): the worktree-isolated fallback reviewer follows the lenses, fix commits are a sha range, and the cap is one .NET slot #918

Merged

timothy merged 5 commits from 916-serialise-review-fallback into main 2026-09-05 14:58:13 +02:00

Conversation 1 Commits 5 Files Changed 6

+155 -63

timothy commented 2026-09-05 14:35:29 +02:00

Owner

Copy Link

Copy Source

fixes #916

Root cause

One .NET slot's review round ran two worktree-isolated reviewers at once — the correctness lens and, when the Codex runner could not run, its fallback — each with its own dotnet build and test host. Measured 2026-09-05 on the 16 GB Mac: three slots in review reached load 82 and swap exhaustion; one slot's round alone took swap from 6.8 GB to 10.8 GB in three minutes. Separately, the finisher's fix attribution was a line-set difference over agent-formatted text and listed all eleven #563 commits as fix commits.

Fix

• review() runs the two lenses in parallel and the Codex runner beside them (it builds nothing); the worktree-isolated fallback starts only after both lenses return. A rubric round whose runner and fallback both fail is an error before the push, not a landed PR whose body claims a substitute reviewed it; cross-family state is per round, so a round-one substitute failure does not doom a run whose landing round had a real Codex review.
• Fix attribution is the sha range the fixer's report head advances (head_sha required on every report, described as the worktree HEAD, not a PR head); the finisher describes exactly the commits git log --oneline lists in those ranges.
• docs/handoffs/orchestration.md and orchestrator-prompt.md state the measured cap — one .NET-building slot at a time on this host, docs and Python slots beside it — with the 20%/10% RAM thresholds by key, why the cap stays at one after the serialisation, and that the referee reads cross_family, not only error, before a verdict. The record says "several", not "three" (Decisions-Edit: yes).

Measured

web/scripts/orchestration-workflow-loop.test.mjs (28 tests) compiles both committed script bodies; lens completions are counted per round from each agent's own label. Reviewer-run mutation sweep on the final head: re-serialising the runner reddens the runner assertion (expected [2] to equal [0]), moving the fallback beside the lenses or into the parallel batch reddens the fallback assertion, removing the per-round reset reddens the round-one-failure case, deleting the failed-substitute guard reddens its case — all in both scripts.

Gate	Result
PYTHONPATH=. python3 -m pytest scripts/tests -q	1580 passed, 3 skipped (reviewer, final head)
npx vitest run scripts/orchestration-workflow-loop.test.mjs	28 passed
npm run lint / npm test	exit 0 / 1371 passed
scripts/check-doc-narrative.py --diff origin/main	0 advisory warnings
scripts/decisions_validate.py	OK

Review

Five cold review rounds (Opus, high, worktree-isolated, same-family — docs, JS prompts and a vitest harness are outside the cross-family rubric). Round one: 1 blocking (a no-undef lint error my own check missed by reading only the tail of the output) and 8 lesser; round two: 1 blocking (the code contradicted Done-when box 1 after I moved the runner beside the lenses — the issue body was amended) and 2 should-fix; round three: 1 should-fix (sticky cross-family state); round four: 1 blocking (a harness assertion that could not fail); round five: no findings, VERDICT: merge. Each finding was verified by the reviewer by execution.

Cross-family review: not required (routine risk class under process.independent-review-rubric).

Deferred

none

🤖 Generated with Claude Code

https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

fixes #916 ## Root cause One .NET slot's review round ran two worktree-isolated reviewers at once — the correctness lens and, when the Codex runner could not run, its fallback — each with its own `dotnet build` and test host. Measured 2026-09-05 on the 16 GB Mac: three slots in review reached load 82 and swap exhaustion; one slot's round alone took swap from 6.8 GB to 10.8 GB in three minutes. Separately, the finisher's fix attribution was a line-set difference over agent-formatted text and listed all eleven #563 commits as fix commits. ## Fix - `review()` runs the two lenses in parallel and the Codex runner beside them (it builds nothing); the worktree-isolated fallback starts only after both lenses return. A rubric round whose runner and fallback both fail is an error before the push, not a landed PR whose body claims a substitute reviewed it; cross-family state is per round, so a round-one substitute failure does not doom a run whose landing round had a real Codex review. - Fix attribution is the sha range the fixer's report head advances (`head_sha` required on every report, described as the worktree HEAD, not a PR head); the finisher describes exactly the commits `git log --oneline` lists in those ranges. - `docs/handoffs/orchestration.md` and `orchestrator-prompt.md` state the measured cap — one .NET-building slot at a time on this host, docs and Python slots beside it — with the 20%/10% RAM thresholds by key, why the cap stays at one after the serialisation, and that the referee reads `cross_family`, not only `error`, before a verdict. The record says "several", not "three" (`Decisions-Edit: yes`). ## Measured `web/scripts/orchestration-workflow-loop.test.mjs` (28 tests) compiles both committed script bodies; lens completions are counted per round from each agent's own label. Reviewer-run mutation sweep on the final head: re-serialising the runner reddens the runner assertion (`expected [2] to equal [0]`), moving the fallback beside the lenses or into the parallel batch reddens the fallback assertion, removing the per-round reset reddens the round-one-failure case, deleting the failed-substitute guard reddens its case — all in both scripts. | Gate | Result | |---|---| | `PYTHONPATH=. python3 -m pytest scripts/tests -q` | 1580 passed, 3 skipped (reviewer, final head) | | `npx vitest run scripts/orchestration-workflow-loop.test.mjs` | 28 passed | | `npm run lint` / `npm test` | exit 0 / 1371 passed | | `scripts/check-doc-narrative.py --diff origin/main` | 0 advisory warnings | | `scripts/decisions_validate.py` | OK | ## Review Five cold review rounds (Opus, high, worktree-isolated, same-family — docs, JS prompts and a vitest harness are outside the cross-family rubric). Round one: 1 blocking (a `no-undef` lint error my own check missed by reading only the tail of the output) and 8 lesser; round two: 1 blocking (the code contradicted Done-when box 1 after I moved the runner beside the lenses — the issue body was amended) and 2 should-fix; round three: 1 should-fix (sticky cross-family state); round four: 1 blocking (a harness assertion that could not fail); round five: no findings, `VERDICT: merge`. Each finding was verified by the reviewer by execution. Cross-family review: not required (routine risk class under process.independent-review-rubric). ## Deferred none 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

timothy added 5 commits 2026-09-05 14:35:29 +02:00

fix(916): the cross-family review step follows the lenses instead of running beside them, and fix commits are a sha range ... f7a0da4051

One .NET slot's review round ran two worktree-isolated reviewers at once — the
correctness lens and, on a rubric change, the Codex fallback — and took swap
from 6.8 GB to 10.8 GB in three minutes on the 16 GB host; three slots reached
load 82. review() now awaits the lenses, then the Codex runner, then its
fallback. The finisher's fix attribution is the sha range the fixer's report
head advances (head_sha is required on every report), replacing a line-set
difference over free text that listed all eleven #563 commits as fixes. The
harness gains a case that records how many lenses were still in flight when
the cross-family agents started (must be zero); moving the fallback back into
the parallel batch reddens it in both scripts. The mechanics page and the
standing prompt state the measured cap: one .NET-building slot at a time.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

fix(916): the Codex runner runs beside the lenses, only the fallback waits; head_sha described everywhere; the docs keep their thresholds ... 929dff835a

The runner builds nothing, so serialising it only added its wait to the
critical path; the worktree-isolated fallback is what must follow the lenses,
and the harness case now records lens count at the FALLBACK's start alone.
setTimeout in the harness is globalThis.setTimeout (the .mjs lint config has
ES builtins only). head_sha carries the same description in both scripts and
every fixer/implementer prompt asks for the worktree HEAD, not a PR head. The
mechanics page says why the cap stays at one after the serialisation and
restores the 20%/10% RAM thresholds by key; the record says "several", not
"three".

Decisions-Edit: yes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

fix(916): a failed substitute is an error, not a landed claim; head_sha described in both schemas; the issue's box says what the code does ... 595c819de1

The runner-beside-the-lenses design is now what Done-when box 1 asks for (body
amended). A rubric round whose runner and worktree fallback both fail returns
an error before the push instead of landing a PR whose body claims a substitute
reviewed it. The harness records lens count at the runner's start too (expects
0, so a re-serialised runner reddens), resets its counter per round, and has a
case for the double failure.

Decisions-Edit: yes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

fix(916): cross-family state is per round, and the orchestrator reads cross_family before a verdict ... b1d5fbefcb

A substitute that failed in round one said nothing about the tree that lands
after round two, yet the flag was sticky and doomed the run; the xfamily
string was never reset either, so clearing the stickiness alone would have let
a stale "substitute ALSO failed" sentence into the PR body. Both reset at the
top of review(). The harness runner is round-aware (ran per round, its own
counter reset) and a two-round case pins the fix; restoring the sticky flag
reddens it in both scripts. Step 4 of the mechanics page tells the referee to
read cross_family, not only error, before posting on a rubric-class PR.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

fix(916): the harness counts lens completions per round, so the runner assertion can fail ... a083c851b3

The runner observer reset the shared counter to zero one line before reading
it, so its assertion held under the very mutant it existed to reject and the
fallback assertion caught that mutant for the wrong reason. Completions are
now keyed by the round in each agent's own label; no stub resets shared
state. Measured: re-serialising the runner reddens the runner assertion
(expected [2] to equal [0]) in both scripts; moving the fallback beside the
lenses reddens the fallback assertion and the round-one-failure case.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

timothy referenced this pull request 2026-09-05 14:36:16 +02:00

Orchestration workflows: a rubric review round runs two worktree-isolated .NET reviewers at once and saturates swap; run the cross-family step after the lenses, and cap the doc at one .NET slot #916

timothy commented 2026-09-05 14:36:19 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ a083c85

Five cold review rounds (Opus high, worktree-isolated, same-family: docs + JS prompts + a vitest harness, outside the cross-family rubric); round five clean after a five-mutant sweep and the full Python suite green on the final head. Cross-family review: not required (routine risk class). Orchestrator session.

Review-verdict: MERGEABLE @ a083c85 Five cold review rounds (Opus high, worktree-isolated, same-family: docs + JS prompts + a vitest harness, outside the cross-family rubric); round five clean after a five-mutant sweep and the full Python suite green on the final head. Cross-family review: not required (routine risk class). Orchestrator session.

timothy merged commit 366a0f9040 into main 2026-09-05 14:58:13 +02:00

timothy deleted branch 916-serialise-review-fallback 2026-09-05 14:58:15 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#918