fix(skills): a churning review loop must convert to an investigation

Measured on one feature: five review rounds, ~10 verified findings each,
49 of 49 real, every round finding defects introduced by the previous
round's fixes. The review was working perfectly. The question it was
answering had no answer in the data, and nothing in these skills could
say so.

review-tier gains two things.

Scope: TARGETED now also includes the writers of any persistent field the
diff reads but does not write. Callers-and-callees follows the call graph;
a defect in what a column MEANS propagates along the schema, and reads and
writes of a column are connected by no call at all. A read-only feature
therefore cannot reach, by any call-graph scoping, the code that decides
what it is reading. In the measured case two writers of one column meant
different things — "new revision" and "archived duplicate" — and five
rounds never reached either, because a read feature calls neither.

Step 2b, the halt rule: stop the loop and open an investigation when a
verified finding blames the previous round's fix two rounds running, or
findings across rounds restate one definitional question, or a candidate
fix must consult a side channel to decide what a column means. The first
rule is mechanical and fires at round 3's triage in the measured case.
Findings-per-token, the existing stop signal, detects a loop gone DRY; it
cannot detect one churning at a steady rate over an unsatisfiable design,
and says "keep going" forever.

verify-landed gains the claim discipline the same week produced counter-
examples for. A verification carries its command and the tool's own
verdict or it is not a verification: a "whole-project test compile" was
reported green having compiled nothing, because the surfaced status was a
trailing grep's — and grep exits 0 when it MATCHES, so it succeeded
because it found FAILED. A quality gate was reported OK over a broken
build, having scored the previous run's coverage. And a new check: when
the claim is about data, query the data — a scratch Postgres settled in
ninety seconds what a careful argument got wrong.

The whole-project note is updated too: one repository fake broke six times
on signature changes to the interface it implements. At that frequency the
remedy is structural, not another warning.

Author: olehsvyrydov

claude/skills/review-tier/SKILL.md

@@ -33,6 +33,10 @@ Look at **changed paths and changed symbols**, not at your opinion of the change
 
 **`TARGETED` — ordinary feature code.** Scope finders to the diff **plus its callers and callees**. This caveat is load-bearing: the review's value is finding the break two files away, so a diff-only scope degrades it into a linter.
 
+**And plus the writers of any persistent field the diff reads but does not write.** Callers-and-callees follows the call graph; a defect in what a *column means* propagates along the schema instead, and reads and writes of a column are not connected by any call. A feature that only reads a column can therefore never reach, by any call-graph scoping, the code that decides what it means. Grep the entity field and the raw column name; if there is **more than one writer**, put those writers in front of the finders and treat the column's semantics as part of the blast radius.
+
+> Measured: a version-history feature read `superseded_by`. Two writers — an ingest re-collect meaning "new revision", an operator PATCH meaning "archived duplicate" — differ in meaning and are distinguishable only in an audit payload. Five review rounds and 49 real findings never reached either writer, because a read feature calls neither. The grep takes seconds.
+
 **`LINT_ONLY` / `SKIP` — no behaviour can change:** documentation, comments, formatting, test-only changes, generated files, dependency lockfiles with no version bumps.
 
 State the classification and the rule that fired, so the decision is auditable.
@@ -50,6 +54,20 @@ So scope the finders to `git diff <round-N-sha>..HEAD` plus the prior findings a
 
 The justification is empirical, not theoretical: on a real loop, **two consecutive rounds each found that the previous round's fixes had introduced fresh defects** — including one where a fix to a data-loss path created an unbounded request loop, and one where a "safe" sweep would have destroyed operator-archived data. The freshly written fix is the riskiest code in the repository, and a full re-read buries it in noise.
 
+## Step 2b — a churning loop is not a review problem, and must convert to an investigation
+
+Step 5's findings-per-token detects a loop that has gone **dry**. It cannot detect one that is **churning** — a review of a design the data cannot satisfy produces real findings at a steady rate *forever*, so the metric says "keep going" indefinitely. Measured: five rounds, ~10 verified findings each, **49 of 49 real**, every round breaking the previous round's fixes. The review was working perfectly; the question was unanswerable.
+
+Halt the loop and open an investigation — a stated question that can have a wrong answer, plus a kill condition (`research-method`) — when **any** of these fire:
+
+1. **A verified round-N+1 finding `git blame`s to a round-N fix commit, two rounds running.** Mechanical: map each finding to the commit that introduced the defective line; if it lands inside the previous round's fixes twice consecutively, stop.
+2. **Findings across two or more rounds restate one definitional question.** "What set?", "what order?", "what number?" are one question — *what is a version* — not three defects.
+3. **A candidate fix must consult a side channel** — an audit payload, a log, a naming convention — to decide what a column *means*. The column's semantics are contested; no amount of reading code settles it.
+
+The next round's budget goes to the investigation, not to finders, and its first question is about the **writers of the data**, not the feature's code. Applied to the case above, rule 1 fires at **round 3's triage** — saving rounds 3, 4 and 5.
+
+Why a review cannot do this itself: a review verifies code against a spec it must assume is coherent. Its output vocabulary is the *finding* — local, with an implied fix — and it contains **no sentence meaning "this question has no answer in the data"**. Only an investigation can return that.
+
 ## Step 3 — spend verifiers in proportion to the claim
 
 - **3 adversarial verifiers** — data-loss, security, authorization, money. A false negative here is expensive.

claude/skills/verify-landed/SKILL.md

@@ -34,10 +34,24 @@ Two real instances, one day apart:
 
 When a comment states a rationale, treat it as a **hypothesis about the code** and check the code. Comments explain *why*; only symbols establish *whether*.
 
+**6. When the claim is about data, query the data.**
+A conclusion *reasoned* from code about what rows will do — which end a `LIMIT` truncates, what a walk returns, whether two columns can desync — is a hypothesis until it runs. A scratch database (`docker run postgres`, four lines of SQL) settles in ninety seconds what argument gets wrong across several rounds.
+
+Observed both ways in one week, in the same file. The scratch query settled a truncation question immediately and correctly. The *argued* analysis of a two-column desync — reasoned carefully, stated to the user as safe — was refuted by a reviewer who found the case the argument had not considered. The difference was entirely whether it was executed.
+
+The same applies to whether a test discriminates: revert the fix and watch it fail. A test written after a fix, which passes immediately, proves nothing. Measured: three integration tests "covering" a change all passed against the implementation they were written to prove wrong.
+
 ## Reporting
 
 State what you verified and how, in one line each — "grepped `X` at `file:line`", "test fails without the change" — not "verified" as a bare adjective. If you could not verify something, say which part and why, rather than letting a green build stand in for it.
 
+**A verification claim carries its command and the tool's own verdict, or it is not a verification.** "The whole-project test compile passes" is a claim; the command, the tool's own exit code, and its verdict line (`BUILD SUCCESSFUL`, `N tests completed, M failed`) are evidence. If the transcript does not contain the command, the check did not happen — say that instead.
+
+Two ways this has produced a confident false report:
+
+- A "whole-project test compile" reported green having compiled nothing, because the surfaced status belonged to a trailing `grep` — and **grep exits 0 when it matches**, so it succeeded precisely *because* it found `FAILED`. End such a chain with a predicate that is true on success (`cmd > log 2>&1 && echo PASS || echo FAIL`), never one true on failure.
+- A **quality gate reported OK over a broken build**: the test task had failed, so the analysis scored the *previous* run's coverage. A gate verdict and a build verdict are independent claims. Read each step's status separately, never just the last line of a sequence.
+
 ## When a whole-project check is the only real check
 
-Module-scoped test runs can pass while the project is broken: a shared test double that drifts from an interface it implements only fails where it is compiled. Before declaring backend work done, run the **whole-project** test-compile at least once. Two separate breakages on one branch came from exactly this — a repository fake in one module missing a method added in another.
+Module-scoped test runs can pass while the project is broken: a shared test double that drifts from an interface it implements only fails where it is compiled. Before declaring backend work done, run the **whole-project** test-compile at least once. On one branch a single repository fake broke **six times** this way, once per signature change to the interface it implements — documentation did not stop it, and at that frequency the fix is structural (an abstract adapter or generated defaults), not another warning.