Skip to content

fix(trace-repair): stop rejecting a repair for how it labels itself - #560

Merged
drewstone merged 1 commit into
mainfrom
fix/repair-budget-label-not-rejection
Aug 10, 2026
Merged

fix(trace-repair): stop rejecting a repair for how it labels itself#560
drewstone merged 1 commit into
mainfrom
fix/repair-budget-label-not-rejection

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

The action budget rejected an intervention whose declared kind disagreed with its measured payload. The scaffold runs the action identically either way, so the rule scored an arm on how it described a repair rather than on the repair itself.

It also made the analyst contract unsatisfiable. The prompt fixes the declared kind to shell and recommends authoring a whole file with a heredoc, and the classifier reads any heredoc as edit, so an answer that followed the prompt was rejected before a container opened.

Measured on the milestone-2 repair corpus

arm findings rejected on the label alone
prime 10 7
bare-framing 1 1
oracle-fix control 43 0
inert-probe control 43 0

The controls could never trip the rule: the admit runner derives their declared kind from classifyActionPayload, the same classifier the check measures with. The gate therefore applied to the analyst arms alone, which is what makes it a comparison defect rather than a shared cost.

Re-graded on the same corpus, the answer that was rejected in 1 ms now reaches execution and returns a real outcome (outcome=measured).

Change

declared joins payload in BudgetMeasurement, and the mismatch stops being a BudgetViolation. Everything the budget still rejects is an executability bound: byte cap, one top-level statement, one authored file, no-op, submit-instead-of-repair.

Checks

  • npx vitest run tests/trace-repair/ — 128 passed (11 files)
  • npx tsc --noEmit — one pre-existing error in src/agent-profile.test.ts, present on clean main and unrelated to this change
  • merges cleanly into origin/main (git merge-tree --write-tree)

The action budget rejected an intervention whose declared kind disagreed
with its measured payload. The scaffold runs the action identically either
way, so the rule scored an arm on how it described a repair rather than on
the repair itself.

It also made the analyst contract unsatisfiable. The prompt fixes the
declared kind to "shell" and recommends authoring a whole file with a
heredoc, and the classifier reads any heredoc as "edit", so an answer that
followed the prompt was rejected before a container opened. On the
milestone-2 corpus that was 7 of 10 prime findings and the bare arm's
finding. The deterministic control could never trip the rule, because it
derives its declared kind from the same classifier the check uses, so the
gate applied to the analyst arms alone.

The declared kind is now recorded beside the measured payload, so a report
can still count how often an analyst misdescribes its own action.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — cd4d62ef

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-08-10T11:07:45Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 94.6s (2 bridge agents)
Total 94.6s

💰 Value — sound

Removes a budget gate that rejected an intervention whose self-declared kind disagreed with its measured payload — a property that never affected execution, so the gate taxed analyst arms (not controls) on labeling rather than on the repair.

  • What it does: Deletes the payload-kind-mismatch BudgetViolation from checkInterventionBudget and stops treating a declared-vs-measured kind disagreement as a rejection. The declared kind is now recorded (new BudgetMeasurement.declared field) alongside the measured payload for reporting, but no longer gates admission. Every remaining rejection (bytes, statements, heredocs, no-op, submit) is an executabil
  • Goals it achieves: (1) Stop rejecting a repair for a label that the scaffold ignores at run time — wrapActionForExec(action, cwd) in grade.ts:296/301/349/370 runs the text identically either way, and response.intervention.kind is read exactly once (grade.ts:141), only to feed this check. (2) Remove a comparison defect: the control arms derive their declared kind from classifyActionPayload (the same classifier
  • Assessment: A clean, correct fix in the grain of the codebase. The file's own header states the budget bounds 'what the scaffold could have done' (one statement, one file, a byte cap); the removed gate was the sole rejection that was not an executability bound. The degenerate-strategies.ts catalog (src/trace-repair/degenerate-strategies.ts:76-83) never enumerated label-mismatch as a defended strategy, confi
  • Better / existing approach: none — this is the right approach. Searched for an existing equivalent: checkInterventionBudget is the single admissibility site and this edits it in place; no parallel classifier or duplicate kind logic exists to consolidate. The control-arm classifier reuse (scripts/tb-repair-milestone1.ts:516 calling classifyActionPayload) is already shared. The only alternative would be to drop the `declar
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound

Removes the one budget rejection that scored an arm on its self-label rather than its executable payload, fixing a comparison defect that only the analyst arms could trip; every remaining rejection is a true executability bound.

  • Integration: Wired through the live grader path. checkInterventionBudget(action, response.intervention.kind, budget) is called at src/trace-repair/grade.ts:139-143, so the declared kind already flows from the analyst response into the check; the measurement flows into the rejection record at src/trace-repair/grade.ts:151 and src/trace-repair/funnel.ts:37. Removing the payload-kind-mismatch variant is type-
  • Fit with existing patterns: Fits the grain and reconciles the code with its own stated design. The module's top doc (src/trace-repair/action-budget.ts:5-8) says the budget bounds what the scaffold can execute, and src/trace-repair/degenerate-strategies.ts:76-83 documents the buy-a-bigger-action defeat purely as a statements/heredocs/bytes bound. The label-mismatch rule was the sole non-executability rejection, so dropping
  • Real-world viability: Holds on all paths. It is a pure-function relaxation: one rejection branch removed, one always-populated recorded field added (required declaredKind param, no null path). An action that mislabels itself now reaches the reproduction gate — correct, because the scaffold runs the text identically either way, which is the load-bearing fact the PR's measured corpus confirms (controls 0/43 rejected on
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260810T113349Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — cd4d62ef

Review health 100/100 · Reviewer score 89/100 · Confidence 70/100 · 3 findings (3 low)

opencode GLM 5.2 opencode DeepSeek v4 Pro opencode DeepSeek v4 Flash aggregate
Readiness 92 95 89 89
Confidence 70 70 70 70
Correctness 92 95 89 89
Security 92 95 89 89
Testing 92 95 89 89
Architecture 92 95 89 89

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision.

🟡 LOW declared kind is recorded but never persisted for admitted rows — src/trace-repair/action-budget.ts

The doc comment says 'A reader who wants the mismatch counts declared against payload', and the commit message says the mismatch is 'measured so a report can count it'. But the measurement only survives into the record path for rejected rows: grade.ts:151 puts check.measurement into RepairRejection.measurement, and for the admissible branch grade.ts discards the check result entirely (produceGrade proceeds without storing check.measurement in any of the not-reproduced/did-not-execute/measured outcomes). So for every admitted row — the rows whose label-vs-payload mismatch the change exists to observe — declared and payload are absent from the recorded grade, and the mismatch count the change promises cannot actually be computed from artifacts. Either surface the measurement on the measured/

🟡 LOW Admission test lives under a 'rejects' describe block — tests/trace-repair/action-budget.test.ts

The describe block at line 76 is framed around rejection, but the renamed test at line 103 now asserts admission. This is pre-existing (the 'admits one command inside the cap' test at line 79 already broke the framing), so not a regression — but the block title now misleads a reader scanning for what is rejected vs admitted. Cosmetic; no behavior impact. Optional: split admits/re

🟡 LOW Rejection-path measurement does not pin declaredtests/trace-repair/action-budget.test.ts

The new test (line 103-111) pins measurement.declared on the admitted path, but the rejected-path test (lines 130-134) asserts only statements/bytes. Because measurement is built once before all reject branches (action-budget.ts:413-425), declared is present there too, so this is not a correctness risk — but pinning declared/payload on a rejected action would lock the invariant that reports can count the mismatch even for rejected repairs. Optional hardening, not a blocker.


tangletools · 2026-08-10T11:36:11Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 3 non-blocking findings — cd4d62ef

Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-10T11:36:11Z · immutable trace

@drewstone
drewstone merged commit 7045151 into main Aug 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants