From cc3ebdb4f4b30803e24365fb55343b6429d19e7f Mon Sep 17 00:00:00 2001 From: Joel Teply Date: Sun, 16 Aug 2026 20:19:43 -0500 Subject: [PATCH] =?UTF-8?q?fix(swe):=20ONE=20env=20root=20=E2=80=94=20a=20?= =?UTF-8?q?second=20cache=20made=20the=20benchmark=20unable=20to=20report?= =?UTF-8?q?=20its=20own=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two SWE env roots existed and neither named the other: ~/.continuum/benchmarks/swe/envs swe_cache_dir()/envs — LIVE. 46 envs, 8 repos. ~/.continuum/cache/swe-envs legacy grade_local.py default — 14 envs, 3 repos, untouched since Aug 4. Nothing failed. Each root was internally consistent, so whichever one you looked at answered confidently and never mentioned the other. WHAT IT COST, 2026-08-17: `ls` on the retired root showed sympy/flask/requests only. That became the reported finding — "77% of staged instances have no environment; the env builder only works for three repos" — presented as the benchmark's root cause, with a design approved on top of it. The live root held 46 envs across all 8 repo families: 95% coverage, astropy and django and pytest and pylint and sphinx all present. Same question, two directories, opposite answers, and the wrong one drove a decision. THE GENERAL DEFECT, which is why this gets a guard and not just a delete: a cache with two roots cannot report its own coverage. Every reader picks one and receives a self-consistent lie. That is not a stale-data problem — it is a missing constraint ([[the-same-bug-at-two-sites-is-a-missing-constraint-not-two-bugs]]). THREE PARTS: 1. Retired root DELETED (154MB, 14 venvs). Nothing read it but the quarantined script. 2. `grade_local.py --env-root` now defaults to the canonical path, so even if the legacy script is run it writes where the core reads. A second cache cannot re-form by accident. 3. GUARD: `the_swe_env_root_has_exactly_one_spelling` scans crate source (comments stripped) and fails if either root appears as a path literal anywhere. Needles are ASSEMBLED at runtime — a literal needle matched its own declaration and the guard failed on itself on first run, which is also its positive control: it demonstrably scans rather than passing vacuously. `swe_cache_dir()`'s doc now carries the incident and the rule: derive the envs dir from the function, never spell the path. Corollary written down for the next person measuring coverage — read the root from the code, never from a directory you found by name. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo --- .../continuum-core/src/cognition/swe_bench.rs | 85 +++++++++++++++++++ legacy/benchmarks/swe/grade_local.py | 17 +++- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/core/continuum-core/src/cognition/swe_bench.rs b/core/continuum-core/src/cognition/swe_bench.rs index a5a2ae19b6..5aed39e1de 100644 --- a/core/continuum-core/src/cognition/swe_bench.rs +++ b/core/continuum-core/src/cognition/swe_bench.rs @@ -208,6 +208,29 @@ pub fn reap_orphaned_solve_runs() -> Vec { /// Where cached datasets and per-instance environments live. A governed cache class, not a /// scratch dir — see the disk-eviction contract. +/// +/// # THIS IS THE ONLY SWE ENV ROOT. `swe_cache_dir()/envs`, nowhere else. +/// +/// A second root used to exist — `~/.continuum/cache/swe-envs`, the default of the retired +/// `legacy/benchmarks/swe/grade_local.py`. Both directories held real venvs. Neither named +/// the other. Nothing failed loudly, because each was internally consistent. +/// +/// On 2026-08-17 that cost a full misdiagnosis with a decision attached: `ls` on the retired +/// root showed 14 envs across 3 repos, which became the reported finding *"77% of staged +/// instances have no environment — the env builder only works for sympy/flask/requests"*. +/// A design was approved on it. The live root held **46 envs across 8 repos** — 95% coverage, +/// every repo present. Same question, two directories, opposite answers. The retired root is +/// now deleted and the legacy script's default points here. +/// +/// The general defect, worth recognising before it regrows elsewhere: a cache with two roots +/// cannot report its own coverage, because every reader picks one and gets a self-consistent +/// lie. If you add a second location for anything cached here — a mirror, a per-node copy, a +/// migration staging dir — it needs to be derived from THIS function, not spelled out again. +/// A path literal repeated in a second file is the whole failure mode +/// ([[the-same-bug-at-two-sites-is-a-missing-constraint-not-two-bugs]]). +/// +/// Corollary for anyone measuring env coverage: read the root from here, never from a path +/// you remember or a directory you found by name. pub fn swe_cache_dir() -> PathBuf { let home = std::env::var("HOME").unwrap_or_else(|_| "/tmp".into()); PathBuf::from(home) @@ -1523,6 +1546,68 @@ pub async fn grade( mod tests { use super::*; + // what this catches: a SECOND swe env root. On 2026-08-17 two roots existed — + // `swe_cache_dir()/envs` (live, 46 envs / 8 repos) and `~/.continuum/cache/swe-envs` + // (retired, 14 envs / 3 repos, the legacy python default). Nothing failed: each root was + // internally consistent, so whichever you `ls`ed answered confidently. Reading the retired + // one produced "77% of staged instances have no environment", which was reported as the + // benchmark's root cause and had a design approved on it. Truth was 95% coverage. + // + // A cache with two roots cannot report its own coverage. So: the env root is derived from + // swe_cache_dir() and appears as a path literal NOWHERE else in the crate. If you need the + // envs dir, call the function. See swe_cache_dir's doc for the full incident. + #[test] + fn the_swe_env_root_has_exactly_one_spelling() { + fn walk(dir: &std::path::Path, out: &mut Vec<(String, String)>) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for e in entries.flatten() { + let p = e.path(); + if p.is_dir() { + walk(&p, out); + } else if p.extension().is_some_and(|x| x == "rs") { + if let Ok(t) = std::fs::read_to_string(&p) { + out.push((p.to_string_lossy().to_string(), t)); + } + } + } + } + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = Vec::new(); + walk(&root, &mut files); + + // The retired root, and any hand-spelled sibling of the live one. ASSEMBLED at + // runtime, never written whole: a literal needle would match its own declaration and + // the guard would fail on itself (it did, first run). Comments are stripped too, so + // this test's prose and swe_cache_dir's doc can name the paths freely. + let retired = format!("cache/{}-envs", "swe"); + let live_spelled_out = format!("benchmarks/{}/envs", "swe"); + let banned = [retired.as_str(), live_spelled_out.as_str()]; + let mut hits = Vec::new(); + for (path, text) in &files { + for (n, raw) in text.lines().enumerate() { + let code = match raw.find("//") { + Some(i) => &raw[..i], + None => raw, + }; + for b in banned { + if code.contains(b) { + hits.push(format!("{path}:{} → {}", n + 1, code.trim())); + } + } + } + } + assert!( + hits.is_empty(), + "a SECOND spelling of the swe env root appeared — this is exactly how the \ + 2026-08-17 coverage misdiagnosis happened (two roots, both real, neither naming \ + the other, opposite answers to the same question). Derive it from \ + `swe_cache_dir()` instead of writing the path:\n {}", + hits.join("\n ") + ); + } + // what this catches: the false-env-void misgrade (pytest-11143 attempt 3, live // 2026-08-12) — a candidate patch that CREATED a file survived `git checkout .`, the // leftover module broke the "pristine" p2p re-run, and a REAL capability regression was diff --git a/legacy/benchmarks/swe/grade_local.py b/legacy/benchmarks/swe/grade_local.py index 5a5773cd27..725dbf0205 100644 --- a/legacy/benchmarks/swe/grade_local.py +++ b/legacy/benchmarks/swe/grade_local.py @@ -199,8 +199,21 @@ def main(): help="build the environment instead: a venv from BASE_PYTHON with " "pytest + the repo installed editable. Cached per instance under " "--env-root, so a re-grade is instant.") - ap.add_argument("--env-root", default=os.path.expanduser("~/.continuum/cache/swe-envs"), - help="where --auto-venv caches per-instance environments") + # THE CANONICAL ENV ROOT, and the ONLY one. This default used to be + # `~/.continuum/cache/swe-envs` — a SECOND cache that the Rust core never reads and + # never writes. Both roots existed, both held real venvs, and neither named the other. + # On 2026-08-17 that cost a full misdiagnosis: `ls`ing the retired root showed 14 envs + # across 3 repos and produced the headline "77% of staged instances have no + # environment", reported and acted on. The live root held 46 envs across 8 repos — + # 95% coverage. Same question, two directories, opposite answers. + # Keep this pointed at `swe_bench::swe_cache_dir()/envs`. One root or the confusion + # regrows. ([[the-same-bug-at-two-sites-is-a-missing-constraint-not-two-bugs]]) + ap.add_argument("--env-root", + default=os.path.expanduser("~/.continuum/benchmarks/swe/envs"), + help="where --auto-venv caches per-instance environments. THE canonical " + "root, shared with the Rust core (cognition::swe_bench::ensure_env) " + "— do not point this somewhere else, a second cache is how the " + "2026-08-17 misdiagnosis happened.") ap.add_argument("--pytest", default="pytest<7", help="pytest pin — repos older than pytest 7 need <7 (conftest using " "`monkeypatch.notset`, removed in 7). The gold gate catches a wrong pin.")