Skip to content

test(_util): match error ANNOUNCEMENTS, not the word "error" (unblocks #70) - #74

Merged
Brian Krabach (bkrabach) merged 2 commits into
mainfrom
lane/wp6-error-regex-scope
Sep 3, 2026
Merged

test(_util): match error ANNOUNCEMENTS, not the word "error" (unblocks #70)#74
Brian Krabach (bkrabach) merged 2 commits into
mainfrom
lane/wp6-error-regex-scope

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

Unblocks #70. model_performance-wp6. Diff is two files, both under tests/.

The defect

tests/_util.py's assert_no_silent_failure used re.compile(r"\berror\b", re.IGNORECASE). That is not a test for an error being emitted — it is a test for the word appearing, and a command is allowed to talk about errors while succeeding.

doctor prints every assumption's own DESCRIPTION on a healthy run, so in a catalogue whose entire subject matter is error handling, one honest sentence turns a green run red. Reproduced locally against PR #70's own head (20557bc), doctor --quick, exit 0:

  [PASS] read.unavailable_not_absent   an infrastructure read failure raises BeadsUnavailableError
  with its cause intact on read/claim and reports UNAVAILABLE (not ERROR) per project, while
  genuine absence on a healthy database still reports plain 'not found'

\berror\b matches ERROR at offset 302 of the combined output → assert_no_silent_failure raises → test_doctor_quick_succeeds_against_the_real_installed_bd FAILS on a completely healthy system.

Rewording #70's description was explicitly not done: the trap is in the predicate, and the next assumption would hit it.

The fix

Five announcement shapes instead of one word. Every shape is justified by output this repo actually emits:

shape pattern why
error-colon \berror\s*: Error: unknown command "reclaim" (the shipped reap bug); ERROR: <cause> (adapter.py's status for an unreadable project, as instances renders it mid-column); ERROR: tokens file not found (gateway.py:141); argparse/cobra <prog>: error: <msg>. Not anchored to line start — a real announcement is routinely preceded by a program name or a column.
json-error-field "error"\s*:\s*<non-empty> supervisor.py:177,189 writes {"error": str(e)} into printed payloads. null/"" is the no error reading and is deliberately not matched.
error-running \berror\s+running\b announcement without a colon
unknown-command \bunknown command\b the shipped bug's own words — new coverage, contains no "error"
python-traceback ^Traceback \(most recent call last\): a crash printed while exiting 0 — new coverage, contains no "error"

error_announcement() returns (shape_name, matched_text) and the assertion message carries both, so a CI reader can tell a real announcement from a false positive without re-deriving the regex.

This is not purely a loosening: two of the five shapes are coverage the parent commit did not have.

The guarantee is NOT retired

tests/unit/test_error_announcement_detection.py pins both directions in one file (per the acceptance criteria — the failure mode being fenced is the two collapsing into each other). Ten parametrised guilty rows plus three assertion-level tests, including Error: unknown command "reclaim" + exit 0 → must still raise.

Eight of those ten pass on the parent commit AND on this branch. That is the proof the guard survived, rather than an assurance that it did.

Fail-before / fail-after

Full log: docs/lanes/wp6-error-regex-scope/evidence/fail-before.txt.

Parent ea233a7 (tests/_util.py restored to the bare \berror\b, this branch's test file in place):

$ .venv/bin/python -m pytest tests/unit/test_error_announcement_detection.py -q
FAILED ...::test_pr70_description_is_prose_not_an_announcement
FAILED ...::test_healthy_doctor_run_carrying_pr70s_description_is_not_a_silent_failure
FAILED ...::test_mentioning_errors_is_not_announcing_one[reports UNAVAILABLE (not ERROR) per project]
FAILED ...::test_mentioning_errors_is_not_announcing_one[this command never reports an error it did not observe]
FAILED ...::test_mentioning_errors_is_not_announcing_one[ERROR and UNAVAILABLE are different readings of the same field]
FAILED ...::test_mentioning_errors_is_not_announcing_one[{"ok": true, "error": null}]
FAILED ...::test_mentioning_errors_is_not_announcing_one[{"ok": true, "error": ""}]
FAILED ...::test_real_error_announcements_are_still_caught[unknown command "reclaim" for "bd"]
FAILED ...::test_real_error_announcements_are_still_caught[Traceback (most recent call last):\n  File "cli.py", line 1\n]
FAILED ...::test_the_failure_message_names_which_shape_fired
10 failed, 13 passed in 0.24s

First seven = the defect (prose read as an emitted error, including #70's real sentence). Next two = new coverage. Last = the new shape-naming API. The 13 passing include eight of the ten real announcements.

Assertions are written against looks_like_error_text/assert_no_silent_failure — the API that existed before this change — so the parent run reports the real diagnosis instead of an AttributeError.

This branch: 23 passed in 0.22s.

Would #70 now pass? Yes.

docs/lanes/wp6-error-regex-scope/evidence/pr70-would-now-pass.txt, from a real run, not a fixture: git worktree of origin/pr-70 @ 20557bc, its own venv, doctor --quick against the real workspace root so sweeps.alive passes (the healthy-CI condition) → exit 0, All 35 assumptions hold.

predicate on those exact bytes
parent \berror\b MATCH at offset 302 → raises → the CI failure blocking #70, reproduced
this branch no announcement → does not raise → check PASSES

Does not re-run #70's CI, and does not touch jyg.

Tiers run (all of them, by name)

make venv installs .[dev,web] and modules/tool-work-tracker[dev], so tier 5 did not silently fail at collection. Raw logs under docs/lanes/wp6-error-regex-scope/evidence/.

tier command result
1 unit make test-unit 813 passed (45.5s)
2 integration make test-integration 354 passed, 3 skipped (17m52s)
3 cli make test-cli 80 passed, 1 failed (6m56s) — see below
4 ledger make test-ledger 26 passed (0.6s)
4b mutation harness make ledger-mutate exit 0, no unproven holes
5 modules make test-module 113 passed (7m32s)
lint+types make check ruff clean, pyright 0 errors

The one failure is model_performance-jyg, not this change. test_doctor_quick_succeeds_against_the_real_installed_bd dies at the earlier assert result.returncode == 0 because sweeps.alive FAILs under the isolated AMPLIFIER_WORK_TRACKER_ROOT (conftest.py:308). Proven pre-existing by re-running that single test with parent's _util.py: 1 failed in 46.35s, identical signature (evidence/jyg-preexisting.txt).

That is also why the regression test is tier 1: tier 3 structurally cannot reach assert_no_silent_failure while jyg is present, which is exactly why this defect was invisible locally and only appeared in CI. I tried to bypass it by exporting the real root; run_cli uses setdefault, so the monkeypatched value wins — attempt recorded rather than dropped (evidence/jyg-cannot-be-bypassed-from-outside.txt).

c0e and the test_supervisor_web.py port flake did not reproduce in these runs. Reported as observed, not claimed fixed.

Why option (1) was NOT taken

Option (1) — doctor emitting descriptions on a channel the check can exclude — is not in this PR.

  1. The owner decision (2026-09-03) pins scope to option (2) and says to file option (1) as a follow-up.
  2. It changes doctor's output contract; this diff is two files under tests/.
  3. It would not have been sufficient alone. The evidence run surfaced a genuine Error: announcement arriving on stderr from bd itself, under a doctor run that healed the condition and exited 0. Filtering doctor's own [PASS] rows would not exclude that. The two options are complements, not alternatives.

I did not conclude option (1) is necessary, so there was nothing to stop for.

Discovered work, filed not absorbed

model_performance-kxk (discovered-from wp6) — a HEALED dirty-schema migration still leaks bd's own Error: line to stderr while doctor exits 0. Observed once, not reproducible on the next run; a real intermittent flake source for any tests/cli test funnelling through assert_no_silent_failure. Product-side output-contract question, not a test-regex one. Capture: evidence/pr70-doctor-quick.run1-transient.stderr.txt.

Deviations, called out rather than slipped in

  1. Regression test is tier 1, not tier 3 (reason above). Tier 3 left untouched.
  2. unknown command + Traceback are new shapes — both named in the item/owner decision, both strengthen the guard, neither can produce the fix: pin the dolt scan directory; stop reporting infrastructure failure as "item not found" #70 false positive.
  3. The JSON "error": <non-empty> shape was not named in the item. It preserves coverage the parent had via \berror\b for supervisor.py's printed payloads, which a colon-only rule would have dropped.
  4. No assumption description was reworded, per the scope-out.

Spend: $0.00 of a $0.00 authority. No API, no DTU, no infrastructure created.

Full note: docs/lanes/wp6-error-regex-scope/DONE-NOTE.md.

@bkrabach
Brian Krabach (bkrabach) marked this pull request as ready for review September 3, 2026 10:13
@bkrabach

Copy link
Copy Markdown
Collaborator Author

Manager verification — FIX, fail-before reproduced in both directions

Head a963da6, base ea233a7.

Fail-before, this PR's tests against main's tests/_util.py:

10 failed, 13 passed

The failures split across both properties, which is what makes this a real fix rather than a loosening:

  • test_mentioning_errors_is_not_announcing_one[...] — the false positives being removed;
  • test_real_error_announcements_are_still_caught[unknown command "reclaim" for "bd"] and the Traceback case — shapes main's bare \berror\b does NOT catch, which this PR adds.

So the guard gets stronger, not weaker: the shipped reap bug this tier is named for (Error: unknown command "reclaim" alongside exit 0) is still caught, and a bare traceback now is too.

Tiers: unit 813 passed · ledger 26 · ruff check + format clean (150 files) · doctor 34/34 (measured; 34 is correct for this base — #72's two assumptions came later) · cli 80 passed, 1 failed.

That one cli failure is test_doctor_quick_succeeds_against_the_real_installed_bd = model_performance-jyg, pre-existing and unrelated: it dies at the first assertion (returncode == 0) because the isolated test root has no sweep heartbeat, so execution never reaches assert_no_silent_failure at all. Worth stating plainly: in this environment jyg masks the very defect this PR fixes, which is exactly why it only ever surfaced in CI.

Does it actually unblock #70? The lane did the right experiment rather than a synthetic one — it ran doctor --quick from #70's own head (20557bc) against the real workspace root (exit 0, the healthy-CI condition):

I am merging on that evidence and will re-run #70's CI next.

Squash + --admin (base-branch policy, as with #72/#67/#353).

amplifier-lane added 2 commits September 3, 2026 03:13
`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).
@bkrabach

Copy link
Copy Markdown
Collaborator Author

Rebased onto 058b4e2 (main moved — my own #72 landed while this verified). Clean rebase, no conflict. Re-verified rather than assumed: unit+ledger 839 passed, doctor 36/36 measured (36 is correct now that #72's two defer/block assumptions are on main — it was 34 on the old base). Head is now 2760376.

@bkrabach
Brian Krabach (bkrabach) merged commit 802c204 into main Sep 3, 2026
2 checks passed
@bkrabach
Brian Krabach (bkrabach) deleted the lane/wp6-error-regex-scope branch September 6, 2026 02:35
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