Skip to content

docs: record the closing-keyword trap firing through its own safeguard #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
topij merged 2 commits into from
Jul 27, 2026
Merged

docs: record the closing-keyword trap firing through its own safeguard #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
703890b
docs: record the closing-keyword trap firing through its own safeguard
Jul 27, 2026
6085d21
docs: stop deriving the closing-keyword mechanism; keep the conservat…
Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 43 additions & 17 deletions docs/kit-friction-log.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Comment on lines +60 to +68

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use surface-accurate terminology for the commit-message measurement.

The commit sample contains backtick-delimited plain text; it does not contain a Markdown inline span. This distinction matters because the incident resulted from applying rendered PR-body behavior to plain-text commit messages.

  • docs/kit-friction-log.md#L60-L68: replace “inline span” with “backtick-delimited text in a plain-text commit message,” and explicitly leave inline Markdown behavior unmeasured.
  • docs/kit-handoff.md#L62-L69: replace “fenced-vs-inline” with “fenced-vs-backtick-delimited text,” while retaining the separate body-vs-commit surface distinction.
📍 Affects 2 files
  • docs/kit-friction-log.md#L60-L68 (this comment)
  • docs/kit-handoff.md#L62-L69
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/kit-friction-log.md` around lines 60 - 68, Update
docs/kit-friction-log.md lines 60-68 to describe the commit sample as
backtick-delimited text in a plain-text commit message, explicitly stating that
inline Markdown behavior remains unmeasured. Update docs/kit-handoff.md lines
62-69 to replace “fenced-vs-inline” with “fenced-vs-backtick-delimited text,”
while preserving the separate body-vs-commit surface distinction.


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;
Comment on lines +85 to +88

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Name the command that establishes this verification claim.

The document says this was “verified” but does not provide the command or evidence, despite the earlier #54 rule requiring every verification claim to name its establishing command. Add the exact repository inspection command(s) used to confirm the squash message and hook coverage.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/kit-friction-log.md` around lines 85 - 88, Update the verification
bullet in docs/kit-friction-log.md to name the exact repository inspection
command(s) used to confirm commit 9c6ab3a’s squash message and verify that
scripts/ contains no commit-message inspection hook. Preserve the existing
findings while adding sufficient command evidence to satisfy the `#54`
verification rule.

- 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
Expand Down
16 changes: 10 additions & 6 deletions docs/kit-handoff.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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`.
Expand Down
Loading