fix(verify): give the per-node break finding its two consumers - #114
Merged
Conversation
The previous change landed the measurement; NOTHING READ IT. `hollow_nodes`,
`node_verdict` and `node_analysis` had zero readers outside `local_verify` itself,
which in this repo is the built-and-forgotten failure mode wearing a fresh coat of
paint. Two consumers, both reporting, neither gating.
1. `runtime_ac._completed_result` DROPPED local_verify's reason on a PASS
(`reason = None if status == "PASS"`), throwing the node names away in exactly
the case they exist for. It now carries the reason when `_hollow_carry_condition`
holds -- named hollow nodes, or a `node_verdict` that is anything but PASS -- and
stays silent on a clean PASS so this cannot become constant noise. Evidence with
no per-node fields at all (an older local_verify) counts as clean, so the prior
behaviour is unchanged for it.
SAFE BY CONSTRUCTION, and checked rather than assumed: `evaluate_results`
computes every verdict from `status` alone and carries `reason` as explanatory
text, so a reason on a PASS cannot flip a gate. The same edit fixes a latent
crash -- the old branch called `.get` on `(parsed or {})` having type-checked
only the verdict lookup, so a JSON array would have raised.
2. `synthesis_promotion` carries `node_verdict` + `hollow_nodes` into the promotion
evidence, and `_break_caveat` qualifies the candidate body's Why sentence, which
CLAIMS "the synthesis passed the recorded ... local/deliberate-break ... gates
... this is a verified delivery opportunity". That claim is graded per COMMAND
and was overstated whenever tautologies rode along on one real test; the caveat
names them in the same sentence. Deliberately STATED, never added as a Task: an
unchecked task drives Keepalive rounds, which would be the gating decision taken
by accident.
`local_verify._name_nodes` is promoted to public `name_nodes`, so both modules share
ONE bounded formatter rather than two that drift.
TWO DECISIONS, recorded on improvement-log item 0 with their revisit triggers rather
than left implicit:
- Hollow nodes do NOT gate, yet. The latched-gate answers are all available (drain =
re-author or delete the node; the drain runs while the gate is shut; one
population, measured and drained by the same list), so it could be built safely --
the reason to wait is evidential: nobody yet knows what fraction of `hollow` flags
are legitimate pre-existing regression guards, and a gate with an unmeasured
false-positive rate gets waived, and a waived gate protects nothing. Revisit once
the outcomes table holds enough PASS-with-hollow-nodes rows to compute it.
- No per-runner probe for npm/go/cargo. Those report `INDETERMINATE` with the runner
named. Building one now would be a new capability with NO caller -- every
deliberate_break check in this fleet is a Python repo -- which is precisely what
rule 0 exists to stop. Revisit on the first non-Python deliberate_break check that
actually runs.
TESTS. `runtime_ac`'s new selftest case units `_completed_result` on local_verify
JSON (clean PASS stays silent, tautologies and INDETERMINATE both carry, legacy
payload unchanged, FAIL untouched, non-object JSON no longer crashes) with a
DELIBERATE BREAK -> REVERT on `_hollow_carry_condition`: silence it and the tautology
case reverts to a PASS indistinguishable from the clean one. Two new pytest tests
cover `synthesis_promotion`, one running the REAL verification path end to end on a
mixed real+tautology file; dropping the evidence fields or silencing `_break_caveat`
each turns them red, both demonstrated.
FLOOR 448 -> 450: exactly the two new pytest tests. `runtime_ac` has a `--selftest`
and `synthesis_promotion` does not, which is the whole reason only one of the two
needed pytest tests. Measured on the merge result at origin/main 8549f84 (re-checked
that main had not moved before writing the number): 450 passed / 450 collected,
85/85 selftests, 5/5 gates, mypy ratchet 12/12 untouched, no skip ceiling moved and
nothing new skipped -- the new tests need only git and pytest.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 30 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 70 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. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
stranske
temporarily deployed
to
agent-standard
August 25, 2026 00:05 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Workflow source neededPR #114 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:
Once a valid source is present, this warning will not be reposted. |
Automated Status SummaryHead SHA: a919f43
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
Contributor
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
stranske
added a commit
that referenced
this pull request
Aug 25, 2026
…in the shape an issue body takes (#118) Two repairs from the Counter_Risk audit of 2026-08-24, and the first one's DIAGNOSIS was wrong in a way worth recording, because the wrong fix was the reasonable inference. 1. `how_to_use: null` where the precondition fails — A TABLE GAP, NOT A SUPPRESSION. The audit saw `how_to_use: null` on all five `applies_to='self'` capabilities plus partitioned-review / role-triage / role-decomposer, every one beside a failed-or-unset precondition, and concluded the answer WITHHOLDS guidance when a precondition fails. There is no such branch: `_attach_how_to_use` stamps unconditionally on both `advise()` return paths. The correlation was a coincidence of populations — those eight were among the **29 of 39 bound capabilities `HOW_TO_USE` had no entry for at all**. A fixer acting on the stated mechanism would have gone hunting a branch that does not exist. * Eight `HOW_TO_USE` entries written, exactly the eight the audit found null. Each names the call and, where one exists, the BOUNDARY. `capability-propensity`'s carries the audit's own Note 8: `--provenance` exists, defaults to `self_reported` at weight 0.25, and silently records a defect-finding verdict as an opinion when omitted — which is why three verdicts from that run landed weaker than their evidence. * `advise()` now returns `guidance = {offered, documented, undocumented}` and the render states the cause in words. Per-entry `null` cannot distinguish a gap from a rule; `2 of 5 documented` can, and nothing about it suggests a mechanism to look for. 2. "the concept may transfer" was AN INVITATION WITH NOTHING BEHIND IT. `precondition_note` carries the important half and stops one clause short. The audit accepted the invitation for `feature-scan`, transferred the concept by hand, produced two dimension-6 findings with it — and had to RECONSTRUCT what the capability's question even was from its name. `CAPABILITY_PRECONDITIONS` gains a third key, `concept`: the question the capability asks, in words that name no repository. `evaluate_precondition` returns it as `transferable_concept` and `format_advice` prints `ASK IT BY HAND:` directly under the note it completes. Declared in the SAME table as `applies_to` on purpose — a second table is how the note and its content would drift back apart, which is exactly how `HOW_TO_USE` came apart from `format_advice`. Two rules, both enforced, not advisory. It rides the SCOPE mismatch only: a `requires` failure means the repo has no observable surface at all, so there is no question left to transfer and offering one rebuilds the empty-invitation defect facing the other way. And every `applies_to: self` row must carry BOTH `concept` and a `HOW_TO_USE` entry — one for the audit that must ask by hand, one for the consult where the instrument does apply. 3. `deliberate-break-verifier` was ONE OUTPUT FORMAT from fitting `repo-audit:phase-4`. The audit called it "a genuinely close match to what I did by hand" and ran the proof itself anyway: AGENT_ISSUE_FORMAT wants the raw before/after console output quoted VERBATIM into an issue body, and `verify()` returns a verdict with that output JSON-escaped inside it. Every audit on record has re-run the same proof for that reason. A packaging mismatch, not a capability one. `local_verify.break_transcript()` renders the two halves the result ALREADY holds (`red`, `green`); `--transcript` prints it. Captures nothing new, changes no verdict, no exit code and no consumer — the exit code is computed above the rendering choice, because a rendering flag that could move a gate would make the artifact and the gate two different answers. A quotable artifact must not overstate, because its caveats do not travel with it. So they are stated INSIDE the block: #114's hollow nodes, an INDETERMINATE per-node pass, and one new guard. `--test-path` takes files and directories, so a pytest NODE ID is silently not copied into the base; the base then runs without that test, fails with "file or directory not found", and the verdict is PASS — red because the test was ABSENT, not because it FAILED. `uncopied_test_paths()` detects it and the transcript leads with THIS IS NOT A VALID DEMONSTRATION. Reported, never gated, and a selftest pins the verdict unchanged. BREAK -> REVERT, 14 breaks, every one confirmed to discriminate and all reverted green. Two of mine did NOT discriminate on the first pass and are recorded because the failure mode is the point: one asserted verbatim quotability THROUGH the formatter's own `_console` helper, so escaping the output escaped both sides equally; the other pinned a heading substring a renamed heading still contained. Fixed by asserting against the raw `stdout_tail`/`stderr_tail` on the result, and by making the section order one expression that a break can actually reverse. NO NEW PYTEST TESTS, so `.verify-floor.json` is untouched: both suites extend an existing module selftest, which changes nothing pytest collects. Nothing new skips — every assertion builds its own tempdir ledger or reads committed tables, so all of it runs on a bare runner. verify.py: 450 passed, 0 failed, 0/26 skipped (450 collected; floor 450), 85 of 85 selftests, 43/43 can-fire, 5 of 5 gates green. Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #100. That change landed the measurement; nothing read it.
hollow_nodes,node_verdictandnode_analysishad zero readers outsidelocal_verifyitself — which in this repo is the built-and-forgotten failure mode wearing a fresh coat of paint. This wires two consumers, both reporting, neither gating.1.
runtime_acthrew it away in exactly the case it matterssrc/runtime_ac.py —
_completed_resultdropped local_verify's reason on a PASS:So a deliberate-break PASS carrying tautologies recorded
reason=None. It now carries the reason when_hollow_carry_conditionholds — named hollow nodes, or anode_verdictthat is anything but PASS — and stays silent on a clean PASS so this cannot become constant noise. Evidence with no per-node fields at all (an olderlocal_verify) counts as clean, so prior behaviour is unchanged for it.Safe by construction, checked rather than assumed:
evaluate_resultscomputes every verdict fromstatusalone and carriesreasonas explanatory text, so a reason on a PASS cannot flip a gate.The same edit fixes a latent crash: the old branch called
.geton(parsed or {})having type-checked only the verdict lookup, so a JSON array would have raised.2.
synthesis_promotionwas making a claim the evidence contradictedThe candidate body's Why paragraph says the synthesis "passed the recorded scope, secret, local/deliberate-break, runtime-AC, and repository gates … this is a verified delivery opportunity". That claim is graded per COMMAND, and was overstated whenever tautologies rode along on one real test.
Now
node_verdict+hollow_nodesgo into the promotion evidence, and_break_caveatqualifies that sentence in place:Deliberately stated, never added as a Task — an unchecked task drives Keepalive rounds, which would be the gating decision taken by accident.
local_verify._name_nodesis promoted to publicname_nodesso both modules share one bounded formatter rather than two that drift.Two decisions, with revisit triggers
Recorded on improvement-log item 0 rather than left implicit:
hollowflags are legitimate pre-existing regression guards, and a gate with an unmeasured false-positive rate gets waived — and a waived gate protects nothing. Revisit once the outcomes table holds enough PASS-with-hollow-nodes rows to compute that fraction.INDETERMINATEwith the runner named. Building one now would be a new capability with no caller — everydeliberate_breakcheck in this fleet is a Python repo — which is precisely what rule 0 exists to stop. Revisit on the first non-Pythondeliberate_breakcheck that actually runs.Tests
runtime_ac's new selftest case units_completed_resulton local_verify JSON: clean PASS stays silent, tautologies and INDETERMINATE both carry, legacy payload unchanged, FAIL untouched, non-object JSON no longer crashes. DELIBERATE BREAK → REVERT on_hollow_carry_condition— silence it and the tautology case reverts to a PASS indistinguishable from the clean one.Two new pytest tests cover
synthesis_promotion, the first running the real verification path end to end on a mixed real+tautology file. Both breaks demonstrated: dropping the evidence fields, and silencing_break_caveat, each turns them red.Floor 448 → 450 — exactly the two new pytest tests.
runtime_achas a--selftestandsynthesis_promotiondoes not, which is the whole reason only one of the two needed pytest tests. Measured on the merge result atorigin/main8549f84, re-checked that main had not moved before writing the number:No skip ceiling moved and nothing new is skipped — the new tests need only git and pytest.
black --line-length 100andmypyclean on all four files.🤖 Generated with Claude Code