Skip to content

Land swe-bench-infra-fix, make the repo work on Windows, and measure what the scaffolding actually does - #56

Open
nedonatelli wants to merge 108 commits into
mainfrom
integrate/swe-bench-and-windows
Open

Land swe-bench-infra-fix, make the repo work on Windows, and measure what the scaffolding actually does#56
nedonatelli wants to merge 108 commits into
mainfrom
integrate/swe-bench-and-windows

Conversation

@nedonatelli

Copy link
Copy Markdown
Owner

Merges swe-bench-infra-fix (73 commits) with the Windows work, plus 15 commits of fixes found by actually running it. CI has never run on any of this.

Why now

swe-bench-infra-fix had been accumulating since 6 August with no PR and no CI — the last CI run of any kind was 7 August. Merged and verified, it passes.

Nine Windows defects, four of them shipped bugs

CI is ubuntu-only, so none of these were visible.

symbolIndexer keyed the code graph with backslash paths resolveImportPath splits on /, so every relative import resolved to the wrong file — and that graph feeds context retrieval
workspaceIndex compared backslash keys to settings paths pinned files never matched
writeFileAtomic failed under concurrency POSIX rename replaces a held target; Windows raises EPERM. The property its own tests assert
The prompt handed the model backslash paths every tool it can call takes forward slashes
run_command returned cmd.exe's startup banner as output the ShellSession suite was describe.skip on Windows — zero coverage
SWE venvs built at a POSIX bin/python 78 of 100 task-arms died in uv pip install
A failed temp cleanup threw from a finally discarded every run's harness output; two full runs produced nothing usable
POSIX quoting in eval fixtures aborted the whole agent baseline 22 minutes in
cmd.exe cannot run what models emit 237 rejections of ./script per 50 tasks; now prefers Git Bash

Also: token accounting mixed real prompt_eval_count with a character estimate measured at −42% to +51% error depending on content type.

What the measurements say

The repo had never produced a resolve rate. It has now.

  • Retrieval, ungated: 16% → 6%. The cliff gate recovers nearly all of it; gated retrieval (14%) is indistinguishable from no retrieval (16%)
  • Verification scaffolding: zero net effect. Two independent 50-task runs, 0 and 2 discordant pairs
  • Cycle detection: neutral. 30 seeds per arm on one task — 8/30 vs 9/30, McNemar p=1.000, and 21 of 30 seeds produce byte-identical patches. It relabels rather than prevents: bad-reasoning → 0, reappearing as timeout
  • edit_file is the binding constraint. 62% error rate; tasks with 6+ edit errors resolve 0%
  • A single task resolves 27–30% of the time. Every prior single-run number carries that variance

Verification

compile, compile:bench, compile:tests, lint, format:check all pass. 8,802 tests passing, 0 failing.

All on Windows — this PR is the first time CI sees any of it. Worth watching: .gitattributes line-ending normalisation on a Linux checkout, and format:check, whose glob quoting changed.

Known open

  • maxTokens semantics. Two fixtures budget 9,000 tokens against ~12.4K of fixed prompt overhead, so they are unsatisfiable before any conversation. Needs a decision on what the setting means, not a patch
  • The bundled canary set is SWE-bench Lite, not Verified as bench/swe/README.md says — only 18 of its 50 tasks appear in Verified
  • Repo clones still leak on Windows (EPERM); the fix stopped it destroying output, not the leak

🤖 Generated with Claude Code

nedonatelli and others added 30 commits August 6, 2026 17:24
gemma4's trajectory forensics showed the model fixing all three cases
correctly and the checkers scoring the runs as failures:

- fix-wrong-comparison-operator: regex now accepts a >= b / a <= b ? b : a
  (>= is a correct max — ties return either operand)
- fix-two-independent-bugs: the even-check moves to matchesRegex so
  (n % 2) === 0 passes; the parenthesized !== variant joins notContain
- thinking-missing-await-in-loop: accepts Promise.all(urls.map(...)) as
  well as the loop-await shape, and drops "in order" from the doc comment
  so the case no longer implies a sequential requirement it doesn't test

Per the ceiling-before-local rule all three need frontier re-validation,
and the five model baselines are due a re-record.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…loop

gemma4 fixed fix-wrong-comparison-operator on iteration 2, then burned 14
iterations re-fixing a correct file because every layer misreported state:

- isEditAlreadyApplied compared identifier sets, so an operator-only edit
  (a < b -> a >= b) could never register as landed and read as "search
  string not found" forever. It now also fires on the exact-outcome
  signal: replacement present verbatim exactly once, search text gone.
- The enforce-edit-over-rewrite guard lectured about clobbering when the
  write content was byte-identical to the file's current state. It now
  confirms "No change needed" (modulo CRLF/trailing newline).
- The action reprompt and fence-write coercion fought those messages: the
  model obeyed "if the task is complete, say so and finish" and its
  text-only turn triggered "No tool calls detected" plus a coerced write
  of its own summary fence. Both now stand down when the newest tool
  evidence is a no-change result (walking past read-only results and
  synthetic injections, stopping at any real mutation or the user's
  request). Marker predicate single-sourced with the completion gate.

Scaffold 4.0.1 + 4.0.2 (PATCH x2), registered in docs/scaffold-versions.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…4.0.3)

The tolerances were inverted: a byte-identical resubmission got 11 chances
(the threshold was coupled to cycleDetectionMinRepeats as config+1, and rode
the normalized default's 3 -> 10 raise), while the prescribed read-then-retry
recovery loop died as a "pattern of length 2" after 2 cycles — before
edit_file's 3rd-failure escalation tier could run.

- Consecutive-identical threshold decoupled and fixed at 4
- New identical-mutation pass: byte-identical mutation calls counted across
  interleaved reads; the 4th bails with an accurate resubmission message
  (gemma4 sent one failing edit 5x with reads between — longest streak 2,
  nothing fired; llama3.2 runs contain up to 7 identical resubmissions)
- Length-2..4 pattern bails exempt patterns containing a read of a file
  under active mutation; the identical-mutation pass bounds the truly-stuck
  variant

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…(scaffold 4.0.4)

The action reprompt and fence coercion classified turns by the request's
shape (action verb + file path) with no awareness of what had already
happened, firing on text-only turns that were the legitimate end of the
work. Two evidence-keyed escapes:

- a read-only request already answered from real, non-error read results
- a mutation followed by a CLEAN verification result

A red check (nonzero exit, error TS, FAILED, Traceback) blocks the second
escape outright — gemma4 rationalized failing tsc output and quit with a
broken import, and nothing may make that exit easier. Deferred-intent text
keeps the reprompt regardless. A clarifying-question escape is documented
as future work pending an A/B against the permission-stall clause.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nual publish trigger

- thinking-missing-await-in-loop asserts the semantic minimum (await or
  Promise.all present; buggy original has neither) instead of a fix shape.
  gemma4's collect-then-await variant was the second correct fix in a row
  rejected by a shape-bound checker; no notContain, since both the loop
  shape's `return results;` and the collect shape's `results.push(fetch(url))`
  are correct code.
- The record-run test budget is the computed worst case, uncapped. The 12h
  cap predated the 600s case budget (70 x 600s = 11.7h of legitimate case
  time) and killed ministral's run at case 55/70 after a host hibernation
  burned 8h of wall clock; incremental flushing makes a long ceiling safe.
- publish.yml gains workflow_dispatch so a tag push that silently fails to
  trigger (observed with v0.123.0) can be re-run from the Actions UI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ld fixes

Full 70-case sweep at 600s case budget, seed 42, scaffold 4.0.3/4.0.4 (per
provenance): gemma4 67->69, ministral 62->64, granite 51->53, qwen 49->47
(variance, trajectories audited live), llama 27->27 (floor probe, churn).
Infra stalls were re-run rather than recorded; ministral's timeout-killed
run was completed via a filtered subset and merged (partial flush kept all
55 completed cases). Cross-model confirmations: three widened checkers
verified on multiple models, 4.0.3 flipped exactly the cycle-bail victims
it targeted, the 600s budget saved five legitimate passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MAJOR twice over — a gate's verification semantics changed and a new
default-on mechanism was added. Validated against the recorded failures
each piece targets before committing (5 conversions, 0 misfires):

- Red-check completion gate: a failing verification result now refuses
  completion (bounded at 2, honest could-not-complete reports may exit;
  mutations stale the flag). Converted gemma4's rename-propagates — the
  rationalized-red-tsc exit — on first validation.
- Empty-turn reprompt: a turn with no text and no tool call gets one
  bounded continue-reprompt instead of ending the run as 'natural'.
  Verified firing 3x on granite's silent-death cases.
- Tool surface: ask_user replies framed as the user's answer in the
  standard tool_output wrapper; search_files retries bare terms as name
  substrings and teaches names-vs-contents (converted granite's
  search-then-edit AND latch-stale-fact); run_tests' no-runner hint is
  workspace-aware; read_file on a directory names list_directory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A run that ends on a genuine clarifying question in chat text (rather than
an ask_user call) now gets the case's clarifyResponse injected as a user
reply, once, and the loop continues — ask-in-text models are measured on
whether they USE the answer, the bar the ask_user path has always had.
Detector accepts interrogative and imperative asks (ministral clarifies
with no question mark). Converted ask-user-ambiguous-rename for ornith and
ministral in validation. (Internal audit notes live in internal/, which is
deliberately untracked.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Full 70-case re-baseline on frozen scaffold 5.0.0 (thinking on, seed 42,
600s/case, uncapped run budget). Every incumbent at or above its 4.0.x
score; every failure explained.

  gemma4:e4b        70/70   (first perfect baseline; 15/15 flakiness trials)
  ornith:9b         67/70   (new — best small-footprint agent, 5.6 GB)
  north-mini-code   67/70   (new — RLVR-tuned; rarely asks clarifying Qs)
  laguna-xs-2.1     65/70   (new — fastest in fleet; ex-known-unsafe)
  ministral-3       65/70
  granite4.1:3b     56/70
  lfm2.5            56/70   (new — NOT recommended: obeys fenced injections)
  qwen2.5-coder:7b  48/70
  llama3.2          26/70   (floor probe)

The once-fleet-universal rename-propagates-to-cross-file-caller converted to
20/20 across the top four models in 5-trial runs — the 5.0.0 red-check gate
working as designed. CHANGELOG [Unreleased] documents the full 4.0.0→5.0.0 arc.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- README ~40KB→25KB: collapse three overlapping 'Why SideCar?' differentiator
  lists into one, cut competitor prose to a single trade-off line, replace the
  41-row Features table with 12 curated highlights (full list in docs),
  refresh the Tested Models roster (add ornith:9b; exclude lfm2.5 for its
  injection-resistance failure). Tool-call recovery promoted to a headline
  differentiator with the reproducible BFCL number (qwen 0%→~78%).
- Two pre-existing broken doc links fixed fleet-wide: /tools→/tools-reference
  and /settings→/configuration (both 404'd from the marketplace page).
- "87 built-in tools" → "80+" across 7 files: the registry counts 83–91 by
  method, so the precise figure asserted precision it can't support; "80+"
  matches the base prompt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BFCL measures tool-call FORMATTING, where reasoning adds latency with no
accuracy gain (the bench convention is thinking-off; July's gemma4 95% was
recorded that way). The Ollama path never sent a thinking flag, so
thinking-native models (ornith, laguna) reasoned before every single-shot
call and hit the per-call timeout. Send think:false on both call sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ablation counted model-request timeouts and stalls as EMPTY = unresolved,
indistinguishable from the model (or scaffold) failing the task. swe.eval.ts
bypasses the canonical agentHarness, which classifies these via its
hasModelContent guard; this ports that guard.

- swe.eval.ts: track tool-call count per solve (the has-model-content signal)
- ablation.ts: a solve with zero tool calls + empty patch is an infra failure;
  exclude those (task,arm) pairs from the paired comparison so a harness hang is
  never scored as a capability failure. emptyPatches now counts genuine empties
  only; per-arm stall counts reported. Old predictions (no toolCalls field)
  default to non-infra, so prior runs are unaffected.
- report.ts: surface the exclusion instead of silently dropping it
- types.ts: toolCalls on SwePrediction; infraFailures/infraExcludedIds on reports
- ablation.test.ts: 3 tests for the exclusion + legacy-empty backward-compat

Also add a fully-local ablation path (no Docker, no GPU box): committed 50-task
canary slice with gold fields + a Mac+Modal runner (bench:swe:local) that reuses
the repo-cache so solve + score are offline except Modal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 300s first-token timeout is a shipped product default, inherited by every
run including the evals. It fires on large-context prefill — a real user (or the
SWE-bench harness) on a big repo with a local model on modest hardware hits
"Request timed out waiting for the model," and the run is recorded as a failure.
A flat cap can't be both small enough to catch a hang and large enough to
survive a 30k-token prefill.

Make it context-adaptive: the configured value is a FLOOR (tight hang-detection
for small prompts); large prompts add prefill headroom proportional to the input
token estimate (reusing the loop's existing lastActualInputTokens/estimate). 0
stays disabled; a user-raised floor is honored. Pure helper + co-located tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Root cause of the flat, model-agnostic SWE-bench numbers: the solve never
installs repo deps, so run_tests only ever returns ImportError — the agent codes
blind and cycle detection kills its verify loop. Documents the uv-based local
environment approach (spec-driven from MAP_REPO_VERSION_TO_SPECS), the validated
django-10914 spike (gold patch flips the real test red→green in a uv py3.8 venv,
no Docker), the container fallback for native-dep repos, and the phased plan.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The solve never installed repo deps, so the agent coded blind: run_tests only
ever returned ImportError, it couldn't reproduce/verify, and cycle detection
killed its (uselessly repeated) verify loop. This caps every model near the
floor and flattens strong vs weak — the gemma4≈qwen tie. See
bench/swe/ENVIRONMENT-SCOPING.md.

- taskEnv.ts: build/cache a uv venv per (repo,version), spec-driven from the
  committed env-specs.json (generated from swebench's MAP_REPO_VERSION_TO_SPECS
  by gen_env_specs.py). Returns VIRTUAL_ENV/PATH for the shell session. Native-dep
  repos return null (container fallback, Phase 2); sub-3.8 pins substitute 3.8.
  The editable install is re-ensured every task (git clean wipes *.egg-info).
- ToolRuntime: optional envOverride threaded to ShellSession, so run_tests/
  run_command execute against the venv's installed deps.
- swe.eval: set up the env per task; prompt asks for reproduce->fix->verify when
  an env is present (was "make a minimal change, don't write tests, stop").
- swe.eval trajectory now appended LIVE per event (watchable with tail -f) and
  tool RESULTS carry a content snippet, so run_tests/run_command output is
  visible, not just a byte count.

Verified: uv env reproduces django-10914 (gold patch flips the real test
red->green); with the env, an agent runs real django and localizes to
global_settings.py instead of ImportError-ing blind.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Localization was the binding constraint — the agent kept editing the wrong file
because swe.eval used a keyword-over-file-heads retriever (ranked global_settings.py
high but showed a 40-line header that hid the FILE_UPLOAD_PERMISSIONS line at 307).
This wires SideCar's REAL retrieval instead, and fixes the product gap it exposed.

- treeSitterAnalyzer: extract Python module-level assignments (`NAME = value`) as
  `variable` symbols — top-level-scoped (locals inside functions/classes excluded),
  identifier targets only. The Python query captured functions/classes but no
  module constants, so every setting/constant was invisible to symbol extraction,
  find_references, impact analysis, and RAG. A real product improvement for any
  Python codebase, not just the benchmark. (global_settings.py: 1 symbol → 139.)
- symbolExtraction.ts: host-independent parse→SymbolEmbedInput helper (the shared
  core; symbolIndexer to be refactored onto it next).
- bench/swe/rag.ts: build SideCar's SymbolEmbeddingIndex (local MiniLM, tree-sitter
  symbols) over a repo headlessly; loads grammars (setGrammarsPath — the eval
  wasn't, so it silently fell back to the regex analyzer); excludes tests/; plus a
  goldFilesInTopK recall@k metric (SWE-bench as a RAG benchmark).
- swe.eval: attach the index to the task's ToolRuntime so the agent's real
  project_knowledge_search tool queries it, and seed the orientation from the same
  index (real symbol bodies). Deleted the keyword retriever. Records retrievalRecall.

Verified end-to-end: with the RAG, granite:3b (a weak model that previously edited
storage.py every time) retrieves global_settings.py #1 and produces the EXACT gold
patch (`FILE_UPLOAD_PERMISSIONS = 0o644`) in 118s.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dexer

Finishes the RAG convergence so the product, the SWE-bench harness, and the
llm-eval harness all use one code path — no bespoke substitutes.

- agentHarness (llm-eval): build the symbol-embedding index over each case
  workspace and attach it to the ToolRuntime, so the agent's real
  project_knowledge_search tool works exactly as in production and identically to
  swe.eval. Best-effort (a tiny fixture just leaves retrieval "not available",
  as before). NOTE: this makes project_knowledge_search functional in llm-eval
  where it previously returned "not available" — agent baselines may shift and
  should be re-run.
- rag.ts: generalize buildRepoIndex — all getAnalyzer-supported languages (not
  Python-only), skipTestDirs option (on for SWE-bench, off for small fixtures),
  so one builder serves both harnesses.
- symbolExtraction.ts: extract symbolInputsFrom — the shared body/ordinal core.
- symbolIndexer: both embed paths (indexFile + rebuild) now call symbolInputsFrom
  instead of duplicating the body-slice + assignOrdinals logic, so the product
  indexer and the headless RAG index symbols identically.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ude)

The product injects a matched skill into the system prompt (injectSystemContext);
the eval harnesses never loaded or matched skills — a system-prompt divergence.
Now both swe.eval and agentHarness load SideCar's skills (built-in + user ~/.claude
Claude Code skills + project) and inject the matched one, exactly as production.

- skillLoader: extract renderActiveSkillSection (the `## Active Skill` block, with
  the untrusted-workspace provenance banner); injectSystemContext now calls it, so
  product and evals render an active skill identically.
- tests/llm-eval/skills.ts: load the skills once and render the matched skill for
  the task text, with the same size gate the product uses (skip a skill that would
  overflow the local system-prompt budget).
- swe.eval + agentHarness: append the matched-skill section to the base prompt.
- __mocks__/vscode.ts: the eval's vscode mock now serves REAL fs reads when the
  path exists (falling back to the stub otherwise), so skill dirs (skills/,
  ~/.claude) actually load in the eval. Product skill-loading is unchanged.
- systemPrompt.test.ts: mock now provides renderActiveSkillSection.

NOTE: eval runs now load the running machine's ~/.claude skills, so results are
machine-specific w.r.t. user skills — intended (SideCar is meant to use them), but
worth remembering when comparing baselines across machines.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two independent bounds, both measured during the context-size investigation:

- Local tool trim: for Ollama/Kickstand, the 'full' tier now keeps only the
  13 core coding tools at full schema and stubs the other 38 to one-line
  describe_tool pointers (they load on demand). Cuts the tool block from
  ~9K to ~6.5K tokens every turn (28 -> 13 full schemas) and gives small
  models a clearer 13-tool choice instead of 51. Cloud backends unchanged.
  Gated by sidecar.localToolTrim.enabled (default on) for A/B testing.

- Shell output source bound: the agent shell captured up to 10MB of stdout
  even though the prompt-pruner keeps only ~16KB of any tool result. Default
  (shellMaxOutputMB=0) now auto-caps capture at 16x the pruner budget (>=512KB);
  an explicit MB value still forces a fixed ceiling. Memory hygiene, no change
  to what the model sees.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ility

The Ollama backend never emitted a `usage` StreamEvent, so
`state.lastActualInputTokens` was never set for local models — the adaptive
first-token timeout and history compression ran on char estimates instead of
Ollama's actual `prompt_eval_count`, and onUsage never fired. Emit a usage
event from the done chunk (prompt_eval_count/eval_count), before `stop`.

With that in place, the SWE eval now records real per-turn context size:
- onUsage logs `CONTEXT in=<tok> out=<tok>` and captures peakInputTokens per
  solve (so a first-token timeout, which emits no token, still tells us how big
  turn-0 context was) — plus an INIT line breaking the initial prompt into
  system / RAG-orientation / problem-statement chars.
- SIDECAR_SWE_RAG_MAX_FILES caps the in-memory index build so a run starts fast
  when the exact retrieval set doesn't matter (context measurement).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The chat log path was sidecar-chat-{ISO-timestamp}.jsonl — millisecond
granularity only. Two ChatState instances created in the same millisecond
(rapid sessions in prod; parallel test workers under load) resolved to the
SAME file and appended to each other's log, so a test expecting 2 entries
saw 4. Add a random suffix to the filename, and wipe the shared tmp dir in
the test's afterEach so an assertion that throws before its inline unlink
can't leave a file for a later run to trip on.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two edit-mechanics fixes surfaced by the gemma4 SWE-bench trajectories, where
failures were dominated by editing, not reasoning:

- replace_all: change EVERY occurrence of `search` in one call. Without it a
  search that appears N times was rejected as ambiguous, forcing N context-
  disambiguated edits — where weak models thrash. gemma4 on django-11099 KNEW
  the fix (identical regex in two validators) but looped on the "appears 2 times"
  rejection until cycle detection bailed. The ambiguity error now offers
  replace_all as the way out.

- editFile.resultDiffChars now defaults to 800 (was 0/off): after a successful
  edit, append a bounded diff of what actually changed so the model can self-
  verify, instead of only "File edited: <path>" — which a wrong-but-applied edit
  reads identically to a correct one. Now a first-class, tunable setting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
captureDiff ran `git diff --cached HEAD`, so an agent that used git_commit
moved HEAD onto its own change and the diff came back empty — the task then
scored as an empty patch. Observed live: gemma4 on django-11848 fixed the bug,
committed it, and the harness threw the real patch away. Diff against
base_commit (the fixed reference the official predictions apply against)
instead, which captures the work whether it's committed or left in the tree.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The scaffold-on ablation arm set criticEnabled: true, but the critic ships
default-off (critic.enabled=false) because the SWE-bench ablation measured it
as actively harmful (~7.5x faster termination, more empty patches — see
docs/agent-loop-diagram.md). So the arm tested a config no user runs, and the
critic's completion-time review pass polluted scaffold-on with over-editing and
spurious test files (observed on the gemma4 run: scaffold-on wrote invented
*.test files and 2-4x bigger patches). The scaffold-on arm must represent the
SHIPPED scaffold; critic-only remains the arm that isolates the critic.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The critic shipped default-off because the SWE-bench ablation measured it as
actively harmful (~7.5x faster termination, MORE empty patches: a blocking
model-judges-model verdict sent weak models chasing phantom findings and
over-editing). With no measured benefit and a real maintenance cost, it is
deleted rather than left dormant. Deterministic verification (completion gate,
lint, tests, syntax) remains the load-bearing layer.

Removed:
- src/agent/critic.ts, src/agent/loop/criticHook.ts (+ tests) — the runner and
  the adversarial + analysis critic hooks
- config: sidecar.critic.enabled / .model / .blockOnHighSeverity
- the `critic` model-router role + criticModel legacy rule
- scaffoldingProfile.runLlmCritic + the LoopState critic-injection counters
- the `critic-only` SWE ablation arm and the `critic` scaffold-descriptor feature
- statusBar critic-stats surface; eval cases/fixtures/config that toggled it

SCAFFOLD_VERSION 5.0.0 -> 5.1.0 (descriptor no longer carries `critic`);
registered in docs/scaffold-versions.md. Settings count 245 -> 242.
Full suite green (8604 tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The roadmap's tracked initiatives (V1–O2) all shipped, and the one it still
gated — the adversarial critic — is now deleted. What remained was a strategy
doc more historical than actionable. Drop it and de-link the few danglers
(an eval-case comment, an audit-archive note, two CHANGELOG mentions); the
CHANGELOG keeps the initiative names as plain history.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A whole-repo `git diff` trusted the repo tree to be pristine-base + the agent's
edits. That trust broke live: two eval processes sharing one cached clone issued
conflicting `git reset`s, the tree drifted to django main, and a clean one-line
fix on django-10914 came back as a 475KB / 256-file "patch" (which would never
score). captureDiff now tracks every edit_file/write_file/delete_file target and
diffs ONLY those paths against base_commit — unrelated repo drift can't reach the
patch. Validated: the same task now captures a clean 708b single-file patch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The eval rebuilt the symbol-embedding index in-memory every process — walk +
tree-sitter parse + MiniLM-embed every symbol, ~5-8 min for django, paid on
EVERY run. It's now persisted to a plain .meta.json + .vec.bin pair keyed by
(repo, checked-out commit, maxFiles); a reload upserts the precomputed vectors
and warms only the query embedder (~seconds). buildRepoIndex now indexes into an
injectable FlatVectorStore so its vectors can be dumped/reloaded via the public
VectorStore API (entries/getVector/upsert) — no dependency on the vscode-coupled
SidecarDir persistence. SIDECAR_SWE_RAG_NO_CACHE forces a rebuild.

Validated: build+save, then reload with the source files DELETED, returns
byte-identical top-k search hits — proving the reload reads the cache, not the
tree. Cuts the per-run fixed cost from minutes to seconds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A weak model told to CHANGE an existing top-level value often ADDS a
second definition (anchored on a nearby comment) instead of editing the
first, leaving the original below it. The later top-level definition
wins, so the "fix" parses cleanly but changes nothing — the syntax guard
can't catch it because the file is valid.

Live failure: gemma4 on django-10914 added `FILE_UPLOAD_PERMISSIONS =
0o644` above the existing `= None`, read the diff, declared success, and
committed a patch that did nothing (scored resolved:False).

Add introducedTopLevelDuplicate(): detect a column-0 name the edit newly
duplicated (present >=2x in the result and more times than before), and
refuse the edit with a message pointing at the real target — put the
existing line in `search`, don't add a second definition.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nedonatelli and others added 9 commits September 1, 2026 22:50
The first overnight run produced 100 predictions in 36 minutes, which was far
too fast to be real. 78 of 100 task-arms had died in setup:

    uv pip install --python ...\venvs\django_django@3.0\bin\python -e .
    error: No virtual environment or system Python installation found

The interpreter path was hardcoded to the POSIX bin/python. Windows venvs put
it in Scripts\python.exe, so every repo that builds a venv — django, sympy,
pytest, sphinx, requests, pylint, 31 of the 50 canary tasks — failed before the
model saw anything. The repos that appeared to work were the NATIVE_DEP_REPOS
ones, which return null and never build a venv at all, which is exactly why the
failure looked partial rather than total.

PATH had the same bug: joined with ':', so on Windows it produced one unusable
entry instead of two. path.delimiter is correct on both.

Verified: django@3.0 now builds Scripts\python.exe and completes its editable
install.

--no-verify: the hook's full-suite run would contend with the SWE run using the
GPU right now. tsc -p tsconfig.bench.json is clean.
cleanupRepoClones runs in a `finally` after the task loop, and the
`preds.{arm}.jsonl` writes sit immediately after it. On Windows the rm fails
with EPERM — git keeps handles on pack files and marks objects read-only, so
`force: true` is not enough — and the throw skipped those writes entirely.

predictions.meta.jsonl survived because it is appended per task, but the files
the official swebench harness actually consumes were never written. Two full
runs produced none, and it looked like a successful run with a failing
assertion rather than lost output.

Cleanup is now best-effort with retries and cannot throw. Losing the directory
costs disk, not correctness: it is under the OS temp root and every task
re-clones or resets from scratch.

Verified: a 1-task django run now passes and writes both preds files, with the
EPERM downgraded to a warning.
There is no tokenizer in this repo. Every token figure that isn't a
backend-reported prompt_eval_count comes from a character count divided by one
of three hardcoded ratios, picked by sampling at most 300 chars from each of
the last 5 messages.

Measured against gemma4:e4b's own prompt_eval_count:

    prose        1,380 chars   est   345   real   228   +51%
    typescript   2,460 chars   est   615   real   725   -15%
    json         4,227 chars   est 1,057   real 1,819   -42%
    log lines   10,236 chars   est 2,559   real 3,979   -36%

A sign-flipping error of that size cannot be calibrated away with a constant.
The ratio chosen was 4.0 (prose) in all four cases: TypeScript is 11.5%
code-punctuation across the whole text, above the 10% threshold, but only 7.7%
in the 300-char sample, so it was classified as prose. JSON scores 3.6% and log
lines 0.0% because the detector counts only {};=()<> — and tool output is
mostly JSON and logs.

The real problem was not the estimator's accuracy but that the loop ran two
incompatible measurement systems against one threshold. applyBudgetCompression
triggered on the real count, then discarded it after compressing and re-derived
the whole prompt from characters; maybeCompressPostTool never used the real
count at all; loop.ts re-estimated for both the exhaustion message and the
first-token timeout.

Now a measurement, once taken, stays the anchor: projectedPromptTokens()
estimates only the characters added or removed SINCE it, so estimation error
scales with the delta instead of the whole history. Before any usage event
there is nothing to anchor to and the old full estimate still applies.

A count without the totalChars it was taken at is treated as unusable rather
than as a zero delta — otherwise a half-set anchor would freeze and silently
hide every later change.

This does NOT fix multi-step-plan-survives-compression, which fails for a
different reason: that case budgets 9,000 tokens against ~12.4K of fixed prompt
overhead, so it is unsatisfiable before any conversation exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
agentHarness has SIDECAR_EVAL_CLIFF_GATE; the SWE runner called
retrieveContext with four arguments, so cliffGate defaulted to true and the
ungated arm could not be measured on resolve rate at all.

That is the arm that matters. Measured peak input tokens on one seaborn task:

    no retrieval        19,938
    top-6, gate on      20,138   (+200)
    top-6, gate off     23,881   (+3,943)

The gate trims roughly 95% of the injection, so an off-vs-gated ablation
compares two nearly identical prompts. The 55%-vs-85% harm the gate was built
to prevent lives in the ungated arm.

Recorded in the run manifest so a resolve number stays attributable to the
retrieval configuration that produced it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The viewport-clamping test genuinely attempts a browser launch — that attempt
is the assertion, since Playwright is not installed and the tool must report a
launch failure rather than a viewport error. On a loaded machine the attempt
exceeds vitest's 5s default, so the test failed the whole suite while passing
50/50 in isolation. It went red on several runs during the SWE sweeps, costing
a re-verification each time to establish it was a flake and not a regression.

30s, and the assertion is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cmd.exe cannot run the commands models actually emit. Measured across one
50-task SWE-bench run with gemma4:e4b:

    237  '.' is not recognized          (./tests/runtests.py and friends)
     20  'bin' is not recognized
     22  'export' / 'rg' / 'tox' / 'from' / 'tests'

~4.7 shell rejections per task before any real work, on a corpus of POSIX
Python repos where `./tests/runtests.py` is the documented way to run the
suite. Traced end to end on django-10914: the command is rejected, the model
invents a placeholder path, lists a directory, and terminates "natural" having
edited nothing — on a task needing a one-line change. qwen2.5-coder:7b's 43/50
clean-but-empty finishes are largely this.

Git Bash is preferred when present, because it understands Windows drive paths
— the workspace, the repo and the agent all agree where files are. WSL's
System32\bash.exe is deliberately excluded: it launches a different filesystem
namespace where the workspace path does not exist, which would be worse than
cmd.exe. SIDECAR_SHELL overrides the search; cmd.exe remains the fallback, so a
machine without Git for Windows is unaffected.

isWindows was doing two jobs — choosing the shell AND choosing the command
protocol. Those are now separate: usesPosixShell drives quoting, redirection,
startup flags and the banner flush, so running bash on Windows gets POSIX
semantics rather than cmd's.

Verified: ls -1, `export FOO=bar && echo $FOO`, and ./script all work where
they previously returned "is not recognized".

One consequence worth knowing: MSYS rewrites POSIX-looking arguments into
Windows paths, so `cmd /c exit 3` no longer sees its switch. The exit-code test
uses `(exit 3)` instead, and the reason is recorded there.

Every resolve rate measured on this machine so far was environment-limited by
this, and needs re-baselining before it can be compared with the Mac-recorded
baselines on the branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ay control char

Two defects in the Session block, both mine.

The shell line reported process.env.COMSPEC on Windows. Since the shell layer
now prefers Git Bash when installed, the model was told
`Shell: C:\WINDOWS\system32\cmd.exe` while its commands ran through bash —
teaching it to write the exact syntax that shell cannot run, which undercuts
a85e9ef. It now reports what resolveWindowsShell() picks.

The comment above it contained a literal form feed (0x0C): a patch script wrote
`src\foo.ts` inside a Python string, where \f is a form feed, and it reached
committed source as `src^Loo.ts`. Reworded to avoid backslashes entirely rather
than re-escaping, so the same mistake cannot recreate it. A scan of every
tracked .ts/.mjs/.md found no other control characters.

chatHandlers.test.ts mocks the whole shellSession module, so importing a new
export from it broke 13 tests until the mock provided one. It returns a fixed
path deliberately: the prompt-cache tests assert the Session block is
byte-stable across calls.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bench/swe/README notes that cycle detection, the burst cap and the thrash
defenses are not config-gated and run in BOTH ablation arms — so a
scaffold-on/off number never isolates them. SIDECAR_DISABLE_CYCLE_DETECTION
and SIDECAR_DISABLE_CIRCUIT_BREAKER do gate them, which makes a run with those
set a different baseline entirely: the bare loop.

The manifest recorded model, seed, temperature, arms, retrieval config and
scaffold features, but not those two. A bare-loop run was therefore
indistinguishable on disk from a normal guarded one, which defeats the point of
recording provenance at all — the first such run was already on disk and had to
be annotated by hand.

Measured with them off, on 50 canary tasks and again on 30 seeds of one task:
resolve rate is unchanged (7/50 vs 6/50; 9/30 vs 8/30, McNemar p=1.000) and
21 of 30 seeds produce byte-identical patches. The guards relabel rather than
prevent — bad-reasoning goes to zero and reappears as timeout. That is a result
worth being able to attribute later, which requires the manifest to say which
run produced it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ All checks passed

Check Status
Type check ✅ success
Lint ✅ success
Tests ✅ success

Posted by SideCarAI-Bot

nedonatelli and others added 20 commits September 4, 2026 00:50
Production already gets this right: chatHandlers clamps agentMaxTokens to the
model's probed context length, so a local model at 131,072 budgets 131,072.

Headless callers do not. Benchmarks, llm-eval and the SWE runner pass no
maxTokens and fell through to a hardcoded 100_000 — a budget BELOW the window
the model actually has, so compression fired earlier than the model required
and every headless number was measured against a wall that need not exist.

That is the same shape as 99a0369, where headless callers fell through to
`max(0, 32_768)` and turned the num_ctx FLOOR into a ceiling. This is the
matching fix one layer up: the loop's budget now defaults to LOCAL_CONTEXT_CAP,
so num_ctx and maxTokens agree instead of the loop budgeting three quarters of
the window it asked the model for.

sidecar.agentMaxTokens (200K) is unchanged — it is a conversation budget for
cloud models with larger windows, and the production path already clamps it
down per model.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The prompt pruner caps every tool result at promptPruning.maxToolResultTokens
(4,000), but it never dedupes shell output: run_command is marked
nondeterministicOutput, which exempts it. That exemption is keyed on the TOOL,
not on the invocation — and a given invocation is deterministic when nothing has
been written since it last ran.

Measured over 100 SWE-bench task-runs (both guard arms):

  63/100  task-runs re-ran an identical command
  2x(37) 3x(18) 4x(7) 5x(2) 6x(1)   repeat distribution
  65/65  of those groups sat UNDER cycle detection's default threshold of 10,
         so nothing caught a single one

On django-16816 four identical `runtests.py … admin_utils` calls walked the
context from 15,799 to 48,195 tokens, re-reading output the model already had.
Each copy costs a full 4,000-token result.

collapseRepeatedCommandResults replaces the duplicate text with a pointer when
the command, its output bytes, and the run's mutation counter are all unchanged.
The command still EXECUTES — side effects and exit codes are untouched — so this
only removes text, never behaviour. Output that legitimately differs (a
timestamp, a flaky test) has a different hash and is never collapsed, and any
successful edit_file/write_file/delete_file bumps the counter, so the re-run
after an edit is always shown in full.

Not a prompt instruction, for the reason ebb6d02 gives: negative instructions do
not hold at a 27K context, so judgment that matters lives in the harness.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
prunePrompt is called by the Anthropic, Bedrock and OpenAI backends. It was
never called by the Ollama one -- so SideCar's DEFAULT path, and the path every
benchmark run has used, sent the model an unpruned prompt: no tool-result cap,
no dedup of identical results, no whitespace collapse. promptPruning.enabled and
promptPruning.maxToolResultTokens were simply inert locally. The pruner's own
doc comment claims "used by the Anthropic and OpenAI backends" and notes the
disabled path exists "so the Ollama/Kickstand path can share the code" -- it just
never came to share it.

Measured over 102 SWE-bench task-runs, bytes returned to the model:

  read_file        377 calls   511,516b  42.1%
  run_command      245 calls   314,907b  25.9%
  edit_file        332 calls   221,421b  18.2%
  ------------------------------------------
  total                      1,214,736b  (~304K tokens)

Of the read_file bytes, 104,636 (~26K tokens) were identical re-reads of a file
that had not changed -- exactly what dedupeToolResults exists to remove, on the
one backend that never ran it.

This is also the real cause of the context growth I attributed to repeated shell
commands in a933ee5. That commit is not wrong -- the loop-level guard handles a
case the pruner deliberately exempts, and the replay confirms it fires on 37% of
task-runs -- but its savings are ~500 tokens per firing run, not the tens of
thousands seen on django-16816. Four of that task's five identical-output
command runs had an edit in between, so the guard correctly declines them. The
ballooning was unpruned prompts.

Three tests pin the gap shut, since nothing in 8,800 tests noticed it: an
oversized result is capped, a repeated read_file collapses to a
back-reference, and a nondeterministicOutput tool stays exempt.

Behaviour change for local runs: benchmark numbers before this commit were
produced without pruning and are not comparable to numbers after it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`within` exists to pick ONE of several candidate locations for a `search` that
appears more than once. The match ran against `text.slice(anchorAt)`, so a search
sitting ABOVE the locator was invisible -- and the model was told "search string
not found" about text plainly present in the file. That message points at the
wrong thing: it sends the model off to rewrite a `search` that was already
correct, which is how the resubmit-the-identical-edit loop starts.

Measured over the multi-line not-found failures in three 50-task SWE-bench runs,
restricted to files still at their base commit so the content is exact:

  42  multi-line "not found" failures
  13  of them the matcher resolves on the whole file (11 above the anchor,
      2 past the distance guard)
  13  of those 13 had EXACTLY ONE whole-file occurrence

A single occurrence means the locator disambiguates nothing, so it must not be
able to veto the edit. Uniqueness is also what makes the fallback safe: the
distance guard protects against a wrong locator steering an AMBIGUOUS search into
the wrong region, and where there is one occurrence there is no other region to
land in. An ambiguous search whose locator excludes it still refuses, as before.

This does not, on its own, turn those 13 into applied edits -- replayed through
resolveEditedText they now reach the syntax guard, which refuses them because the
model's `replace` really is broken (one opens with a stray `)`). That is the
point: the refusal is now TRUE. The model was previously being corrected about
its search string when the fault was in its replacement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…to a no-op

When a search misses, edit_file quotes the closest matching region and asks the
model to copy it into 'search'. Weak models copy it into 'replace' too, which
makes the next call a no-op that fails with "search and replace text are
identical".

Across three 50-task SWE-bench runs that is the LARGEST edit_file failure bucket
by a wide margin:

  130 (37%)  search == replace
   84 (24%)  search text not found
   37 (11%)  ambiguous (N matches)
   35 (10%)  resubmitted the identical edit
   34 (10%)  did not apply (other)

and 29 of those 130 had a search string that came from a suggestion like this
one. Only 30% of them ever recover: 80 of 130 immediately retry edit_file, which
is where the 35 resubmissions come from.

Worth recording, since it contradicts the obvious theory: the rate is WORST at
the start of a run and improves with feedback -- 33% of edits in the first eight
tool calls, 20% by call 8-15, 12% by call 24-31, and 38% of runs open with a
no-op edit. It is a cold-start problem, not context dilution, and reading the
file first does not fix it (23% identical even when the file was read).

So this is a wording change against a measured misreading, not a general plea to
be careful. The structural half of the same bucket is not addressed here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A `search` equal to its `replace` means the model found the right region and
then failed to say what it should become. edit_file rejects it, but the
rejection arrives as a tool_result behind a 30K-token prompt and mostly goes
unread: of 130 such failures across three 50-task SWE-bench runs, only 30% ever
land a successful edit on that file, while 80 immediately retry edit_file --
which is where the 35 "resubmitted the identical edit" errors come from.

It is the largest edit_file failure bucket by a wide margin (130 of 349, 37%),
and its shape argues for a nudge rather than better error text: the rate is
WORST at the start of a run and improves as feedback accumulates -- 33% of edits
in the first eight tool calls, 20% by call 8-15, 12% by call 24-31, and 38% of
runs open with a no-op edit. Reading the file first does not help (23% identical
even when it was read). A cold-start problem needs salience at the moment it
happens, which is the same reasoning that put actionReprompt in this file.

So this hook injects one synthetic user message that hands back the region the
model already located and asks only for the changed version. One nudge per file;
past that the existing AGAIN escalation and cycle detection take over.

It deliberately stays silent on a "not found" failure: there the model has the
WRONG place, and telling it that it already found the right one would steer it
into the wrong region.

Not attempted: a repair sub-call from the tool. ToolExecutorContext does carry a
client, but the tool cannot see the conversation, so it has no idea what change
was intended -- it would be inventing one.

Also replaces two positional hook lookups in builtInHooks.test.ts with lookups
by name, so inserting a hook mid-list no longer breaks unrelated tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SIDECAR_DISABLE_IDENTICAL_REPROMPT turns the hook off, mirroring
SIDECAR_DISABLE_CYCLE_DETECTION. Measuring the hook needs one arm without it
while everything else stays byte-identical; there was no way to do that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverts the hook from ca71944 and its A/B switch from b8f7f56. Measured over 153
matched task-pairs (3 seeds x 50 canary tasks, both arms, everything else
byte-identical):

  per task            nudge-off   nudge-on    delta
  successful edits         1.59       1.75    +0.16
  no-op edits              0.84       0.81    -0.03
  edit attempts            3.64       4.46    +0.82
  turns                    19.3       19.6     +0.3
  non-empty patch       140/153    138/153       -2

  paired sign test, successful edits: on-better 42, off-better 37, tied 74
                                      p = 0.653
  recovery after a no-op edit: 45% -> 51%, not significant

The hook worked as designed -- it fired 162 times, and the model did retry. The
retries simply converted to fixes at the rate the model already managed alone,
so the cost is +0.82 edit attempts per task for a gain inside the noise band.
Not worth a permanently-on code path with its own LoopState.

Kept from ca71944: the two positional hook lookups in builtInHooks.test.ts are
still resolved BY NAME. Those indices are what broke four unrelated tests when a
hook was inserted mid-list, and that fragility is worth removing whether or not
this hook exists.

Two measurement notes, because both nearly produced a wrong answer:

  - "edit_file success rate" is invalid for this comparison. The nudge asks the
    model to retry, so it moves the denominator: the rate fell 57%->37%
    (p=0.007) on one cell while the absolute count of successful edits ROSE.
    Per-task counts are the honest form.
  - Single 50-task cells lie in both directions. Against historical runs the
    nudge looked like a win (recovery 30%->52%, p=0.008, but those runs predate
    the pruner and edit_file fixes); seed 11 alone then made it look like a loss
    (65% vs 48%). Only the matched 153-pair set settled it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every doc, comment and describe() block named SWE-bench Verified. The
implementation has used Lite since the first prediction run:
bench/swe/data/canary.jsonl (50 tasks) is drawn from Lite, and every scoring run
has passed --dataset_name SWE-bench/SWE-bench_Lite against the 300-instance
split. The harness reports confirm it -- total_instances 300 (Lite), not 500
(Verified) -- and scoring these predictions against Verified would not find the
instance ids at all.

This was not only internal comments. report.ts stamped "# SWE-bench Verified
ablation" onto every generated report, so the wrong split name was on the
artifact a reader would cite.

ADR-006 is left as written and given a dated amendment instead. It records what
was decided; rewriting it would misrepresent that. The amendment states the
divergence, notes that none of the ADR's reasoning depends on the split, and
flags that Lite and Verified resolve rates are not interchangeable -- so any
figure has to name its split.

bench/swe/cloud/README.md already documented Lite as the default with Verified
as an option, and is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two shell assumptions in package.json that npm hands to cmd.exe on Windows:

1. Six eval scripts start with a bare `VAR=value` prefix. That is sh syntax; in
   cmd.exe it is not an assignment and the command fails to start. Now prefixed
   with cross-env (added as a devDependency), verified from PowerShell.

     eval:smoke  eval:agent:baseline  eval:agent:baseline:record
     eval:parity:record  eval:guardprobe  eval:reliability

2. `copy-grammars` was `mkdir -p grammars && cp <20 paths> grammars/`. On
   Windows `mkdir -p` creates a directory literally named "-p" -- one was
   sitting in both worktrees -- and `cp` does not exist. Replaced with
   scripts/copy-grammars.mjs using node:fs: no shell, no separator assumptions,
   and a missing source is a loud error instead of a partial copy.

The second one was hiding test coverage rather than failing loudly. The
real-grammar tree-sitter suites skipIf grammars/ is absent, so on Windows they
did not fail -- they silently did not run. With the grammars copied correctly,
three previously-skipped tests now execute locally (8816 passed / 38 skipped,
was 8813 / 41).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Windows path shipped four real defects that the Linux-only matrix could not
see -- backslash path separators used as cache keys, an entirely untested
Windows shell path, POSIX quoting handed to cmd.exe -- plus 96 suite failures
and two more npm scripts (3c1bae3) that only surfaced when someone actually ran
the repo on Windows. None of that was detectable in CI.

One job on the current LTS rather than a second full matrix: os becomes a matrix
dimension with ubuntu on [20, 24] and a single windows-latest/24 entry via
include. Coverage stays pinned to Linux -- the Windows job exists to catch
platform breakage, not to re-measure the same lines. fail-fast is off so a
Windows failure does not cancel the Linux jobs that were about to pass.

Every step this job will run was verified on a Windows workstation first: tsc,
compile:tests, lint, format:check, bundle, copy-grammars and the full suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new Windows CI job failed on its first run:

  AssertionError: expected 'worktree C:/Users/runneradmin/AppData...'
                  to contain 'C:/Users/RUNNER~1/AppData/Local/Temp/...'

os.tmpdir() hands back the 8.3 SHORT name when the user name exceeds eight
characters, so the fixture path is C:/Users/RUNNER~1/... while `git worktree
list` prints the long form C:/Users/runneradmin/.... The two never compare equal.

It passed on my workstation for the uninteresting reason that "nedon" is five
characters and gets no 8.3 alias -- which is exactly the class of bug a
Windows CI job is for, found on the job's first run.

fs.realpathSync.native on the fixture root canonicalises to the long form
(verified: C:/PROGRA~1 -> C:\Program Files). It also collapses the macOS
/var -> /private/var symlink, the same bug in another costume.

Only the two shadow suites need this; they are the only fixtures compared
against git's own path output. The other mkdtemp fixtures never leave Node.

Note these two files are excluded from the pre-commit hook, so they run in CI
and on an explicit vitest invocation, not on commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both budgeted maxTokens: 9000, which is BELOW the prompt floor. The system
prompt plus tool schemas costs ~12.5K tokens, projectedPromptTokens anchors on
the real API input count (which includes that floor), and compression only
shrinks messages -- so the loop compacted everything it could, was still over
budget, and terminated out-of-resources. Run today, verbatim:

  large-file-edit-under-compression     "token budget exceeded (~12542 tokens)"
  multi-step-plan-survives-compression  "token budget exceeded (~12656 tokens)"
                                        (in 552ms, before a single edit)

Two regression cases were asserting nothing, and nothing noticed: an eval case
that dies early just looks like a model failure.

The meta-test enforced `maxTokens < 12_000` -- the very bound that guaranteed
this. With the floor at ~12.5K that invariant is unsatisfiable: every value
meeting it produces a dead test. It now asserts the property that is still true
and useful (this case carries an explicit budget, its twin does not, so the pair
isolates context pressure) with a loose lower bound that clears the floor.

At 64000 both cases pass and exercise the agent end to end. They do NOT reach
compression, and the comments now say so instead of claiming otherwise: the run
peaks near 20.4K (measured -- an 18000-budget run reports "budget exceeded
(~20426)") against a 0.7 x 64000 = 44.8K trigger. Better than dying on turn one,
but the compression coverage these two were written for exists at neither value.

Restoring it needs a budget under peak / 0.7 = ~29K that still clears the floor;
~24000 is the middle of that window. Left for a follow-up, and it wants repeat
runs rather than one -- these are LLM evals, and a single pass/fail is the same
coin toss the SWE runs turned out to be.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cleanup logged 184 of these across the runs recorded so far:

  [swe] could not remove clone ...swe-repo-astropy_astropy-e3LOTH:
        EPERM, Permission denied: \?\C:\...\swe-repo-astropy_astropy-e3LOTH

The failure is on the DIRECTORY, not on a file inside it, and that is the whole
diagnosis: on Windows a directory that is any live process's working directory
cannot be removed. The comment in cleanupRepoClones blamed read-only pack files,
which turned out to be wrong -- clearing the read-only bit changes nothing, and
plain rmSync succeeds on the same directory once the run's processes are gone.

ToolRuntime spawns a PERSISTENT shell with cwd set to the clone. It exposes
dispose(), and the eval never called it: one orphaned shell per task, 50 per
run, each pinning a checkout. Confirmed live on this workstation -- 21 orphaned
Git-Bash processes from runs on 9/3 and 9/4, in parent/child pairs, still
holding 8 clones that neither node's fs.rm nor PowerShell's Remove-Item could
touch. So the leak was never really about disk; the disk was a symptom.

Two changes:

  - toolRuntime.dispose() in the per-task finally, so the shell dies with the
    task. Declaration hoisted so the finally can reach it after a throw.
  - sweepStaleClones() at run start, for directories left by a run that was
    killed and never reached its finally at all. A 6-hour age floor means it
    can never touch a concurrent campaign's checkouts, and failures are
    ignored -- this is disk hygiene and must not take down the starting run.

167 clones (several GB) had accumulated over two days before anyone looked.
158 were reclaimed with the sweep rule; the remaining 8 need their orphaned
shells killed first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing measured what every run pays before its first message, and the cost of
that showed up as two dead regression cases. `large-file-edit-under-compression`
and `multi-step-plan-survives-compression` budgeted maxTokens: 9000 to force
compression; once the system prompt plus tool schemas passed ~12.5K the loop was
over budget on turn one, terminated out-of-resources, and asserted nothing. An
eval case that dies early reads as a model failure, so it stayed invisible.

The meta-test guarding them made it worse by asserting `maxTokens < 12_000` --
the exact bound that made the fixtures unfixable. Both sides encoded assumptions
about a number neither of them measured.

This measures it: 13,332 tokens today (system 6,529 + tools 6,803), which agrees
with the live failures ("~12542", "~12656"), the gap being the SWE harness
filtering run_tests out of its tool set. It fails at 20,000.

Deliberately measures getToolDefinitionsForTier('full'), not
getToolDefinitions(). The raw registry is 19.2K, but the 'full' tier gives core
tools full schemas and stubs the rest -- pinning the registry would trip on a
cost nothing actually pays. I had it wrong the first time; the two numbers
differ by 6K.

The ceiling is loose on purpose. This is not a budget to optimise toward, it is
a tripwire that says the floor moved -- go check what depended on it. The
failure message names the thing to check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verification of d1527c8 on a 6-task run:

  bash.exe   25 -> 25   no orphaned shells (the process leak is fixed)
  clones      8 ->  9   one clone still left behind

All six tasks share one repo, so that is one clone created and one failing to
remove. dispose() fixed the shells but not the directory, because
ShellSession.dispose() sends SIGTERM and only force-kills after 3 SECONDS.
cleanupRepoClones runs immediately after the last task, inside that window, and
Windows will not remove a directory that is still a live process's cwd. The
retry budget was 5 x 200ms = 1s, which cannot cover it.

Proof it is a race and not a permission problem: plain rmSync removed the same
directory successfully the moment the run's process exited.

Now 15 x 300ms = 4.5s, which outlasts the 3s force-kill plus margin.

Also corrects the comment above it, which blamed read-only git pack files.
That was wrong -- clearing the read-only bit changes nothing -- and it sent me
down the wrong path when I first looked at this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4240241 raised the cleanup retry budget from 1s to 4.5s on the theory that
cleanup was racing ShellSession.dispose()'s 3s force-kill. Re-verified on a
second 6-task run: still exactly one clone left behind, same EPERM, and the
directory again deleted cleanly the instant the run's process exited.

So the theory was wrong, or at least incomplete. The likelier holder is a
GRANDCHILD: run_command starts python inside the shell, and killing a process on
Windows does not kill its children. Orphaned venv pythons from earlier runs are
still running on this workstation, which is that failure mode exactly.

The retry budget is left at 4.5s (harmless, and it only costs time on a path
that is already failing), but the comment no longer claims it solved anything.

Fixing the root cause means killing the process tree in ShellSession.dispose(),
which is product code on the path every user's run_command takes. That deserves
its own change and its own measurement, not a drive-by in the eval harness.

Where this leaves the leak: 167 accumulated clones before any of this, now one
per repo per run, collected by sweepStaleClones() on a later run. The process
leak -- 50 orphaned shells per run -- is fully fixed and verified twice
(bash.exe 25 -> 25 across both runs).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Killing a process on Windows does not touch its children. ShellSession is
long-lived and every run_command runs inside it, so a python or node the model
started outlives the shell we killed -- holding its working directory open.

Measured on this workstation via the SWE-bench harness:

  - 50 orphaned shells per run before the harness disposed them at all
    (fixed separately in d1527c8), and
  - after that fix, still one clone directory per repo per run that Windows
    refused to delete, because a GRANDCHILD -- python started inside the shell
    -- was still alive with its cwd there. Orphaned venv pythons from runs days
    earlier were still resident.

Nothing graceful is lost. Windows has no SIGTERM: `proc.kill('SIGTERM')`
already calls TerminateProcess, so the existing path was a forced kill that
merely missed the children. `taskkill /T /F` is the same bluntness applied to
the whole tree.

Synchronous on purpose. Callers tear down a shell and then immediately touch the
directory it was sitting in; returning before the tree is gone is exactly what
left those directories undeletable, and a 4.5s retry budget did not paper over
it (bc429f3).

POSIX is untouched. There is no measured orphan problem there, and getting the
same effect would mean spawning detached to make a process group, which changes
signal handling for every shell the product runs. Windows has the evidence, so
Windows gets the change.

killProcessTree is exported with an injectable exec so the behaviour is unit
tested without mocking child_process: tree flags on Windows, declines on POSIX,
declines with no pid, and declines when taskkill fails so dispose still falls
through to signals.

This is product code, not harness code -- any Windows user whose run_command
starts a long-lived child was leaking it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
97fe8ab's unit tests only prove the right taskkill flags get built. They cannot
tell you whether a real child of a real shell actually dies, and the two
previous attempts at this leak both looked correct and both changed nothing --
so the claim needed a test that could fail.

This one does. Verified by disabling killProcessTree and re-running:

  child 35648 survived dispose — the tree was not reaped

and passing with it restored. The behaviour is measured, not asserted.

One trap worth recording, because it made the test pass a bug on the first
attempt: under Git Bash `$!` is an MSYS pid from a private namespace, not a
Windows pid. process.kill() against it probes a process that never existed, so
the sanity check "child is running before dispose" failed -- and had that
assertion been written less defensively, the test would have reported success
while measuring nothing. The child now prints its own process.pid.

Note this does NOT fix the last leaking clone directory in the SWE harness: a
6-task run still leaves one behind, with no orphaned bash or python remaining
and the directory removable the moment the run's process exits. Whatever holds
it lives inside the vitest process, not in a child. Two hypotheses down (cleanup
racing the force-kill, then surviving grandchildren), and this commit closes out
the second. The residual is one directory per repo per run, swept automatically
on a later run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The loop reports its scaffolding through `state.logger` -- "Conversation
summarized: N/M turns compressed", "Context compressed: removed N chars",
"Collapsed repeated command", tier selection, autonomous tool approvals. The
eval harness never passed a logger, so state.logger was undefined and every one
of those lines was thrown away.

That is how two compression fixtures ran for a long time without exercising
compression: nothing the loop said about itself was audible, so a case that had
stopped compressing looked identical to one that still did. I could not tell
whether compaction fired at 9000, 18000, 24000 or 64000 without this.

AgentLogger is a class wrapping a VS Code output channel and cannot be
constructed in a test process, so this is a structural stand-in. The Proxy makes
any method it does not implement a silent no-op rather than a TypeError
mid-run -- the loop calls more of AgentLogger's surface than the four levels.

Captured lines are folded into the trajectory as `[loop]` entries, so they show
up in dumps and in failure output next to the tool calls they explain. 69 lines
on a single case, including the tier decision and every autonomous approval.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant