fix(capabilities): say what the concept IS, and ship the break proof in the shape an issue body takes - #118
Conversation
…in the shape an issue body takes
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: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 50 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 (3)
Comment |
Workflow source neededPR #118 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: 9a80f31
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
|
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
…ase (#125) The overlay is meant to add the candidate tests to the base tree and nothing else. When the fix itself lives IN TEST FILES, the default `--test-path` scope — "every changed test file" — is every changed file, so the base tree after the overlay is identical to the worktree in every file that differs. RED and GREEN then run the same code and neither verdict means anything. WHAT IT REPORTED INSTEAD, captured from the fixture with the guard disabled: verdict: FAIL_HOLLOW reason: candidate tests still pass against the base implementation; 2 of 2 candidate nodes do not discriminate: tests/test_guard.py::TestGuard::test_guard, tests/test_regression.py::TestRegression::test_regression A confident statement about the TESTS, and a false one — the tests were fine and the RUN was misconfigured. Worse, the per-node pass corroborates it by naming both nodes as tautologies, so the diagnosis reads well-evidenced and its implied fix (rewrite the tests) is the opposite of the real one (scope `--test-path` to the new module). Measured on Counter_Risk #964, where getting the scope right was load-bearing and had to be known in advance. `overlay_covers_every_change()` is the EXACT condition — every path that differs from the base was copied into it — not a heuristic about which files look like tests. Conservative by construction: it fires only when NOTHING is left uncovered, so the correct usage (overlay the new test, leave the fix behind in the base) can never trip it, and a deletion cannot be copied so a removing worktree is never flagged. ONE PREDICATE, TWO CAUSES. `invalid_demonstration()` folds this together with the node-id case #118 added, so a consumer has a single question to ask. And it reaches the RESULT DICT, not only the transcript: the JSON consumer is the one being misled, and a finding that only the renderer can see is the delivery defect `how_to_use` already paid for once. It rides `reason`, which `record_verdict` writes to `outcomes.notes`, so the evidence stream sees it too — with the computed verdict preserved inside the sentence. REPORTED, NEVER GATED. `verdict`, `ok` and the exit code are computed ABOVE this and are untouched, matching `--transcript`'s rule that a reporting feature which could move a gate would make the artifact and the gate two answers to one question. A selftest pins the verdict unchanged. `verify()` now reads `_changed_paths` ONCE and filters it for the default overlay, because the whole changed list is what answers the coverage question; two git calls could disagree about what changed. BREAK -> REVERT (four; two of them fire on the previously-silent path, which is the direction that matters most for a new warning) * `overlay_covers_every_change` returns False -> `AssertionError: overlay_covers_every_change( degenerate)` with the full FAIL_HOLLOW result quoted above. * the finding stops riding `reason` -> `AssertionError: candidate tests still pass against the base implementation; 2 of 2 candidate nodes do not discriminate...` * `changed_paths` no longer carried -> the guard goes blind and fires on the SOUND run: `AssertionError: "THIS IS NOT A VALID DEMONSTRATION" not in sound_text` * `all(...)` -> `any(...)` -> the same false positive, from the other side. Reverted; selftest green. The fixture asserts BOTH directions on one repo: the default scope is flagged, and the same fix with `--test-path` scoped to the new module alone is silent — a warning that fires on every run is a warning nobody reads. verify.py: 458 passed, 85 selftests, 5 of 5 gates green. Collection unchanged (458), no floor move. Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…bility with two routes (#126) * fix(advisor): the fix arc could not be offered the two instruments it actually uses `repo-audit:fix` bound codemod-campaign, epic-decomposition and testgen-lane (plus offload, inherited). Three independent implementation runs entered it for real on 2026-08-25 — the first runs ever to supply the filed issue and commit target the delivery capabilities need — and both capabilities those runs relied on were missing from the declared set. 1. `deliberate-break-verifier` was TRIGGERED and scored `useful` six times at this surface across the three runs, reaching the caller only through the KEYWORD CLASSIFIER. So the one consult whose free text said "verbatim console record / red / green / goes red when the trim helper is neutered" instead of "pytest" classified as `task_types: []` and was offered a fix arc with no way to PROVE its gate — then used the capability successfully on that very issue. 2. `frontend-verifier` was bound to `ux-review`, `repo-audit:phase-2` and `repo-audit:dimension-4` only. A UI fix at `repo-audit:fix` classifies as `ux_review`, and the run had to consult a DIFFERENT surface to be offered the one instrument that verifies UI. THE MATCHER WAS NOT WIDENED, and that is the point. The reported diagnosis was "the binding was correct and the matcher failed"; the binding was the half that was wrong. `binding_for` already survives a classification miss — `advise()` returns the declared set when `classify_task` finds nothing — so the fix arc got exactly the four capabilities that WERE declared. The binding is the layer that does not depend on the classifier, which is why the answer is a declared entry with its reason rather than a wider `TASK_SIGNALS` vocabulary (widening it to raise a hit rate corrupts the learned associations). Resolved set goes 4 -> 6, inside the 1..10 ceiling the selftest enforces. `frontend-verifier`'s declared precondition still annotates the offer, so a repo with no UI dismisses it in one line. STALE PROSE CORRECTED IN THE SAME CHANGE. `CONSULT_SITES["repo-audit:fix"]` said the surface is NAMED but "no run actually ENTERS it". True when written, false since 2026-08-25 — a cached reason outliving its evidence, which is the defect this workspace names. ARCHITECTURE.md carried the same sentence as its worked example and now carries the correction plus what produced it. BREAK -> REVERT: unbind `frontend-verifier` from `repo-audit:fix` -> `AssertionError: a UI fix at this surface classifies as ux_review and had no UI verifier to be offered`. Reverted; selftests green. The pin reads `binding_for("repo-audit:fix")` — the RESOLVED set a caller receives — not the table entry, so inheritance and ledger promotions are included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(propensity): promote on "triggered here and it helped", the signal the loop could not see The binding fix in the previous commit had to be made BY HAND, and the detection loop could not have found it. That is the durable half of the defect. `propose_bindings` promoted on ONE external signal: hand-work, a regex over a surface's own records guessing that it did the capability's job manually. It cannot see the case that actually happened — the surface used the capability, the capability HELPED, and nothing bound it there. WHY THAT IS NOT ALREADY COVERED BY "it was reached anyway". It was reached by the KEYWORD CLASSIFIER, which depends on the caller already using the capability's vocabulary. Measured: `deliberate-break-verifier`, six useful verdicts at `repo-audit:fix`, and the one consult whose free text missed the keywords got the fix arc without it. Binding is the layer that does not depend on classification, so "repeatedly useful at a surface that does not bind it" is a binding defect. WHY IT IS NOT THE RATCHET. Promotion from the CONTROL ARM (named and not triggered) is forbidden because the advisor's own naming would feed back into more naming. This is the opposite population: named, TRIGGERED, and an evidenced `useful` verdict recorded afterwards. That is an outcome, which is what the learning rules permit — and it is strictly stronger evidence than a prose signature, which is why it sorts first. The anti-ratchet assertion is untouched and still passes. LATCHED-GATE ANSWERS 1. What decrements it? Binding the capability at that surface — `already_bound` drops the row. That is the action the proposal asks for, not "someone notices". 2. Can the drain run while it is closed? Yes, unconditionally: the proposal is report-only, never withholds the capability and never lowers its propensity, so the surface keeps triggering it — which is how the count rose in the first place. 3. Same window both ways? `WINDOW_DAYS`, the one constant `experiments()` already uses for the trials counted here and the declines counted opposite. Runtime rule: `missed_selection` reports `useful_here` on every row INCLUDING below the floor, so "no proposal" can never read as "nothing is accumulating". Floor is 2, deliberately lower than hand-work's 3 — the same reasoning `DEMOTION_MIN_DECLINES` (2) uses against `DEMOTION_MIN_TRIALS` (8): better evidence clears a lower floor. Each `useful` verdict is refused without evidence, so two are two evidenced outcomes. BREAK -> REVERT (two, on either side of the floor) * `by_use = False` in `propose_bindings` -> `AssertionError: []` (nothing proposed at all) * `PROMOTION_MIN_USEFUL_UNBOUND = 1` -> `AssertionError: one evidenced use is an anecdote, not a binding` Reverted; selftests green. The two fixtures differ ONLY in how many evidenced uses they carry and both carry ZERO hand-work records, so neither can pass on the old signal and the floor is the only thing that can separate them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(advisor): an invented surface name says "no such surface", not "nothing applies" A run opened with `--surface 'audit-implementation-run'`, a name nothing declares. `binding_for` returned `{}`, the free text did not classify, and the answer came back `bound_count: 0`, `useful: false`, `capabilities: []` with a long `not_applicable` list — which reads as "the advisor has nothing for issue-filing work". It has three capabilities for exactly that, at `file-agent-issue`. The caller acted on the wrong sentence and filed no record at all. Silent absence in the advisor itself, which is the same class as a binding with no caller and a capability bound nowhere: the answer to a question nobody could ask looks identical to the answer "there is nothing here". `surface_status()` gives the surface FOUR states, and the three that are not `unknown` are the reason it can be switched on at all: * `unspecified` — no surface was passed. Never asked, so never a defect; a caller with no `--surface` must not be told it invented one. Same three-valued discipline as the precondition axis. * `declared` — the name is in `SURFACE_BINDINGS`, `CONSULT_SITES`, `KNOWN_UNCONSULTED` or this instance's promotions. A surface that is declared and deliberately binds NOTHING is `declared`, not `unknown` — that distinction is the whole point of the check. * `inherited` — a phase of a known surface. `binding_for` resolves every prefix, so `repo-audit:phase-9` legitimately inherits; calling that "unknown" would fire on the normal case and the check would be switched off within a week. * `unknown` — no prefix is declared anywhere. `known_surfaces()` is DERIVED from the same tables `binding_for` resolves against. A hand-kept list of valid names would be free to drift from the names that actually resolve, which is the parallel-inventory defect this tree keeps paying for. REPORTED, NEVER ACTED ON. The axis annotates and changes neither the candidate set nor its order — the same restraint the precondition and contraindication axes keep, and a selftest pins the two candidate lists identical with the axis firing and quiet. Being a diagnosis it carries its remedy: the closest declared surface names, so the retry is one line away. Both the answer dict (what MCP callers receive) and `format_advice` (what CLI callers read) carry it, because a field only the renderer reads is the defect this tool already paid for once with `how_to_use`. BREAK -> REVERT (both directions, because a false positive here is as bad as a false negative) * return `"inherited"` for an undeclared name -> `AssertionError: {'status': 'inherited', ...}` * drop `SURFACE_BINDINGS` from `known_surfaces()` -> a declared-but-empty surface is called unknown: `AssertionError: {'surface': 't-empty', 'status': 'unknown', ...}` Reverted; selftests green. Live probe on the real tables: 'audit-implementation-run' -> status `unknown`, did_you_mean ['implementation-verification', 'repo-audit:dimension-8', ...]. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(advisor): testgen-lane has two routes; the guidance named only the one that was unavailable `how_to_use` read "label the issue `testing`; the lane adds the testgen_gate.py acceptance gate to the prompt". That is the LANE route, and it is unavailable to a seat that is IMPLEMENTING rather than dispatching — labelling the issue hands the work to a remote agent and races it against the PR being written. The declared entrypoint has always been `testgen_lane.py/testgen_gate.py`, and `testgen_gate.py --help` has always shown a complete standalone CLI. The consequence is measured in both directions on 2026-08-25. One run "checked the binary rather than trusting the prose", ran the gate directly, and produced this capability's FIRST trigger in 44 offers — two `useful` verdicts and a propensity move from 0.5 to 0.7647. Another declined it. So guidance that names one of two routes does not merely omit: it STEERS AWAY. Both routes are now named, with which one applies stated first (dispatching vs implementing), plus the two invocation rules that have each cost a wasted run — `--source` takes an importable module and not a file path, and the pytest-arg strings are split shell-style so a spaced `-k` must be quoted inside — and the real boundary: the coverage-delta check measures a PRODUCTION source module, so a change touching only test files has nothing for `--source` to point at. This is yesterday's `how_to_use` delivery fix being incomplete rather than wrong: #118 made the field reach the caller on both answer branches; this makes the field itself complete. BREAK -> REVERT: restore the one-line lane-only entry -> `AssertionError: the DIRECT route must be named — the lane route is unavailable in-seat, and an entry that names only it converts a usable capability into a decline: label the issue `testing`...` Reverted; selftests green. The pin sits in `_selftest_how_to_use` PART 1 beside the offload and adversarial-review clauses, which are the existing precedent: each is a standing repair proposal's evidence, asserted as WORDS rather than as key presence. The general rule is stated there — a capability with two routes must document both — and the specific clauses are what a later tidy cannot silently drop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Tim Stranske <tim@stranskemo.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Two repairs from the Counter_Risk capability-consult audit of 2026-08-24, both through the repair
channel. The first finding's diagnosis was wrong, and that is recorded rather than quietly
corrected — the wrong fix was the reasonable inference from what the caller could see.
1.
how_to_use: nullwhere the precondition fails — a TABLE GAP, not a suppressionThe audit saw
how_to_use: nullon all fiveapplies_to='self'capabilities pluspartitioned-review/role-triage/role-decomposer, every one beside a failed-or-unsetprecondition, and concluded the answer withholds guidance when a precondition fails.
There is no such branch.
_attach_how_to_usestamps unconditionally on bothadvise()returnpaths. The correlation was a coincidence of populations: those eight were among the 29 of 39
bound capabilities
HOW_TO_USEhad no entry for at all. Acting on the stated mechanism wouldhave meant hunting a suppression branch that does not exist.
HOW_TO_USEentries, exactly the eight found null. Each names the call and, where oneexists, the BOUNDARY.
capability-propensity's carries the audit's own Note 8:--provenanceexists, defaults to
self_reportedat weight 0.25, and silently records a defect-finding verdictas an opinion when omitted — which is why three verdicts from that run landed weaker than their
evidence warranted.
advise()returnsguidance = {offered, documented, undocumented}and the render states thecause in words. Per-entry
nullcannot distinguish a gap from a rule;2 of 5 documentedcan.2. "the concept may transfer" was an invitation with nothing behind it
precondition_notecarries the important half and stops one clause short. The audit accepted theinvitation for
feature-scan, transferred the concept by hand, produced two dimension-6 findingswith it — and had to reconstruct what the capability's question even was from its name.
CAPABILITY_PRECONDITIONSgains a third key,concept: the question the capability asks, in wordsthat name no repository.
evaluate_preconditionreturns it astransferable_concept;format_adviceprintsASK IT BY HAND:directly under the note it completes. Declared in thesame table as
applies_to— a second table is how the note and its content would drift apart,which is exactly how
HOW_TO_USEcame apart fromformat_advice.Two rules, both enforced by
_selftest_how_to_use:requiresfailure means the repository has no observablesurface at all, so there is no question left to transfer, and offering one would rebuild the
empty-invitation defect facing the other way.
applies_to: selfrow must carry bothconceptand aHOW_TO_USEentry — one for theaudit that must ask by hand, one for the consult where the instrument does apply.
3.
deliberate-break-verifierwas one output FORMAT from fittingrepo-audit:phase-4The audit called it "a genuinely close match to what I did by hand" and ran the proof itself
anyway:
AGENT_ISSUE_FORMATwants the raw before/after console output quoted verbatim into anissue body, and
verify()returns a verdict with that output JSON-escaped inside it. Every audit onrecord 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);--transcriptprints it. It captures nothing new and changes no verdict, no exit code andno consumer — the exit code is computed above the rendering choice.
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
INDETERMINATEper-node pass, and one new guard.--test-pathtakes files and directories, so a pytest node id is silently not copied into thebase tree; the base then runs without that test, fails with "file or directory not found", and the
rolled-up verdict is
PASS— red because the test was absent, not because it failed.uncopied_test_paths()detects exactly that and the transcript leads withTHIS IS NOT A VALID DEMONSTRATION. Reported, never gated, and a selftest pins the verdict unchanged.Real output — the guard firing on the node-id case
Deliberate break → revert
14 breaks, every one confirmed to discriminate, all reverted green. Two of mine did not
discriminate on the first pass, and they are recorded because the failure mode is the point:
_consolehelper, so escaping theoutput escaped both sides of the comparison equally — a test using the very thing it guards. Fixed
by asserting against the raw
stdout_tail/stderr_tailon the result.order a single expression a break can actually reverse.
Verification
python3 src/verify.py— 450 passed, 0 failed, 0/26 skipped (450 collected; floor 450), 85 of 85selftests, 43/43 can-fire, 5 of 5 gates green.
.verify-floor.jsonis untouched, deliberately: both suites extend an existing moduleselftest, so nothing pytest collects has changed. Nothing new skips either — every assertion builds
its own tempdir ledger or reads committed tables, so all of it runs on a bare runner.
ARCHITECTURE.mdupdated (the capability-layer / precondition-axis contract). No stage, component,rail/role classification, feedback surface or
roles.pyentry changed, so the diagram is unaffected.🤖 Generated with Claude Code