Skip to content

fix(verify): grade the deliberate break per test NODE, not per test COMMAND - #100

Merged
stranske merged 1 commit into
mainfrom
claude/friendly-benz-8fba9c
Aug 24, 2026
Merged

fix(verify): grade the deliberate break per test NODE, not per test COMMAND#100
stranske merged 1 commit into
mainfrom
claude/friendly-benz-8fba9c

Conversation

@stranske

Copy link
Copy Markdown
Owner

The defect

local_verify.verify() ran test_cmd twice — once in the worktree, once against the extracted base — and returned ONE verdict from red["ok"]. red["ok"] goes False as soon as any test in the command fails against the base, so one genuinely discriminating test earned a PASS for every tautology sitting beside it in the same file, and named none of them.

Hit for real: a three-test file where 2 of 3 were tautologies returned a bare PASS (Fine-Art-Archive audit finding F3).

The change

_analyse_nodes re-runs the candidate paths alone against the same extracted base with pytest -v --tb=no and reads the per-node outcomes:

pytest outcome disposition
FAILED, ERROR discriminates — part of the proof
PASSED hollownamed by node id
SKIPPED, XFAIL, XPASS inconclusive

One extra subprocess, not one per node, sharing the caller's timeout. The result gains hollow_nodes, node_verdict and node_analysis.counts.

Verified end to end on an F3-shaped repo — 3 tests, 1 real, 2 tautologies:

verdict:      PASS                 <- unchanged, exit code still 0
node_verdict: FAIL_HOLLOW_NODES
hollow:       ['tests/test_geom.py::test_area_type_is_int',
               'tests/test_geom.py::test_module_imports']
counts:       {'nodes': 3, 'discriminates': 1, 'hollow': 2, 'inconclusive': 0}

Advisory by construction

verdict, ok and the CLI exit code keep their exact meaning, so runtime_ac, synthesis_promotion and record_verdict read what they always read. The finding travels on reason, which is what record_verdict writes to outcomes.notes, and node ids go into the completion event's test_ids — the only test-identity field the payload schema has, since an extra key is REJECTED by feedback._sanitize_completion_payload.

Scoping the probe to test_paths rather than the whole test_cmd tightens the evidence a second way: a failure in some unrelated test the command happens to run can no longer read as this change's proof.

It says what it did not check

No pytest, a collection error, non-Python paths, a timeout → node_verdict: INDETERMINATE with the prerequisite named, and a PASS carrying an indeterminate analysis appends that to its own reason:

reason: candidate tests pass live and fail against the base implementation;
        per-node attribution unavailable: the candidate paths do not import
        against the base (pytest reported a collection error), so the file as
        a whole fails there but no single node is attributable

An empty hollow list and "could not look" read identically otherwise, which is the masking this change exists to remove.

Two under-reporting traps closed, each proven by break→revert

  • The probe parses UNTRUNCATED stdout. _run's 4000-char tail drops the earliest nodes of any sizeable file.
  • The probe clears inherited PYTEST_ADDOPTS. Measured with the guard removed: -x stops at the first failing node → false clean node_verdict: PASS with zero hollow nodes; -n auto activates xdist, whose verbose lines put the outcome BEFORE the node id → nothing parses.

Dedup (CLAUDE.md §0)

Hollow detection exists twice, both at coarser grain — local_verify.verify per COMMAND and objective_anchor.arm_signals per ARM (a base_pass boolean) — and testgen_gate.pytest_cmd(collect_only=True) builds a pytest collection command but reads only ok. No per-node parser or attribution exists anywhere in the tree. The improvement log records this precision limit as STILL OPEN. Extended local_verify rather than building anything new; the deliberate-break-verifier capability's lifecycle state is unchanged.

Deliberately not recorded as a capability_propensity.record_repair: that capability's repair proposal was drained on 2026-08-23 for the unrelated NO_BINDING-surface defect in #32, and this precision limit was never in that proposal's defect evidence. Recorded as its own finding on improvement-log item 0.

Tests

Selftest adds the mixed real+tautology fixture on the existing _init_repo, with a DELIBERATE BREAK → REVERT on NODE_OUTCOME_DISPOSITIONS: count a base-passing node as part of the proof and the run reverts to exactly the old masking PASS. Also pins the two parse shapes that would under-report (an outcome word inside a summary error message; xdist's outcome-first line) and the named-prerequisite INDETERMINATE path. Needs only git and pytest, so it runs on a bare runner — no ceiling moved and nothing new is skipped.

No floor change, measured not assumed: the new coverage is in the module selftest, so pytest collects the same 442. Re-measured on the merge result after rebasing onto origin/main 146b458 (#98 and #91 landed underneath):

pytest:     442 passed, 0 failed, 0/26 max skipped (442 collected; floor 442)
selftests:  85 of 85 modules ran, 0/7 max skipped
5 of 5 gates green
mypy ratchet: 20/20 max of 99 exempt, 79 checked  (local_verify stays off the list)

black --line-length 100 clean; mypy src/local_verify.py clean.

Docs: ORCHESTRATOR.md's local_verify entry now tells a reader to read hollow_nodes as well as verdict. No ARCHITECTURE.md/diagram change — local_verify remains a rail gate and no rail/role classification moved.

🤖 Generated with Claude Code

…OMMAND

`local_verify.verify()` ran `test_cmd` twice -- once in the worktree, once against
the extracted base -- and returned ONE verdict from `red["ok"]`. `red["ok"]` goes
False as soon as ANY test in the command fails against the base, so ONE genuinely
discriminating test earned a PASS for every tautology sitting beside it in the same
file, and named none of them. Hit for real: a three-test file where 2 of 3 were
tautologies returned a bare PASS (Fine-Art-Archive audit finding F3).

`_analyse_nodes` re-runs the candidate paths ALONE against the same extracted base
with `pytest -v --tb=no` and reads the per-node outcomes: FAILED/ERROR
`discriminates`, PASSED `hollow` (named by node id), SKIPPED/XFAIL/XPASS
`inconclusive`. ONE extra subprocess, not one per node, sharing the caller's
`timeout`. The result gains `hollow_nodes`, `node_verdict` and
`node_analysis.counts`.

ADVISORY BY CONSTRUCTION. `verdict`, `ok` and the CLI exit code keep their exact
meaning, so `runtime_ac`, `synthesis_promotion` and `record_verdict` read what they
always read. The finding travels on `reason`, which is what `record_verdict` writes
to `outcomes.notes`, and node ids go into the completion event's `test_ids` -- the
only test-identity field the payload schema has, since an extra key is REJECTED by
`feedback._sanitize_completion_payload`.

Scoping the probe to `test_paths` rather than the whole `test_cmd` tightens the
evidence a second way: a failure in some unrelated test the command happens to run
can no longer read as this change's proof.

WHEN IT CANNOT ATTRIBUTE IT SAYS SO, with the prerequisite NAMED -- no pytest, a
collection error, non-Python paths, a timeout -- and a PASS carrying an
INDETERMINATE analysis appends that to its own reason. An empty hollow list and
"could not look" read identically otherwise, which is the masking this change
exists to remove.

Two under-reporting traps closed, each proven by break->revert:
- the probe parses UNTRUNCATED stdout. `_run`'s 4000-char tail drops the earliest
  nodes of any sizeable file.
- the probe clears inherited `PYTEST_ADDOPTS`. `-x` stops at the first failing node
  -> false clean `node_verdict: PASS` with zero hollow nodes; `-n auto` activates
  xdist, whose verbose lines put the outcome BEFORE the node id -> nothing parses.
  Both measured with the guard removed.

DEDUP (CLAUDE.md 0): hollow detection exists twice, both at coarser grain --
`local_verify.verify` per COMMAND and `objective_anchor.arm_signals` per ARM
(base_pass boolean) -- and `testgen_gate.pytest_cmd(collect_only=True)` builds a
pytest collection command but reads only `ok`. No per-node parser or attribution
exists anywhere in the tree. The improvement log records this precision limit as
STILL OPEN; extended `local_verify` rather than building anything new, and the
`deliberate-break-verifier` capability's lifecycle state is unchanged.

Selftest adds the mixed real+tautology fixture on the existing `_init_repo`, with a
DELIBERATE BREAK -> REVERT on `NODE_OUTCOME_DISPOSITIONS`: count a base-passing node
as part of the proof and the run reverts to exactly the old masking PASS. Also pins
the two parse shapes that would under-report (an outcome word inside a summary error
message; xdist's outcome-first line) and the named-prerequisite INDETERMINATE path.
It needs only git and pytest, so it runs on a bare runner -- NO ceiling moved and
nothing new is skipped.

NO FLOOR CHANGE, and that is measured not assumed: the new coverage is in the module
selftest, so pytest collects the same 442. Re-measured on the merge result after
rebasing onto origin/main 146b458 (#98 and #91 landed underneath): 442 passed / 442
collected / floor 442, 85/85 selftests, 5/5 gates, mypy ratchet 20/20 unchanged and
`local_verify` stays off the exempt list.

Deliberately NOT recorded as a `capability_propensity.record_repair`: the
`deliberate-break-verifier` repair proposal was drained on 2026-08-23 for the
unrelated NO_BINDING-surface defect in PR #32, and this precision limit was never in
that proposal's defect evidence. Recorded as its own finding on improvement-log
item 0 instead.

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

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 34 minutes.

View limit details

Limit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f49da03a-1025-4924-96d5-7bfae0e6d030

📥 Commits

Reviewing files that changed from the base of the PR and between 146b458 and c0d3d68.

📒 Files selected for processing (2)
  • ORCHESTRATOR.md
  • src/local_verify.py

Comment @coderabbitai help to get the list of available commands.

@agents-workflows-bot

Copy link
Copy Markdown
Contributor

Workflow source needed

PR #100 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely.

Please do one of:

  • Add <!-- meta:issue:123 --> or a normal Closes #123 / Related to #123 line.
  • Check one Workflow Source option in the PR body.
  • Add a hidden marker such as <!-- workflow-source:local_request -->, <!-- workflow-source:manual_remote -->, <!-- workflow-source:review_followup -->, <!-- workflow-source:sync_campaign -->, or <!-- workflow-source:dependabot -->.
  • Add a workflow source label such as workflow:source-direct-pr, workflow:source-local-request, workflow:source-review-followup, workflow:source-sync, or workflow:no-automation.

Once a valid source is present, this warning will not be reposted.

@stranske
stranske merged commit 529c236 into main Aug 24, 2026
17 checks passed
@stranske
stranske deleted the claude/friendly-benz-8fba9c branch August 24, 2026 05:32
stranske pushed a commit that referenced this pull request Aug 24, 2026
#94 and #100 landed floor/gate work while this branch was open, conflicting only on
.verify-floor.json. Resolved by KEEPING MAIN'S note and re-applying just the two facts this branch
owns — the exempt bound (16) and the collected count measured on the MERGE RESULT (448, up from 442
because #94/#100 added tests). Re-verified on the result rather than assumed: 448 passed, 0 failed,
0 skipped, 85/85 selftests, 43/43 can-fire, 5/5 gates; mypy clean over 99 modules.
stranske added a commit that referenced this pull request Aug 24, 2026
…cked) (#99)

Batch four: 338 -> 308 findings, exempt bound 20 -> 16, 83 of 99 modules checked.

The lesson, learned at the cost of four failing tests: an ANNOTATION is not a behaviour change, a COERCION is. Two fixes in feedback.py were coercions dressed as type fixes — dict(selector[role]) discarded mutations the function returns through 'selector', and str(validate_resolved_worker_model(...) or '') turned a deliberate None refusal into an empty string. Both reverted to non-mutating forms; test_feedback_model_provenance x3, test_model_profile_trial and periodic_report's selftest caught them.

Also fixed a batch-3 mistake: the repo -> repo_arg rename rewrote keyword ARGUMENT names too.

Cleared: codemod_lane, switch_review (stale_runners' 'now: int' was wrong — the body floats it), issue_readiness (normalize_title declared str while its own selftest asserts normalize_title(None) == ''), feedback.

Merged main mid-flight (#94/#100 floor work); resolved by keeping main's note and re-applying only this branch's two facts, re-verified on the merge result: 448 passed, 0 failed, 0 skipped, 85/85 selftests, 5/5 gates.
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