feat(improvement): raw-trace context for code-surface proposer (meta-harness edge) + SWE-bench repo-state scoring#495
Conversation
Root-fix a scoring bug proven live: on django-12419 @ glm-4.6 the agent made the EXACT gold fix but scored 0 because the harness extracted the patch by parsing the model reply for a fenced diff block, and the model wrote prose instead. Two changes, standard SWE-bench practice (SWE-agent/OpenHands read the diff from repo state and pre-stage the checkout): - boxSetup(task): harness clones the instance repo at base_commit into a fixed /work BEFORE the agent shot (same session) so the agent only edits. A stochastic model cannot be trusted to clone to an exact path (observed cannot-change-to-/work failures). Adapter-agnostic seam on BenchmarkAdapter. - boxExtract(): git diff of the agent edits in /work (test files excluded; the judge applies the gold test_patch itself), preferred over the event-stream parse which stays the fallback. Prompt updated: repo pre-staged, agent only edits. Typecheck clean; calibration still green. Live glm-5.2 verification blocked on transient sandbox box-provisioning failures (box unavailable before start on a green health) - re-run when the platform host-agent recovers.
…proposer The code-surface proposer fed the coding agent the ~400-char DISTILLED findings (generationFailureDistiller), not the raw traces. The meta-harness edge (yoonholee.com/meta-harness) is raw-trace filesystem context: the coding agent greps/cats the FULL run traces of failed candidates to diagnose, rather than reading a pre-summary. Add rawTraceDistiller() — an additive analyzeGeneration producer that, instead of summarizing, points the proposer at the prior generation's real run traces already durable on disk under runDir (per-cell spans.jsonl event logs + cached-result.json scores + artifacts) with a grep/cat-to-diagnose instruction. It emits AnalystFinding[] with ABSOLUTE paths (the harness runs from a worktree cwd) so it drops into the same analyzeGeneration slot and renders through the same agenticGenerator prompt path. The existing distiller stays the default. improve() gains a one-line enable: opts.rawTraceContext = true wires rawTraceDistiller() when the caller has not supplied their own analyzeGeneration (explicit analyzeGeneration still wins; null still disables). Self-test builds a real tmp runDir with fake candidate + trace files and asserts the findings reference the actual absolute trace-file paths + the grep/cat instruction, plus worst-candidate-first ordering and the clean-generation fallback. tsc clean, biome clean, 3/3 new tests + 7/7 existing improve tests pass.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 92a8c17c
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-07-08T17:11:03Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Concerns | 2 (2 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 540.3s (2 bridge agents) |
| Total | 540.3s |
💰 Value — sound-with-nits
Adds a raw-trace findings producer for code-surface improvement and switches SWE-bench scoring to read the actual repo diff — both coherent, worthwhile improvements with only a minor architectural overlap with the existing Deliverable abstraction.
- What it does: Two stacked changes. (1) In
src/improvement/, introducesrawTraceDistiller()and arawTraceContextflag onimprove(surface:'code')so the coding proposer receives absolute paths into the prior generation's raw trace files (spans.jsonl,cached-result.json, artifacts) plus agrep/catdiagnosis instruction, instead of the default ~400-char distilled summary. (2) Inbench/, adds `box - Goals it achieves: (1) Bring the meta-harness recipe — feed the proposer the full raw traces rather than a pre-summarized digest — to the code-surface improvement loop, so the coding agent can diagnose failures from ground truth. (2) Fix the live SWE-bench scoring bug where an agent made the exact gold fix but scored 0 because the harness only parsed a fenced diff from the model's prose output; scoring from repo sta
- Assessment: Both changes are good on their merits and fit the existing seams. The raw-trace distiller is a clean drop-in replacement for the
analyzeGenerationproducer (improve.ts:394-402), emits the sameAnalystFinding[]shape the rest of the loop expects, and is backed by a focused test. The SWE-bench fix is a durable correctness improvement: it owns the checkout (so the model can't clone to a random - Better / existing approach: Searched the repo for existing trace context producers (none other than
generationFailureDistiller) and for existing box artifact extraction patterns. The only overlap is withDeliverableinopenSandboxRun(src/runtime/sandbox-run.ts:51-53), which already supports post-turn file reads viakind: 'artifact'. The newboxSetup/boxExtracthooks (bench/src/run-benchmarks.ts:212-221, `ben - Model: opencode/kimi-for-coding/k2p7
- Bridge attempts: 1
🎯 Usefulness — sound-with-nits
Two well-designed, pattern-fitting additions wired onto live paths (the default improvement loop and the default bench shot runner); one lifecycle question on the SWE-bench pre-stage hook deserves a correctness verify.
- Integration: Both halves are reachable. (1)
rawTraceDistilleris exported from src/improvement/index.ts, wired intoimprove()at src/improvement/improve.ts:399 behind therawTraceContextflag, and drops into the exactanalyzeGenerationslot — same signature and precedence as the existinggenerationFailureDistiller(improve.ts:231). A 195-line test (raw-trace-distiller.test.ts) proves trace paths reac - Fit with existing patterns: Excellent.
rawTraceDistillermirrorsgenerationFailureDistillerbyte-for-byte in signature, fallback-to-seed behavior, and the 0.999 pass threshold (raw-trace-distiller.ts:42 vs improve.ts:253) — it is the intended swappableanalyzeGenerationproducer, not a competing path.boxSetup/boxExtractcleanly extend the existingBenchmarkAdapterseam with an opt-in two-hook protocol (pre-stage - Real-world viability: The raw-trace distiller handles the expected edge cases well: in-memory
mem://runs (warns via isDurable at raw-trace-distiller.ts:298), missing trace dirs (listTraceFiles returns [] at :275), clean generations (falls back to seed findings at :117), and emits ABSOLUTE paths so the coding harness in a worktree cwd can cat them (:304). The SWE-bench side has one lifecycle subtlety:boxSetupruns - Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 boxSetup/boxExtract overlaps with the existing Deliverable abstraction [better-architecture] ``
openSandboxRunalready has a canonicalDeliverable<'artifact'>seam for post-turn box FS reads (src/runtime/sandbox-run.ts:51-53). The PR addsboxSetup/boxExtracttoBenchmarkAdapter(bench/src/benchmarks/types.ts:59-65) and the runner (bench/src/run-benchmarks.ts:212-271) rather than extendingDeliverableto support command-based extraction and a pre-stage hook. This is a workable shim, but it splits artifact-extraction semantics across two layers. Consider unifying later: ext
🎯 Usefulness Audit
🟡 boxSetup accesses run.box/run.sessionId before run.start() creates the session [robustness] ``
run-benchmarks.ts:212-221 calls
run.box.exec(..., { sessionId: run.sessionId })inside theadapter.boxSetupblock, which executes beforerun.start()at :222. In src/runtime/sandbox-run.ts thebox(line 266) andsessionId(line 270) getters both throw 'unavailable before start()' whilehandleis undefined;handleis only assigned insidestart()(line 305). If confirmed, this throws on every SWE-bench shot before the agent runs. This is an implementation/correctness detail (the hoo
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.
❌ Needs Work —
|
| deepseek | kimi-code | aggregate | |
|---|---|---|---|
| Readiness | 47 | 50 | 47 |
| Confidence | 70 | 70 | 70 |
| Correctness | 47 | 50 | 47 |
| Security | 47 | 50 | 47 |
| Testing | 47 | 50 | 47 |
| Architecture | 47 | 50 | 47 |
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 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision.
Blocking
🟣 CRITICAL boxSetup runs before SandboxRun handle exists — bench/src/run-benchmarks.ts
openSandboxShot calls
run.box.exec(..., sessionId: run.sessionId, ...)at lines 214-220, beforerun.start(...)at line 222. TheboxandsessionIdgetters on the object returned byopenSandboxRunthrow whenhandleis undefined (src/runtime/sandbox-run.ts:266-272), andhandleis assigned only insidestart()(src/runtime/sandbox-run.ts:305). Therefore any adapter that definesboxSetup(i.e., the SWE-bench adapter added in this PR) will fail every shot with 'box unavailable before start()' or 'sessionId unavaila
Other
🟠 MEDIUM SWE-bench test-file exclusion is incomplete and overbroad — bench/src/benchmarks/swe-bench.ts
boxExtractexcludes edits with the pathspec:(exclude)*/test*. This misses top-level test files such astest_foo.py,_test.py, andconftest.py, so an agent edit to those files could be included in the submitted patch and collide with the goldtest_patchduring judging. It is also overbroad: it would strip a legitimate source file under a directory likesrc/testutils.pybecause the directory component matchestest*. Reuse the existingisTestPathrules frombench/src/swe-bench-env.ts(or an equivalent pathspec) so the exclusion matches the documented intent.
🟠 MEDIUM No test coverage for boxSetup/boxExtract delivery — bench/src/run-benchmarks.ts
The new boxSetup (lines 212–221) and boxExtract (lines 230–271) code paths have zero test coverage. The test suite (run-benchmarks.test.mts) injects a stub shot via
runShotthat completely bypassesopenSandboxShot, so the primary deliverable extraction, the fallback to event-stream, the boxSetup timeout (300s), the boxExtract guard (non-zero exit + empty stdout), and the BENCH_ARTIFACT_DIR debug path are all untested. These are the central behavioral changes of the SWE-bench reliability fix. Verified: grep for boxSetup/b
🟠 MEDIUM TOCTOU race between existsSync and readdirSync — src/improvement/raw-trace-distiller.ts
Line 275 checks
existsSync(dir)before line 277 callsreaddirSync(dir, ...). If the directory is deleted between these two calls (concurrent cleanup, external mutation),readdirSyncthrows ENOENT. The comment on line 273 says 'throws loud' by design, but for a production improvement loop that reads potentially stale generation dirs, this is unnecessarily brittle. The `
🟠 MEDIUM listTraceFiles inner readdirSync has no error handling — src/improvement/raw-trace-distiller.ts
Line 282:
readdirSync(full, { withFileTypes: true })inside the directory loop has no try/catch. If a subdirectory is unreadable (EACCES), a symlink target is missing (ENOENT), or the dir is deleted between the outer readdir (line 277) and this inner call (TOCTOU), the entire function throws, crashing the distiller. The function handles self-absent dirs gracefully ([line 275](
🟡 LOW Shell interpolation of metadata in boxSetup without sanitization — bench/src/benchmarks/swe-bench.ts
${repo}and${base}fromtask.metadataare interpolated directly into the shell command string (line 85). If a non-trusted adapter's metadata contained shell metacharacters (e.g.,repo; rm -rf /), this becomes command injection running inside the sandbox. The SWE-bench adapter itself loads from the curatedprinceton-nlp/SWE-bench_Verifieddataset so real risk is minimal, but theboxSetuphook in types.ts is defined as a general-purpose adapter seam that other adapters may implement with untrusted input.
🟡 LOW Test-exclude pathspec misses root-level test files — bench/src/benchmarks/swe-bench.ts
The pathspec
:(exclude)*/test*requires a directory separator beforetest, so root-level files named liketest_utils.pyortesting_config.pyare NOT excluded from the generated diff. If the agent edits such a root-level test file the goldtest_patchwill collide on apply, producing a false negative. Most SWE-bench repos usetests/directories so realistic impact is low, but the exclude should also cover root-level test files. Fix: add':(exclude)test*'or use:(exclude,glob)**/test*.
🟡 LOW Dynamic import('node:fs') in shot hot path — bench/src/run-benchmarks.ts
import('node:fs')is called dynamically inside the boxExtract try block (line 248) and again in the artifact dump block (line 284). For a built-in module this is unnecessary overhead on the shot's hot path — it should be a staticimport { mkdirSync, writeFileSync } from 'node:fs'at the top of the file. No correctness impact, but violates the established pattern (e.g.,_harness.tsuses static imports for the same fs functions).
🟡 LOW Non-zero exit from boxExtract silently ignored when stdout is non-empty — bench/src/run-benchmarks.ts
Lines 242–245:
if (boxArtifact.length > 0) artifact = boxArtifact; else if (res.exitCode !== 0) boxExtractError = .... Ifgit diff --cachedproduces some stdout AND exits non-zero (corrupted repo state producing a partial diff), the partial stdout is accepted as the artifact and the error is never signaled. Forgit diff --cachedthis is a catastrophic edge case (broken repo), but the pattern of silently accepting partial+errored output is a correctness hazard if replicated to other extract commands.
🟡 LOW opts.runDir override path untested — src/improvement/raw-trace-distiller.test.ts
The
RawTraceDistillerOptions.runDiroption (line 49) overridesinput.runDirfor path anchoring but is never tested. The test at line 170 verifies the DEFAULT behavior (no override), and line 192 tests fallback findings, but neither test passes{ runDir: '/some/other/path' }to verify override precedence. The code path at [line 96](https://github.com/tangl
🟡 LOW listTraceFiles follows symlinks to directories — src/improvement/raw-trace-distiller.ts
Line 281:
entry.isDirectory()on aDirentfromreaddirSync({ withFileTypes: true })resolves symlinks on Linux/macOS — it follows the symlink target. If a trace directory contains a symlink pointing outside the runDir (e.g.,cell -> /etc),listTraceFileswould enumerate files outside the intended scope and include them in findings fed to the proposer LLM. While trace dirs are loop-controlled, the@experimentaltag means callers could use custom runDirs. Fix: skipDirententries where the underlying type is uncertain by checkingentry.isSymbolicLink()first (available in Node 20+), or simply don't recurse into directories at all since t
tangletools · 2026-07-08T17:35:49Z · trace
tangletools
left a comment
There was a problem hiding this comment.
❌ 1 Blocking Finding — 92a8c17c
Full multi-shot audit completed 2/2 planned shots over 7 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 7 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-07-08T17:35:49Z · immutable trace
# Conflicts: # bench/src/benchmarks/swe-bench.ts # bench/src/run-benchmarks.ts # docs/api/primitive-catalog.md # src/improvement/raw-trace-distiller.test.ts # src/improvement/raw-trace-distiller.ts
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 8c78eaae
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-07-08T19:32:48Z
Two session harness improvements, stacked.
1. Raw-trace context for
improve(surface:'code')— the meta-harness edge (92a8c17)Meta-harness's differentiator (yoonholee.com/meta-harness) is feeding the coding-agent proposer the FULL RAW TRACES of failed runs (grep/cat, ~10M tokens) instead of a ~400-char distilled summary — it matches prior optimizers' accuracy with ~10× fewer evaluations. Our code-surface proposer only got the distilled findings.
Adds
rawTraceDistiller({ runDir, ... })+ arawTraceContext: trueshortcut onimprove(surface:'code')that points the coding agent at the prior generation's raw trace files (candidate source + full event/trace logs + scores) so it can diagnose from ground truth. Additive — the existing distiller stays the default. 195-line test proves the trace file paths reach the findings.Proven end-to-end this session (results in supervisor-lab
docs/results/meta-harness-poc.md): a coding agent used it to read real supervisor traces, diagnose why delegation fired 1/3, and propose a grounded harness edit — which the eval gate then correctly REJECTED as a null (3/5 = same-model control 3/5). The loop has teeth.2. SWE-bench scores from repo STATE, not the model's printed text (305fea5)
Root-fixes a scoring bug proven live: an agent made the exact gold fix but scored 0 because the harness parsed its prose reply for a fenced diff. Now
boxSetuppre-stages the repo at base_commit andboxExtractreadsgit diffof the agent's edits (standard SWE-agent/OpenHands practice). Calibration green (gold→resolved, empty→not).Both typecheck-clean. The raw-trace capability is the durable meta-harness primitive; the SWE-bench fix is the durable eval-correctness primitive.