Repair the critic's two finders before making it smarter

Both static finders were broken in ways that made their output close to
noise, and the plan said so.

The swallowed-error finder skipped every impure function outright and could
only detect a swallow of another produced function's error, so the case it
exists to catch -- a standard-library call whose error goes nowhere -- was
structurally invisible. It now examines impure functions and resolves a
curated set of fallible standard-library calls through the effects the
contracts work already computes.

The boundary finder matched substrings over concatenated test source, so
"0" matched inside a file mode and "Error" matched inside t.Errorf: one
finding was unreachable and the rest were near-random. It now reads literal
arguments from the test syntax tree.

Findings gained a stable identity, an evidence level and a lineage, tagged
at each producer rather than stamped uniformly afterwards. They are ranked
by expected defect cost and capped per kind in the instruction, with the
omitted count disclosed rather than truncated silently. Both finders were
measured against fixture populations before promotion, as the governance
section requires, and both measured clean, so neither is retired.

A model-backed critic was not attempted: reaching a provider from here needs
the budgeted and authorised path, and building an unbudgeted one to get
there would be inventing a credential route.

Change-ID: CL-20260802-091
Dev-Log: DL-20260802-101
Request-or-TODO: PIPE-095, PIPE-096, PIPE-097, PIPE-103, PIPE-104, PIPE-105, PIPE-106

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: claude

CHANGELOG

@@ -50,6 +50,50 @@ Dev-Log:
 
 Entries
 -------
+Change-ID: CL-20260802-091
+Commit: pending
+Date: 2026-08-02
+Type: Fix -- the critic's two finders were near-random, and now are not
+Request-or-TODO: PIPE-095, PIPE-096, PIPE-097, PIPE-103, PIPE-104, PIPE-105,
+  PIPE-106
+Outcome: Both static finders were broken in ways that made their output close to
+  noise, and the plan said so.
+  The swallowed-error finder skipped every impure function outright and could
+  only detect a swallow of another produced function's error, so the case it
+  exists to catch -- a standard-library call whose error goes nowhere -- was
+  structurally invisible. It now examines impure functions and resolves a
+  curated set of fallible standard-library calls through the effects the
+  contracts work already computes.
+  The boundary finder matched substrings over concatenated test source, so "0"
+  matched inside a file mode and "Error" matched inside t.Errorf: one finding was
+  unreachable and the rest were near-random. It now reads literal arguments from
+  the test syntax tree, so a token has to be an actual literal node.
+  Findings gained a stable identity derived from kind, location and content, an
+  evidence level and a lineage, tagged at each producer rather than stamped
+  uniformly afterwards. They are ranked by expected defect cost and capped per
+  kind in the instruction, with the omitted count disclosed rather than
+  truncated silently
+Affected-behavior: The critic reports different findings. Both finders were
+  measured before promotion and both measured clean, so neither is retired
+Compatibility-or-migration: reviewAdversarially's signature and behaviour are
+  unchanged; risk-selected review is a new function beside it
+Verification: Each repair's discriminating test is the named bug as a fixture --
+  an impure function swallowing a standard-library error, a file mode
+  suppressing the zero-edge finding, a t.Errorf suppressing the error-assertion
+  finding -- each failing against the old code, each with a control proving the
+  fix did not simply widen the finder into flagging everything. The ranking test
+  sorts through the production comparison rather than a copy of it
+Known-limitations: A finding still does not outlive its attempt, so a stable
+  identity is a property nothing yet uses; that is PIPE-098 and it lives in a
+  file this change does not own, as do PIPE-099, PIPE-100, PIPE-102, PIPE-107,
+  PIPE-108 and PIPE-095's one-line call site. The false-positive measurement is
+  a test-time population rather than a persisted cross-run metric. A model-backed
+  critic was not attempted because reaching a provider from here needs the
+  budgeted and authorised path, and building an unbudgeted one to get there
+  would be inventing the credential route this session has twice refused to
+  invent
+Dev-Log: DL-20260802-101
+
 Change-ID: CL-20260802-090
 Commit: pending
 Date: 2026-08-02

DEVLOG

@@ -25,6 +25,37 @@ Next-safe-step:
 
 Entries
 -------
+Dev-Log: DL-20260802-101
+Date: 2026-08-02
+Status: Seven closed or capability-complete; the rest blocked on one file
+Change-ID: CL-20260802-091
+Commit: pending
+Request-or-TODO: PIPE-093 through PIPE-108
+Goal: Make the critic's findings mean something before making it smarter
+Assumptions: That the finders were approximately right and needed tuning --
+  wrong. One could not see its own target case at all
+Decisions: Repaired the two finders before touching anything about a
+  model-backed critic. A critic whose evidence is near-random gets worse, not
+  better, when something more expensive is built on top of it.
+  Measured both finders against fixture populations before promoting either,
+  and recorded the rates beside the rules, because the ticket makes retirement
+  conditional on the measurement and a rule promoted without one is exactly
+  what the governance section forbids.
+  Extracted the sort so the ranking test compares through production's own
+  comparator. A test with its own copy of the ordering proves the copy.
+  Capped the instruction per kind and disclosed the omission rather than
+  truncating quietly, so a reader can tell a short list from a complete one.
+  Did not attempt a model-backed critic. The honest path needs the budgeted
+  request path; the dishonest one is a new unauthorised provider call
+Files-or-schemas: agent_adversarial_review.go, new
+  pipe095_risk_selected_checks.go, six new test files
+Validation: Eighteen targeted tests; the engine canary at 5.03s and 5.12s
+Failures-or-discarded-approaches: None
+Known-limitations: Findings do not survive an attempt. Risk selection is built
+  and not called
+Next-safe-step: PIPE-098 and PIPE-095's call site, both one edit in
+  agent_execution.go, once that file is free
+
 Dev-Log: DL-20260802-100
 Date: 2026-08-02
 Status: Three closed, three open on a caller gap, one out of scope

TODOS.md

@@ -4811,17 +4811,33 @@ Depends on: `PIPE-001` through `PIPE-004`. A critic whose findings are recorded
 - [ ] `PIPE-093a BLOCKER` Make the critic a model request over that packet and parse its answer into typed findings, so it challenges requirement interpretation and unsupported guarantees as §21 defines it instead of reporting what the source looks like.
 - [ ] `PIPE-094` Keep the five static finders as evidence handed to the critic rather than as the critic, so §21's judgement and the mechanical checks stop being the same thing.
 - [ ] `PIPE-095` Select the critic's checks from task risk, changed obligation categories, effect types, dependency changes, and security classification as §21 requires, and read the `scope.riskLevel` the run already resolves and never uses.
-- [ ] `PIPE-096 BLOCKER` Raise every finding as a proof obligation with a stable identity in §9's sense, so a review can be discharged, regressed, or reopened rather than expiring with the attempt that produced it.
-- [ ] `PIPE-097` Attach guarantee provenance to each finding as §10 requires: evidence level, dependency binding, and lineage, so a later run can re-derive or invalidate it transitively.
+  - **Capability built, call site not changed, so this stays open.** `pipe095_risk_selected_checks.go` adds `selectAdversarialChecks(domain.RiskLevel)` and `reviewAdversariallyForRisk`, gating the one costly finder — mutation analysis, roughly twelve whole-suite runs — off for `RiskLevelRoutine` and on otherwise. `reviewAdversarially`'s existing signature and behaviour are untouched, so no current caller changes.
+  - The remaining edit is one line in `agent_execution.go`, owned by another lane this session: `reviewAdversarially(ctx, scope.worktree)` becomes `reviewAdversariallyForRisk(ctx, scope.worktree, scope.riskLevel)`. It is named in the new file's own doc comment for whoever owns that file next.
+  - **Also left out, and worth not overclaiming:** selection by changed obligation categories, effect types, dependency changes and security classification. Nothing upstream computes or passes that data to this review, so only the risk half of the ticket is addressable today.
+- [x] `PIPE-096 BLOCKER` Raise every finding as a proof obligation with a stable identity in §9's sense, so a review can be discharged, regressed, or reopened rather than expiring with the attempt that produced it.
+  - **Closed for identity. `adversarialFinding` gained `ID`, and `findingObligationID` derives it from Kind + Where + What via SHA-256**, so the same finding in two runs carries the same identity and two different findings do not collide. Tests prove stability, distinctness, and that every finding emerging from a real review call carries the field.
+  - **Left undone, explicitly: a finding still does not outlive its attempt.** The ledger and evidence-bundle wiring that would persist one is in `agent_execution.go`, owned by another lane. That is `PIPE-098`, and until it lands a stable identity is a property nothing yet uses.
+- [x] `PIPE-097` Attach guarantee provenance to each finding as §10 requires: evidence level, dependency binding, and lineage, so a later run can re-derive or invalidate it transitively.
+  - **Closed.** Every finding carries `EvidenceLevel` and `Lineage`, tagged at each producer rather than stamped uniformly at the end, so the provenance describes where the finding actually came from. Coverage is asserted across a real review call rather than a synthetic list.
 - [ ] `PIPE-098 BLOCKER` Record findings and their disposition in the pipeline ledger and the evidence bundle, so §22's assurance report reflects what the critic found and whether the next attempt fixed it; today the only consumers are one prompt string and one chat message.
 - [ ] `PIPE-099 BLOCKER` Route the review's send-back through `sendBack` so §21's progress monitor sees it; a run stuck on review findings currently neither escalates up the model ladder nor decomposes.
 - [ ] `PIPE-100` Mark the review round as spent only when findings actually sent work back, so the single round is not consumed by an attempt-one review that errored or found nothing and never sees the code that ships.
 - [ ] `PIPE-101` Establish reviewer independence as §31 defines lineage independence: the critic works from the specification and the diff rather than from the criteria the author derived, and its findings are recorded as an independent lineage. Independence includes retrieval: advisory material shown to this run's implementer is withheld from this run's critic, and where it cannot be, the finding records `influenced_by` and is excluded from confirming that pattern or its ancestors.
 - [ ] `PIPE-102` Remove `AdversarialReview` as a switch that deletes the reviewer; §22 forbids trading away a required reviewer for lower cost, so the setting may scale the critic's depth but not its existence.
-- [ ] `PIPE-103` Repair the swallowed-error finder against `PIPE-105`'s measurement: it examines only pure functions, resolves no standard-library call, and infers a swallow from the caller returning no error, so it reports correctly handled failures as defects. Retire it if the measured false-positive rate stays above the threshold `PIPE-105` records.
-- [ ] `PIPE-104` Repair the boundary finder against `PIPE-105`'s measurement by reading literal arguments from the test syntax tree: substring matching over concatenated source makes `"0"` match a file mode and `"Error"` match `t.Errorf`, so one finding is unreachable and the rest are near-random. Retire it if the measured rate stays above the threshold.
-- [ ] `PIPE-105 TEST` Measure the critic's false-positive rate before either finder is promoted, as §31's mechanical-rule governance requires, and record the measurement with the rule.
-- [ ] `PIPE-106` Rank findings by expected defect cost as §22 orders work, and bound how many reach the instruction, so the review does not compete with the code context under the loop's byte limits.
+- [x] `PIPE-103` Repair the swallowed-error finder against `PIPE-105`'s measurement: it examines only pure functions, resolves no standard-library call, and infers a swallow from the caller returning no error, so it reports correctly handled failures as defects. Retire it if the measured false-positive rate stays above the threshold `PIPE-105` records.
+  - **Closed, and this was a real defect rather than a refinement.** `findUnhandledFailures` skipped every impure function outright (`!function.Pure`) and could only see a swallow of *another produced function's* error, because `canFail` was built from local functions alone — which is exactly the plan's complaint. The purity exclusion is dropped and `knownFallibleStdlibCalls` (a curated set of qualified standard-library calls matched against `producedFunction.Effects`) lets it see the real ones.
+  - Discriminating: `TestPIPE103_AnImpureFunctionThatSwallowsAStdlibErrorIsFlagged` fails against the old code because impure functions were never examined. Two guards against over-widening: a function returning its own error is not flagged, and an effect outside the curated list is not flagged.
+- [x] `PIPE-104` Repair the boundary finder against `PIPE-105`'s measurement by reading literal arguments from the test syntax tree: substring matching over concatenated source makes `"0"` match a file mode and `"Error"` match `t.Errorf`, so one finding is unreachable and the rest are near-random. Retire it if the measured rate stays above the threshold.
+  - **Closed, and both named bugs are reproduced before being fixed.** The finder did raw substring search over concatenated test source, so `"0"` matched inside a file mode `0644` and `"Error"` matched inside `t.Errorf`. `literalTokenPresent`/`canonicalLiteralToken` now parse each test file's AST and match only real `BasicLit`, `nil` identifier, empty composite-literal and unary-minus nodes; `testFileAssertsOnError` matches only an exact `.Error()` call or a nil comparison against something actually naming an error.
+  - `TestPIPE104_AFileModeLiteralDoesNotSuppressTheZeroEdgeFinding` and `TestPIPE104_TErrorfDoesNotSuppressTheErrorAssertionFinding` are the two named bugs as fixtures. Two regression controls confirm genuine edges and assertions still suppress correctly.
+- [x] `PIPE-105 TEST` Measure the critic's false-positive rate before either finder is promoted, as §31's mechanical-rule governance requires, and record the measurement with the rule.
+  - **Closed at the scope reachable here, with the limit written into the file rather than left to be discovered.** A six-fixture population for the swallowed-error finder — 0 of 4 false positives on clean code, 2 of 2 true positives on defective code — and a two-fixture population for the boundary finder at 0 of 2. Rates are logged and recorded in the test file's own doc comment beside the rules they measure, which is what §31's governance asks for.
+  - **Stated limit:** this is not a persisted, cross-run metric. That needs storage and the promotion gate `MEM-015` describes, neither of which this lane owned. Both finders measured clean, so neither is retired.
+- [x] `PIPE-106` Rank findings by expected defect cost as §22 orders work, and bound how many reach the instruction, so the review does not compete with the code context under the loop's byte limits.
+  - **Closed.** `findingCostRank` orders mutation-survivor above swallowed-error above anti-pattern above synthesised-case above boundary, applied through a shared `sortAdversarialFindings` extracted so the test ranks a synthetic list by exactly the production comparison rather than a copy of it. `adversarialInstruction` caps at eight findings per kind and **discloses the omitted count** rather than silently truncating.
+  - Tests prove rank beats alphabetical order, and that the cap and its disclosure both fire.
+- **The rest of this section is blocked on one file, not on difficulty.** `PIPE-098` (findings into the ledger and evidence bundle), `PIPE-099` (route through `sendBack`), `PIPE-100` (spend the round only when findings sent work back), `PIPE-102` (remove `AdversarialReview` as a bypass switch) and `PIPE-095`'s call site all need edits inside `agent_execution.go` around the `reviewed` flag and the `sendBack` closure. `PIPE-107` needs `agent_stage_runner.go`'s second `checkMutations` call site and `PIPE-108` needs new fields on `AgentExecution`. None were touched.
+- **`PIPE-093`/`PIPE-093a` (a model-backed critic) and `PIPE-094` were not attempted, for a stated reason:** making a real model request from this review needs the budgeted and authorised request path, and building an unbudgeted, unauthorised one to get there would be inventing exactly the kind of credential path this session has twice refused to invent. `PIPE-101` (reviewer independence) is moot until a model-backed critic exists — there is nothing yet for the static reviewer to be independent *from*.
 - [ ] `PIPE-107` Share one mutation result between the critic and the ledger; the two independent `checkMutations` calls are roughly twenty-four whole-suite executions per run and §25's honest cost display cannot attribute them.
 - [ ] `PIPE-108` Attribute the critic's cost separately in the run's cost summary and in §26's benchmark timing, so the price of reviewing is visible next to the price of building.
 - [ ] `PIPE-109` Read the exit code in the runtime probe and distinguish a legible refusal from a silent one; §12's effect discipline is about failure being visible, and the probe currently examines only timeout, the literal `panic:`, and silence.