Repository navigation
Rework Strict review for the official model: deep passes, per-comment verification, self-proving evidence - #71
Merged
Merged
Conversation
Collaborator
Author
|
Wave-2 holdout measured post-merge-base: ARM 14 — Opus 5 16 on 30 blind verdicts over 10 never-seen PRs; arm precision 0.748 vs 0.672 (above baseline), recall 0.492 vs 0.651. Details and disclosures in #72. Cumulative fresh-data verdicts for v13: 29—31 at ~1/3 cost. 🤖 Generated with Claude Code |
Contributor
|
dsv4pro-0831已经撤回发布,不如预期 |
… verification, self-proving evidence Rebuilds the Strict PR-review pipeline around the official deepseek-v4-pro (RFC: doc/RFC-strict-review-deep-engine.md; campaign report: doc/EVAL-goal-strict-vs-opus5.md). Measured on the 20-PR campaign splits vs the recorded Claude Code + Opus 5 baseline (blind gpt-5.6 judge, 3 replicates): val 8-7 (first head-to-head win; was 1-4 pre-campaign), one-shot frozen test 7-8 - combined parity at ~1/3 the cost. Engine (all behind default-on settings with kill switches): - Hybrid pass set: deep investigator + adversary (32-iter budgets, budget discipline) alongside the behavior/verification breadth lenses. - Per-comment agentic verify pass: refuted drops, unverifiable demotes, confirmed may tighten; fail-open; traced skip reasons. - Self-proving evidence: every comment quotes the decisive code verbatim with file:line (drafting contract + verifier + renderer) - the single biggest measured lever (4-11 -> 8-7). - Coverage promotion mines lens findings/blockers for uncovered protected-class concerns; corroboration-ranked 8-comment budget with rich-only overflow. Recall mechanics: value_flips planner signal (default flips never rule light), verification lens in the planner fallback, evidence caps 120k->260k / reducer 60k->280k, commit timeline + changed-symbol consumer sweep + hunk-location paging targets in the evidence pack. Knowledge: strict-review-checklist page injected via new manifest key knowledge.review_checklist (linter extended); vllm-omni module map 8->17 with platform/diffusion/distributed risk-tiered. Also includes the previously-untracked review-comment anchoring module (anchor.py + tests) this pipeline depends on, ARM_JOBS parallelism for eval sweeps, and the v13 arm artifacts + judgments backing the report. Note: config.py, manifest.yaml and run_copilot_arm.py carried small pre-existing in-flight local edits that ship here as-is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arm, wave-2 holdout wiring - run_campaign_pipelined.py: overlap generation and blind judging per item (ARM_JOBS / JUDGE_ITEM_JOBS parallelism, resumable, judge retries) - run_cursor_arm.py: headless cursor-agent arm on the pinned PR-time tree; detective leakage audit over the full stream-json tool log (no tool allowlist exists), gh auth stripped, items with violations disqualified - judge_val.py: holdout split (wave-2, 10 fresh PRs, human-only GT) - run_copilot_arm.py / run_baseline_pinned.py / build_pr_snapshots.py: wave-2 items, ARM_JOBS concurrency, commit timeline in frozen snapshots (information parity across arms) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…parison results
Arms: DS v4-pro v13 (r1+r2), MiMo-v2.5 v13, cursor Composer-2.5 and
Grok-4.5 harness arms (with tool-log audits; Grok 9/10 contaminated),
plus the v7-v13 campaign iteration arms. Judgments: GPT-5.6 and
claude-sonnet-5 sets, never pooled. results/: aggregator +
model_comparison.{md,json,csv} computed from raw verdicts.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ness
LLM_MIXTURE members accept {"model": ..., "provider": "cursor"}: the
lens is delegated to that vendor CLI via run_harness_step (same bridge +
audit path as a harness backend), independent of the run's STRICT_BACKEND.
No BudgetedLLM wrap — a subscription harness has no per-token spend for
the USD cap to govern; sessions stay bounded by strict_backend_timeout_s.
Unknown/unshipped provider ids degrade to member-skipped at resolve time.
- moa.Member.provider + label model@provider
- registry.transport_for_id (explicit id, factored out of transport_for)
- run_harness_step(provider_id=, model=) pins transport and model
- ensemble._lens_overrides routes harness members via harness_member
- runner.run_agent_step(harness_member=) takes the harness branch
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cursor-agent emits worktree-relative paths for in-tree reads/greps; the audit realpathed them against the auditing process's own cwd, flagging every relative in-tree access as out-of-roots (live finding: 5 of 10 items in the composer cursor-backend arm carried false violations). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…olidated results Wave-2 holdout arms running the v13 Strict pipeline through the new cursor-agent backend (STRICT_BACKEND=cursor, MCP tool bridge), blind claude-sonnet-5 judging vs the CC+Opus 5 baseline: - Composer-2.5: 4-25 (1 tie) R .32/.43 P .73/.78 - Grok-4.5: 5-24 (1 tie) R .35/.50 P .78/.81 - Grok-4.6: r1/r2 tainted by the grok skill-leak (kept for the ledger, INVALID_/QUARANTINE_ dirs); the clean canonical r3 runs in a peer session - MoA r1 (composer+grok4.6+mimo on the DS spine, MOA_WHEN=always): 5-24 (1 tie) R .29/.42 P .80/.79 — precision fine, worst recall of any arm; mixing weaker proposers dilutes coverage Conclusion: no cursor-served configuration beats the baseline; every arm lands inside DS's replicate band and strict-credit recall stays the universal gap. Contamination ledger in results/model_comparison.md documents the three-location grok skill-hunting behavior, the countermeasure history (prompt ban and dot-renames defeated; only 000-perm off-path quarantine held), and per-arm audit classifications. Harness: ARM_TIMEOUT_S override for slow harness-backend items; run_moa_mixture_env.py composes the MoA member env without printing keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The baseline's raw column wobbles across judgment sets (independent judge calls, pairwise context, item subsets), so cross-row comparison now uses within-set ratios (1.00 = parity) plus tie-adjusted win share. Normalized, the story is unambiguous: recall index 0.55-0.75 for every arm, precision index 0.91-1.05 — the raw wobble was judging noise, the recall gap is not. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Peer audit caught the prose tables lagging the computed JSON: Grok-4.6 r3 interim is 2-16 (not 0-18) and MoA r2 is FINAL at 5-25 over 30/30 (not 5-22 partial). The JSON/CSV were already correct — only the hand-written markdown rows raced the live judgment dirs. Table stamped INTERIM until the peer session's 4-item r3 backfill lands; one final regeneration follows its ping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…haeology tools Teacher-trace distillation (Claude Fable 5 on the train split) + wave-2/3 forensics, per doc/RFC-review-recall-v14.md: checklist duties 13-23 (claim ledger, sibling contrast, PR-numbers falsification, merge audit, producer census, test/gate epistemics + CI economy, differential parity); docs claims-audit/IA pass replacing code lenses on docs-heavy diffs; read-only archaeology tools (diff_stat/file_at_base/show_commit/search_history/calc, bridged to harness backends); coverage-driven second round seeded per-hunk (deep-engine RFC open q3) with uncapped per-file diff slices; ranked verification ledger with [resolved]/[claim-*] channels; ledger-residue promotion; verify pass extended to light depth. Machinery: agent-loop retries on empty/token-ceiling-cut finals; coercion repair head+tail window at full llm_max_tokens; reducer candidates-only retry + failure-path near-dup collapse; promotion text-similarity dedup; gray-zone planner max_tokens 400->8000 (reasoning model consumed the old cap invisibly - silently broken on every gray item across two campaigns); evidence caps 260k->340k / reducer 280k->360k. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
build_wave3/4.py reuse the audited wave-2 machinery (seeded stratified draws; exclusions cover prior waves + the 6 never-drawn extras whose GT was materialized during the wave-2 build). Splits wired through every runner, the pipelined orchestrator, snapshots, and judge_val. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wave-3 gate attempts 1-2 (disclosed; att.1 invalidated by machinery, wave-3 then spent for forensics), wave-4 clean gate (v15-DS 10-20, r .261/.335 p .816/.787 - precision above baseline on a fresh holdout), cursor-backend Composer row (4-24-2, contamination-clean), Fable-5 teacher arm on train, v14/v15 smokes, train probe. DS 402-outage replicate quarantined (INVALID_apierror_goal_v15r2_holdout4_sonnet). Wave-4 baseline pr5720/pr5864 carry the benign self-read audit flag, disclosed in their cost.json. Consolidated table: results/model_comparison.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
review_promotion_model (with review_planner_model, owner direction: both ride v4-flash; precision roles stay on the tier model) and review_lens_backends — a pass-name -> "provider:model" map routing individual review passes through a harness provider via the existing MoA harness-member seam (per-lens tier-model fallback preserved). Enables the measured Composer-recall / DS-precision complementarity as one arm: breadth passes (investigator/docs/round2) on the subscription CLI, adversary/verify/reducer on the precision model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
git-apply of f625192 (providers/claude-code-codex-backends) onto the v15 tree: claude-code transport (built-ins denied, bridge-only — fully preventive), codex transport (read-only sandbox), sanitized_env in base, bridge knowledge tools (doc_search/doc_read/repo_map via the runtime factories). Conflict resolved in tool_bridge.py: the M2 knowledge-tools registration and the v15 archaeology-tools registration now coexist. MoA member test updated: a claude-code member resolves post-M2. Enables Fable in the recall seats via review_lens_backends (adversary/round2 -> claude-code:claude-fable-5). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
v15 r2 (flash cheap seats) 11-19 replicates r1 — precision above the baseline in both independent replicates. Role-split (Composer breadth seats) train probe: no gain. v16 (Fable adversary/round-2 via the claude-code backend) train probe 9-11 with BOTH means above; wave-4 gate 6-23-1: best fresh-split recall (.336, ratio .81) at precision parity — train promise did not transfer; measured baseline judge drift across sets (opus r .335/.338/.416 on identical mds) now bounds single-split claims. Pooled wave-4 DS-core: 27-62-1, p .805/.790, r .293/.363. Consolidated in results/model_comparison.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
paired_analysis.py: the correct test for this design — pair the two candidates inside each verdict (one judge call scores both, cancelling its leniency) and cluster replicates by item. Applying it WITHDRAWS the "precision above baseline" claim: pooled wave-4 DS-core Δrecall -.070 [-.161,+.021], Δprecision +.015 [-.025,+.055] — both span zero. The honest wave-4 result is parity within measurement precision at ~1/3 cost. Cross-set raw-mean comparison was invalid: the baseline's own recall mean read .335/.338/.416 on identical mds (±.08 judge drift). Wave 5 drawn and frozen (build_wave5.py, holdout5 wired everywhere) as a power extension — ~32 items are needed to resolve .07 at the observed sd, so wave4+wave5 = 20 tightens the bound. goal-eval/PREREG-wave5.md fixes arm, items, judge, analysis and the decision rule BEFORE any wave-5 number exists, including "CI contains 0 -> report the bound, do not run a third wave hunting a favorable draw". Results table and RFC corrected; correction posted to issue #72. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
doc/EVAL-v14-v16-recall-campaign.md: the honest close-out. Goal (both means strictly above CC+Opus 5) NOT demonstrated; what the data supports is parity within measurement precision at ~1/3 cost, a concentrated rather than diffuse recall deficit, and configuration effects that replicate in sign (v15 precision-leaning, v16 recall-leaning and the only config to reproduce the GOLD-gap catch in-pipeline). The measurement lesson is recorded as the campaign's most transferable output: pair inside the verdict, cluster replicates by item, and check power before spending a holdout — wave-4's 10-item gates could never have resolved the .07 effect three configs were submitted against. .env.template now documents both supported configurations with their measured tradeoffs, and the planner's reasoning-headroom trap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Primary analysis per goal-eval/PREREG-wave5.md, v16 pooled over waves 4+5 (20 items, 60 verdicts): Δrecall -.049 [-.097,-.001] — CI excludes zero, so the pre-registered rule "the deficit is real; stop claiming parity" fires. Δprecision -.000 [-.041,+.040] = parity. Marginal result (upper bound -.001): a real but SMALL deficit, and concentrated — 6 of 20 items favour the arm, one item (pr5978, -.37) supplies ~a third of the gap. Per the pre-registration, no third wave was run. Campaign close-out: the arm matches CC+Opus 5 on precision and trails on recall by ~5 rubric points at ~1/3 the cost. Goal (both means strictly above) NOT achieved. Wave-5 baseline pr5871 was lost to the arm/baseline worktree race and regenerated serially; two items carried the benign self-read audit flag and were regenerated once per the pre-registration's failure rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A Fable-5 quota exhaustion made every routed harness session fail at the
transport and return a contract-shaped `status: blocked` with zero tokens.
Three defects turned that into a mislabelled measurement:
- the ensemble's zero-yield retry dropped `**overrides`, silently moving a
routed seat onto the default backend (an arm labelled "Fable in the
adversary seat" measured DeepSeek there);
- the second round has no retry of its own, so its routed seat failing
deleted the whole coverage pass invisibly — 16 of 20 holdout items
reported "8 uncovered hunks, 0 added";
- nothing checked, at arm level, that the configured seats ran at all.
Fixes: retry keeps the route and traces a capability_gap; the second round
falls back to the run's own backend with disclosure; run_copilot_arm gains
a routing gate that fails the sweep when a REVIEW_LENS_BACKENDS seat
produced no work on any item (verified: it flags all 20 wave-5 failures).
outcome_blocked() distinguishes a dead seat from a quiet one.
Prompt duties from the wave-4/5 class forensics: 22a name the VICTIM of
every guard/fallback ("unreachable, therefore harmless" is the measured
way this review talks itself out of a real defect), 22b names/taxonomy/
placement (the LARGEST class of real maintainer comments; the arm filed
~2% where GT is ~47%), 22c verify off-fixture (a claim checked against the
PR's own demo/test/benchmark is unverified; a false [resolved] closes the
thread a reviewer would have opened). Unverifiable suspicions may now be
filed as an explicit QUESTION comment instead of being suppressed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wave-4/5 class forensics (3 agents, 20 items) overturned the working theory. The missing concern classes are ALREADY in the prompts and the duties DO fire — what fails is conversion from internal reasoning to a scored comment. Four measured leaks, four fixes: 1. `[resolved]` lines render into the unscored "Validated" block, and GT on merged/amended heads is ~70% "the fix landed, what does it still not cover?". _promote_resolved_residuals now re-files every [resolved] line that states a residual as a comment (grounded by construction — it re-files a line the run already verified; outside the coverage cap). 2. A pass that burns its completion ceiling returns no contract-conformant final, and BOTH ensemble retries required a truthy output — so the lens was dropped silently. Measured: 5 whole passes and ~2.1M input tokens discarded on one holdout, 3 of them on its worst item. The retry now fires on empty output too, with a fit-your-final instruction. 3. The comment contract had no field for a patch, so the renderer could not emit one: the baseline ships dozens of code suggestions per review and was scored more actionable on every measured item. Added `suggestion` plus a render path, and a severity-is-a-decision rule (53 of 66 shipped comments were `minor`, which collapses the budget's severity key and hands the cut to corroboration — structurally favouring mechanically convergent findings over single-lens judgement ones). 4. The findings ledger ranked but never deduped, so parallel passes shipped one fact three times; judges docked precision for it in five rationales. Also: overflow bullets clipped mid-word (one ended at "replaces `u") — now word-boundary clipping with a visible marker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
val alone cannot answer the question (three of its five items scored an exact 0.000 paired delta at v13, item sd 0.171 vs train's 0.041), so train is added and disclosed here rather than after seeing the val number. Fable quota is still exhausted, so the probe arm is core-only by construction and is labelled that way instead of being routed into dead seats again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pose Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
val 4 items -.047 [-.108,+.013]; train 10 items -.052 [-.130,+.026]; pooled 14 items -.050 [-.104,+.003] recall, +.005 precision. That is the same number the pre-registered holdout produced (-.049) from a third independent draw, so 'the deficit is unchanged at about five rubric points' is now the reading supported by wave 4+5, val and train alike. No CI here excludes zero; nothing is resolved, and both splits were already spent, so this is a probe and is labelled one. The train row is confounded on purpose and says so: v17 gained the conversion fixes AND lost the two Fable seats v16 had (quota still exhausted), so it measures core-only+fixes against core+Fable. What the probe bought, which is the point of running it: * A DeepSeek 400 -- thinking blocks not passed back -- hit 8 of 15 items 14 times. Most absorbed it via retries, losing whole passes silently; pr4825 hit it four times and died. Quarantined and reported MISSING per the probe's own decision rule rather than scored as a zero. * run_campaign_pipelined.py judged that crashed stub and printed 'complete: 5/5 ok'. A blocked artifact must never reach the judge. * The cap-8 comment budget binds on exactly the items we lose: four items hit the cap and spilled into an appendix the judge called truncated, averaging -.100 against -.031 for the other ten. Same conversion-failure class as the [resolved] block, different valve. * The dedupe gap cost precision on both splits, exactly as predicted in the probe doc and deliberately left unfixed to keep the probe honest. * suggestion blocks cut both ways: they won replicates on actionability and lost two of three on pr4810, where a major finding shipped with a patch that inverts the guard. They also render broken indentation. Eval runs also mutated adapters/vllm_omni/skills/_candidates.json; that is product state, not eval output, and is reverted rather than committed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MOA_WHEN was set to off on every arm from v13 to v16 and was omitted when I launched v17, so moa_when fell through to its code default of 'full' and dispatched round-1 lenses to mimo-v2.5 and qwen3.6-plus. qwen was 403 Unpurchased on all 24 attempts; mimo wrote 24% of the productive round-1 lenses. The existing judgment sets therefore do not measure the arm they are named for. They are kept and relabelled rather than deleted, and the corrected run is pre-registered here before it is generated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A reader who opens arms/copilot_v17_val without reading the campaign doc would otherwise believe it holds a DeepSeek core-only arm. It does not: 24% of its productive round-1 lenses were written by mimo-v2.5. Each affected directory now says so on its own, since directories get read out of context. Names left alone deliberately -- every verdict's _roles block records the arm directory name, so renaming would point existing provenance at a directory holding different data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
I reported that 24% of productive round-1 lenses were written by mimo-v2.5. That was inferred from seat names, not measured, and it is wrong by ~2.6x. Attributing by the model recorded on each agent_dispatch that produced usable output: deepseek-v4-pro 155 (89.1%), mimo-v2.5 16 (9.2%), qwen3.6-plus 3 (1.7%). DeepSeek still wrote ~90% of the work. Both member figures are upper bounds. BudgetedLLM.create reserves an estimated cost per LLM call and, on a refused reservation, reruns that call on the tier model while the lens keeps its original dispatch label; with moa_max_usd=1.50 against per-item spend of 0.85-1.66 that budget trips partway through most items. Only the dispatch carries a model and the per-call fallback emits none, so the trace cannot say how much lower the true member share is -- a second provenance defect of the same family as the manifest recording env strings instead of resolved settings. The correction shrinks the contamination; it does not remove it. A ~10% vendor substitution concentrated in two round-1 lenses is still not the pre-registered arm, and the 104 wasted minutes distort every latency number, so the corrected run stays warranted and is still in flight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… items The MoA-off re-run reached 7 clean items before the account emptied. All seven were verified MoA-free by the contamination gate armed at launch, so the fix itself is confirmed working -- moa_dispatch appears in none of them. Three items died as rc=3 stubs (pr4816, pr4926, pr4977) and two verdicts were scored against the pr4977 stub before the judges were stopped. Both are quarantined; the stub .md files are moved out of the arm directories so a resumed sweep regenerates those items rather than skipping them, since the runner treats any existing non-empty .md as done. The 7 surviving items are NOT a partial result. The 8 missing ones are whichever the sweep had not yet reached, which is not a random subset, so no delta may be computed until the run completes. Incidental confirmation: pr4825, which died twice in the contaminated run, completed normally here. Its fatal 400 came through the unguarded /tier fallback call in _one_lens -- a path that only exists when MoA is on. Second 402 of the campaign; the first invalidated wave-4 replicate 2 on 2026-08-15. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Seek does not Same code (ae1b6f1), same 5 val items, same sonnet judge, same CC+Opus 5 baseline, MOA_WHEN=off verified on both. api / deepseek-v4-pro Drecall -0.059 [-0.202,+0.085] Dprec -0.057 win .20 cursor / composer-2.5 Drecall -0.003 [-0.222,+0.215] Dprec -0.040 win .40 -0.003 is the closest to parity any arm has reached, but n=5 gives it a +-0.22 interval and the two CIs overlap almost entirely, so this does NOT establish Composer above DeepSeek. What it does do is invert the v15 ordering, where Composer trailed DeepSeek (-.081 vs -.075) on the same comparison. The conversion fixes are the only thing that changed. Per-item, the two arms fail together and win apart: pr4893 sinks both by about the same amount (-0.250 vs -0.267), so that item is a PIPELINE failure rather than a model one, and it carries val's entire deficit. Composer's edge comes almost wholly from pr4825 (+0.217 vs +0.033). Composer is 2-4x faster (350-680s/item vs 1000-2900s) and rides the subscription, but every item logged tok_out=0 -- harness sessions do not surface tokens to span accounting -- so cheaper is assumed, not measured. Contamination: all three imreview copies vaulted to /data/zhoutaichang/.skill-vault-v17cb (RESTORE.md inside) before the cursor run, per the 2026-08-14 skill-leak ledger. Post-run sweep over packed traces including IO blobs found 0 imreview references. The one /skills/ hit is a URL to vllm-omni's own .claude/skills/precheck-pr/SKILL.md -- repo content under review, not our methodology skill. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same 10 train items, same code, same judge: api / deepseek-v4-pro Drecall +0.024 [-0.030,+0.077] Dprec +0.016 win .47 cursor / composer-2.5 Drecall -0.052 [-0.165,+0.061] Dprec -0.097 win .20 Val had put Composer ahead (-0.003 vs -0.059) and I reported it as inverting the v15 ordering. Train says otherwise, and train is the split that can resolve it: item sd .075 for api against .158 for cursor, on 10 items rather than 5. The val row was a small-sample mirage and the caveat I attached to it deserved more weight than I gave it. Composer's spread is the story: pr4970 +0.317 against pr4923 -0.233, while the api arm stays within +-0.06 on seven of ten items. Same conclusion the campaign keeps reaching -- every alternative to the plain DeepSeek core (Composer, grok, MoA, Fable routing) has cost something and none has bought recall. Contamination clean: 0 imreview references across packed traces including IO blobs. All three skill copies were vaulted off-$HOME before both cursor runs and are now restored and verified readable; the vault is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pr4893 costs both backends about a quarter of a recall point (-0.250 api,
-0.267 cursor) and carries val's entire deficit. The arm FINDS the
ground-truth concern on nearly every replicate and loses anyway, because of
how the finding is presented. Four defects, measured across 26 v17 artifacts:
* 25 broken `?:NNN` anchors. The overflow appendix inlined
`c.get('line', c.get('_declared_line','?'))`, which returns None whenever
the key EXISTS holding None -- exactly what anchor resolution does when the
diff index cannot corroborate a position. Comments had the ~declared
fallback; the appendix never got it. Now a shared _anchor().
* 31 mid-sentence truncations. The appendix clipped at 320 chars, and on
pr4893 the ground-truth match landed there and was cut mid-claim: "X only
gestures at this in a truncated, cut-off 'additional observations' bullet
ending in [...]". The appendix is a scored surface, not a footnote -- 900
chars now, and _clip ends on a sentence boundary where one exists.
* 18 duplicate findings. Judges docked precision on both backends and both
splits ("~4 near-duplicate restatements", "5+ near-duplicate findings").
Deduped on word-set overlap within a file rather than a prefix, because the
real duplicates diverge only in trailing words -- which any prefix key lets
through, as the first version of this fix demonstrated.
* Unparseable suggestion blocks. A wrong claim welded to an applyable diff is
refutable in a way a hedged one is not, and judges refuted two of ours by
name. Python suggestions must now parse (bare, dedented, or as an indented
fragment) or they ship as prose instead of a patch.
dsh: the bundled runtime compiles in 122 plugins and dsh-mcp-client is not
one of them, so our tool bridge is unreachable and lenses run on the
harness's native bash + str_replace_editor. Naming an absent plugin does not
fail fast -- the runtime dies on load while the SDK waits in initialize,
measured at 30 minutes per lens -- so _assert_plugins_bundled scans the
executable once and raises immediately, naming the plugin. Every session
handed a bridge spec it cannot honour traces a capability_gap, so a dsh arm
can never be described as having run with tools bridged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each of these corresponds to something that actually happened in the last 48 hours and was caught only by hand, hours later. 1. Manifests record RESOLVED settings, not env strings. The v17 manifest said moa_when "(default)" while the code resolved "full", so a 15-item probe ran as a three-vendor mixture and nobody could see it in the artifact. Provenance that records the INPUT cannot detect a default that changed underneath it. Now also records the mixture members, strict backend, lens routing, and the planner/promotion models. 2. Preflight balance check, refusing to start rather than dying halfway. Two 402 Insufficient Balance events in three days (wave-4 replicate 2 on 08-15, v17ds on 08-17) each turned an empty account into rc=3 stubs that the pipeline then judged as zeros, and both needed manual quarantine after the fact. Refuses on is_available=false or under 20 CNY. A probe outage is NOT a gap -- an unreachable endpoint must never block a sweep. 3. Mixture gate. The routed-seat gate only inspected REVIEW_LENS_BACKENDS, so the MoA path walked straight past it. Verified against real arms: flags 7/7 contaminated items on copilot_v17_train and 0 on copilot_v17ds_train. It reports rather than fails, because a deliberate MoA arm is legitimate -- it just has to be visible in the sweep output instead of discoverable only by grepping traces afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The native-tools smoke on pr4816 works: sandbox held (git status clean on the worktree despite 55 editor calls under read-only), tools fire, and two of three lenses completed normally at ~40 steps / ~50 tool calls each in about 11 minutes. The third ran away: 558 steps, 564 tool calls, a 7MB session log, still looping 50 minutes later when the arm's outer timeout killed the whole item. Cause: dsh owns its loop and the bundled runtime exposes no step cap, so AgentSessionRequest.max_iters -- which maps to a native cap where one exists, as with claude --max-turns -- has nothing to bind to. request_timeout_seconds did not save it either; it bounds a request, and a busy loop keeps making progress. minimal also mounts no compaction, so a runaway grows its context the whole way down. Both bounds are now ours: a step ceiling counted off step/end notifications and a hard wall-clock deadline that closes the runtime from a watchdog thread. The ceiling is 6x max_iters rather than 1x because the harness legitimately plans more steps than our in-process loop for the same work -- the two healthy lenses used ~40 against a budget of 14 -- so a tight cap would truncate normal sessions while 6x still stops a runaway an order of magnitude short of what was measured. Also recorded while verifying: session events key off `type`, not `kind` (all 13682 entries carried kind=None). The parser already reads both, so the tolerant version written before the schema was known turns out correct -- but it is now verified rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The doc/ refactor is about to move ~210 files. Before touching any of them, these three measure what is about to break — and what was already broken. check_doc_links: relative links across every .md we own. Skips fenced code blocks and <placeholder> targets, and excludes eval/dataset (generated review artifacts that quote the TARGET repo's doc paths — checking them produced 250+ false positives) and knowledge-templates (whose links resolve only after the file is copied into knowledge/). check_doc_citations: the safety net that did not exist. 29 places in src/, test/ and skills/ cite a doc/ page from a docstring or comment; 17 of them point at one file. No import resolves them, no test touches them, no linter looked. It also parses paths built from pathlib SEGMENTS — ROOT / "docs" / "codex" — because a string scan cannot see those, and exactly one such reference later survived the path sweep and broke a test. check_spec_freshness: SPEC coverage and staleness. It reads a page's declared `verified-against:` date rather than its commit date, because git dates cannot survive the migration: `git mv` rewrites a page's last-commit date to the move, which would make every July-era spec look permanently current. The gate would have been destroyed by the very refactor it exists to guard. First run on the current tree: links and citations clean; SPEC reports 45 pages with no declared verification and 17 modules with no page at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Writing the per-file contracts for providers/ meant reading each transport against what the registry says about it. Two statements did not survive. 1. registry.py declared `mcp_tools` for the deepseek backend. That transport measurably cannot use the tool bridge: its bundled runtime compiles in 122 plugins and @deepseek-ai/dsh-mcp-client is not among them, so sessions run on the harness's native bash, the outcome reports mcp_bridged=False, and a review.mcp_tool_bridge capability_gap is traced on every session handed a bridge spec it could not honour. providers/deepseek.py has said so in its own docstring the whole time. base.ProviderSpec defines capabilities as "the flags the integration can rely on", so this was a false declaration. Nothing branches on it — which is precisely why it survived: an inert wrong fact only ever misleads readers, and it had already misled the first draft of guide/backends.md into telling users their deepseek runs were tool-bridged. 2. tool_bridge.py's module docstring contradicted its own code, stating that repo_map is not reconstructed for harness sessions while lines 270-282 reconstruct it. Corrected, and the real boundary written down: skill/memory retrieval is deliberately NOT bridged, because those tools propose knowledge candidates and a cross-process write path for them was declined. No behaviour change: the flag had no consumers and the docstring is prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doc/ was 204 files: 127 of one-off migration forensics, 49 SPEC pages, and 20
top-level files spanning six genres with no signposting. Alongside it sat docs/
— one letter apart — holding user guides AND two config templates that are not
documentation at all.
The tree now groups by genre, which the filenames already encoded:
guide/{hosts/,samples/} operator-facing; hosts/ is Direct-mode install
features/ the four RFCs, three of them shipped features
architecture/{SPEC/} CODE_TOUR, DESIGN, KNOWLEDGE, per-file contracts
contributing/ style, knowledge extension, release maintenance
evaluation/ the EVAL/RESEARCH reports, frozen
archive/ IMPLEMENTATION_STATUS, dead PLANs, reorg-audit
docs/ is dissolved; codex/config.toml and cursor/mcp.json move to
integrations/config-templates/ because they are templates, not docs.
reorg-audit/ is archived in place, NOT deleted: knowledge/SCHEMA.md and the
rebase workflow page cite it as the provenance baseline of record.
Moving files broke 29 in-code citations and 17 relative links, exactly as
check_doc_citations predicted. Both are fixed here: repo-root citations across
src/, test/, skills/, knowledge/, adapters/ and the workflow, plus per-file
relative-depth fixes inside the five documents that changed directory. One
reference was built from path segments (ROOT / "docs" / "codex") and escaped
the string sweep — it broke test_imcifix_skill and is fixed too.
Content updates ride along where they share file identity with a move:
CODE_TOUR gains the providers/ and tool_bridge sections it never had (it
claimed to cover "all 80 .py files"; there are 92, and the 9 missing were
exactly the v3-era additions), loses the inventory tables that now belong to
GUIDE, and DESIGN gains Part III for the v3 era — Direct mode, the provider
registry, and dual-path mode. The four RFCs gain feature-doc headers stating
status, kill switches and measured result; auto-run is now marked UNIMPLEMENTED
at the top, which it never was.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Includes the five new documents the tree needs to be navigable — doc/README.md
as the index, doc/GUIDE.md as the single overview, guide/backends.md and
guide/hosts/ for the host-vs-backend split, and evaluation/README.md — because
the moved files link to them. Splitting them into a later commit left this one
with dangling links, which is precisely what the linters exist to catch.
The three linters now run on every push. links and citations are plain calls. SPEC runs with --strict, which is only satisfiable as of this branch: all 92 modules have a page, all 62 pages carry a verified-against date, and none is behind its source. Before this refactor the same check reported 44 of 45 pages silently stale and 17 modules with no page at all. Also lands the plan itself, with its completion record — including the four places executing it proved the plan wrong: - the freshness gate could not use git dates, because `git mv` resets them and the migration would have made every July-era page look permanently current; - 62 pages, not 59: nested-underscore and dunder page names were being misclassified as cross-cutting; - only 15 of the 45 existing pages actually needed rewriting — the other 30 were touched by exactly one rename sweep, so instead of rewriting blind, a programmatic public-contract check ran over them. 29 were clean; one had real drift (profiles/establish.md still listed LANGUAGE_SUFFIXES, consolidated into profiles/languages.py). A blind rewrite copies that drift forward; - string scanning cannot see paths built from pathlib segments. The plan documents what was NOT done, too: four guide pages were moved and link-fixed but never verified line-by-line against source, and QUICKSTART and DESIGN remain English in an otherwise Chinese tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… most
Every judge rationale on the four items Composer lost named the same three
faults, and none of them is a model fault -- the pipeline finds the concern
and then buries it:
"buries the same insight under ~10 near-duplicate 'Validated'/'claim-refuted'
log entries and 4 nearly-identical inline comments restating the same
PR-description staleness point, hurting signal density" (pr4977)
"~6 near-verbatim restatements labeled 'resolved', which inflates apparent
coverage without adding new signal and makes the actually-actionable items
hard to find" (pr5009)
"two of its findings cite broken locations ('?:102', '?:33') instead of real
file paths, hurting both precision and actionability" (pr4817)
Replaying the SHIPPED dedupes over the twenty measured artifacts showed why
they never stopped it: both were inert.
* the ledger key was the normalized first 90 chars. Restatements of one fact
open with whichever evidence that lens happened to cite, so the key removed
0 of 138 entries on Composer and 0 of 136 on DeepSeek.
* `_dedupe_comments` scored word overlap >= 0.8 WITHIN A FILE. The commonest
duplicate class anchors one fact differently -- `?:34` and
`flash_attn_hub.py:34` -- so the buckets never met: 0 of 68 removed.
* `_promote_resolved_residuals` deduped promoted residuals against existing
comments but never against EACH OTHER, so five `[resolved]` lines about one
residual promoted four times and saturated its own cap. That is the pr4977
cluster, generated at source.
`_same_finding` replaces both with two sufficient conditions, because the
duplicates arrive in two shapes: near-verbatim claims (word overlap, no subject
needed -- some findings name no identifier at all), or same-subject-same-claim
with different citations (identifiers AND claim words). Rule 2 stays
conjunctive: keying on the subject alone merged pr4977's `:~81` "add a cache
test" into `:~58` "that test sits on no CI path", and the judge credited `:~58`
by name. A false merge deletes a finding and costs recall; a missed merge only
pads the review.
Also: unknown-file findings render as `PR description` or `general` rather than
an invented `?:34` (they are overwhelmingly about the PR's own prose, which has
no file:line anywhere); the ledger cap drops 14 -> 6 (14 saturated on 9 of 10
items in BOTH arms, so it was a quota being filled, not a ceiling); and the
skill-curation queue plus the raw task repr leave the judged deliverable for
DIAGNOSTICS.md -- operator state about the copilot, ~1000 chars on every item,
and a direct violation of report.py's own stated contract.
Replayed over the measured artifacts, ledger 138->59 and 136->58 (57% both),
broken anchors 13->0 and 6->0, artifact bulk -32% / -21%. The asymmetry is the
point: comment-level redundancy is 5x worse on Composer (26% cut vs 5%), which
is precisely what its rationales were docking. Offline replay predicts the
direction of the noise cut, not the final artifact -- the source-level fix
frees budget slots earlier, so a live run promotes different comments.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One lens writes `orchestrator.py`, another writes the full `vllm_omni/engine/orchestrator.py`. As raw strings those share nothing, so `_topic` scored zero overlap and two comments about one subject both shipped. Caught by reading the v19 val output: the same "are these CI reds pre-existing?" question was asked twice at `orchestrator.py:1290`. Replayed over the 168 comments in the three measured v17 arms this merges 1 more comment (0.6%) -- too small to justify regenerating the v19 probe now in flight, and that probe has not been judged, so it is not being kept for its score. Committing the fix for the runs after it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ropping
v19's dedupe traded recall for precision, and both splits agree it was a real
trade, not sampling:
Drecall Dprecision
v17cb train -.052 -> v19cb train -.092 -.097 -> +.060
v17cb val -.003 -> v19cb val -.122 -.040 -> +.104
Precision is fixed and reproducible (+.157 train, +.144 val, above baseline on
both, win share .20 -> .37). The recall cost has a mechanism: redundant
restatements were each an independent attempt to match a ground-truth concern,
and `_dedupe_comments` kept whichever arrived FIRST. That choice is arbitrary
in exactly the dimension the judge scores -- restatements differ in how
precisely they state the causal mechanism, and precision of mechanism is what
earns recall credit:
"Y states the causal mechanism precisely ('older get_kernel doesn't accept
the kwarg, which would have made every fallback attempt fail') matching the
GT reasoning, while X buries the same insight"
So duplicates now MERGE rather than drop: the survivor per finding is the one
that names the most code and says the most about it (evidence included, since
a claim welded to a quoted hunk is the actionable one). Severity still wins
first, so a wordy nit can never displace the blocker it restates. Same rule
applies to the Validated ledger.
Diagnostics that redirected this work, both of which contradicted my prior:
* Routing vs coverage, measured per ground-truth concern: only 5 of 64 train
misses were "found but rendered outside the comments". Burial was never the
main recall lever, so the residual-promotion path is not where the gap is.
* Against a baseline control, the arm ENGAGES MORE ground-truth concerns than
the baseline on train (27 vs 16) while losing judged recall. The deficit is
depth on what it touches, not breadth of what it finds. (First run of that
control scored the winning baseline 0 of 505 because the parser only knew
our own markdown shape -- fixed with a paragraph fallback before drawing any
conclusion from it.)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The remaining gap is recall, and the routing/coverage split says it is not
where I first looked: only 5 of 64 train misses were rendered outside the
comments, and against a baseline control the arm ENGAGES MORE ground-truth
concerns than the baseline (27 vs 16) while losing judged recall. The deficit
is depth and legibility on what it touches.
Reading the baseline artifact for an item it won on recall (.62 vs .38) shows
the shape difference plainly. It titles each finding with the CLAIM ("Seeded
requests still take the N-scalar-calls path -- and the seed is ignored
anyway"), develops the causal chain over paragraphs, and weaves quoted code
into the argument. Ours put the entire finding in one paragraph and appended
its evidence as an `(evidence: ...)` parenthetical that routinely ran past 500
characters. That is the measured complaint, in judges' own words: "makes the
actually-actionable items hard to find". Recall is scored on concerns COVERED,
and a finding the reader cannot pick out does not get credit for being present.
So a finding now renders as headline, location, argument, evidence-as-quote.
The headline is taken from the sentence carrying the concern, searched from the
SECOND sentence on: the reducer contract puts the change the diff makes in the
first, and a tell-word lands there incidentally often enough to matter -- "This
diff centralizes loading ... so each repo loads once" contains "so " and
headlined the change instead of the concern until the search skipped it.
This is a legibility change, not a judge trick: it makes our output look like
what a good reviewer writes, which is what the baseline is.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first v21 generation exposed the flaw before it cost a judge run: 34 of 49 headlines ended on a dangling ";", "," or connector, because they were the first 160 characters of a comment that is one long sentence. A fragment reads worse than the plain paragraph it replaced, so this was a defect being shipped as an improvement. Headlines are no longer clipped, and a comment that has no distinct concern sentence -- one sentence, or no sentence carrying the claim -- renders in the plain `file:line [sev] — text` shape instead of a stub heading. The evidence quote block, which was clean on its own, is unchanged. The lesson is that the claim/argument split cannot be recovered deterministically from prose that was not written with one; getting the baseline's structure reliably needs the reducer to emit a title field, which is a model change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Measured on train, 10 items x 3 replicates against the pinned Opus 5 baseline: v20cb Drecall -.005 [-.176,+.167] Dprecision +.046 win .37 v21cb Drecall -.093 [-.178,-.007]* Dprecision +.079 win .35 The v21 recall interval EXCLUDES zero -- this is not replicate noise, it is the strongest negative signal any arm has produced in the campaign. The hypothesis was that a claim headline plus an evidence quote would make findings easier for a reader to credit, since judges had complained ours were "hard to find". It bought precision (+.046 -> +.079) and lost twice that in recall. Promoting one sentence to a heading and demoting the rest of the argument below the location line evidently fragments the finding rather than clarifying it. Whatever the mechanism, the render layer is not where the remaining recall lives -- consistent with the routing/coverage split, which already showed only 5 of 64 train misses were rendered outside the comments. v20 stands as champion: recall parity (-.005) with precision above baseline (+.046), from -.052/-.097 where Composer started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Render is measurably exhausted as a lever -- v21's restructure lost recall with
a CI excluding zero, and the routing/coverage split had already shown only 5 of
64 train misses were rendered outside the comments. So the remaining recall has
to come from what the lenses and reducer PRODUCE.
Measured on the same ten train items, per finding:
arm 6.0 findings @ 405 chars
baseline 7.8 findings @ 1238 chars
Three times the depth, and reading them side by side the difference is entirely
the causal chain. Ours flags ("no test covers this path"); the reference states
the trigger, the path it propagates through, and what is observed:
"the only test constructing it is CUDA-gated, and the merge-gate job selects
-m 'core_model and cpu', so a reload regression reaches main unseen"
That is also exactly what the rationales reward on the items we lose: "Y states
the causal mechanism precisely ... matching the GT reasoning, while X brushes
this tangentially" and "skews almost entirely toward test-coverage nitpicks".
Recall is scored on ground-truth concerns COVERED, and a flag covers a concern
less than a developed argument does.
The lens contract and the reducer rewrite rule now both require the mechanism:
triggering condition, code path (named and quoted), observed consequence, plus
whether a claimed benefit actually materializes. Both carry an explicit
anti-padding clause -- length must come from the chain and its evidence, never
from restatement, which v19/v20 penalize separately as redundancy.
This is a generation change, not a formatting one; v21 established that
rearranging the same 405 characters does not help.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v22 asked each finding to carry trigger -> code path -> observed consequence. It did what it was told: claims grew 407 -> 500 chars and findings 3.0 -> 4.4 per item, and spot checks show real causal chains, not padding. Judged recall still fell. v20 (champion) drecall -0.005 [-0.176, +0.167] dprecision +0.046 v22 drecall -0.082 [-0.153, -0.011] dprecision +0.040 train, n=10, 3 replicates, judge claude-sonnet-5, vs baselines/claudecode_opus5. v22's recall CI excludes zero; with v21 these are the only two resolved recall intervals in the campaign, and both are negative. Depth was the wrong lever. Distinct source files reasoned about per review: baseline 13.8, v20 12.2, v22 10.8 -- the same order as recall. v22 anchored findings on more files (2.3 -> 3.2) while exploring fewer overall, i.e. it spent lens budget elaborating ground already covered. The deficit is exploration breadth, not per-finding depth. Reverts only the prompts.py hunk of 82d682f; that commit also swept in an unrelated knowledge/ -> doc/knowledge/ tree move and a thin_mcp_server.py change, which are left intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The four round-1 lenses partition the CHECKLIST; none partitions the DIFF. Every lens sees the whole change and self-selects hunks, so coverage is emergent and they converge on the same salient ones. Measured: distinct files reasoned about per review is 12.2 for us against 13.8 for the reference reviewer, and the arm's worst recall items are its largest diffs. Adds a fifth lens that owns coverage and nothing else: enumerate every changed file, one pass each, a finding or an explicit no-issue conclusion, and name what it could not resolve. Deliberately shallow -- v21 (claim headlines) and v22 (per-finding causal chains) are the only two recall intervals in the campaign whose CI excludes zero and both are negative, so depth here would be a regression, not an improvement. Runs at full depth; available to the planner at standard; not added to DEFAULT_STANDARD_LENSES and does not affect light. On train that is 7 of 10 items, including all four recall losers. Sweep candidates compete for the comment budget on normal merit -- no deprioritisation, which would blunt the effect under test. Tests that pinned a reply count to four lenses now derive it from len(_REVIEW_LENSES), matching the pattern already used elsewhere in the file. Pre-registration with falsifiable predictions and a kill criterion in eval/dataset/goal-eval/PROBE-v23-sweep-lens.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lens did its job -- anchored files/item 2.3 -> 3.5, evidence files 6.4 -> 8.8, findings 3.0 -> 5.1, on FEWER tool calls (288 vs 306) and without depth drift (claim chars 441 vs 407; v22 was 511). Judged recall still fell and precision fell with it. v20 (champion) drecall -0.005 dprecision +0.046 win 0.37 v23 drecall -0.093 dprecision +0.024 win 0.27 train, n=10, 3 replicates, judge claude-sonnet-5, vs baselines/claudecode_opus5. That falsifies the breadth hypothesis this probe was built on, the same way v22 falsified the depth one. Predictions P1/P4 passed and P2/P3 both failed, which is the cleanest possible refutation: the mechanism worked and the outcome did not follow. Also records a methodological correction. Tested DIRECTLY against v20, paired within item, none of the three interventions is distinguishable from the champion: v21 vs v20 drecall -0.088 [-0.258, +0.082] v22 vs v20 drecall -0.077 [-0.217, +0.063] v23 vs v20 drecall -0.089 [-0.239, +0.062] Earlier commits leaned on each arm's CI vs the BASELINE excluding zero; that supports "worse than Opus 5" but not "worse than v20", and the two were conflated. The noise floor is the reason: pr4970 runs at light depth -- no lenses -- in both v20 and v23, so neither v22 nor v23 could touch it, yet its drecall moved +0.567 -> +0.317 between runs. A 0.250 swing on an unchanged path exceeds every effect being interpreted at n=10. Reverting anyway: no evidence it helps, both point estimates negative. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4e3b324 stated pr4970 moved "+0.567 -> +0.317" between v20 and v23, a 0.250 swing. Wrong: +0.317 is v17cb's value. paired_analysis prints its per-item block for the LAST row of the table, not the first, and it was read as the first. Actual per-arm pr4970 drecall: v17cb +0.317 | v19cb 0.000 | v20cb +0.567 v21cb -0.017 | v22cb 0.000 | v23cb 0.000 The claim holds and was understated -- v20 +0.567 against 0.000 for both v22 and v23, a 0.567 swing on a light-depth item whose code path neither change touches. Five of six versions score this PR at ~0.000; only v20 does not. That prompted a leave-one-out the campaign should have run much earlier: v20 train drecall -0.005 -> -0.068 without pr4970 v20 val drecall +0.003 -> -0.046 without pr4825 Every other single-item removal moves train by <=0.03. So the champion's recall parity rests on one PR per split, and without it v20 sits level with v19/v22/v23 (-0.09 to -0.10). Precision does not share this: every val leave-one-out stays positive, +0.021 to +0.155. Variance decomposition, same data: re-judging a fixed review 3x gives sd 0.075; re-generating from unchanged code gives sd 0.164 (2.2x). The variance is generation-side, so more judge replicates cannot reduce it -- only multiple generation runs per PR can. Revised reading: the precision gain is the campaign's robust result. The recall parity is a single-item artifact on both spent splits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… claim corrected origin/rfc/strict-review-deep-engine was a single squashed commit (52acaf1) developed in parallel with this branch from the same base b9cfa7f, so neither history contains the other. This branch is being force-pushed over it, which would otherwise drop this file. Five of the six files that existed only on the remote were deliberately removed here by 9647b1b ("Refactor doc/ into a signposted tree"), so they stay removed. This one is different: absent at b9cfa7f, created only by 52acaf1, never deleted by any local commit, and it is the design document PR #71 is built on. Restored from 52acaf1 with one factual fix. The Rollout section claimed all three mechanisms sit behind kill switches while naming only two. config.py carries exactly two booleans (review_verify_comments, review_deep_engine); every other evidence* setting is an int or dict, so self-proving evidence rendering has no switch of its own and cannot be rolled back independently. The text now says so, which also reconciles it with Open Question 2. Caught by the pre-commit reviewer; I first argued provenance should exempt a verbatim carry-forward and that was wrong -- the file is new to this branch, so this branch owns its accuracy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tzhouam
force-pushed
the
rfc/strict-review-deep-engine
branch
from
August 19, 2026 02:01
52acaf1 to
17b77c3
Compare
Two checks fail on this branch, both fallout from relocating the contributing docs out of the knowledge tree. tests / test_intra_knowledge_anchor_links_resolve: six intra-knowledge links still pointed at contributing/*.md relative to knowledge/, which no longer holds them. Repointed at doc/knowledge/contributing/ from CLAUDE.md, general/remote/_index.md, general/debug/guides/retrospective-to-rules.md and skills/claudeception/SKILL.md, plus the two CONTRIBUTING.md entry links in CLAUDE.md, which now resolve to doc/knowledge/CONTRIBUTING.md. vLLM-Omni release drift audit / stale_pin: doc/architecture/KNOWLEDGE.md recorded the afd-plugin mirror SHA a432692e as "main @ <sha>". release_baseline.yaml lists that file as a pin_document and the auditor requires every marker in that form to equal audited_sha (807db6ef), so a SHA belonging to a different repository read as a stale vllm-omni pin. Now a plain backticked SHA with an explicit note that the pin syntax is reserved for vllm-omni. The one real vllm-omni pin is untouched and still matches. CLAUDE.md and doc/architecture/KNOWLEDGE.md are staged surgically from HEAD rather than taken from the working tree, because the working copies carry more than path repair and the pre-commit reviewer was right to block both. The KNOWLEDGE.md working copy is a full translation and restructure that also drops recorded upstream provenance and the authorization/licensing basis. The CLAUDE.md working copy weakens a governance gate: its owner-selection bullet no longer requires recording the content, nearest owner, owner evidence and target directory before writing. Neither belongs in a commit that claims to fix CI, so both stay uncommitted and intact in the working tree for the doc-refactor commit to land on its own terms. Known and deliberately not fixed here: CLAUDE.md still tells the reader to run tools/check_knowledge_tree.py, but that script now lives at knowledge/tools/. It breaks no check and the working-tree rewrite already corrects it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A static SVG comparing every arm against the pinned Claude Code + Opus 5 baseline: the six Composer 2.5 pipeline versions this branch went through (v17cb through v23cb) above, and the other generators measured on the same pipeline family with the same judge (DeepSeek v4-pro, DeepSeek in its own dsh harness, grok-4.5, grok-4.6, MiMo v2.5, Composer standalone) below. The chart draws the resolution limit as a shaded band -- plus/minus 0.099 recall and 0.115 precision, the 95% half-width at n=10 computed from the observed item-level spread -- because most of the differences on it fall inside that band and should not be read as real. Whiskers are 95% CIs, dots are point estimates, paired inside each verdict and clustered by item. Generator committed alongside the asset so the numbers can be re-derived rather than trusted. The SVG uses presentation attributes only, with no script, no style block and no external font reference, so GitHub renders it through its image proxy, and it paints an explicit light background so it stays legible under either GitHub theme. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The knowledge authoring rules moved from knowledge/ to doc/knowledge/ in 82d682f, but the pages were left mid-migration: their internal links still pointed at the old tree, and check_doc_links reported 31 broken targets across nine files. That was the remaining red step on this PR. This is not a link patch and is not titled as one. writing.md is a full rewrite (+197/-325), the templates are substantially reworked, and CONTRIBUTING.md and maintainer-walkthrough.md gain prose alongside repointed links; only DEVELOPMENT.md, README.md, doc/README.md and tree-map.md are pure link repair. All of it already existed in the working tree and is committed unmodified. Scope corrected mid-way, on the pre-commit reviewer's second finding. The first attempt staged only the nine files named in the broken-link report, which sliced the refactor incoherently: templates/README.md newly requires every owner rules.md to open with a Direct code quick-entry producer-consumer chain, but templates/rules.md -- where that skeleton lives and which writing.md names as the authoritative one to copy -- was left at its old revision without the section. Every new rule page would have violated a convention the same commit introduced. doc/knowledge/ now lands as a whole, so the requirement and its skeleton arrive together. One edit of my own, from the reviewer's first finding: templates/repo-index.md advertised rules.md, components/, models/, review/ and ci/ as unconditional rows, while writing.md's directory skeleton creates rules.md only when a repository-level gate actually exists. A new repo copying it verbatim would link files it must not create or fail check_doc_links, and placeholder pages are forbidden. Added a comment marking the rows optional. doc/architecture/ is deliberately NOT included -- that is the separate broader refactor, and doc/architecture/KNOWLEDGE.md remains the surgical pin-only change so its recorded provenance and licensing basis are not dropped without review. check_doc_links reports OK across 290 files; the offline suite is green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…AUDE.md Two leftovers from the knowledge/ to doc/knowledge/ move, both only visible once the authoring pages landed. check_doc_links still excluded doc/knowledge-templates and doc/contributing/knowledge-templates. Templates are deliberately broken in place -- their relative links resolve only after the file is copied into a repo tree -- so with the tree now at doc/knowledge/templates the checker started reporting five of them as real breakage. Exclude path updated to match. knowledge/CLAUDE.md is now taken from the working tree, reversing the surgical version I committed in d7cd6b9. That commit preserved HEAD's owner-selection wording, which requires recording the content, nearest owner, owner evidence and target directory, and linked it to layout.md#写入前-owner-凭证. The refactor deletes that section from layout.md outright -- it survives under no other name -- so the preserved sentence pointed at a heading that no longer exists and test_intra_knowledge_anchor_links_resolve failed on it. The pre-commit reviewer had flagged the working-tree wording as an undeclared policy weakening, and on the evidence then available that was the right call. With layout.md's rewrite committed it reads differently: the requirement was removed from the rules page as part of the refactor, so CLAUDE.md's shorter wording is the coherent counterpart rather than a silent regression. Recording the reversal here because the earlier commit message argues the opposite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous two commits landed doc/knowledge/ and the link checker, and each exposed the next seam: the refactor only passes its own linters as a whole. check_doc_citations, check_spec_freshness --strict, check_knowledge_tree and check_wiki_lint all fail on a partial tree and all pass on the complete one, so this lands the remaining 93 files -- doc/, the knowledge/ pages, skills/, adapters/vllm_omni/manifest.yaml, AGENTS.md and QUICKSTART.md. eval/ is deliberately excluded. The 18 uncommitted entries there are arm artifacts and judgment sets from the measurement campaign, not part of this refactor; they are a separate decision about how much evidence to ship. doc/architecture/KNOWLEDGE.md is also excluded from the wholesale take and stays at the surgical pin-only version. The rewrite deletes the recorded upstream provenance and, with it, the authorization from the knowledge base's author plus the statement that upstream ships no LICENSE file so that recorded authorization IS the licensing basis. That text survives nowhere else in the tree. It cannot be dropped as a side effect of a documentation migration, and the refactor's own new doc/knowledge/README.md still routes readers to this file for exactly that information, so removing it would leave the new navigation pointing at a page that no longer answers the question. Raised twice by the pre-commit reviewer and correct both times. With this, all five doc and knowledge linters pass and the offline suite is green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
check_spec_freshness --strict reported all 62 SPEC pages stale in CI while reporting 0 stale on the same commit locally. The difference is clone depth, not content. The tool dates each source file with git log -1 --format=%ct -- <path>. actions/checkout defaults to a depth-1 shallow clone, where the single commit has no parent, so git reports every tracked file as created in that commit. Every module therefore gets a code date of today, and every page with an earlier verified-against: date reads as behind its source. Modules nothing in this branch touched -- ui, tracing, providers/* -- were flagged alongside the ones it did, which is what gave the mechanism away. The check has been ineffective for as long as the clone has been shallow: it passed only while the tip commit predated the pages' verified-against: dates, and the first commit made on a later day failed all 62 at once. fetch-depth: 0 gives it the history it needs, so it starts comparing real per-file dates instead of one repo-wide timestamp. Not fixed by bumping the 62 dates: that would assert a human re-verified each page against its source today, which did not happen, and it would fail again on the next day's commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
congw729
pushed a commit
to congw729/InferMatrixCopilot
that referenced
this pull request
Aug 24, 2026
main is red on check_spec_freshness --strict: thin_mcp_server verified 2026-08-18, code 2026-08-19. The file was edited on both sides of the InferMatrixLab#71 merge -- f203f0d added the mandatory review guides, 82d682f changed how update_knowledge resolves its entry -- so the merged content differs from both parents and dates to the merge. The check is correct; the page really had not been read against the combination. Three things were actually behind, not just the marker: - mandatory_review_guides was undocumented. It is a governance field, so it belongs in the governance-by-data invariant: cross-owner review procedures from _DIRECT_MANDATORY_REVIEW_GUIDES, failing closed if a path does not resolve, and counted into execution_budget.knowledge_file_reads so an unreadable budget cannot be issued. - The page states that _knowledge_path prevents escaping the knowledge root, which is now not the whole truth: _contributing_entry deliberately resolves outside it, because the contribution entry moved to doc/knowledge/ with the docs. Recorded as the single explicit exception, with why it is safe -- a hardcoded constant path, not caller input, and escape protection is about caller input. - LOC ~806 was stale by about 480 lines; the file is 1286. The refactor note is updated too, since 'about 350 lines of _direct_* helpers' no longer describes it. I am NOT carrying f2e7c42 from the merged branch, which skipped the synthetic PR merge when dating files. It would not have fixed this -- main fails on a push event, where the merge commit is real history -- and it was the wrong idea anyway: on a pull_request run the synthetic merge is exactly what main is about to become, so a file going stale there is a true warning that main will go red after merge. Suppressing it would have turned a correct early failure into a green PR and a red main, which is what just happened. The failing PR run was right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements RFC #72 — Strict review reworked for the official
deepseek-v4-pro: hybrid deep-investigation passes, a per-comment agentic verify pass, and self-proving verbatim evidence. Design, alternatives, and open questions in the RFC issue (#72, also committed asdoc/RFC-strict-review-deep-engine.md).Performance vs the Opus 5 baseline
Every arm is measured against the same opponent — Claude Code + Opus 5 reviewing the same PRs — judged blind by
claude-sonnet-5, 3 replicates, paired inside each verdict and clustered by item. Regenerate withpython tools/mkchart_vs_opus5.py.Pipeline versions (Composer 2.5 backend, train n=10):
Other generators on the same pipeline family and judge: DeepSeek v4-pro (v17 clean, train) +0.024 / +0.016; DeepSeek in its own
dshharness (v18, val) +0.038 / +0.023 — the best recall row of the campaign; DeepSeek v4-pro (v13, wave-2) −0.115 / +0.040; grok-4.6 −0.080 / +0.004; Composer 2.5 standalone −0.127 / −0.048; grok-4.5 −0.164 / +0.030; MiMo v2.5 −0.212 / −0.084.How to read these numbers
pr4970, val +0.003 → −0.046 withoutpr4825. Every other single-item removal moves train by ≤0.03.claude-sonnet-5throughout. Earlier gpt-5.6-judged results from this branch (val 8—7, frozen test 7—8) credit recall more leniently and are reported separately indoc/EVAL-goal-strict-vs-opus5.md; do not read them on the same axis.testsplit is deliberately unspent.Contents
review_deep_engine); per-comment verification (review_verify_comments); self-proving evidence quotes end-to-end; coverage promotion; corroboration-ranked budget with rich-only overflow.value_flipsplanner signal, verification lens in the planner fallback, evidence caps 120k→260k / reducer 60k→280k, commit timeline + changed-symbol consumer sweep + hunk-location paging in the evidence pack.api | cursor | claude-code | codex | deepseekbehindSTRICT_BACKEND, with the MCP tool bridge — this is what let the same pipeline be measured across the generators above._same_finding: verbatim-or topic∧claim), merge-to-richest instead of drop-later, no invented?:NNanchors, ledger capped at 6.strict-review-checklistpage injected via new manifest keyknowledge.review_checklist(linter extended); vllm-omni module map 8→17 with platform/diffusion/distributed risk-tiered.anchor.py+ tests this pipeline imports, andARM_JOBSparallel eval sweeps.Disclosures:
config.py,manifest.yaml, andrun_copilot_arm.pycarried small pre-existing in-flight local edits that ship as-is; superseded local arms (v7–v12) are not included; both tuning splits are now spent — further iteration should use the wave-2 snapshots (RFC open question 1). One quarantined arm (INVALID_moa_*) is excluded: it ran with Mixture-of-Agents on, so its lenses were partly served by other vendors.🤖 Generated with Claude Code