docs(conventions): add the detector-findings owner doc - #2692
Conversation
…plicitly Implements ADR 0010. The findings-file shape is the whole integration contract -- nothing authenticates the writer -- so any component that persists a conforming file reaches the apply relay. That made a second producer a silent data-loss bug: the fix action took the newest *.md declaring type: review-findings and merged nothing, so a detector running after a full review shadowed the entire review with no error, no warning, and a green run. This is the failure class the liveness-assertion convention exists to prevent, and it is settled before the second producer ships rather than after. The fix action now consumes the set of conforming files for the exact current branch and unions the coverage fields, because reporting one producer's ## Surfaces line would hide a surface that ran and returned nothing -- moving the hidden-findings failure up a layer instead of closing it. Dedup is presence-only: identical Location and identical Finding text. The tempting key, normalised path plus a plus-or-minus-three-line bucket, sits behind a Sonnet semantic stage the fix action does not run, and adopting the bucket without the semantics inverts the pipeline's own minimise-false-merge rule -- two distinct defects at foo.ts:42 and foo.ts:44 would collapse, and since each row carries its own Action, one producer's remediation would be discarded with no trace. A false split adds a row an operator can see; a false merge drops a finding. The applied-plan record becomes the consumption ledger and is written on every path that applies. It was headless---yes-only, so a bound anchored on it was a no-op on the dominant interactive path and the merge set would have grown without limit, re-injecting findings the required post-fix re-review had already resolved. source-findings is pinned as a YAML block sequence of consumed file names -- always a sequence, never a bare scalar, because a writer emitting a scalar where the reader expects a sequence under-matches silently and re-consumes what the record was written to retire. Names rather than paths, because the resolved memory_dir can differ between the session that wrote the findings and the session that applies them, while the branch findings directory is a single home with unique timestamped names. The exact-branch filter binds records as well as candidates, so a slug-collided branch's record cannot truncate the set. Consumption is per file, not per row: rows surfaced by the low-confidence fence or narrowed out by the operator are named in the record body and recovered by re-running the review, never by re-consumption. Documented rather than left for a reader to discover, since a reader implementing "exclude the consumed file" would otherwise reasonably conclude those rows vanish. A one-file set reduces to the previous behaviour byte-for-byte and an empty set keeps the clean STOP; eval 23 flips (an interactive apply now writes a record) and cases 24-27 pin the two-producer merge, the ledger subtraction on the interactive path, the both-sides branch filter, and the false-merge guard. Closes #2678 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fresh-context reviewer audited the diff against ADR 0010 with the rationale withheld and found four blocking defects. All four are prose-contract bugs: this skill is read and executed by an agent, so an instruction that can be read two ways is a real defect. Recovery from a deferred row claimed more than it delivers. Step 5 said rows surfaced or narrowed out are recovered by re-running the review, but re-running this skill re-fans-out this skill's own reviewers -- it regenerates nothing a script detector produced. Since consumption is per file, an operator narrowing to "only the correctness ones" retired a detector's cleanup rows permanently, and pre-change that loss was impossible on the interactive path because no record was written at all. Recovery now names the row's own producer as the thing to re-run, and the record body must attribute every deferred row to the file it came from, which is what tells the operator which producer that is. The ## Surfaces union was computed and then discarded. Step 2 ordered the union, but neither the plan template, the report bullets, nor the record body had a slot to print it -- reproducing the exact harm ADR 0010 Decision 1 names, one layer further along. All three now carry it. default-mode.md still told producers "the fix action parses only ## Findings and ## Unparsed" while fix-pass-mode.md now requires ## Surfaces and tier: from every consumed file. A producer reading the writer contract and a consumer reading the consumer contract would execute differently. The clause is narrowed rather than deleted -- presentation-additive stays true of ## By dimension alone. Records written by 0.19.0 carry source-findings as a bare scalar and survive the upgrade untouched, because the findings directory is gitignored local state. The new reader expected a sequence, so a legacy record would have subtracted nothing and re-injected findings already applied. The reader now tolerates a scalar as a one-entry sequence compared by base name, and the CHANGELOG carries the migration note. Also from the same pass: the collapsed row had no rule for Tier, Confidence, or Action (now MAX tier, MAX confidence, every distinct Action retained) and merged Rank collided at 1 across files (now renumbered, which also fixes the apply order); "identical" is defined as byte-for-byte after unescaping; an abnormally terminated apply writes no record, since re-consuming an applied fix is recoverable and a silently retired row is not; a minimally conforming producer that omits tier: or ## Surfaces is consumed with tier: unstated rather than skipped or guessed at; the DEGRADED blockquote is collapsed to its first line; the names-not-paths rationale is replaced -- it claimed a cross-directory property the design does not have, when the true reason is that both sides are always read from the same single directory; and the empty-set message no longer says "run the review first" to an operator who already did. Stale single-file language cleared from the SKILL.md description and the plugin README. Eval 22 gains the sequence assertion, 25 trades a restated expectation for an orthogonal one, 27 covers the collapsed-row fields and renumbering, and 28 pins the legacy-scalar path. Refs #2678 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A plugin installs standalone and cannot resolve a repo-relative docs/ path, so the citation form is a raw URL to main -- which means this doc must be on main before anything cites it. That makes it a hard merge barrier for the pilot rather than a companion to it. Published as a deliberate stub. The registry rule in PLUGIN-PHILOSOPHY.md is a deadline -- a new cross-plugin convention lands in an owner doc before a second plugin adopts it -- and the first detector pilot is itself that second adopter. Depth trails the pilot because the pilot is what produces the evidence to harden against; the deadline is why the stub cannot wait for it. The findings-file schema is cited, never copied: default-mode.md owns it, and a second statement of a table is a second thing to drift. What this doc owns is what the shape alone does not settle -- the four fields a producer computes for itself, coexistence between producers, and minimal conformance. The confidence rule is the payload. A detector author will reach for `low` to express uncertainty, and `low` ranks BELOW emitting nothing at all: the order is high > medium > unscored > low, and absent confidence resolves to unscored. Emitting `low` therefore buries the finding under the one you never wrote. This also moves the multi-producer rule out of the review plugin. 0.20.0 stated it in context/default-mode.md, but it binds every component that writes a conforming file, and a rule binding three plugins cannot live inside one of them. default-mode.md keeps only what fanout's own writer does and points here for the rest. Review plugin 0.20.1 -- no behaviour change. Enforcement is deferred with event triggers rather than dates: a conformance gate has nothing to run against until the first detector reaches main, and the doc's own depth waits on the pilot or a second adopter, whichever comes first. The adopters table lists only review:fanout, because tabling a planned adopter would assert what a reader cannot rely on. Closes #2679 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 41s —— View job Security Review
Skip gate triggered — no security-relevant surface. This PR is docs-only: two new markdown files ( No code execution paths, no authorization/access-control logic, no injection surfaces (command/SQL/path/template), no secret/token handling, and no GitHub Actions workflow files are touched. The added raw-URL citation ( No security issues found. |
|
Claude finished @kyle-sexton's task in 2m 42s —— View job Claude finished the code review
SummaryThis is a docs-only PR (owner-doc stub + a pointer-only edit in No blocking or important findings. The doc is unusually well-verified for a PR description, and I independently re-checked the load-bearing claims rather than taking them on faith:
Not independently run (sandboxed environment blocked script execution for this session): No inline comments filed — nothing rose to the "a careful senior reviewer would block or flag" bar. |
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca078f51f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…view found A fresh-context reviewer audited the doc against issue #2679 with the rationale withheld. Its factual-accuracy and link lanes came back clean; its structural lanes did not. The doc could not be acted on. Its single normative instruction is "write a conforming file into the current branch's findings directory", and it never said how to resolve that directory. The existing chain dead-ends for exactly the audience this doc serves: default-mode.md points at the review SKILL.md, which resolves through a ${CLAUDE_PLUGIN_ROOT}-relative path only that plugin can expand. A "Where the file goes" section now routes tier resolution to the topic-docs convention and states the three producer-facing specifics that pointer does not carry -- the reviews/<branch-slug>/ sub-path, the lossy slug rule, and the self-ignore guard, whose omission would commit findings meant to stay checkout-local. Tier was the doc's first producer-owned field and it never named the vocabulary or its owner. severity.md "Severity tiers" owns CRITICAL/IMPORTANT/SUGGESTION and carries a consumer-precedence rule that binds a producer too: where the consuming project defines its own vocabulary, map to that instead. A detector emitting P1 or HIGH would have been non-conforming with nothing in this doc to say so. Confidence cited the wrong owner. The enum is defined by severity.md "Confidence axis"; findings-normalization.md is fanout's internal pipeline, which this doc's own Boundary excludes from a producer's concern. The citations were accurate but pointed at a downstream consumer of the vocabulary rather than its source. The definition now cites the owner and the rank order stays only as the consequence that makes `low` worse than omission. Also added: Confidence is confidence-of-realness, not confidence in the fix -- a detector sure a defect is real but unsure of the remediation says so in Tier and Action wording, never by downgrading Confidence. The doc claimed "This doc never restates it" and then restated four things: merge-set construction, consumption subtraction, per-file consumption, and the presence-only dedup rationale with its borrowed FALSE-MERGE quote -- plus verbatim copies of the cell-escaping characters and the path-relativization rule. Duplication runs one direction here (convention copying plugin), so each is reduced to the producer-facing consequence plus a pointer and the claim on line 10 becomes true. The adopters table contradicted the doc's own scope: the title says "from outside review:fanout" and the sole tabled adopter was review:fanout. It ships empty, with review:fanout named in prose as the reference writer -- which is the honest state of a stub. Also: date: had no stated status for a producer and is now required; the admission test is cited rather than restated; default-mode.md's closing sentence no longer overstates the split while the sentence above it still frames the shape's cross-producer status; severity.md, topic-docs, and enforceability-tiers join External authority, matching both siblings; and the review CHANGELOG's "no behaviour change" is replaced with what actually did and did not change, since instruction text an agent reads is not nothing. Refs #2679 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 37s —— View job Claude finished the code review
SummaryReviewed under the code-review lane ( No blocking or important findings. The new "Where the file goes" section in
I also re-verified the rest of the diff against this commit:
Not independently run (sandboxed environment blocked script/lint execution for this session): No inline comments filed — nothing rose to the "a careful senior reviewer would block or flag" bar. |
|
Claude finished @kyle-sexton's task in 50s —— View job Security Review
Skip gate triggered — no security-relevant surface. New commits since the last security review (
All four are prose-only edits to markdown documentation. No code execution paths, no authorization/access-control logic, no injection surfaces, no secret/token handling, and no GitHub Actions workflow files are touched. This update also happens to close the Codex P1 comment on the prior revision (missing findings-location resolver) by adding the new "Where the file goes" section — a documentation-completeness fix, not a security fix, since the prior state was an incomplete pointer rather than an exploitable gap. No security issues found. |
…tion resolver
A detector following the owner doc alone resolved only "the current branch's
findings directory" and three restated specifics, so a consumer that configures
memory_dir or declares a review-artifacts location in CLAUDE.md left the
producer writing where the fix action never scans — every finding silently
hidden.
Point at plugins/review/reference/topic-docs.md "Resolution" instead: the
binding SKILL.md "Shared inputs" itself resolves through. Naming it by repo
path is what makes it followable — the ${CLAUDE_PLUGIN_ROOT}-relative form is
unexpandable outside the review plugin, but it is the same document. It carries
the rung order, sub-path, slug rule, and guard, so the restatements of all
three are deleted rather than extended.
Stated in their place is only what the binding leaves to a producer: run the
whole rung order rather than its default, take the contract's non-interactive
rule rather than inventing an answer to a rung that asks, match on branch:
frontmatter rather than the directory, and owe the self-ignore guard.
The re-emission rule no longer asserts how the ledger identifies a consumed
file — that is fix-pass-mode "Step 5"'s to own and to change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The no-restatement claim was absolute while two clauses under it still carried borrowed facts: that the slug mapping is lossy, and that the non-interactive rule is contract-owned so bindings cite rather than redefine it. Both go false if their owner changes them. Keep the producer-owned consequence — branch: frontmatter is what proves a file is this branch's — and let the binding's slug rule say why; soften the claim to what is true as written. Invoke the non-interactive rule on its own precondition rather than on an assumed producer shape: the contract scopes it to any context that cannot ask the user or persist config, which a headless detector cannot, while this doc's own format-only contract admits interactive producers too. Cite it by its exact name, "Non-interactive / forked mode", matching every other citation here. Qualify the shared-resolver claim: producer and consumer resolve through one binding, but the operative texts state different rung counts, and the non-interactive collapse is what makes them coincide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
… reviewer-burden term (#2694) Closes #2682 Closes #2683 Phases 5 and 6 of the boris-routines-adoption plan, in **one PR by design** — one autonomy version bump, one CHANGELOG entry. Splitting them either duplicates the bump or strands one without its changelog line, and `check-changelog-parity.sh` fails on both. Independent of the detector chain (#2690 / #2692); nothing here blocks on those merging. ## Why rows for classes we are not building A catalog that lists only what shipped cannot be reasoned from. A reader asking *"why is there no clone unifier?"* finds silence, and silence reads as an oversight rather than a decision. **Nine rows added, one existing row amended**, and every derivation run **through the mapping rules** (`routines.md` "Mapping rules (catalog to matrix)"), never by hand — that is this phase's whole discipline. ## Existing-row sweep (first work item) Run against `routines.md` itself, not the research record. Per candidate class, "no row" or the existing row's identity: | Candidate | Sweep result | |---|---| | dead code | **Row exists** — `dead-code-sweep`, `DET detect \| DC (review-gated PR) \| repo \| n/a — no agent session \| not-a-routine`. → **amended**, not duplicated. | | clone work | **No row.** The nearest row, `coverage-mutation-watch`, is a **different observable** — coverage and mutation score, not clone density — so it is no match and was not amended. → new row. | | the other eight | **No row.** → new rows. | `clone-trend-gate` ships a **byte-identical cell signature** to `coverage-mutation-watch` (`DET | R (digest/gate) | repo | n/a — no agent session | not-a-routine`). The rows are genuinely distinct — clone density and its trend versus coverage and mutation score — and because nothing in the six cells says so, a class parameter now states it. Two rows that read as a copy-paste duplicate need the difference written down somewhere. **The issue's line citations had drifted.** It cites `dead-code-sweep` at `:192` and `coverage-mutation-watch` at `:195`; they are at `:191` and `:193` on `main`. The row *identities* and their full cell text match exactly, so the sweep result stands — but the offsets are not uniform (1 and 2), so the numbers were re-resolved rather than adjusted. ## ADR-0004 incumbent gate — the search record #2682: *"ADR-0004's incumbent gate applies to rows here, not only to detectors."* ADR-0004 D-1 requires the search carry `path:line` evidence. It is recorded here rather than in `routines.md` because that contract is deployment-agnostic — an adopting org does not ship this marketplace, and a binding parameter grounded in what *this* repository happens to contain is the same defect as grounding one in this fleet's hardware. What the search *changed* in the contract is in the contract; the evidence trail is here. | # | Class | Verdict | Evidence | |---|---|---|---| | 1 | `formal-logic-modeling` | **No incumbent** | Searched `TLA+`, `Alloy`, model-checking, invariant-verification, design-by-contract vocabulary across `plugins/`. No hits that were not false positives. | | 2 | `cant-fail-test-repair` | **Partial** | `plugins/mutation-testing/skills/audit/SKILL.md:126` — surviving-mutant disposition surfaces tests that cannot fail a given mutant. Remediation (`:201-209`) authors **new** killing tests via `/testing:write`; it does not repair the existing tautological assertion, which is this class's whole content. | | 3 | `layering-enforcement` | **Incumbent** | `plugins/review/agents/architecture-guardian.md:3` — "Reviews code for dependency-direction violations, layer boundary breaches…"; `:33` checks "inner layers must not reference outer layers; follow the project's stated layer rules." Close match, same inform-human posture. **Already reflected in the row**: its join trigger requires "a recurring manual pattern the incumbent reviewer does not already cover", and its parameter requires clearing the incumbent gate against the existing architecture-review surface. | | 4 | `clone-trend-gate` | **No incumbent** | Searched clone-detection tool names, `clone densit`, `copy-paste detector`, `code clone`. `plugins/review/agents/code-reviewer.md:46` flags "Duplicated Code" ad hoc during diff review — not a density trend or a gate. | | 5 | `stale-flag-removal` | **Partial (weak)** | `plugins/code-tidying/skills/tidy/templates/host-wiring-lane.template.md:16` lists "stale feature-flag branches" as one example under the Dead Code tidying. A bare example phrase inside a glob-scoped lane; no flag-age tracking, no staleness detector, no single-variation-everywhere trigger. | | 6 | `logic-simplification-sweep` | **No incumbent, at either altitude** | Above expression level: `tidyings.md` has no cross-function control-flow restructuring; Beck's Guard Clauses (`plugins/code-tidying/skills/tidy/reference/tidyings.md:15`) restructures within one method only. At expression level: searched `boolean expression`, De Morgan, `redundant condition`, `simplif.*expression` across `plugins/code-tidying/` — **zero matches**. `plugins/code-tidying/skills/batch-simplify/SKILL.md:140` delegates to an external agent not present in this repo, with instructions naming no altitude. | | 7 | `abstraction-flattening` | **Partial — the closest call in the set** | `plugins/architecture/skills/improve/research/deepening/scan-briefing.md:49-51` (two-adapter rule → "speculative indirection") and `research/deepening/vocabulary.md:28` (the deletion test: "If complexity vanishes, it was a pass-through") detect the same smells this class targets. But the lens's remedy is *deepening*, it applies no fix itself, and it routes through a human interview and planning handoff (`SKILL.md:63`) — there is no autonomous change path. | | 8 | `ant-only-shipper` | **No incumbent** | Searched champion/challenger, canary, shadow deploy, A/B, promote-to-production, competing-implementation vocabulary. `plugins/prototype/skills/explore-directions/SKILL.md` is adjacent — throwaway UI variations picked by a human in-browser — not an evidence-based promotion recommender. | | 9 | `gui-crash-fuzzing` | **No incumbent** | Searched `fuzz`, monkey-test, random-click, GUI-crash, UI-stress vocabulary. All hits false positives. | | 10 | `dead-code-sweep` (amended) | **Partial** | `plugins/code-tidying/skills/tidy/reference/tidyings.md:21-25` — Beck #2 Dead Code covers detection and verified deletion inside a scoped lane. It has **no quarantine-then-judge staging**, which is the entire normative content of the amendment. | ### What the search changed - **`abstraction-flattening`'s "no validated detector" was under-qualified.** Heuristic detectors for exactly these smells ship — including in this marketplace. The parameter now says what is actually true and what the join trigger actually names: *a scanner is not the join trigger; a published validated detector is*, and none surveyed is validated against a fault-outcome ground truth. - **`logic-simplification-sweep`'s "already covered by structure-only tidying surfaces" was false**, and the search is what established that — there is no incumbent at either altitude. The claim is removed rather than restated. - **No verdict removed a row.** Two partials (`cant-fail-test-repair`, `dead-code-sweep`) are "detection exists, the remediation shape does not", which is a capability gap rather than a duplicate. `layering-enforcement`'s real incumbent was already gated in the row before this search. ## Rows added **Tier 1 — join triggers** | Class | Judgment | Output | Access | Derived | Status | |---|---|---|---|---|---| | `formal-logic-modeling` | AGT | R | repo | `C1` | join: a stated invariant or specification artifact exists to model against | | `cant-fail-test-repair` | hybrid: DET detect; AGT repair judgment is the routine | DC (PR) | repo | `C3` | join: proven recurring manual pattern | | `layering-enforcement` | AGT/HUM | R | repo | `C1`; disposition human-gated | join: layering rules stated as text, and a recurring manual pattern the incumbent reviewer does not already cover | **Tier 2** | Class | Note | |---|---| | `dead-code-sweep` | **Amended.** Was `DET detect` only; the quarantine-exit judgment makes it hybrid, and that portion derives `C3` — liveness is *not* mechanically checkable, since reflection, dynamic dispatch, and out-of-tree callers each defeat the build that would otherwise prove it. Window floor **30–90 days with staged quarantine, never one day**, and not tunable downward by an org binding. | | `clone-trend-gate` | `DET \| R (digest/gate) \| repo \| n/a \| not-a-routine`. Detection and trend gating **only**. | | `stale-flag-removal` | hybrid; removal portion derives **`C4`** — a flag definition is a configuration surface, and the structural-blast-radius rule composes above `C2`/`C3`. Disposition human-gated. | **Tier 3 — recording why not** | Class | Why not | |---|---| | `logic-simplification-sweep` | `C3`; `join (external)`: published effectiveness evidence exists. | | `abstraction-flattening` | `C4` (structural surface); `join (external)`: a validated detector is published. The fault data also runs *backwards* — Speculative Generality and Middle Man are in some studies associated with **fewer** faults. | | `ant-only-shipper` | `AGT/HUM`, `C1`, human-gated. The promotion decision is a product call. | | `gui-crash-fuzzing` | `not-a-routine` (DET). Reported crash-replay reproducibility is low enough that filing every crash would degrade the governed queue rather than feed it, so its `WI` output is replay-gated. The row also carries the isolation consequence the catalog had skipped: unattended GUI actuation requires `L3`. | ## Exclusions are exclusions, not deferrals Recorded as such, because a deferral invites a future PR to "finish" the class: - **`clone-trend-gate`** — the unify *decision* is excluded, not a deferred posture. No surveyed clone-detection tool automates deciding which clones to unify, across a detection literature the survey found spanning two decades. On that record there is nothing to defer *to*, so adding a unify posture re-opens the class rather than extending it. - **`gui-crash-fuzzing`** — judging a filed crash beyond replay is excluded the same way. A filed crash is ordinary queue intake, owned by the issue-lifecycle classes. Without this the row's `DET → not-a-routine` exit would be skipping a judgment portion, which is precisely the misreading `routines.md` §"What a routine is" is worded to prevent. - **`stale-flag-removal` / `ant-only-shipper`** — the disposition never automates. Which branch survives, and whether to promote, are product calls. That is why both rows are `AGT/HUM`. - **`layering-enforcement`** — inform-human posture only; a direct-change posture derives `C4` and no surveyed precedent supports one. ## The contract is now three tiers, explicitly The new `Class parameters` section carries normative detail six table cells cannot hold — stated as **binding**, so a leaf contradicting one is non-conforming. That created a contradiction with three statements in the same file saying definition depth exists only for the `v1` classes, while `clone-trend-gate` and `gui-crash-fuzzing` — neither of which will ever gain a leaf — carry parameters. **Resolved by widening, not collapsing.** The hub now declares three tiers: catalog plus mapping rules; class parameters, binding any class leaf or not; leaf-level depth for the ten `v1` classes. Collapsing the parameters back into row cells was the alternative and was rejected — the section exists precisely because the cells cannot carry the detail, and several parameters bind `v1` and hybrid rows generally rather than one deferred class. ## Four general derivation rules now live in the mapping rules `## Mapping rules` exists, in its own words, *"so an adopting org can classify a **novel** routine class end-to-end… without a contract change."* Three general rules were filed under `### Class parameters` instead — two of them titled with a class token while generalizing in their last sentence (*"The same reading applies to every `AGT/HUM` row"*; the structural axis *"keys on blast radius"*). An org reading the mapping rules would not have found them. All three moved: the hybrid portion-split rule onto the hybrid bullet under *Judgment and output*; the `AGT/HUM` clarification onto its existing bullet; the target-not-file rule onto *Structural blast radius*. A **fourth** was stranded the same way inside a `dead-code-sweep` note and is now stated generally: a risk-raising axis evaluates **per item** as well as class-wide, so a class whose axis fires on only some items derives the lower class and records the escalation rather than deriving the higher one wholesale. **One honest limitation now stated in the rules themselves.** The structural axis keys on the change's *target*, and **no catalog column records a target** — which is why `logic-simplification-sweep` and `abstraction-flattening` carry byte-identical axis cells (`AGT | DC (PR) | repo`) and derive `C3` and `C4`. Rather than leave a reader to conclude the three axis columns are sufficient when they are not, the rule now says the target comes from the class's own definition and that a row turning on it says so in its `Derived row` cell. ## A second `join` semantic, separated Every pre-existing `join:` trigger is a condition the adopting **org** can satisfy: connect a telemetry surface, write the layering rules down, accumulate a manual pattern. `published effectiveness evidence exists` and `a validated detector is published` are world state no adopter can act on. One token carrying both semantics made those two rows read as backlog items. A `join (external): …` legend row now separates them. Rewording the two triggers into adopter-actionable form was the alternative and would have been dishonest — the point is that the org *cannot* fire them. An `excluded:` status was also rejected: these classes are genuinely deferred pending evidence, unlike the unify decision, which is excluded outright. Every other `join:` row in the table was re-checked against the new definition; none belongs under the new one. ## #2683 — the predicate side - **What may never enter a predicate.** An acceptance or merge rate is never a promotion input and is not an efficacy signal, in either role, at any cell, at any threshold. - **Two shipped terms sit close to that line, and both are now distinguished.** `0 human-reverted merges` is a *correctness* signal — a human asserting the change was wrong after it landed — not an acceptance rate. And `≥ 20 autonomous C2 merges over ≥ 14 days` is a **volume floor**, not a rate: a ratio rises when its denominator shrinks, so attempting less — or attempting only what is certain to land — raises it with no change in the work. A count has no denominator to shrink. Selectivity leaves it flat. - **The term inventory is now complete.** It previously enumerated four term types and the table has **seven** — merge counts, advisory-review counts, and missed-blocking-finding counts were all missing. All three are correctness- or volume-side, so nothing in the argument moved. - **Reviewer-burden term: deferred with an explicit trigger, not omitted.** It needs a denominator, and a denominator needs three org-scale things this contract does not have — a population to divide by, a non-merge outcome signal, and a lookback window with a demotion rule. Without them the term moves with *volume* rather than trustworthiness, which rewards a cell for producing less. Trigger: the volume **and** a non-merge outcome signal; volume alone is not the trigger. - **Standing constraint on any future tuner** — its signal set stays **disjoint** from promotion evidence. Overlap is a self-dealing loop: a tuner optimizing a signal that also promotes a cell can raise that signal to reduce the scrutiny applied to the tuner's own output. Binds the tuner's inputs, not its intent, and binds whether or not the reviewer-burden term is ever activated. ## Two claims in #2682 that did not verify — and one of the replacements did not either **#2682 says logic simplification above expression level is "excluded by name in `tidyings.md`".** It is not: `plugins/code-tidying/skills/tidy/reference/tidyings.md` contains **zero** occurrences of `simplif`, case-insensitive. **The first replacement for that claim was also false.** An earlier revision of this description said expression-level simplification is "already covered by structure-only tidying surfaces" and that simplification above that level "changes behavior in the general case". Both fail: - The incumbent search found **no incumbent at either altitude** — no cross-function control-flow restructuring in `tidyings.md`, and zero expression-simplification matches anywhere in `plugins/code-tidying/`. - "Changes behavior in the general case" is contradicted by `plugins/code-tidying/skills/batch-simplify/SKILL.md:174` — *"Simplification is behavior-preserving"* — which treats behavior alteration as a **regression** its verification exists to catch. It also contradicted the row's own cell, which claims only that equivalence above expression level is not mechanically checkable: weaker, and correct. A checkable-and-false claim had been replaced by an uncheckable one. The parameter now uses the row cell's own wording and drops the coverage comparison entirely; the `path:line` evidence lives in the incumbent table above, where it cannot rot a deployment-agnostic contract. ## Deployment-specific fact removed from normative text A binding parameter read: *"The fleet ships no GUI to fuzz, so the class has no observable here at all"* — inside a section declaring *"A parameter here binds the class; it is not commentary"*, in a contract whose own closing line is *"The contract assumes no machine, org size, or budget"* and whose hosting stance makes substrate a deployment-owned binding. An adopting org that ships GUIs would read a binding parameter grounded in **this** fleet's inventory. Struck. The deployment-independent half stays, and the class's `not-a-routine` derivation never depended on the inventory in the first place — it follows from Judgment `DET`, which holds for any deployment. **On #2682's present-observable admission check** (*"Drop any candidate anchored to nothing"*): the observable here is crashes surfaced by actuating a GUI — a real, nameable artifact class, not nothing. A deployment without a GUI simply never schedules the class, which is true of every access-gated row in the catalog and is not an admission question. The row is admitted; what was wrong was grounding its *parameter* in one deployment's inventory, not the row's presence. ## Empirical claims, hedged to the register the evidence supports The supporting research record is gitignored, so no in-tree reader can check any empirical claim in this contract. Several were stated at a confidence the reader has no way to audit: | Was | Is | |---|---| | "Published practice at scale runs an order of magnitude longer than a day" — ~10 days, which does not reach the 30-day floor it was offered to support | The floor derives from what the window must **out-last**: 30 days is the shortest window spanning a monthly invocation cadence at all, 90 spans a quarterly one, and a one-day window spans nothing | | "twenty years of clone-detection tooling produced no production automation" — a universal negative stated as fact | "no **surveyed** clone-detection tool automates the choice…, across a detection literature the survey found spanning two decades" | | "The flag-lifecycle tools that lead this space" | "The flag-lifecycle tooling **surveyed**" | | "No published effectiveness evidence supports automating it" — universal negative | "No effectiveness evidence… **surfaced in the surveyed literature**" | | "a direct-change posture … has no evidence behind it" | "**no surveyed precedent** supports one" | | "reproducibility around 36.6%" — two significant figures, no citation | "low enough that filing every crash would degrade the queue" — and the population shift is fixed too: the figure measured replay of *crashes*, and was being applied to *filed work items* | | "weakest precedent of the catalog" — a superlative over all forty-nine rows | "among the weakest precedent the survey found" | | "Three independent lines of evidence — peer-reviewed observational work, large-N regression, and a randomized trial", citing exactly one | The rule now rests on the one finding **verified at primary source** (Lenarduzzi et al., quoted verbatim); the other two design families are named as survey context, explicitly not checked | The target register is the one already-well-calibrated claim in the section — *"in some studies associated with fewer faults"* — which is left as it stands. ## Fresh-context verification, and the two contradictions it caught The edits above were checked by a **separate fresh-context agent** with the authoring session's reasoning withheld, briefed to judge whether each finding actually no longer holds, whether any fix introduced a new in-tree contradiction, whether the remaining empirical claims are calibrated, and whether every row still derives correctly. It re-derived six rows from the mapping rules and axis cells alone before reading the `Derived row` column: **six of six matched.** It found two real contradictions, both now closed: - **`gui-crash-fuzzing`'s Output cell (`R + WI`) contradicted its own binding parameter**, which said filing would degrade the governed queue. The previous revision masked this — *"the fleet ships no GUI to fuzz"* made the whole row moot, so the Output cell never had to agree with the parameter. Removing that escape exposed it. The `WI` output is now admission-constrained: an item is filed only where re-running the recorded input sequence reproduces the crash, and the rest stay in the `R` half. That gate is a re-run rather than a judgment, so it adds no `AGT` portion and the `DET` exit stays complete. - **`work-classes.md` deferred the reviewer-burden term on "the org-scale trust-path requirements this contract already defers"** — a phrase with **zero** other occurrences anywhere in the repository. A back-reference leaked from the gitignored research record into normative text, asserting a prior decision no in-tree reader can locate. The three requirements were already enumerated inline, so the phantom reference is dropped rather than manufactured. It also caught the stranded per-item rule, the missing-target-column limitation, the four over-claimed hedges above, and one SSOT gap outside the three files in play: **`guardrails/isolation-ladder.md` scoped `L3` to untrusted-provenance (`C5`) work alone**, while the catalog's access rule has always also required it for unattended GUI actuation — a demand that reaches classes deriving no work class, so it cannot travel through the matrix's min-isolation column. That leaf is the contract's source of truth for when a level applies, and was incomplete against its own charter. Fixed at the root rather than papered over in `routines.md`. ## Other corrections in this pass - **The posture-qualified-identity mandate was unsatisfiable for seven of the nine hybrid rows.** The new parameter made it mandatory for every hybrid row, while *Routine identity* says posture tokens are owned by the class's definition leaf — and seven hybrid rows are `join:`-deferred with no leaf. Scoped: tokens are minted with the leaf, so the obligation binds at leaf time; until then a deferred class records its split in its Judgment cell and binds nothing. - **The `L3` on `gui-crash-fuzzing` does not come from the matrix.** The guardrail matrix's min-isolation column is indexed by work class (`C1`–`C5`); a row deriving no class cannot reach it. The floor comes from the GUI-actuation mapping rule directly, and the row and its parameter now say so rather than implying a matrix lookup that would not resolve. - **`## Precedent pointers`' scope line claimed coverage it did not have.** It declared itself pointers "for the deferred and deterministic rows"; all nine new rows are deferred or deterministic and none got one. Narrowed to the rows where a shipped pattern was surveyed, with an explicit note that absence is not a claim that no pattern exists — rather than inventing nine pointers this pass has no evidence for. - **`dead-code-sweep`'s precedent pointer was stale against the amended row.** It described deletion pipelines with no quarantine stage while the row's whole normative content is staged quarantine — so it read as precedent for the unstaged form. It now says the staged quarantine is the row's own normative content, not a property read off those pipelines. - **`### Reviewer-burden term` sits under a preamble that did not cover it.** The preamble said "the threshold values below are suggested defaults the org binds"; a deliberately-not-live term with no threshold is not a suggested default. Promoting it to a sibling `##` was the alternative and was **rejected** — #2683 names §"Suggested default predicates" as its placement. The preamble was narrowed to the table instead, and now states that the two subsections after it are not defaults and carry no bindable threshold. - **CHANGELOG bullets re-filed per Keep a Changelog**, which the file's own header cites. The `dead-code-sweep` amendment and the derivation-rule bullet change existing content and moved to `### Changed`. The row count is now exact: **nine added, one amended** — class rows go 40 → 49, and "ten rows added" double-counted the amendment. (Commit `21fce8eb`'s body carries the same off-by-one; history is not rewritten, the durable surface is fixed.) ## Sanity checks — actual output, re-run at `c0795c93` - **No two rows share a class token** — `awk -F'|' '/^\| [a-z]/ {print $2}' … | sort | uniq -d` returns **empty**. Verified non-vacuous: the same pipeline yields **51** distinct tokens, against **41** on `origin/main`. The delta is **+10, not +9**: nine new class rows plus the new `join (external): …` status-legend row, which the pipeline also matches because legend rows begin `| join…`. Class rows alone go 40 → 49. - `grep -n "388\|180 merged" plugins/autonomy/reference/routines.md` → **empty**. - `grep -ci "never a promotion input\|no acceptance-rate\|not an efficacy signal" …work-classes.md` → **1**. - `grep -Eci "predicate .*(merge|acceptance) rate|(merge|acceptance) rate .*(threshold|>=)" …work-classes.md` → **0**. - `scripts/check-changelog-parity.sh --check-bump origin/main` → **exit 0** (autonomy 0.16.12 → 0.17.0). - `scripts/check-changed-skills.sh origin/main` → "No changed skills under `plugins/*/skills/`". - `markdownlint-cli2` on all four changed files → **0 issues**. ## Declared human gate — evidence recorded, disposition open #2682: *"a reviewer who did not author the rows re-derives three of them from the mapping rules and the row's own axis cells alone. No command self-clears this."* **A fresh-context agent performed the re-derivation. No human re-derived these rows, and this PR does not claim the gate is cleared.** An earlier revision of this description said CLEARED; that was the authoring session grading its own homework, and a fresh-context same-vendor agent is the middle rung of the independence hierarchy, not the reviewer #2682 names. The full record — the withheld-answer protocol, the agent's unprompted provenance statement, the three derivations, and this same caveat — is posted as a durable comment on **both** #2682 and this PR, so it does not live only in a commit body and a PR description written by the session that produced the rows. | Class | Independently derived | Authored | Match | |---|---|---|---| | `dead-code-sweep` | `C3` | `C3` | yes | | `stale-flag-removal` | `C4` | `C4` | yes | | `abstraction-flattening` | `C4` | `C4` | yes | Three of three. **The run predates three subsequent commits**, so it was performed against a superseded revision — but the later verification pass re-derived all three classes against the current text and reached the same three answers, so the result survives the edits rather than merely predating them. More useful than the match: the derivation showed the mapping rules *imply* three things they never *stated*, each of which the agent had to reason out to avoid a wrong answer. All three are now stated in `## Mapping rules` itself, alongside the fourth the verification pass found stranded. **What a human still owes this gate:** deciding whether an agent re-derivation satisfies it. The record exists so that call can be made against the actual protocol rather than against a summary. ## Related Independent of #2690 / #2692. Blocks the can't-fail test detector's row amendment (#2684). --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fix action's merge-set ledger keyed on file names, and names are not unique: the timestamp has second resolution, the topic is producer-chosen, and nothing requires a producer to put a timestamp in a name at all. Two defects followed. Write side — the consumption record was `<UTC-timestamp>-fix-pass-applied.md`, so two applies on one branch finishing in the same UTC second wrote the same path and the second clobbered the first. That was harmless while the record was audit-only prose; this branch made it the ledger Step 1 subtracts by, so a lost record leaves its files unsubtracted and re-injects already-applied findings — the failure Step 5 exists to prevent. The record is now staged through `mktemp` outside the findings directory, digested, and moved in as `<UTC-timestamp>-fix-pass-applied-<sha256-12>.md`. A content digest rather than a nonce because the only case that still collides is two byte-identical records, which name the same consumed set, so the overwrite is a no-op. Read side — `source-findings:` held bare names and matching was by name, so a producer writing a different file under a name an old record already named was subtracted unread. Entries are now `name:` + `sha256:` mappings, and Step 1 computes each candidate's digest at read time from the candidate's own bytes. A candidate is subtracted only when an entry matches BOTH halves. An entry with no digest matches by name alone — the whole of the 0.19.0 bare-scalar tolerance — and is bounded twice: an entry that has a digest never falls back, and a digest-less entry is honored only when the candidate's `date:` is not newer than the record's `date:`. Declared frontmatter instants, never filesystem mtimes, which a copied or restored memory tier rewrites and whose `stat` format flag differs between GNU and BSD userland. The second bound is load-bearing because a conforming detector may write one fixed name it overwrites every run, and without it a single stale legacy record would retire every future version of that file silently and forever. A candidate with no readable `date:` fails the test and stays in the set: `date:` is not part of the admission test, so the case must be decided, and it is decided toward a recoverable re-application over an unrecoverable drop. The producer-facing findings-file shape is unchanged — the identity is computed by the consumer, so a conforming detector needs no edit. `default-mode.md`'s writer contract gains a never-overwrite rule for a producer's own path, stated as hygiene rather than a second identity mechanism. The same under-specification reached the directory itself. Both skills glossed the findings home as `<memory_dir>/reviews/<branch-slug>/` unconditionally, but only two of the binding's five rungs compose that segment; a producer and a consumer disagreeing about it land in different directories, and the symptom is a clean empty-set STOP an operator cannot tell from "no findings". Both `SKILL.md` "Shared inputs" bullets, fanout's Step 1, and the README now cite `reference/topic-docs.md` as the authority instead of restating a path shape, and that binding states which rungs compose the segment. The binding also now cites the topic-docs convention's "Non-interactive / forked mode" section, which is contract-owned and which it previously neither cited nor covered — leaving the headless `--yes` path with no stated behavior at the rungs that need a user. The record body gains a rendered `## Not applied` table — location, finding, why, and source file — so every row that did not land is individually recoverable. Step 5 already required that attribution; the template rendered it on one of three lines, reporting surfaced rows as a bare count and giving operator-narrowed rows no home at all. Recovery means re-running the producer that found the row, and a count cannot say which one that is. Also corrects the CHANGELOG's stale names-not-paths rationale, which cited a cross-`memory_dir` property the design does not have; scopes `default-mode.md`'s "required" coverage sections to fanout's own writer, which contradicted Step 1's minimal admission test; pins `## Surfaces` attribution to the consumed file's name, the only identifier the shape carries; and softens the one-file "byte-for-byte" claim to the applied set, which is what is actually preserved. Evals: 19, 22, 28 and 30 updated; 29, 31, 32, 33 and 34 added to pin the read-side collision, the attribution table, the fixed-name legacy producer, the undated candidate, and the minimally conforming detector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oked
An independent verifier found the not-newer test had reintroduced, through
its tiebreak, the failure it was added to prevent.
`date:` is producer-DECLARED, not machine-observed. `default-mode.md` declared
it as a bare `date: <ISO-8601 UTC>` with no stated semantics, so a detector
deriving it from the commit under review, a scan date, or a template constant
is making an ordinary implementation choice. With a constant `date:`, EQUALITY
IS THE NORMAL STATE — and subtract-on-equal then let one pre-0.20.0 record
retire every future version of a fixed-name file, silently and forever.
The tiebreak now fails open: a digest-less entry is honored only for a
candidate strictly OLDER than the record, and equal keeps the candidate. That
is the direction every other clause in the comparison already takes — an
unreadable `date:` keeps the candidate, a missing digest narrows rather than
widens — because re-application is recoverable and silent retirement is not.
The old justification ("preserving the behavior the record was written under")
preserved nothing: 0.19.0 ran no date comparison at all.
The producer-facing shape now states what `date:` MEANS — the instant the file
is written, not a commit date, scan date, or constant — and that the file name
must end in `.md`, since the consumer's scan is the only way a file is ever
seen. Both bind fanout's own writer; the consumer still assumes neither.
Comparison is now specified: normalize to UTC and compare as instants, accept
an explicit `Z` or numeric offset, treat date-only or designator-less values as
unreadable. The string-comparison shortcut is withdrawn — it inverts on
fractional seconds, where `…01.123Z` sorts before `…01Z` while being later.
The empty-set STOP now prints the resolved directory and the rung that resolved
it, and on a non-interactive run says the asking and persisting rungs were
skipped. Step 1 diagnosed a wrong-directory resolution as indistinguishable
from "no findings", then printed nothing an operator could tell them apart
with; this is also the skill's half of the binding's cited non-interactive rule,
which requires surfacing the assumption rather than silently taking a default.
Smaller items from the same review: `date` added to the shape's required list,
so the parity claim between the shape and the admission test is true for the
field this change turns on; a cell-escaping rule for the `## Not applied` table,
whose rows carry producer text and whose loss costs the source attribution that
makes a row recoverable; the `/simplify` staleness note scoped to files that
declare a readable `date:`, since a fixed-name producer has no age to read; and
`SKILL.md`'s inlined timestamp spec replaced by the binding it says owns it.
The pattern behind the finding is the reason for the last fix here: this doc
had twice stated an optimistic bound as a property of the system when the
admission test does not guarantee it. A pass over the remaining absolutes found
one more — the candidate scan assumes `*.md` — now stated as a requirement of
the shape rather than left as an inference about producer naming.
Evals: 28 and 32 updated for the flipped tiebreak; 35 added to pin the
constant-`date:` producer, the case the old tiebreak silently retired.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s-convention The parent absorbed origin/main and landed the content-digest identity, so this stacked branch had to take both. One conflict, in the paragraph both branches legitimately rewrote. Resolved as the union of the two intents rather than by picking a side. The parent's precision survives -- "by content digest, not by file name" is the whole point of its change and the sentence is where a producer author learns it. This branch's pointer to the detector-findings convention survives too, because the producer rules are a cross-plugin concern and one owner doc per shared concern is what moved them out of this file. Dropped from both: "A producer needs nothing beyond this shape; it must NOT append into another producer's file." That is a coexistence obligation binding every non-fanout producer, which is exactly what the convention now owns and states. Keeping it here would reintroduce the duplication this branch exists to remove, one line after pointing at the doc that owns it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
The `hygiene` job failed on the `typos` step: `unparseable` should be `unparsable`, twice -- `context/fix-pass-mode.md` Step 1's date-readability rule and eval 33's expectation for the same rule. Both are prose written in this PR. Worth noting where the gates did not catch it: `check-changed-skills.sh` and `markdownlint-cli2` both passed on these files, because neither is a spell checker. `typos` runs only in CI, so a misspelling in newly authored skill prose is invisible locally no matter how carefully the skill gates are run. The third occurrence in the tree, `plugins/source-control/.../babysit_checks.py`, is pre-existing and was not flagged -- the run emitted exactly two warnings, both in this plugin. Left alone rather than swept up: it is outside this PR's scope and touching it would put an unrelated plugin in this diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 1's compact bolded rule still said a digest-less entry matches "only if the candidate is not newer than the record" -- a <= condition that includes equal -- while the rule six lines below, default-mode.md's writer contract, the CHANGELOG migration note and eval 35 all specify strictly older, equal keeps the candidate. The summary sentence is the one an implementer skimming for "the rule" extracts, and read at face value it authorises exactly the failure the strictly-older test was added in this same PR to prevent: a producer with a constant date: retired permanently by one pre-0.20.0 record. Self-inflicted and worth naming: the tiebreak was flipped from not-newer to strictly-older in an earlier commit, and every downstream statement was updated while the upstream summary was not. Swept the plugin for the old phrasing -- no other occurrence survives, so the class is closed rather than the instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cers' into docs/detector-findings-convention
|
Claude finished @kyle-sexton's task in 1m 33s —— View job Security Review
Skip gate triggered — no security-relevant surface. Diffed this head ( (The intervening merge commits — No code execution paths, no authorization/access-control logic, no injection surfaces (command/SQL/path/template), no secret/token handling, and no GitHub Actions workflow files are touched. The No security issues found. |
#2690 squash-merged, so main carries its changes as one commit while this branch carries the same content as real history. Git sees divergent ancestry for identical content, which is why three files conflicted despite nothing actually disagreeing. All three resolved to this branch's side, because it already absorbed #2690's final state -- including the typo fix and the strictly-older tie-case correction -- through the parent merges before the squash landed: - plugin.json: 0.20.1, which is this PR's bump on top of #2690's 0.20.0. - CHANGELOG.md: keeps the 0.20.1 entry above the 0.20.0 one main just gained. - default-mode.md: keeps the union resolved earlier -- "by content digest, not by file name" from #2690, plus the pointer to the detector-findings convention from this branch. Verified after resolution rather than assumed: no conflict markers survive anywhere under plugins/ or docs/, the version reads 0.20.1, and both changelog headings are present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude finished @kyle-sexton's task in 46s —— View job Security Review
Skip gate triggered — this head already has a successful security review that still applies. Current head (
No new security-relevant surface since the last clean review. No security issues found. |
|
Claude finished @kyle-sexton's task in 4m 32s —— View job Claude finished the code review
SummaryReviewed this docs-only PR (owner-doc stub for One finding, posted inline on Everything else I independently re-checked against current file state came back accurate:
Not independently run (no lint/script execution tooling available in this session): |
This doc said "The two texts state different rung counts -- SKILL.md inlines the rungs it operates on -- and the non-interactive rule below is what makes them coincide." True when written. Not true now. #2690 rewrote SKILL.md "Shared inputs" to delegate the ladder entirely: "Resolve the home; never assume its shape ... Read the binding rather than working from the default's shape." It inlines no rung count at all. So there are no longer two texts to reconcile, and the non-interactive rule is not what reconciles them -- verified against SKILL.md:32 on this branch rather than assumed from the commit message. The failure mode is worth naming, because a stack invites it: this branch described a sibling PR's pre-fix state, the sibling then fixed it, and the merge brought the fix in without touching the sentence describing the problem. Nobody edited a wrong line -- the line went wrong when a different file got better. And it landed in the one section this doc points readers at for authoritative resolution behaviour, which is the citation-accuracy bar the doc holds everything else to. Replaced with what is now true: SKILL.md does not restate the ladder, so a producer and the consumer read one text rather than two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 36s —— View job Claude finished the code review
SummaryReviewed under the code-review lane (
That one change (
No new findings. Nothing else in the diff changed, so the rest of the doc stands as previously reviewed (all citations, line references, and relative links independently re-checked across the last several passes and found accurate). Not independently run (no lint/script execution tooling available in this session): |
…-persist-findings Squash divergence from #2692: main carries the detector-findings convention as one commit while this branch carries the same content as real history. Resolved to this branch's side for the Adopters row and the enforceability-verdict flips, which this PR owns, and took main's newer "Where the file goes" paragraph, which this branch predates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…le behind a flag (#2715) Closes #2680. Phase 3 of the boris-routines-adoption plan — the **integration slice**: the smallest end-to-end demonstration that a non-fanout producer reaches the apply relay, run against a detector that already existed rather than one invented alongside the contract. Stacked on #2692 → #2690 → `main`. Base is `docs/detector-findings-convention`. ## What this does `/mutation-testing:audit --persist-findings` writes the run's survivors as a findings file the `review:fanout` `fix` action consumes. Opt-in; bare invocation still reports and stops. No fanout edit, no registration, no dispatch wiring — the producer writes one conforming file and that is the whole integration. Mechanics live in the new spoke `plugins/mutation-testing/skills/audit/context/persist-findings.md`, which reads the producer contract for this plugin and points at it rather than restating it. ## The open question the issue handed us: zero survivors **Settled: the discriminator is whether the run *examined* anything, not whether it *found* anything.** - **≥1 mutant examined, no rows to emit** → **write**, with the `## Findings` header row and no data rows. The payload is `## Surfaces`. The consumer unions `## Surfaces` across producers precisely so "this surface ran and returned nothing" is not lost; a merged report saying only "the reviewers found three things" reads differently from one that also says the mutation surface ran over these files and found nothing, and the second is the true one. An empty table still meets the admission test, so the file is consumed and its coverage reaches the plan. - **No mutant examined** — empty scope, everything dropped by coverage/suppression/cap, or a Phase 0 refusal → **write nothing.** There is no coverage to report, and a `## Surfaces` line claiming the surface ran would assert coverage never attempted. The reasoning is recorded in the skill's own text, not only here. ## Severity is machine-computed, and two classes emit nothing | Phase 4 class | Row | `Tier` | |---|---|---| | Productive | emitted | IMPORTANT | | Unclassified (equivalence claimed, no demonstration) | emitted | IMPORTANT | | Arid | **none** | — | | Equivalent | **none** | — | Productive is IMPORTANT rather than CRITICAL because a survivor demonstrates that *the suite* fails to detect a change — it establishes no input, caller, or future edit that produces a wrong result, which is what all three of CRITICAL's limbs require. **Arid emits no row** because its only remediation is a suppression entry the user must accept, and handing a consent-gated write to an apply relay would launder that gate; the human still sees it in the Phase 5 report. **Equivalent emits no row** because it is not a defect. The map is deliberately flat — every emitted row makes the same claim, so manufacturing a tier spread would mean re-deriving tier from prose, which is what a class-keyed map exists to prevent. `Confidence` is `high` on every emitted row (Phase 3 executed the mutant) and **never `low`**, which ranks below omitting the field. ## Sanity checks | # | Check | Result | |---|---|---| | 1 | `check-changed-skills.sh origin/main` exits 0 | **PASS** — evals present, all 5 base-ref trigger phrases preserved verbatim | | 2 | `branch:` equals `git branch --show-current`; filename matches the timestamp pattern | **PASS** — verified mechanically on both specimens | | 3 | With a fanout findings file also present, `fix` plans findings from **both** producers and names both files | **PASS** — executed, see below | | 4 | `git status --porcelain` empty after a persist run except the gitignored write | **PASS** — clean tree; `git check-ignore` confirms the specimens ignored | ## Sanity check 3, executed A fresh-context agent read `fix-pass-mode.md` and executed Steps 1–3 against two real files in the resolved findings directory, with my reasoning withheld. It resolved the destination by running the rung order itself, admitted both files, merged, and hit the non-interactive gate (no `--yes`) — **STOP after the plan, mutate nothing, write no record**, which is why the demonstration cannot damage the tree. ```text Fix-pass plan — consumed 2 findings file(s), 4 findings after merge - 20260815T143000Z-review.md (tier: medium) - 20260815T144500Z-mutation-survivors.md (tier: unstated) - Surfaces (union) — ran: [code-reviewer, doc-drift-detector, architecture-guardian (20260815T143000Z-review.md), mutation-testing:audit (20260815T144500Z-mutation-survivors.md)]; returned no result: [architecture-guardian (20260815T143000Z-review.md) — no module or layer structure touched] - Cleanup-class (1) → /simplify - Correctness-class (2) → sequential scope-fenced fix - Surface-only (1, need human judgment / unparsed) ``` Both producers are consumed and named, both tiers reported — the detector's absent `tier:` renders as `unstated` rather than being invented — and `## Surfaces` is unioned with each producer's line attributed to it. Merged ranks: 1 `SKILL.md:216` (fanout), 2 `check-skill-precompute-compose.sh:30` (**detector**), 3 `skill-leaf-name-registry.txt:55` (fanout), 4 `README.md:11` (fanout). **The plan block evidences the merge, not row authenticity** — it would look the same for invented rows. Authenticity is proved separately, below. ## Row authenticity — measured, not asserted An earlier round of this work had hand-authored rows citing *fabricated* code at real `file:line` values. That is the failure this contract's `Location` rule exists to prevent, and it was caught in review. Both specimens were rebuilt from measurement. The detector row comes from an executed mutation run in a scratch copy (no tracked file touched; restore verified byte-identical): ``` === BASELINE === exit=0, 5 passed, 0 failed ROR line30 (-gt 0 -> -ge 0) SURVIVED while [[ $# -ge 0 ]]; do ROR line47 (-eq 0 -> -ne 0) KILLED ROR line86 (line_count > 1 -> >= 1) KILLED LCR line86 (&& -> ||) KILLED ROR line86 (git_count == 1 -> != 1) KILLED ROR line126 (scanned == 0 -> != 0) KILLED ROR line134 (STRICT == 1 -> != 1) KILLED SBR line89 (drop return 1) KILLED RESTORE: scratch script byte-identical to pristine ``` The survivor is genuine and its cause is precise: every test in that suite enters through the `-- | --all | --paths)` branch, which `break`s out before `$#` reaches 0, so no test drains arguments through the `*)` fall-through and the loop's termination condition is unasserted. A differential run of the mutant from a real checkout differs on **5 of 6** invocation shapes (dies on `case "$1"` with an unbound variable under `set -u`); only `--all` is identical. **That oracle gap is a real defect in this repo's tooling and is filed separately as #2708** — it is not part of this PR. Every `Location` cell in both specimens was then swept mechanically against tracked files; all four resolve to a real tracked line. One miscite (`skill-leaf-name-registry.txt:52`, actually `:55`) was caught by review and fixed. ## The second survivor: equivalent, and therefore absent from the table `scripts/check-docs-only.sh:76` (`break` removal) survived and is classified **equivalent**, so it emits no row and is reported in `## Surfaces` instead — the specimen demonstrates the omission rule on real data. Evidence: 8 differential inputs identical on stdout, exit code and stderr; `$prefix` dead after the loop; no iteration counter; `matched=1` idempotent. **Scope note, stated plainly:** the 8 differential inputs were executed by me, not re-executed by the independent verifier. The verifier confirmed the *structural* argument (dead variable, no counter, idempotent assignment) and said explicitly that it did not re-run the inputs. Read that limb as single-sourced. The equivalence probe itself was wrong twice before it was right — the allowlist was committed *with* the change (so the flag was decided before the mutated loop was reached), and the script `cd`s to its own parent, so invoking it from outside a checkout resolved a different repo. Both produced an all-`IDENTICAL` result that proved nothing. The harness now asserts that both `docs_only=true` and `false` appear, so a degenerate run announces itself. ## Defects found in review and fixed - **The tree self-check was vacuous.** It compared `git status --porcelain` before and after — but a `.gitignore` containing `*` matches *itself*, so a memory root inside tracked space leaves porcelain byte-identical whether the write was ignored or not. Replaced with a positive `git check-ignore -q` proof before any write, plus the three states it catches that reasoning about the guard alone does not. - **"The only write it makes"** — there are two; the self-ignore guard may create a `.gitignore`. Both are now subject to the same proof. - **The spoke restated contract premises** under a no-restatement banner, including one the parent branch had just deleted. Rewritten to point. - **The CRITICAL dismissal addressed 1 of 3 limbs.** Now addresses all three. - **"A wording correction, not a loosening"** was false — the invariant *did* widen. The CHANGELOG now discloses it as a widening. - **Contract-unfetchable path** added: report and stop, never invent a destination. ## Invariant amendment `SKILL.md` and `scripts/skill-leaf-name-registry.txt` now read **read-only with respect to tracked source**. Mutants are still applied, measured and reverted, and tracked source is still byte-identical when the run ends. This is a real widening of what the skill may do, disclosed as one — what did not change is the skill's standing under the naming doctrine, whose verb table already permits mutation behind an explicit user override. ## Contract tables updated here The `Adopters` row lands in the commit that makes it true, per the contract's own rule that a row is tabled only once a producer actually conforms — adding it in #2692 would have asserted what a reader could not rely on. The conformance-gate recheck trigger it named ("the first detector reaches `main`") is recorded as **fired**, and the two enforceability rows reading "**Not built**: no producer exists yet" now read as buildable. No gate is invented. Convention CHANGELOG 1.0.0 → 1.1.0. ## Known limitation, routed not solved A mutation finding's remediation lands in the covering test, not at its `Location`, so a consumer fencing each fix to `Location` cannot reach the target. `Location` is **not** retargeted (that would destroy the row's cross-producer dedup key) and **no column is invented**. Naming the test file in `Action` makes the conflict explicit rather than resolving it. Disposition belongs to #2681. ## Deliberately out of scope - **A mechanical conformance gate.** The contract defers it; this PR makes it buildable and says so, rather than shipping one unasked. - **#2708.** A real defect in this repo's own test coverage, found by this pilot, filed separately so it is fixed on its merits. - **A structured field for "equivalent".** An equivalence disposition currently lives in free prose and cannot be machine-checked. That is a crosswalk/vocabulary question routed to #2681, not a field to invent here. - **Consumed-file identity.** The text says nothing about how the consumer identifies what it consumed, so it is correct under both name-based and content-digest ledgers. ## Related - #2692 — the detector-findings owner doc this PR is the first adopter of. **Direct base of this PR.** Its `Adopters` and enforceability tables are updated here rather than there, because this is the commit that makes the row true. - #2690 — merge findings across producers and mark consumption explicitly. Grandparent in the stack; its gate is what this PR proves in situ rather than in a fixture. - #2681 — findings crosswalk. Carries two dispositions this pilot surfaced and deliberately did not solve: a producer whose remediation site is not its `Location`, and the absence of any structured field for an "equivalent" disposition. - #2708 — `check-skill-precompute-compose.sh`'s argument parser is never exercised by its test suite. A real defect in this repo's tooling, found by this pilot's measured mutation run and filed separately so it is fixed on its own merits. Not closed by this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ity crosswalk (#2737) Phase 4 of the boris-routines-adoption plan. Closes #2681. Builds on merged #2715 → #2692 → #2690; rebased onto `main`. ## The trap this closes `plugins/review/context/severity.md` defines tiers by **tests**, not examples — "The test decides the tier". CRITICAL is "you can name a concrete input, caller, or subsequent otherwise-correct change that the defect makes produce a wrong result". A bare threshold cannot evaluate that predicate, so a threshold-to-tier table with no test in it is nominal closure. **The argument is the row.** Every crosswalk row carries the test its mapping asserts, and a rule whose tier cannot be argued from the test is **not admitted** — its detector reports to a human, which the contract's boundary already places outside this convention. The crosswalk is seeded with the four rules the first adopter evaluates, two of which argue a *non-emission* rather than a tier. ## The three judgment calls the issue handed over **1. Shared emitter — three implementations accepted, no registry cluster declared.** Decided on what the mechanism can actually detect, not on preference. `scripts/check-cross-plugin-source-drift.sh` clusters files by path-within-plugin under `plugins/*/` and compares hashes, so a `docs/` convention can never be a cluster; and a registered path that is not a live byte-identical 2+-plugin cluster fails as `REGISTRY STALE` — verified by adding one such line and running `--check`. There is also no emitter *code* to share: both existing emitters are prose a model executes. What prevents drift is one owner per mechanic, reached by pointer. The revisit trigger fires itself — the first emitter code copied across two plugins is reported `UNREGISTERED` by that same script. Nothing mechanical catches producer drift today and Enforceability says so rather than dressing it up. **2. The scope fence (comment 1) — a real disposition, not a column.** `Location` still names the detection site and is never retargeted; the producer names the remediation target in `Action`. `fix-pass-mode.md` Step 2 routes such a row to surface-only and Step 4 surfaces it under a fourth named trigger. Deciding it at classification rather than apply time is what keeps Step 3's counts honest — the correctness count is what Step 4 will attempt. A remediation-target column is recorded as considered and rejected: what it would enable is an unattended two-file apply, which is the one thing the fence exists to forbid, and the existing contained-fix criterion already reaches that verdict without new structure. **3. The equivalence disposition (comment 2) — a property of the rule, not of the finding.** Three outcomes, three homes. A declined candidate is coverage, reported as a count per rule id in `## Surfaces`. A real finding an operator accepted belongs to `finding-suppression/` — and binding an automatic decline there would launder a consent gate, so a producer *proposes* an entry and never writes one. A not-a-defect claim without the rule's stated evidence emits a row. Because disposition belongs to the rule, it is declared once in the crosswalk and only the count is per run — no field is added to the 7-column shape and no new file type with no reader is invented. ## Also in scope - **Rule and threshold vocabulary.** Every emitted row leads its `Finding` cell with the rule that fired and the threshold it crossed in the run's own values. A rule id is `rule-<slug>`, qualified `<plugin>/<skill>/rule-<slug>` — exactly the `check:` constituent `finding-suppression` hashes into a `finding_id`, so it is one vocabulary rather than two. - **`REVIEW.md` cited as the consumer-precedence override**, with the reason it is not decorative: this repository's own vocabulary folds Critical and Important onto one marker, so the tier name does not survive the fold and the row's argued test is what lets a reader re-derive which side of it a finding sat on. - **Auto-applicability settled per rule at contract time.** Cross-file and architectural-judgment rules are never auto-applicable — layering, abstraction, and coupling detectors are *designed to inform a human*, which is the intent of the route rather than a limitation in it. Shaping a rule to look auto-applicable, by narrowing `Location` or lowering `Confidence`, is named as the failure it is. - **The tier argument moved rather than being copied.** Why a productive survivor is IMPORTANT and not CRITICAL is a rule-to-tier argument every consumer of the rule needs; it now appears in exactly one file, and `persist-findings.md` keeps only the class-to-rule map a mutation run owns. - **Enforceability re-rated.** `Tier` is machine-computed moves from reasoning-only to detect-then-judge, because a rule id in every row gives a gate something to check. Three rows added, including one recording honestly that a declined-candidate count is greppable but no gate can know what a run examined. ## Versions `review` 0.20.1 → 0.21.0, `mutation-testing` 0.2.0 → 0.3.0, detector-findings convention 1.1.0 → **2.0.0** (major under the contract's own rule: three producer-owned obligations are added, and a producer ignoring any of them stops conforming). ## Sanity checks (issue #2681) ```text grep -c "finding-suppression" docs/conventions/detector-findings/README.md -> 3 grep -c "REVIEW.md" docs/conventions/detector-findings/README.md -> 2 scripts/check-cross-plugin-source-drift.sh --check -> exit 0 scripts/check-detector-findings-crosswalk.sh --check -> exit 0 "Crosswalk OK: 4 rule row(s), every disposition argued from a stated test." ``` ### One stated divergence from the issue's literal check The issue specifies `awk -F'|' '/^\| rule-/ …'` and `grep -c "^| rule-"`, both of which presume a **bare** rule id at the start of each row. Independent verification found that bare ids are exactly what makes the table unsafe: it is a shared cross-producer registry, and the gate it enables resolves an emitted id against a row by exact match, so two producers sharing a slug would resolve to the wrong row. Rule ids are now fully qualified, and `^| rule-` matches zero rows. **The bar is unchanged and now stronger; only the pattern that measures it moved.** `scripts/check-detector-findings-crosswalk.sh` enforces the same two properties the issue's checks were proxies for — every row has a non-empty test cell, and the row set is exactly the rule set — plus four the greps could not reach: a prose-free cell (an em dash passes a non-empty test while arguing nothing), an unqualified or duplicated id, a row whose cells an unescaped pipe has shifted, and a restatement of the findings-file table. It locates the table by its exact header rather than a row prefix, so a neighbouring table can neither satisfy it nor be dragged into it. It ships with 13 discriminating self-tests and runs in CI as `detector-findings-crosswalk-gate`, so the bar outlives the issue that stated it — which is what the Enforceability row previously claimed falsely. Also green after re-syncing from the parent (which moved three times, once with a conflict): `check-changelog-parity.sh --check / --check-order / --check-bump`, `check-changed-skills.sh`, `check-contract-slice-prune.sh --check`, `check-shell-portability.sh`, `shellcheck`, `markdownlint-cli2` (1127 files, 0 issues), `typos` (0). Pointer-not-copy guard `! grep -q "^| Rank | Tier | Confidence" docs/conventions/detector-findings/README.md` still passes, and is now also enforced by the crosswalk gate. ## What independent verification changed A fresh-context verifier (rationale withheld) returned 2 blocking, 4 should-fix and 7 nits. All were applied; the two blocking ones were real defects in the first commit: - **The crosswalk falsified itself.** A bare-id table sat three lines below prose claiming a bare id "is only ever read inside one producer's own file". Fixed by deleting the short form rather than repairing the clause. - **`rule id == finding-suppression check:` was false.** `suppression.md` keys its `check:` to the mutation *operator*, because a suppression retires per mutant while a rule classifies a disposition. The pilot's file was right and this doc was wrong; the claim is downgraded to shape-compatibility and `suppression.md` is untouched. - **A consumption loop.** Every rule this producer emits is off-site, so Step 2 routed every row to surface-only, so nothing applied, so no record was written, so Step 1 never subtracted the file — re-merged and re-surfaced forever. See below. - Both non-emitting crosswalk rows argued from `severity.md`'s **Action** column, not its **Test** column, failing the admission test they sit under. - Two pointers landed on the wrong heading (`Findings-file shape` vs `Findings-writer contract`), including one pre-existing instance. ## The consumption-record repair The record's trigger was "the action applied anything". That conflated a gate the operator **declined** with a pass that **ran to completion and surfaced every row** — only the first is what the no-record rule was for. The trigger is now a **consented gate followed by a pass that ran to completion**, whether or not it mutated the tree. A declined gate and the non-interactive STOP still write nothing: both emit a plan and process nothing, and a record there would retire files the action never opened — a worse silent drop than the loop being closed. `Consumption is per FILE, not per row` is **extended** to zero-applied rather than merely applied to it, and the PR says so: its wording covered a *partly* surfaced file, which presupposes a non-empty applied set, so the zero case was silent rather than decided. Two zero shapes are argued separately because the recoverability argument is vacuous for the second — a file whose rows were all surfaced is retired because each is rendered in the "Not applied" table with its producer; a **coverage-only** file (no data rows, the ordinary output of a clean detector run) is retired because it carries coverage rather than findings and has nothing to recover. The trade is stated rather than sold: re-running the producer is the only route back, and for a mutation run that is a full re-audit. ## Deliberately out of scope - **A mechanical conformance gate.** The contract still defers it and now records four detect-then-judge verdicts naming what a gate could check. Shipping one unasked would be a different PR. - **A remediation-target column and a disposition field.** Both considered, both rejected in the doc with the reasoning recorded so they are not silently re-litigated. - **Rules for detectors that do not exist.** The crosswalk is seeded with the four rules the one live adopter actually evaluates; inventing rows for a hypothetical layering detector would be fabrication, and the admission test is what the next detector brings its rules to. ## Related - #2715 — the mutation-testing pilot (**merged**). Its two pilot observations are this issue's spec, and its `persist-findings.md` is edited here so the Adopters row stays true. - #2692 — the detector-findings owner doc this PR hardens. Parent in the stack. - #2690 — merge findings across producers and mark consumption explicitly. Grandparent; its Step 2 collapse key and Step 5 record are what the off-site disposition composes with. - #2684 — the can't-fail test detector, named in the pilot's first comment as the next producer to land on the off-site problem. Not closed by this PR; it is the first consumer of the disposition added here. - ADR 0010 — the findings coexistence decision the merge set rests on. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…sub-threshold miss (#2905) Closes #2865. ## What this fixes `scripts/silent-revert-incidents.txt` pins a `clean` row that is supposed to be the closest a non-incident got to the 200-line threshold without crossing it — the row that breaks first if a threshold change starts taxing ordinary development. The pinned commit was not that, and its note named a number that is not a pull request. **Defect 1 (the re-pin).** Measured over the file's own 500-commit corpus (`7b47d2253~500..7b47d22`) at the pinned invocations (#2843, `GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1`), the pinned `c8470efd0` scores **136** blamed lines — sixth-closest of the eight commits in the 100–199 band. The true closest miss is **`9a2307c43` (#2189) at 195 lines** from `3584ae1fa` (#2183) — a margin of 5 lines, not the ~71 the old row implied. `9a2307c43` is now the lead `clean` row. **Defect 2 (the wrong PR number).** The old note credited the deleted content to #2679, which is a closed issue in this repository, not a pull request (`gh pr view 2679` cannot resolve it). The blamed lines trace to `6370a44e7`, the squash commit that landed #2715 — `gh api .../commits/6370a44e7/pulls` returns only #2715, and the commit's own body says `Builds on merged #2715 → #2692 → #2690`. The row is kept as a second guard with its note corrected, rather than dropped: it is still a verified-legitimate quiet commit, and keeping it costs a few lines of prose. **Defect 3 (the "once a month" rate)** was already fixed by #2847, which removed the rate claim from `scripts/check-silent-revert.sh` entirely. Nothing in this PR touches it. ## What this does NOT do Neither clean-row figure is CI-asserted, before or after this change. `clean` rows carry no bracketed attribution field (#2879) — their assertion is the absence of findings, which has no per-culprit count to pin — so the 195 and 136 are hand-measured prose, not watched numbers. The old row's recorded 129 drifting to a measured 136 under the pinned invocations without anything going red is exactly that gap, and the section comment now states it so a reader does not mistake the re-pin for an assertion. Both counts are written as floors ("no fewer than") because `attribute_file` drops `git blame`'s stderr (#2880), so any line blame fails on is silently not counted. ## Verification - Spot-checked both figures against PR #2843's pinned invocations before editing: `9a2307c43` reproduces **195** (75 lines `song-forms-examples.md`, 67 `box-model.md`, culprit `3584ae1fa`), `c8470efd0` reproduces **136** (109 lines `persist-findings.md`, culprit `6370a44e7`). - `bash scripts/check-silent-revert.test.sh`: **101 passed, 0 failed** on this branch. - `scripts/check-silent-revert.sh --verify-known-incidents`: exit 0 — all three `fires` rows reproduce their attributions exactly, and both `clean` rows stay quiet. - `scripts/check-silent-revert.sh --verify-restoration`: exit 0 — all 5 markers present. - A fresh-context verifier independently swept all 500 corpus commits twice (complete coverage: 487 `ok` + 2 `acknowledged` + 11 finding commits = 500) and reproduced every figure in the file. Its verdict: **195 at `9a2307c43` is the highest sub-threshold score** — the next highest is 188 (`3d69448cb`) — so the lead `clean` row pins the true closest miss. The two acknowledged commits were re-run with the ack file disabled and score 447 and 323, both above the threshold, so neither could displace it. ## Related - #2843, #2847, #2873 — the three PRs that reshaped the canary and this file ahead of this change; the figures here are measured under #2843's pinned invocations. - #2879 — records that `clean` rows carry no bracketed attribution field, which is why neither figure in this PR is CI-asserted. - #2880 — records that `attribute_file` drops `git blame`'s stderr, which is why both counts are written as floors. - #2831 / #2832 — the same wrong-PR-number defect shape, corrected earlier on the `fires` rows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LwdkpWf6bptu3AqTMoeg2H --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes #2679
Phase 2 of the boris-routines-adoption plan.
Why a stub now rather than the full doc later
docs/PLUGIN-PHILOSOPHY.md"Convention registry" — "A new cross-plugin convention lands in an ownerdoc before a second plugin adopts it" — is a deadline, not a licence to author it late. The
detector pilot makes a second plugin the adopter of the multi-producer rule, so the owner doc must
precede it. Depth then trails the pilot, because the pilot is what produces the evidence to harden
against. Both halves are load-bearing: the deadline is why the stub cannot wait, and the evidence is
why the stub is a stub.
This is also a hard merge barrier: a plugin installs standalone and cannot resolve a
repo-relative
docs/conventions/path, so the citation form is a raw URL tomain(
plugins/architecture/reference/topic-docs.md:6is the repo's exemplar). Nothing can cite this docuntil it is on
main.What the doc owns — and deliberately does not
Owns: why the contract is format-only, the four producer-owned fields, coexistence between
producers, consumption semantics as they bind a producer, minimal conformance, the liveness
relationship, enforceability, and adopters.
Does not own, and points at instead:
plugins/review/skills/fanout/context/default-mode.md"Findings-fileshape". Pointer, never a copy — a second statement of a table is a second thing to drift.
! grep -q "^| Rank | Tier | Confidence"passes.context/fix-pass-mode.md"Step 1: Build the merge set".context/findings-normalization.md. A detector emits final values,not pipeline inputs.
The four producer-owned fields
A detector has no severity crosswalk, no confidence filter, and no normalization stage behind it, so
it computes these itself — and each fails silently:
Tieris machine-computed, never guessed — derived from the rule that fired, identically forevery finding of that class, or rank order stops meaning anything across runs.
Confidenceishighor OMITTED — neverlow. This is the rule a first detector author willget wrong, because it inverts the intuition: the rank order is
high>medium>unscored>low(findings-normalization.md:72) and absent confidenceresolves to
unscored, which ranks abovelow.:62states it outright — "Absentconfidence ≠ low." Emitting
lowto express uncertainty buries the finding beneath the one younever wrote. Both line references verified against the file, not recalled.
Locationis a repo-relativefile:line— the fix action fences each remediation to it, andan absolute path is not portable across the checkout that applies the fix.
pipelines, type unions, regex alternation), and an unescaped one splits the row into phantom
columns that parse wrong rather than not at all.
Moving the multi-producer rule out of the review plugin
#2690 (0.20.0) stated the multi-producer rule in
context/default-mode.md. That rule binds everycomponent that writes a conforming file, and
PLUGIN-PHILOSOPHY.md"Convention registry" is oneowner doc per shared concern — a rule binding three plugins cannot live inside one of them. This PR
therefore also:
default-mode.mdrecording only what fanout's own writer does, plus a raw-URL pointer,reviewto 0.20.1 with a matching CHANGELOG entry. No behavior change.This is the #2679 work item "Own the multi-producer rule here, not in a review-plugin context file
— #2678's file points here", executed rather than deferred.
Sanity checks
All four of the issue's checks, plus the gates:
ls docs/conventions/detector-findings/README.md docs/conventions/detector-findings/CHANGELOG.md→ exit 0.
grep -c "detector-findings" docs/PLUGIN-PHILOSOPHY.md→ 1 (the registry row).! grep -q "^| Rank | Tier | Confidence" docs/conventions/detector-findings/README.md→ passes.Phrased as a negated
grep -qrather than checkinggrep -cfor 0, becausegrep -cexits 1on a zero count — a check written as "returns 0" fails on success and passes on the condition it
was meant to catch.
gh pr view --json state -q .statereturnsMERGEDbefore the pilot phase opens — the onecheck this PR cannot satisfy itself. It is why the PR exists.
scripts/check-changelog-parity.sh --check-bump origin/main→ exit 0.scripts/check-changed-skills.sh origin/main→CHECK-SKILL fanout: PASS — 0 errors, 1 warning(s)(the pre-existing no-Gotchas-surface warning).
markdownlint-cli2on all five changed/added markdown files → 0 issues.Shape
Follows the majority sibling shape (12 of 20 convention directories carry a CHANGELOG):
README.md+CHANGELOG.md, SemVer with a stated bump policy, a Boundary section, an Enforceabilitytable classified per
melodic-software/standardsenforceability-tiers.md, and an Adopters table.Enforcement is deferred with event triggers rather than dates — a conformance gate has nothing to
run against until the first detector reaches
main, and the doc's own depth waits on the pilot or asecond adopter, whichever comes first. The adopters table lists only
review:fanout, because tablinga planned adopter would assert what a reader cannot rely on.
Independent review
A fresh-context reviewer audited the diff against #2679 with the authoring rationale withheld. Its
factual-accuracy lane came back clean (every cited line reference and quoted phrase resolves in the
real file — nothing asserted from memory), links 6/6 correct, the raw-URL form matching the exemplar
character-for-character, and sibling-shape conformance strong. Its structural lanes did not, and all
findings are fixed in
53252eed:the current branch's findings directory" — and it never said how to resolve that directory. The
existing chain dead-ends for exactly this audience:
default-mode.mdpoints at the reviewSKILL.md, which resolves through a${CLAUDE_PLUGIN_ROOT}-relative path only that plugin canexpand. A "Where the file goes" section now routes tier resolution to the
topic-docsconvention and states the three producer-facing specifics that pointer does not carry: the
<memory_dir>/reviews/<branch-slug>/sub-path, the lossy slug rule, and the self-ignoreguard — whose omission would commit findings meant to stay checkout-local.
Tiernever named its vocabulary or its owner.plugins/review/context/severity.md"Severitytiers" owns CRITICAL/IMPORTANT/SUGGESTION and a consumer-precedence rule that binds a producer
too — where the consuming project defines its own vocabulary, map to that instead. A detector
emitting
P1would have been non-conforming with nothing here to say so.Confidencecited the wrong owner —findings-normalization.mdis fanout's internalpipeline, which this doc's own Boundary excludes from a producer's concern. The definition now
cites
severity.md"Confidence axis"; the rank order stays only as the consequence that makeslowworse than omission. Added:Confidenceis confidence-of-realness, not confidence in thefix.
merge-set construction, consumption subtraction, per-file consumption, the presence-only dedup
rationale with its borrowed FALSE-MERGE quote, the cell-escaping characters, and the
path-relativization rule. Duplication runs one direction here (convention copying plugin), so each
is reduced to the producer-facing consequence plus a pointer — and line 10's claim becomes true.
review:fanout",sole tabled adopter was
review:fanout. It ships empty, withreview:fanoutnamed in proseas the reference writer. That is the honest state of a stub.
Also fixed:
date:had no stated status for a producer and is now required; the admission test iscited rather than restated;
default-mode.md's closing sentence no longer overstates the split;severity.md,topic-docs, andenforceability-tiers.mdjoin External authority (matching bothsiblings); and the review CHANGELOG's "No behavior change" is replaced with what actually did and did
not change, since instruction text an agent reads is not nothing.
Not taken: folding 0.20.1 into 0.20.0 while the stack is unmerged. Per-PR bumps are this repo's
convention and rewriting the parent branch's entry from a stacked branch buys nothing.
Related
Depends on #2678 / #2690. Blocks the Pattern-C pilot (#2680) and the hardening phase (#2681).
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com