diff --git a/docs/kit-friction-log.md b/docs/kit-friction-log.md index c3ec722..578843c 100644 --- a/docs/kit-friction-log.md +++ b/docs/kit-friction-log.md @@ -47,23 +47,49 @@ Everything above now lives in [`kit-friction-log-archive.md`](kit-friction-log-a proposed fix: contract item 5 should say the restore belongs in a `finally` (or a git-clean check) and that any harness must verify the tree is clean *after* it exits, not only after a successful run. -- **The closing-keyword rule as written is wrong, and measuring it changed the fix.** - Last session's entry says never to write one "even negated". Acting on that, I rewrote - a PR body sentence that mentioned a backticked `` `Closes #61` ``. Then I measured: - - ``` - PR #68 body contains `Closes #61` (backticked) → closingIssuesReferences: [] - PR #63 body contains "Does NOT close #60" → closingIssuesReferences: [58, 60] - ``` - - **GitHub excludes inline code spans**; what it does not excuse is negating prose. So - the real rule is narrower than the entry below claims, and the obvious mechanical fix - — grep for `(close|fix|resolve)\w*\s+#\d+` — would fire on every legitimate backticked - reference and train people to ignore it. **M** — proposed fix: a `pr-watch` / `wrap-up` - check that strips fenced blocks and inline code **first**, then flags a closing keyword - within a few tokens of a negation (`not`, `n't`, `NOT`, "stays open"), which is the - actual failure mode. Correct the 2026-07-26 entry's rule at the same time; it is - currently stated more strongly than the evidence supports. +- **The closing-keyword trap fired again, because I weakened a correct rule on the + strength of an experiment that did not test what I thought.** `#61` was closed by the + squash-merge of `#68` and reopened by hand. + + **What is actually measured** — three data points, with their confounds stated, + because two earlier write-ups of this stated more than the data supports: + + | artifact | form | outcome | + | --- | --- | --- | + | `#68` body | `Closes #61` inside a **fenced block** | inert (`closingIssuesReferences: []`) | + | `9c6ab3a` commit message | `Closes #61` in an **inline** span | **fired** — closed `#61` | + | `#63` body | `Does NOT close #60` **and** a plain `fix #60` | fired; which one is **not isolated** | + + **What is NOT measured, though I twice wrote as if it were:** whether an *inline* span + is inert in a PR body. The two backtick data points differ in **two** variables at once + — fenced-vs-inline *and* body-vs-commit — so "GitHub excludes inline code spans" and + "fenced blocks are inert everywhere, inline spans fire everywhere" fit the evidence + equally well. Under the second reading, the fix I proposed last round (strip inline + code from a PR body before checking) reopens the exact hole that closed `#61`. + + Likewise the *negation* hypothesis: `#63`'s body carries a plain, non-negated `fix #60` + alongside the negated mention, so a check that only flags keywords **near a negation** + would have passed that body clean. + + **H** — **stop deriving a mechanism; take the conservative rule.** Three attempts to + state this precisely have each been wrong, which is rule 1's threshold. The original + 2026-07-26 rule below — never write a closing keyword adjacent to an issue number you + do not intend to close, in any form, on any surface — would have prevented all three + incidents; the measurement talked me *out* of a correct rule. It stands as written, and + the proposed check follows it rather than any theory of markdown: + + - flag **every** match on **every** surface — PR body, every commit message, and the + squash message — with **no stripping** of code spans or fenced blocks; + - cover the forms the naive regex misses and GitHub honours: `owner/repo#61`, a full + issue URL, and `Closes: #61` (a colon, not whitespace); + - check the **squash message at merge time** specifically. It is composed *after* the + PR body was reviewed, and nothing reviews it — verified: `9c6ab3a`'s message matches + neither `#68`'s title, its body, nor any of its three branch commits, and no hook in + `scripts/` inspects commit messages at all; + - the operator confirms each match. Over-firing is the acceptable failure here. + + Occurrence count, corrected: **three occurrences across two sessions** (`#63` and `#64` + were the same session; `9c6ab3a` is this one). - **A review bot with an incremental model keeps a stale review across a rewrite.** CodeRabbit reviewed the pre-split head, then declined `@coderabbitai review` ("does not re-review already reviewed commits") and hit its Fair Usage limit on diff --git a/docs/kit-handoff.md b/docs/kit-handoff.md index cded8c3..c0dd0d1 100644 --- a/docs/kit-handoff.md +++ b/docs/kit-handoff.md @@ -59,11 +59,14 @@ stood (355 tests), by my own mutation run at that head, or by CodeRabbit. wrap-up's own handoff block then miscited `#36`, overstated a test count, and got a GitHub rule backwards — all caught by a review lens, none by me. This is the third consecutive session where claim-vs-artifact drift is the most common finding. -- **A closing keyword inside backticks does NOT fire.** Measured: - `closingIssuesReferences` is empty for a PR whose body carries a backticked - `` `Closes #61` ``, while `#63`'s *un-backticked* "Does NOT close #60" closed it. So - the hazard is negated **prose**, not any mention — a grep that ignores code spans - would have over-fired on every legitimate reference. +- **An under-determined measurement talked me out of a correct rule.** `#68`'s + squash-merge closed `#61` (reopened by hand) because I had weakened the standing + "never write a closing keyword next to an issue number, even negated" rule after + measuring one PR body as inert. That experiment varied **two** things at once — + fenced-vs-inline *and* body-vs-commit — so it never established the thing I concluded + from it. Three attempts to state the rule precisely have each been wrong; the + conservative original would have prevented all three incidents. Stop deriving the + mechanism (rule 1). - **Two of my tests pinned nothing**, including the one whose stated thesis is "don't restate the list": it re-derived its expectation from the real `KIT_OWNED` with the prefix filter left out, so deleting that filter left the suite green. @@ -79,7 +82,8 @@ stood (355 tests), by my own mutation run at that head, or by CodeRabbit. `#67` (the same hardcoded triple `#59` just fixed, where it *writes* bad config). `init.sh` has no automated test coverage and **no issue tracks that** — `#36` is the `pre-push` twin, and `#67`'s body miscites it for `init.sh`; both need correcting. -- **`#61`** — still open, never closed: the hook-detection half, with the panel's +- **`#61`** — open (closed in error by `#68`'s squash-merge, reopened by hand): the + hook-detection half, with the panel's evidence, the shape a correct fix needs, and a table of 9 `git config` value forms of which the current scan misparses 5. - **`#47`** still the highest-leverage unbuilt thing, and it subsumes `#67`.