fix: pin the dolt scan directory; stop reporting infrastructure failure as "item not found" - #70
Conversation
Manager verification (independent, scratch clone)Verified in a fresh clone at PR head Classification: FIX. Three read paths report infrastructure unavailable as The failure is actually removed, not tolerated — measured end to endBeyond the unit tests, I drove the fixed helper directly from a 20,000-entry The directory Fail-before, reproducedNew tests at parent …including The no-blurring guardrail is real, not asserted. All tiers
The
Scope16 files: The type-based distinction is the right call over a substring callers must grep: Squash + |
Manager: NOT merging — CI is red, and this PR is the cause. Back to draft.Everything in my verification comment above still holds: the fix is real, the Root cause — one word
_ERROR_WORD_RE = re.compile(r"\berror\b", re.IGNORECASE)
Why neither the lane nor my first pass caught it — worth knowingThis is the same test that already has an unrelated pre-existing failure …so the run never reaches What it needsReword the description to avoid a bare The bigger finding, filed separately
No other gate failed. This is a one-line prose change away from merging. |
|
Correction to the comment above: the follow-up item filed for the underlying trap is |
`assert_no_silent_failure` used a bare `\berror\b`, which is a test for the word appearing rather than for an error being emitted. `doctor` prints every assumption's own DESCRIPTION on a healthy run, so in a catalogue whose subject is error handling one honest sentence turns a green run red -- `read.unavailable_not_absent`'s "... reports UNAVAILABLE (not ERROR) per project ..." matched at offset 125 and failed test_doctor_quick_succeeds_against_the_real_installed_bd on a completely healthy system, blocking PR #70. Replaced with five announcement shapes, each justified by output this project actually emits: `error:` (the shipped `reap` bug, adapter's `ERROR: <cause>` project status, gateway's `ERROR: tokens file...`, argparse's `prog: error: msg`), a JSON `"error": <non-empty>` field (supervisor writes one into printed payloads), `error running`, `unknown command`, and a Python `Traceback`. The last two are NEW coverage -- neither contains the word "error" at all. tests/unit/test_error_announcement_detection.py pins both directions in one file: the innocent case (PR #70's real description text) and the guilty one (`Error: unknown command "reclaim"` with exit 0 still raises).
… the lane artifact root
…#70) (#74) * test(_util): match error ANNOUNCEMENTS, not the word "error" `assert_no_silent_failure` used a bare `\berror\b`, which is a test for the word appearing rather than for an error being emitted. `doctor` prints every assumption's own DESCRIPTION on a healthy run, so in a catalogue whose subject is error handling one honest sentence turns a green run red -- `read.unavailable_not_absent`'s "... reports UNAVAILABLE (not ERROR) per project ..." matched at offset 125 and failed test_doctor_quick_succeeds_against_the_real_installed_bd on a completely healthy system, blocking PR #70. Replaced with five announcement shapes, each justified by output this project actually emits: `error:` (the shipped `reap` bug, adapter's `ERROR: <cause>` project status, gateway's `ERROR: tokens file...`, argparse's `prog: error: msg`), a JSON `"error": <non-empty>` field (supervisor writes one into printed payloads), `error running`, `unknown command`, and a Python `Traceback`. The last two are NEW coverage -- neither contains the word "error" at all. tests/unit/test_error_announcement_detection.py pins both directions in one file: the innocent case (PR #70's real description text) and the guilty one (`Error: unknown command "reclaim"` with exit 0 still raises). * docs(lane wp6): DONE-NOTE + fail-before, tier, and #70 evidence under the lane artifact root --------- Co-authored-by: amplifier-lane <lane@localhost>
…re as "item not found" Root cause (measured by lane model_performance-rpz, harness probes/rpz-dolt-error-misreport/repro.sh): the `dolt` CLI enumerates its data directory and lstat()s every entry on EVERY invocation -- including pure client mode against the already-running shared server -- and with no --data-dir given that directory is the INHERITED cwd of whatever agent happened to call us. If an entry vanishes between readdir and lstat, dolt aborts the whole query. Same query, same server: 0.031s from a 2-entry cwd, 0.805s from /tmp (52,281 entries). Under a churning 40k-entry directory: 6 failures in 25 attempts. 1. REMOVE the failure. `_dolt_scan_dir()` -- a stable, empty, memoised directory we own -- is applied two independent ways, each measured at 0/25 under the same load: `cwd=` on the two hot helpers, and `--data-dir` via `_dolt_conn_args` so every dolt invocation in the module gets it by construction. 2. RETRY it. The retry classification lived only in `Beads._run` (which wraps `bd`); the direct dolt-SQL path had none, across 19 call sites. `_run_dolt_sql_bounded` gives it the same bounded transport retry, and "failed to load database names" joins `_RETRYABLE_CONNECTION`. 3. STOP LYING past the budget. `BeadsUnavailableError` makes "infrastructure unreachable" structurally distinct from "not found" -- a TYPE, not a substring callers must grep. `claim_item` (9 of 12 measured attempts said "item not found" about an item that existed) and `get_readonly` (2 of 8 denied an item the session HELD, discarding the cause entirely) re-raise it untouched. `project_summary` reports `UNAVAILABLE: ...`, distinct from both `ok` and `ERROR` (5 of 10 attempts printed a healthy project as ERROR with null counts). 4. FENCE it. New contract assumption `read.unavailable_not_absent` checks BOTH directions -- and genuine absence on a healthy database still reports absence in exactly the same words, because a fix that makes every "not found" say "maybe transient" is a second lie. `doctor` now measures 35/35 (read off doctor, not computed); AGENTS.md updated in both places.
…ane artifact root
20557bc to
a097365
Compare
Manager: rebased onto
|
|
CI GREEN. Merging: FIX with a reproduced fail-before (18 failed at the parent with the parent's own |
…he service does not serve `_check_sweeps_alive` joined a SERVICE-scoped fact (`describe_service()` -- one singleton unit per user) to a ROOT-scoped one (a heartbeat file under whichever workspace root `doctor` was pointed at). That join is only sound when the two refer to the same root. Against any other root the heartbeat is absent BY CONSTRUCTION, and the check reported that absence as "no heartbeat ever recorded ... the loop may never have started" -- a hard FAIL, exit 1, on a machine whose sweep loops were provably running fine. Every isolated test root is such a root, so tests/cli/test_cli_surface.py::test_doctor_quick_succeeds_against_the_real_ installed_bd failed on any developer machine whose service serves a real workspace -- and, because `assert returncode == 0` came first, it MASKED every later assertion in that test. Measured twice: model_performance-wp6 (announcement-predicate collision, invisible locally, CI-only, blocked PR #70 for days) and model_performance-kxk. The assumption is right; it is simply not evaluable against a root the supervisor was never given. `_serve_argv_tail` already bakes `--root` into the unit as an explicit argument, so the served root is readable: `ServiceInfo.served_root` reads it back (systemd ExecStart, launchd ProgramArguments), and both sweep checks report `unknown` on a mismatch -- following the precedent `sweeps.reclaiming` set in model_performance-oy4 (46d7da4) for exactly this shape. The FAIL path is untouched where it matters: against the SERVED root, a missing, stale, or dead-pid heartbeat is still a loud failure, and a sweep that failed on every project still fails `sweeps.reclaiming`. When the served root cannot be determined at all, the checks evaluate the heartbeat anyway and can still FAIL -- "cannot tell" must never become a way to make a real dead-loop failure disappear. Separately, the masking itself is fixed: the cli test now checks its three properties independently and reports them together (`_util.assert_doctor_run_is_clean`), so the next environmental exit-1 from any cause cannot hide the next real defect. `assert_no_silent_failure` and its announcement predicate are called unchanged. doctor MEASURED at 38/38 (unchanged).
…he service does not serve `_check_sweeps_alive` joined a SERVICE-scoped fact (`describe_service()` -- one singleton unit per user) to a ROOT-scoped one (a heartbeat file under whichever workspace root `doctor` was pointed at). That join is only sound when the two refer to the same root. Against any other root the heartbeat is absent BY CONSTRUCTION, and the check reported that absence as "no heartbeat ever recorded ... the loop may never have started" -- a hard FAIL, exit 1, on a machine whose sweep loops were provably running fine. Every isolated test root is such a root, so tests/cli/test_cli_surface.py::test_doctor_quick_succeeds_against_the_real_ installed_bd failed on any developer machine whose service serves a real workspace -- and, because `assert returncode == 0` came first, it MASKED every later assertion in that test. Measured twice: model_performance-wp6 (announcement-predicate collision, invisible locally, CI-only, blocked PR #70 for days) and model_performance-kxk. The assumption is right; it is simply not evaluable against a root the supervisor was never given. `_serve_argv_tail` already bakes `--root` into the unit as an explicit argument, so the served root is readable: `ServiceInfo.served_root` reads it back (systemd ExecStart, launchd ProgramArguments), and both sweep checks report `unknown` on a mismatch -- following the precedent `sweeps.reclaiming` set in model_performance-oy4 (46d7da4) for exactly this shape. The FAIL path is untouched where it matters: against the SERVED root, a missing, stale, or dead-pid heartbeat is still a loud failure, and a sweep that failed on every project still fails `sweeps.reclaiming`. When the served root cannot be determined at all, the checks evaluate the heartbeat anyway and can still FAIL -- "cannot tell" must never become a way to make a real dead-loop failure disappear. Separately, the masking itself is fixed: the cli test now checks its three properties independently and reports them together (`_util.assert_doctor_run_is_clean`), so the next environmental exit-1 from any cause cannot hide the next real defect. `assert_no_silent_failure` and its announcement predicate are called unchanged. doctor MEASURED at 38/38 (unchanged).
… service does not serve (model_performance-jyg) (#77) * fix(doctor): `sweeps.alive` reports `unknown`, not FAIL, for a root the service does not serve `_check_sweeps_alive` joined a SERVICE-scoped fact (`describe_service()` -- one singleton unit per user) to a ROOT-scoped one (a heartbeat file under whichever workspace root `doctor` was pointed at). That join is only sound when the two refer to the same root. Against any other root the heartbeat is absent BY CONSTRUCTION, and the check reported that absence as "no heartbeat ever recorded ... the loop may never have started" -- a hard FAIL, exit 1, on a machine whose sweep loops were provably running fine. Every isolated test root is such a root, so tests/cli/test_cli_surface.py::test_doctor_quick_succeeds_against_the_real_ installed_bd failed on any developer machine whose service serves a real workspace -- and, because `assert returncode == 0` came first, it MASKED every later assertion in that test. Measured twice: model_performance-wp6 (announcement-predicate collision, invisible locally, CI-only, blocked PR #70 for days) and model_performance-kxk. The assumption is right; it is simply not evaluable against a root the supervisor was never given. `_serve_argv_tail` already bakes `--root` into the unit as an explicit argument, so the served root is readable: `ServiceInfo.served_root` reads it back (systemd ExecStart, launchd ProgramArguments), and both sweep checks report `unknown` on a mismatch -- following the precedent `sweeps.reclaiming` set in model_performance-oy4 (46d7da4) for exactly this shape. The FAIL path is untouched where it matters: against the SERVED root, a missing, stale, or dead-pid heartbeat is still a loud failure, and a sweep that failed on every project still fails `sweeps.reclaiming`. When the served root cannot be determined at all, the checks evaluate the heartbeat anyway and can still FAIL -- "cannot tell" must never become a way to make a real dead-loop failure disappear. Separately, the masking itself is fixed: the cli test now checks its three properties independently and reports them together (`_util.assert_doctor_run_is_clean`), so the next environmental exit-1 from any cause cannot hide the next real defect. `assert_no_silent_failure` and its announcement predicate are called unchanged. doctor MEASURED at 38/38 (unchanged). * docs(lane jyg): DONE-NOTE + before/after evidence (doctor MEASURED 38/38; five tiers green) --------- Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
2 participants
Pin the dolt scan directory; stop reporting infrastructure failure as "item not found"
Fixes
model_performance-8zv. Diagnosis is lanemodel_performance-rpz's(
ai-notesbranchlane/rpz-dolt-error-misreport,a6c6e03); this PR fixes,measures, and fences it. $0 spend — no API calls, no DTU, no infrastructure.
Full lane note:
docs/lanes/8zv-dolt-scan-dir/DONE-NOTE.md.Raw evidence:
docs/lanes/8zv-dolt-scan-dir/evidence/.The defect
The
doltCLI enumerates its data directory andlstat()s every entry on everyinvocation — including the pure client mode (
--host/--portagainst the already-runningshared server) that every
_dolt_*helper uses, where no local database is relevant.With no
--data-dir, that directory is the inherited cwd — whatever directory thecalling agent happened to be in. If an entry vanishes between
readdirandlstat,dolt aborts the whole query. So the failure rate of a work-tracker read was set by an
unrelated process's litter in
/tmp.Past that, three sites flattened the failure into a claim about the data:
claim_itemget_readonlyproject_summaryERRORwith null counts — 5 of 10get_readonlyis the worst: it is the exact pathcontext/awareness.mdhazard #6 tellsagents to trust as the safe recovery after an ambiguous write.
The change
_dolt_scan_dir()— a stable, empty, memoised directory we own —applied two independent ways:
cwd=on the hot helpers, and--data-dirin_dolt_conn_args()so every dolt invocation gets it by construction._run_dolt_sql_bounded()gives the direct-SQL path (19 unretried callsites) the same bounded transport retry
Beads._runalready had forbd;"failed to load database names"joins_RETRYABLE_CONNECTION.BeadsUnavailableErrormakes "unreachable" a type,not a substring callers must grep.
claim_item/get_readonlyre-raise it untouched;project_summaryreportsUNAVAILABLE: …, distinct from bothokandERROR.doctorassumptionread.unavailable_not_absent, both directions.CLI/web follow-on:
instancesprints the new status verbatim; the dashboard paints anunreachable project amber Unavailable, not crimson Broken.
Measurement — rpz's harness, run verbatim, raw counts
25/25 succeed with the scan directory pinned, by either remedy, in both runs.
Run B's load was harsher (12/25 raw failures vs 7/25), so the fixed CLI was not merely
luckier. The item predicted 6/25 unpinned; measured today it is 7/25 and 12/25 — same
defect, worse rate (this host's
/tmpgrew from 52,281 to 66,292 entries). Stated ratherthan smoothed.
Captures:
.amplifier/evaluation/treatment-validation/2026-09-02-model_performance-8zv/.The no-blurring guardrail
A fix that makes every "not found" say "maybe transient" replaces one lie with another.
Every transient-failure test here is paired with a healthy-database test asserting the
absence wording is unchanged — one of them byte for byte. The three CLI-tier
guardrails pass on the parent commit and on this branch. That is the point of them.
Fail-before
New tests run against the parent (
2468a69,src/reverted, tests kept) —docs/lanes/8zv-dolt-scan-dir/evidence/fail-before.txt:The passes are exactly the unchanged-behaviour guardrails.
Tiers — all four, by name, plus the modules tier
make test-unitmake test-integrationmake test-clitest_doctor_quick_succeeds_against_the_real_installed_bd(model_performance-jyg)make test-ledgermodules/tool-work-tracker/teststest_explicit_resolve_refusal_after_reap_…(model_performance-c0e)make checkpyright0 errorsdoctorjygwas checked, not assumed: its failing line is[FAIL] sweeps.alive — no heartbeat ever recorded…, and the newread.unavailable_not_absentreports[PASS]inside thatsame run.
doctor's 35 is measured, not computed;AGENTS.mdupdated in all threeplaces carrying the old count.