open/portfolio-optimiser
1
0
Fork 0
Code Issues Releases 3 Activity

docs(major2): trekreview av proposal-review-doeren - BLOCK, 1 BLOCKER / 3 MAJOR / 1 MINOR

Browse source 
Voyage /trekreview over bcf3337..c66dddb (de tolv MAJOR-2-commitene; mtime-fallbacken
ga da5f10f og ble innsnevret fordi de tre mellomliggende commitene er verdict-gaten).
review.md ligger local-only i .claude/projects/2026-09-04-major2-proposal-review-door/
(gitignored som brief/plan); denne fila er en byte-identisk sporet kopi.

Funn: BLOCKER CLAUDE.md:1843 (SC5 - announce-avviket paastaas uttalt i invariantraden,
raden sier det ikke) · MAJOR run.py:2738 (--proposal-review nektes med --checkpoint-dir,
men --resume krever --checkpoint-dir, saa stien nekten peker paa er unaabar) · MAJOR
tests:1344 (compose-with-resume-armen sender aldri --resume) · MAJOR tests:820
(debatt-tellingen briefen krever paret med hver genereringstelling mangler) · MINOR
run.py:1202 (OSError fra finally-skriveren fortrenger stopp-unntaket).
M29/last_ruling: begge reviewere - riktig som det er, unaabar ved konstruksjon.
Umaalt mot levende modell: SC1 (utfall-halvdelen), SC4, SC8, SC13-premisset.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-05 21:48:24 +02:00
commit 39d585ec47
1 changed files with 221 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

221
docs/2026-09-05-major2-trekreview.md Normal file
View file

@ -0,0 +1,221 @@
<!-- Tracked archive of the Voyage /trekreview artefact `.claude/projects/2026-09-04-major2-proposal-review-door/review.md` (local-only, gitignored like brief.md and plan.md). Byte-identical copy at commit time; `/trekplan --brief` consumes the local-only original. Order 20260905T191506Z-9345814922-from-.claude. -->
---
type: trekreview
review_version: "1.0"
created: 2026-09-05
task: "MAJOR-2: a door for human feedback on the proposal on the table — approve | revise(feedback) after the validator's outcome, one further bounded attempt, history verbatim in RunResult and outbox, CLI --proposal-review, hosting refuses"
slug: major2-proposal-review-door
project_dir: .claude/projects/2026-09-04-major2-proposal-review-door/
brief_path: .claude/projects/2026-09-04-major2-proposal-review-door/brief.md
scope_sha_start: bcf3337
scope_sha_end: c66dddb1da3332e5fdf90f6d466f8ba171968d44
reviewed_files_count: 10
findings:
- ada5f264ceb5027dc102ac972e7123d2365302ab
- 5935d942631ddf34714805ec324b5717d475540e
- 02159d215c23337196ef0a777628a8626244de2b
- da48592851334aa669f75f80635d475176d5987b
- d71a5d72d4a3057232f6adfe8f02f7de912419dd
---
# Review: MAJOR-2: a door for human feedback on the proposal on the table — approve | revise(feedback) after the validator's outcome, one further bounded attempt, history verbatim in RunResult and outbox, CLI --proposal-review, hosting refuses
## Executive Summary
BLOCK: 1 BLOCKER, 3 MAJOR and 1 MINOR finding survived all passes; nothing was suppressed, the `unverified` bucket is empty, and both expected reviewers reported. Scope note: no `progress.json` exists, so the command's mtime fallback (brief mtime 2026-09-04T17:13:26Z) resolved to `da5f10f`, which would have included three commits of the separate, already-closed verdict-gate order (`e7d1a31`, `0a83bb8`, `bcf3337`); the orchestrator narrowed the range to `bcf3337..c66dddb`, the exact 12-commit MAJOR-2 series (`ce4c15e` … `c66dddb`), dropping `docs/2026-09-04-s2c-debatt-k2.md`, `src/portfolio_optimiser/explore.py`, `src/portfolio_optimiser/okf.py` and `tests/test_verdict_layer_refusal_loadbearing.py` (all verdict-gate, none MAJOR-2); `scope_sha_start` is recorded in short form because the coordinator may not run git. Effort is `high` (brief signal), so Pass 3 (reasonableness) was skipped; all five rule_keys are catalogue keys and no normalisation was needed. Read first: the BLOCKER at CLAUDE.md:1843 (SC5 — `proposal_review.py:196-200` says the announce-rule departure "is stated in the invariant row"; the orchestrator's grep over the MAJOR-2 row finds no announce / "never consulted" / zero-is-silence vocabulary while the control label "M33" is present). Read second: the MAJOR at run.py:2738 — the orchestrator confirmed from source that `--proposal-review` is refused whenever `--checkpoint-dir` is set (run.py:2738-2745, "pass --proposal-review at --resume instead") while `--resume` itself requires `--checkpoint-dir` (run.py:2776-2782), so the composition the refusal text, the run.py:2218 help and README.md:546 recommend is unreachable; its test-side twin (MAJOR, tests/test_proposal_review_loop_loadbearing.py:1344) shows the arm named for that composition never passes `--resume`. The conformance reviewer observed plan/execute drift that is stated or plan-internal and raised no finding for it: the K2 script used a four-step list and two revise/approve pairs where plan Step 9(c) said two-entry/one pair (explained in the doc, not labelled a deviation); the plan diagram says "26 arms, 37 mutations" vs Step 9's M1–M40 (plan-internal inconsistency); plan T19 was folded into T13 (stated in T13's docstring); scope creep: none. The executor's own numbers (1364 passed / 5 skipped, 40 red / 1 green, golden `ea8c534…`) are quoted from `docs/2026-09-04-major2-proposal-review-k2.md` and the CLAUDE.md row and were NOT re-run by any reviewer (read-only); the orchestrator confirmed `uv run pytest --collect-only -q` → 1369 tests collected at HEAD.
## Coverage
| File | Treatment | Reason |
|------|-----------|--------|
| CLAUDE.md | summary-only | default |
| README.md | summary-only | default |
| docs/2026-09-04-major2-proposal-review-k2.md | summary-only | default |
| src/portfolio_optimiser/generate.py | summary-only | default |
| src/portfolio_optimiser/hosting.py | summary-only | default |
| src/portfolio_optimiser/outbox.py | summary-only | default |
| src/portfolio_optimiser/proposal_review.py | summary-only | default |
| src/portfolio_optimiser/run.py | summary-only | default |
| tests/test_okf.py | summary-only | default |
| tests/test_proposal_review_loop_loadbearing.py | summary-only | default |
No files were skipped and no working-tree (`[uncommitted]`) changes were in scope.
## Operator focus points
1. `last_ruling` carrier (M29 green): BOTH reviewers independently classified it "correct as it stands — path unreachable by construction; carrier is documentation/defensive". Conformance reviewer's analysis: the loop tail after `for i in range(max_attempts)` is reached only via `continue` on the Rejection branch which sets `last`; every ValidatedProposal branch returns in-loop (reviewer None, approve, revise with remaining <= 0), and on the final iteration `max_attempts - i - 1 == 0` so a revise there returns in-loop. Only divergence: `max_attempts <= 0` (ValueError vs the old AssertionError), `pragma: no cover`, default `max_attempts=3`, not exposed by `run_project`. Residual (no catalogue key fits): T2b's docstring (tests/test_proposal_review_loop_loadbearing.py:438-443) still says "Detach point: reverting to `assert last is not None` (M29)" and repeats the falsified prediction — the `generate.py` comment was corrected, this docstring was not (Fase-3 class). Also docs/2026-09-04-major2-proposal-review-k2.md:59-62 says the generate.py comment "må rettes i neste økt" while :245-248 says it "er RETTET i samme commit" — §1 is stale.
2. `coverage` rows never name a revise as cause (correctness reviewer): row produced at run.py:504-512 with detail "budget exhausted before this approach was evaluated"; literally true when an expert's revises consumed the shared round ledger (tick_round is called only from generate.py:413 and :510 — the debate does not tick rounds). The row under-explains but does not read false; the link is recoverable by arithmetic over `{run_id}-proposal-reviews.json` attempt indices, only when `outbox_dir` is set. Classified: honesty limit correctly stated (CLAUDE.md row, docs § 4), not a correctness gap. Caveat: `honoured=True` does not encode "this revise defunded approach N+1".
3. F4 traceback asymmetry (correctness reviewer): `PlanReviewInputError` is an `ExplorationError` (RuntimeError) (explore.py:551, raised :616); the CLI explore/resume block (run.py:3059-3110) catches only `PlanReviewParked` and `BudgetExceeded`, so plan-review EOF escapes as a traceback, while `ProposalReviewInputError` is caught by name at run.py:3404. In the documented composition `--explore --plan-review --proposal-review` the operator gets a traceback for silence at door 1 and `run stopped:` for silence at door 2. The new channel is correct (RuntimeError, off the ValueError refusal tuple at :3412, outside `_fetch_parsed`'s catch at generate.py:521). Pre-existing, stated, no cross-file regression; the diff added the channel built for exactly this class and left the sibling off it — the cheapest place to fix it. No finding (not anchored to a diff line).
4. Maintenance note (correctness reviewer, no catalogue key at the right tier): hosting.py:109 defines `_REFUSED_BY_NAME = ("proposal_review",)` and a test asserts on the constant, but the check at hosting.py:144 is the literal `if "proposal_review" in payload:` — two copies of one fact (kø-(p)); iterate over `_REFUSED_BY_NAME` at :144.
## Unmeasured against a live model
- **SC1** — mechanics (verbatim injection, bound, composition, byte-identity) fully measured offline; T1's "outcome differs" half is produced by a flip-keyed double (`_FeedbackAwareChatClient:309-335`) — that a live model changes its proposal in response to the words is UNMEASURED.
- **SC2** — fully measured offline (deterministic mutations; counts are the executor's, not re-run by the reviewer).
- **SC3** — fully measured offline (byte hash).
- **SC4** — UNMEASURED against a live model: the second attempt's different amount comes from the script's second entry, not from a model reading the feedback.
- **SC5** — fully measured offline (EOF/`run stopped:` channel is structural; the validated candidate that precedes it is scripted, but the criterion's assertions do not depend on model content).
- **SC6** — fully measured offline (structural refusal, zero calls by design).
- **SC7** — fully measured offline (structural refusals, zero calls).
- **SC8** — UNMEASURED against a live model: outcome 200 000→150 000 is the scripted position, prices are synthetic, and the doc says so (docs/2026-09-04-major2-proposal-review-k2.md:237-241); only token accounting and plumbing are measured.
- **SC9** — fully measured offline (lint/type; not re-run by the reviewer).
- **SC10** — fully measured offline (documents).
- **SC11** — fully measured offline (structural `is` on a recorder).
- **SC12** — fully measured offline (the zero-remaining path is independent of model content).
- **SC13** — mechanism (D6 last-ruling, record keeps the reviewed candidate) fully measured offline with a scripted rejection; whether a live model's follow-up would be rejected is a property of the model, not of the seam, and is UNMEASURED — as is the premise that a live model follows the feedback at all.
Criteria standing unmeasured against a live model: SC1 (outcome-differs half), SC4, SC8, SC13 (model-follows-feedback premise). All others are deterministic/structural and fully measured offline.
## Findings (BLOCKER)
### ada5f264ceb5027dc102ac972e7123d2365302ab
- file: CLAUDE.md
- line: 1843
- rule_key: BROKEN_SUCCESS_CRITERION
- brief_ref: SC5 — "a deliberate departure from the announce rule's zero-is-silence half, stated as such in the invariant row"
- title: State the announce-rule departure in the MAJOR-2 invariant row, as the code claims it already is
- detail: proposal_review.py:196-200 says the zero-reviews-with-reviewer notice is 'a deliberate departure from the announce rule's zero-is-silence half' and that 'The departure is stated in the invariant row'. The MAJOR-2 row starting at CLAUDE.md:1843 (full hunk read) never mentions the announce rule, 'offered, never consulted', or a zero-case departure; grep 'announce' in CLAUDE.md hits only older rows (lines 80, 418-419, 766). The only trace is the mutation label 'M33 rendereren returnerer None på null (3)', which names a mutation, not the decision. This is a claim the surface makes about itself that the row does not back (the repo's Fase-3 class), and SC5 names the row explicitly. The remedy is one sentence; the catalogue fixes the severity.
- recommended_action: Add one sentence to the MAJOR-2 row: proposal_review_notice returns a line on ZERO reviews when a reviewer was offered ('offered, never consulted'), a deliberate departure from the announce rule's omission-on-nothing half, gated by M33 and M37.
## Findings (MAJOR)
### 5935d942631ddf34714805ec324b5717d475540e
- file: tests/test_proposal_review_loop_loadbearing.py
- line: 820
- rule_key: MISSING_TEST
- brief_ref: SC1 — "every 'N generation prompts' assertion in this brief ... is paired with the assertion that the debate's prompt count is unchanged"; SC4 — "the debate's prompt count equal to the control's"
- title: Pair every generation-prompt count with the debate-count assertion the brief requires
- detail: T5-run (:820-826) counts generation prompts via _GENERATION_MARK and T13 (:1145) uses the record's attempt indices as a proxy; no arm in the file asserts the debate turn count between treated and control (grep 'debate' hits only a comment at :657 and a filename at :887). The classifier is sound (both marker lines are emitted only by generate._build_messages, generate.py:330-332), but the brief made the pairing the guard against miscounting debate turns as generation. T13 also asserts none of SC4's prompt-level facts (count by header, feedback in the 2nd prompt, debate count): the 40 000 outcome proves a second attempt was bought, not that its prompt carried the words, because the step-list proposer does not read the prompt.
- recommended_action: In T5-run and T8 assert equality of the non-generation (debate) entries between treated_sink and control_sink; in T13 either document that the record indices are SC4's count proxy or write the child's scripted sink to disk and classify it.
### 02159d215c23337196ef0a777628a8626244de2b
- file: src/portfolio_optimiser/run.py
- line: 2738
- rule_key: PLAN_EXECUTE_DRIFT
- brief_ref: NFR — code correctness (CLI door composition)
- title: --proposal-review refused with --checkpoint-dir, so the --resume path it points to is unreachable
- detail: run.py:2738-2745 refuses whenever args.proposal_review and args.checkpoint_dir is not None, printing 'pass --proposal-review at --resume instead'. But --resume requires --checkpoint-dir (run.py:2776-2782: 'run refused: --resume requires --checkpoint-dir'). So '--resume X --checkpoint-dir D --proposal-review' is refused at :2738 by the very message that recommends it, and '--resume X --proposal-review' without a checkpoint dir is refused at :2776. The composition claimed at run.py:2218 (help text), README.md:546 and the CLAUDE.md MAJOR-2 row does not exist; the fall-through at run.py:3142 -> :3373 would thread the reviewer if the argv ever reached it. Determined by reading both guards; not executed.
- recommended_action: Change the guard at run.py:2738 to 'if args.checkpoint_dir is not None and args.resume is None:' so a parked exploration is refused while a resume passes, and add a test that drives '--resume ... --checkpoint-dir ... --review-inbox ... --proposal-review' with run_project recorded.
### da48592851334aa669f75f80635d475176d5987b
- file: tests/test_proposal_review_loop_loadbearing.py
- line: 1344
- rule_key: MISSING_TEST
- brief_ref: NFR — code correctness (test discriminates the claimed behavior)
- title: The 'composes with resume' arm never passes --resume, so the claimed path has no witness
- detail: test_the_door_composes_with_resume (:1328-1361) claims in its docstring to verify that the resume block falls through to the full-run dispatch with the reviewer, but the argv at :1344-1359 contains no --resume, --checkpoint-dir or --review-inbox; it is a plain single-project run already covered by T13. It therefore cannot detect that run.py:2738 refuses every real resume argv before run_project is reached. Mutation M39 was witnessed only by the park arm (:1294), which asserts the refusal fires, not that the recommended alternative works.
- recommended_action: Rewrite the arm to stage a parked plan-review question and answer (or monkeypatch hitl.read_plan_review_question/load_parked/hitl.load_plan_review_answer and resume_exploration) and pass '--resume RUN --checkpoint-dir D --review-inbox I --outbox-dir O --bundle-dir B --proposal-review', asserting the recorded run_project kwargs carry a non-None proposal_reviewer.
## Findings (MINOR)
### d71a5d72d4a3057232f6adfe8f02f7de912419dd
- file: src/portfolio_optimiser/run.py
- line: 1202
- rule_key: MISSING_ERROR_HANDLING
- brief_ref: NFR — code correctness (error channels)
- title: OSError from the always-on review writer in finally displaces the in-flight stop exception
- detail: run.py:1200-1208 calls outbox.write_proposal_reviews(...) inside the finally at :1184 on every run where a reviewer was given. An OSError from mkdir/write_text while BudgetExceeded or ProposalReviewInputError is propagating replaces the in-flight exception; neither 'except ProposalReviewInputError' (:3404) nor the refusal tuple (:3412) catches OSError, so the CLI tracebacks and the 'run stopped:' / 'run refused:' line is never printed. The parse-failures writer at :1191 has the same shape but runs only when a parse failed; this one runs on every reviewed run.
- recommended_action: Wrap the two finally writers in try/except OSError that records the write failure on stderr and re-raises only when no exception is already in flight, so the original stop reason survives.
## Findings (SUGGESTION)
None.
## Suppressed Findings
None suppressed. No finding was removed in Pass 2, Pass 3 was skipped, and the `unverified` bucket is empty — ALLOW is not withheld by any suppression; the verdict is BLOCK on the counted BLOCKER alone.
Coordinator notes on the passes (no findings altered):
- Pass 1: no `(file, line, rule_key)` collisions among the five findings; the two `MISSING_TEST` findings differ on line (820 vs 1344).
- Pass 2 Succinctness: the code-correctness-reviewer re-emitted its payload with shortened titles (packaging only; every other field identical to the first emission, confirmed by re-reading the file). Title lengths counted by the coordinator: 96 / 85 / 97 / 88 / 90 chars, all ≤ 100; every detail ≤ 800 chars (longest ≈ 750).
- Pass 2 Accuracy: all five `file` values resolve under the repo root and each cited line was read and is plausible (CLAUDE.md:1843 opens the MAJOR-2 row; run.py:2738 is the `--checkpoint-dir` guard; run.py:1202 is the `write_proposal_reviews` call; the two test lines exist). Nothing dropped as `accuracy:refuted`.
- Pass 2 Actionability: every `recommended_action` is non-empty and imperative; the `MISSING_TEST` finding at :820 opens with a locative phrase ("In T5-run and T8 assert …") before its imperative — judged imperative in mood, not a hedge or title restatement, kept.
- Pass 3 skipped (high effort). For the record, all five emitted severities already match their catalogue tiers, and all five rule_keys are in `RULE_KEYS`; no `PLAN_EXECUTE_DRIFT` normalisation was applied.
- IDs: computed by the orchestrator with the shipped 3-arg `computeFindingId(file, line, rule_key)` (`lib/parsers/finding-id.mjs`); the coordinator has no execution tool and did not re-derive the digests. The agent prose's 4-tuple (with title) is not producible by the shipped helper (`coordinator-contract.mjs` header).
## Remediation Summary
- 1 BLOCKER → must address before next plan iteration (CLAUDE.md:1843, one sentence in the MAJOR-2 row).
- 3 MAJOR → should address before next plan iteration (run.py:2738 guard composition; tests/test_proposal_review_loop_loadbearing.py:820 debate-count pairing; :1344 resume-path witness).
- 1 MINOR → nice-to-have (run.py:1202, OSError in the `finally` writer).
- 0 SUGGESTION.
If running `/trekplan --brief review.md`, the planner will consume the BLOCKER + 3 MAJOR findings as plan goals (their `recommended_action` becomes the step intent); the MINOR is skipped for v1.0 plan-input.
```json
{
"verdict": "BLOCK",
"counts": { "BLOCKER": 1, "MAJOR": 3, "MINOR": 1, "SUGGESTION": 0 },
"allow_blocked_by": [],
"unverified": [],
"missing_reviewers": [],
"findings": [
{
"id": "ada5f264ceb5027dc102ac972e7123d2365302ab",
"severity": "BLOCKER",
"rule_key": "BROKEN_SUCCESS_CRITERION",
"file": "CLAUDE.md",
"line": 1843,
"brief_ref": "SC5 — \"a deliberate departure from the announce rule's zero-is-silence half, stated as such in the invariant row\"",
"title": "State the announce-rule departure in the MAJOR-2 invariant row, as the code claims it already is",
"detail": "proposal_review.py:196-200 says the zero-reviews-with-reviewer notice is 'a deliberate departure from the announce rule's zero-is-silence half' and that 'The departure is stated in the invariant row'. The MAJOR-2 row starting at CLAUDE.md:1843 (full hunk read) never mentions the announce rule, 'offered, never consulted', or a zero-case departure; grep 'announce' in CLAUDE.md hits only older rows (lines 80, 418-419, 766). The only trace is the mutation label 'M33 rendereren returnerer None på null (3)', which names a mutation, not the decision. This is a claim the surface makes about itself that the row does not back (the repo's Fase-3 class), and SC5 names the row explicitly. The remedy is one sentence; the catalogue fixes the severity.",
"recommended_action": "Add one sentence to the MAJOR-2 row: proposal_review_notice returns a line on ZERO reviews when a reviewer was offered ('offered, never consulted'), a deliberate departure from the announce rule's omission-on-nothing half, gated by M33 and M37.",
"raised_by": ["brief-conformance-reviewer"]
},
{
"id": "5935d942631ddf34714805ec324b5717d475540e",
"severity": "MAJOR",
"rule_key": "MISSING_TEST",
"file": "tests/test_proposal_review_loop_loadbearing.py",
"line": 820,
"brief_ref": "SC1 — \"every 'N generation prompts' assertion in this brief ... is paired with the assertion that the debate's prompt count is unchanged\"; SC4 — \"the debate's prompt count equal to the control's\"",
"title": "Pair every generation-prompt count with the debate-count assertion the brief requires",
"detail": "T5-run (:820-826) counts generation prompts via _GENERATION_MARK and T13 (:1145) uses the record's attempt indices as a proxy; no arm in the file asserts the debate turn count between treated and control (grep 'debate' hits only a comment at :657 and a filename at :887). The classifier is sound (both marker lines are emitted only by generate._build_messages, generate.py:330-332), but the brief made the pairing the guard against miscounting debate turns as generation. T13 also asserts none of SC4's prompt-level facts (count by header, feedback in the 2nd prompt, debate count): the 40 000 outcome proves a second attempt was bought, not that its prompt carried the words, because the step-list proposer does not read the prompt.",
"recommended_action": "In T5-run and T8 assert equality of the non-generation (debate) entries between treated_sink and control_sink; in T13 either document that the record indices are SC4's count proxy or write the child's scripted sink to disk and classify it.",
"raised_by": ["brief-conformance-reviewer"]
},
{
"id": "02159d215c23337196ef0a777628a8626244de2b",
"severity": "MAJOR",
"rule_key": "PLAN_EXECUTE_DRIFT",
"file": "src/portfolio_optimiser/run.py",
"line": 2738,
"brief_ref": "NFR — code correctness (CLI door composition)",
"title": "--proposal-review refused with --checkpoint-dir, so the --resume path it points to is unreachable",
"detail": "run.py:2738-2745 refuses whenever args.proposal_review and args.checkpoint_dir is not None, printing 'pass --proposal-review at --resume instead'. But --resume requires --checkpoint-dir (run.py:2776-2782: 'run refused: --resume requires --checkpoint-dir'). So '--resume X --checkpoint-dir D --proposal-review' is refused at :2738 by the very message that recommends it, and '--resume X --proposal-review' without a checkpoint dir is refused at :2776. The composition claimed at run.py:2218 (help text), README.md:546 and the CLAUDE.md MAJOR-2 row does not exist; the fall-through at run.py:3142 -> :3373 would thread the reviewer if the argv ever reached it. Determined by reading both guards; not executed.",
"recommended_action": "Change the guard at run.py:2738 to 'if args.checkpoint_dir is not None and args.resume is None:' so a parked exploration is refused while a resume passes, and add a test that drives '--resume ... --checkpoint-dir ... --review-inbox ... --proposal-review' with run_project recorded.",
"raised_by": ["code-correctness-reviewer"]
},
{
"id": "da48592851334aa669f75f80635d475176d5987b",
"severity": "MAJOR",
"rule_key": "MISSING_TEST",
"file": "tests/test_proposal_review_loop_loadbearing.py",
"line": 1344,
"brief_ref": "NFR — code correctness (test discriminates the claimed behavior)",
"title": "The 'composes with resume' arm never passes --resume, so the claimed path has no witness",
"detail": "test_the_door_composes_with_resume (:1328-1361) claims in its docstring to verify that the resume block falls through to the full-run dispatch with the reviewer, but the argv at :1344-1359 contains no --resume, --checkpoint-dir or --review-inbox; it is a plain single-project run already covered by T13. It therefore cannot detect that run.py:2738 refuses every real resume argv before run_project is reached. Mutation M39 was witnessed only by the park arm (:1294), which asserts the refusal fires, not that the recommended alternative works.",
"recommended_action": "Rewrite the arm to stage a parked plan-review question and answer (or monkeypatch hitl.read_plan_review_question/load_parked/hitl.load_plan_review_answer and resume_exploration) and pass '--resume RUN --checkpoint-dir D --review-inbox I --outbox-dir O --bundle-dir B --proposal-review', asserting the recorded run_project kwargs carry a non-None proposal_reviewer.",
"raised_by": ["code-correctness-reviewer"]
},
{
"id": "d71a5d72d4a3057232f6adfe8f02f7de912419dd",
"severity": "MINOR",
"rule_key": "MISSING_ERROR_HANDLING",
"file": "src/portfolio_optimiser/run.py",
"line": 1202,
"brief_ref": "NFR — code correctness (error channels)",
"title": "OSError from the always-on review writer in finally displaces the in-flight stop exception",
"detail": "run.py:1200-1208 calls outbox.write_proposal_reviews(...) inside the finally at :1184 on every run where a reviewer was given. An OSError from mkdir/write_text while BudgetExceeded or ProposalReviewInputError is propagating replaces the in-flight exception; neither 'except ProposalReviewInputError' (:3404) nor the refusal tuple (:3412) catches OSError, so the CLI tracebacks and the 'run stopped:' / 'run refused:' line is never printed. The parse-failures writer at :1191 has the same shape but runs only when a parse failed; this one runs on every reviewed run.",
"recommended_action": "Wrap the two finally writers in try/except OSError that records the write failure on stderr and re-raises only when no exception is already in flight, so the original stop reason survives.",
"raised_by": ["code-correctness-reviewer"]
}
]
}
```