LLP corpus hygiene: illustrative @refs are marked and no gloss uses an em dash (#463 items 2 and 3) - #468
Conversation
… item 3) LLP 0000 and LLP 0001 spell the annotation `@ref LLP NNNN#anchor [relation]: gloss`. 164 annotation lines separated the gloss with a U+2014 instead, which is both a convention violation and a house-style one: CLAUDE.md forbids the character in code, comments, and docs alike. Mechanical and separator-only where the em dash stood where the colon belongs. The 21 sites where the dash sat inside prose rather than in the separator slot got the punctuation the sentence wanted instead (a comma, a colon, a semicolon, or parentheses), never a reworded rationale. Comment text only: no runtime string, and no non-comment line, is touched outside the LLP documents. Also folds in the pre-existing em dash in `matcher.js`'s machine-local list doc comment, named in the issue. Co-Authored-By: Claude <noreply@anthropic.com>
…em 2)
Nine annotations cite deliberately fictional targets: six LLP-form
(`LLP 0042#token-strategy`, `LLP 0074#focus-trap`, `LLP 0074#focus-management`,
`LLP 0007#layout`, `LLP 0042#anchor`) and three path-form (`path/to/doc.md`,
`docs/vendor/spec.md` twice), all inside the `ref-check` and `ref-story` skill
docs. They are correct as documentation and wrong as data, which is why they are
marked rather than repointed at a real section: a ref invented to satisfy a
checker teaches the reader something false, and deleting the examples would gut
the syntax reference.
The marker is keyed on the line, not the target, so it covers both citation
forms (an exclusion rule keyed on `LLP NNNN#anchor` would silence six of nine and
the tool would still exit 1 forever). `ref-check:ignore` suppresses one line;
`ref-check:ignore-start` / `ref-check:ignore-end` suppress a region, written as
HTML comments outside the fence so nothing renders inside the example. Both are
documented in the skill's extract step and in LLP 0001's conventions, and the
regions stay tight because a suppressed annotation is checked nowhere.
`test/core/llp-ref-hygiene.test.js` is the extractor plus resolver LLP 0001
§Tooling asked for and marked unbuilt: it indexes all three anchor forms the
corpus uses (heading slug, `{#slug}` on any line, inline `<a id>`), resolves
every annotation, honors the markers, and fails on an em-dash gloss. The
`LLP 0103#cli` sites that PR #461 owns are tolerated by name, not asserted, so
that branch stays free to fix them. The duplicate-number invariant is written
down but skipped: 0098, 0099, and 0111 are each claimed by two documents, and
choosing between renumbering and a filename-qualified citation form is a corpus
decision, not a drive-by fix.
Co-Authored-By: Claude <noreply@anthropic.com>
🤖 neutral: this PR carries
|
| #463's scope | Status here |
|---|---|
| 9 placeholder refs | Fixed, 9 → 0, via a documented ref-check:ignore marker rather than by inventing targets |
em-dash @ref glosses |
Fixed, 164 → 0 on @ref lines |
| 3 duplicate LLP numbers | Not fixed, left as a skipped test with the reason in the skip message |
The duplicate-number slice was left because it is a corpus-level convention choice (renumber the later claimant of 0098/0099/0111, versus adopt a filename-qualified citation form in LLP 0001), not a mechanical repair. That was the right call, and PR #461 does not fix it either, its own body asks for it to be re-filed.
Since both #463 and #457 name that slice and neither PR fixes it, merging both would have closed both issues with the work silently dropped. It is now tracked in #469, labelled neutral:fix and neutral:stuck with the decision fork spelled out for a human.
Also worth noting for review: the issue's headline count of "459 em-dash @ref glosses" does not reproduce. The verified figure is 164 @ref lines carrying a U+2014 (132 in the separator slot, 32 elsewhere). 2628 lines repo-wide contain one, which is likely where 459 came from, and that wider prose sweep is out of this PR's scope.
git merge-tree was verified clean against fix/issue-457, fix/issue-452 and fix/issue-453. It was not checked against fix/issue-464 or fix/issue-465, which did not exist yet when this branch was built and which both add a new LLP document. If this PR's ref-hygiene test and those branches disagree, it will surface as a normal CI failure on whichever lands second.
Review of #468 found the marker self-triggering: the prose in `ref-check/SKILL.md` that documents `ref-check:ignore-start` matched the plain `includes()` test, so lines 80-146 of that file (the whole "validate a reference" section) were a suppressed region nobody asked for, and LLP 0001's bullet plus this test's own constants opened regions too. That is the exact failure the marker is warned against, reached by writing it down. A marker now counts only when it is written as a comment and outside an inline code span, so documentation can name it without arming it. The gap before the marker text is bounded rather than `*`-quantified so the pattern does not match its own source line. Regions are now exactly the three the author intended. Suppression is also made reviewable, since a suppressed annotation is checked nowhere: the gate fails on a region that is opened and never closed, and on a marker in any file outside an enumerated list of the syntax documentation, so a new suppression has to show up in a diff as an edit to that list. Co-Authored-By: Claude <noreply@anthropic.com>
Review round 1, head
|
| file | change |
|---|---|
test/core/llp-ref-hygiene.test.js |
marker requires a comment context and no inline code span; unclosed-region and marker-location gate; two new tests |
.claude/skills/ref-check/SKILL.md |
documents the comment requirement and that suppression is itself gated |
llp/0001-adopting-llp.plan.md |
same, on the {#illustrative-refs} bullet |
Verified landed with git diff 68d21ef4..origin/fix/issue-463: 3 files, 89
insertions, 10 deletions. Not merged, not marked ready, nothing pushed to
master.
Round 2 review of #468. The round-1 fix stopped the marker from arming itself when named in prose, but it still accepted any comment opener in any file type, so four ordinary ways of documenting the marker in Markdown still opened a real suppressed region: a bullet (`* ref-check:ignore-start`), a heading (`# ref-check:ignore-start`), a fenced code sample, and a four-space indented one. Each was verified to hide a genuinely broken `@ref` with the suite green. Markdown's only comment is `<!-- -->`; `*` is a bullet and `#` is a heading there, so marker recognition is now per file type, and a marker inside a fenced block or an indented block does not count, which is the multi-line form of the inline-code-span exclusion already present. Extraction over the corpus is unchanged: the same 1365 annotations, the same nine suppressed. The gate now also rejects a region closed without being opened and a region opened while another is open. Both previously passed silently, which meant the author and the extractor could disagree about which lines were suppressed. Also from the round-1 findings: - `TOLERATED_BROKEN` is keyed on the file plus the target it cites, with a count, instead of on `file:line`. One blank line added to `policy.js` used to produce five spurious failures. The count keeps it from being a loosening: a seventh broken `LLP 0103#cli` in `clients.js` still fails. - LLP 0001 no longer claims the colon separator is enforced. Only the em dash is gated; 27 annotations still open a gloss with a hyphen, and the doc now says so rather than describing a rule the checker does not apply. Co-Authored-By: Claude <noreply@anthropic.com>
Review round 2 of 2, head
|
| Form | Result at 8a47ea67 |
|---|---|
* ref-check:ignore-start opens a region (Markdown bullet) |
armed, broken ref hidden, suite green |
# ref-check:ignore-start (Markdown heading) |
armed, broken ref hidden, suite green |
| the marker shown inside a fenced code sample | armed, broken ref hidden, suite green |
| the marker shown as a four-space indented code sample | armed, broken ref hidden, suite green |
Control: the same broken ref with no marker fails the suite
(LLP 9999 does not exist), so the suppression above was real and silent.
This matters because a bullet list and a fenced sample are precisely how the
ref-check skill documents everything else, and MARKED_FILES concentrates the
hazard in exactly the two files whose subject is the annotation syntax. Section 4
of that skill is a bullet list explaining these markers; it survives today only
because every mention in it happens to be backticked.
Two further forms escaped the gate without hiding anything by themselves:
| Form | Result at 8a47ea67 |
|---|---|
ignore-start, ignore-start, ignore-end (nested) |
inner end closed the outer region, gate silent |
ignore-end with no ignore-start |
gate silent |
Both mean the author and the extractor can disagree about which lines are
suppressed, which is the unreviewable state the round-1 gate exists to prevent.
Fixed. Marker recognition is now per file type: in Markdown only
<!-- ... -->, since that is Markdown's only comment. A marker does not count
inside a fenced block (the multi-line form of the inline-code-span exclusion
already present) or on a line indented four spaces or more (CommonMark's indented
code block). The extractor and the gate now read markers through one shared
per-file walker, markersFor, so what the gate polices is exactly what the
extractor obeys. The gate additionally rejects a region closed without being
opened and a region opened while another is open.
Verified:
- All six attacks above now fail the suite. The four Markdown ones surface the
hiddenLLP 9999ref; the two pairing ones report
region closed without being openedand
region opened while the one at 193 is still open. - Extraction over the corpus is unchanged: the same 1365 annotations at the
same sites under the old and the new marker logic on the same tree, so the nine
intended suppressions are intact and nothing live became suppressed. The change
can only ever reduce suppression, never create it. - New regression test
a marker shown as documentation or as a code sample does not activate itcovers bullet,-bullet, heading, line-marker-in-a-bullet,
fenced,~~~fenced, four-space indent and tab indent, plus positive cases
that the intended forms still suppress in Markdown and in a source file. It
fails when the pre-fix marker logic is restored.
Forms I tried that did not arm suppression, at 8a47ea67 and after:
inline code span, bare prose, uppercase REF-CHECK:IGNORE-START, mixed case
Ref-Check:Ignore-Start, a gap of nine or more spaces after the opener,
ignore-startle and ignore-starting (the word boundary holds), and a marker in
any file outside MARKED_FILES, which the gate catches. A dangling
ignore-start was already caught by the round-1 gate and still is.
FINDING 2 (low, FIXED): TOLERATED_BROKEN keyed on file:line
test/core/llp-ref-hygiene.test.js:86-103 at the reviewed head.
Round 1 left this as a maintenance note on the grounds that it fails loud. It
does, but the loud failure is a false one, and this PR itself edits both files
it lands on. Demonstrated: inserting one blank line near the top of
src/core/commands/policy.js produces five spurious failures at 8a47ea67.
Fixed without weakening it. The list is keyed on the file plus the target the
annotation cites, with a count spent as a ceiling:
['src/core/commands/clients.js LLP 0103#cli', 6],
A line number is not a property of the defect, so shifts no longer matter, while
the count preserves the property round 1 correctly insisted on: a seventh broken
LLP 0103#cli in clients.js is new breakage and still fails, which a blanket
pattern would have let through. Counts are ceilings and never floors, so pruning
them when #461 lands is still a cleanup and not a failure. The 16 file:line
entries reduce to exactly 5 identities, verified against the resolver's own
output.
Verified: the one-blank-line shift above now passes, and a new unit test,
a tolerated reference is tolerated only as often as it is listed, pins the
ceiling, the per-anchor scoping and the per-file scoping.
FINDING 3 (low, FIXED): LLP 0001 described a rule the gate does not apply
llp/0001-adopting-llp.plan.md:62-64 said "A colon opens the gloss: no other
separator". The gate bans only the em dash, and 27 corpus annotations open a
gloss with a plain hyphen (27 on master too, so this PR neither caused nor
changed it).
Of the three options, I took the honest minimum: align the doc to what is
enforced, rather than widen the gate. Widening it means a third mechanical
sweep of 27 sites in files this PR does not touch, one of them inside the slice
PR #461 already owns, in the last review round. The doc now states that only the
em dash is gated, that the colon is convention the checker does not yet enforce,
how many sites diverge, and why the sweep is deliberately later. That is a
recorded gap instead of a false claim.
llp/0001 and the ref-check skill were also updated to describe the new marker
rule and the new pairing checks, in the same commit as the code, per the LLP
living-docs rule. The marker machinery now carries
@ref LLP 0001#illustrative-refs [implements].
Verification of the pushed change
npm run typecheckclean.npm test: 2888 tests, 2878 pass, 8 fail, 2 skip. The 8 are
test/core/leave-command.test.js, which round 1 confirmed fail identically on
origin/master. The test delta is exactly the two tests added here.git diff 8a47ea67..HEADtouches exactly three files:
test/core/llp-ref-hygiene.test.js,.claude/skills/ref-check/SKILL.md,
llp/0001-adopting-llp.plan.md.- CLAUDE.md compliance: zero em dashes on any added line, no
semicolon-terminated statements added, JSDoc types only, no@typedef, no
inlineimport()types.
Left for triage
- INFO, not fixed, and nothing tracks it. Em dashes remain in ordinary prose
repo-wide, including in files this PR touches:llp/004566,llp/004154,
hypaware-plugin-kernel-types.d.ts36,llp/000119,
src/core/commands/clients.js13, around 2628 lines overall. CLAUDE.md bans the
character everywhere, so this is a real standing violation, but it is wider than
this PR, which scoped itself to annotation lines, and wider than issue LLP corpus hygiene: 3 duplicate LLP numbers, 9 placeholder refs that keep /ref-check from ever exiting 0, and 459 em-dash @ref glosses #463 item
3, which asks only about glosses. I searched the tracker and no open or closed
issue covers the prose sweep, so one needs filing. This PR should not be
blocked on it. - Residual limits of my own fix, stated so nobody assumes more than holds.
Fence and indent awareness applies to.mdonly, deliberately, so that a stray
triple backtick in a source comment cannot silence a real marker. In a source
file*and#are therefore still openers, which is correct for JSDoc and
shell comments but means a Markdown-shaped sample embedded in a.jsstring
could still arm a marker. That is bounded byMARKED_FILES, whose only
non-Markdown member is the hygiene test itself. Separately, a.mdfile with an
odd number of fence lines would invert fence state for the rest of the file and
make later markers inert; that direction only ever reduces suppression, and the
new pairing check catches the unbalanced region it produces. - Issue LLP corpus hygiene: 3 duplicate LLP numbers, 9 placeholder refs that keep /ref-check from ever exiting 0, and 459 em-dash @ref glosses #463 item 1 (three duplicate LLP numbers) stays for a human, tracked
in LLP 0098, 0099 and 0111 each have two claimants; the renumber-vs-qualified-citation choice is unowned and needs a decision #469 and held by the skipped invariant. Still the right call.
No blocking objection to merging.
|
Neutral triage rung (LLP 0017), PR #468, head What I verified by running code, not by trusting claims1. Tried to defeat round 2's marker-suppression fix with forms it did not
None of the forms I tried, including ones not on round 2's list, produced a 2. 3. Duplicate-LLP-number skip. 4. Em-dash gloss rewrites. Spot-checked 20+ sites across One thing I found that round 2 did not flagThe new "no Non-blocking items, filed
VerdictEverything above is non-blocking. Round 2's previously-unreviewed fix commit |
What this fixes
Items 2 and 3 of #463: the placeholder refs that keep
/ref-checkfrom everexiting 0, and the em-dash
@refglosses. Item 1 (three duplicate LLPnumbers) is deliberately left alone, see below.
Verified counts, before and after
Measured on this branch with the checker added here, which indexes all three
anchor forms the corpus uses (heading slug,
{#slug}on any line, inline<a id>) exactly as PR #456 specifies:@refannotations, totalLLP 0103#cliand the wrapped LLP 0135 ref (PR #461's slice)Two counts in the issue needed correcting:
issue is right that the distinction decides the fix: a rule keyed only on
LLP NNNN#anchorsilences six of nine and the tool still exits 1 forever.slot; the rest carry one inside the gloss or in the prose of a line that also
mentions
@ref. All 164 are fixed. 459 does not reproduce under any reading Icould construct: the repo has 2628 lines containing a U+2014 in total, so that
figure looks like a count of em dashes at large rather than of annotations. The
wider prose sweep is not attempted here.
Item 2: the nine placeholders are marked, not repointed
They live in
.claude/skills/ref-check/SKILL.mdand.claude/skills/ref-story/SKILL.mdand citeLLP 0042#token-strategy,LLP 0074#focus-trap,LLP 0074#focus-management,LLP 0007#layout,LLP 0042#anchor,path/to/doc.md, anddocs/vendor/spec.mdtwice. They arecorrect as documentation and wrong as data, so repointing them at a real section
was rejected: a ref invented to satisfy a checker teaches the reader something
false, and deleting the examples would gut the syntax reference.
Of the issue's three options (a skipped example block, a sentinel LLP number, an
ignore list) the marker is the only one that covers both citation forms, because
it is keyed on the line and not on the target:
ref-check:ignoreanywhere on the line suppresses that line.ref-check:ignore-start/ref-check:ignore-endsuppress a region, written asHTML comments outside the fence so nothing renders inside the example.
Documented in the skill's extract step and in LLP 0001's conventions, with the
warning that the regions must stay tight because a suppressed annotation is
checked nowhere. A unit test pins that: a marked ref is skipped, an unmarked one
on the neighbouring line is not.
The illustrative annotations in
ref-storyalso used the em dash separator, sothey were teaching the wrong spelling; they now show the colon.
Item 3: the em-dash sweep
Its own commit, separator-only where the dash stood where the colon belongs. The
21 sites where the dash sat inside prose instead got the punctuation the sentence
wanted (a comma, colon, semicolon, or parentheses), never a reworded rationale.
Every JS change is comment text:
git diffover*.jsoutside the new test filecontains no non-comment line. The pre-existing em dash in
matcher.js'smachine-local list doc comment, which the issue flagged as worth folding in, is
included.
Reproducing test
test/core/llp-ref-hygiene.test.js, the extractor plus resolver LLP 0001§Tooling asked for and recorded as unbuilt.
On
origin/masterwith only the test file added:On this branch all five pass (one skipped, see below). Full suite:
npm test, 2884 tests, 2874 pass, 8 fail, 1 skip. The 8 failures aretest/core/leave-command.test.jsand reproduce identically onorigin/master(2879 tests, 2870 pass, the same 8 fail), so they predate this branch.
npm run typecheckis clean.Left to PR #461
@ref LLP 0103#clisites and the wrappedLLP 0011#interactive-ref in LLP 0135. The test tolerates those sixteen by name, tolerated and
never required, so LLP 0103 names its CLI and reporting sections; the usage-policy fail-safe refs cite a live doc #461 stays free to fix them and pruning the list afterwards
is a cleanup rather than a failure. Verified:
git merge-treeof this branchwith
origin/fix/issue-457is clean, and the test still passes on the mergedtree even though both branches touch
matcher.js,clients.js, andllp/0135. Also clean againstfix/issue-452(Codex session identity is the rollout's session container, not CODEX_THREAD_ID, and the ignore caveat names the fork (#452, #455) #456), which edits the sameref-check/SKILL.md, andfix/issue-453.Left to a human: item 1, the three duplicate numbers
Not fixed here, and not fixed by #461 either (that PR's body asks for it to
be re-filed, which is how this issue exists).
0098,0099, and0111are eachclaimed by two documents. The issue offers renumbering the later claimant versus
adopting a filename-qualified citation form in LLP 0001, and says outright it is
worth deciding deliberately rather than by whoever touches it first. Renumbering
would sweep dozens of inbound refs and rewrite six documents' history; I am not
picking that in passing.
The invariant is written down and skipped in the test, with the reason in the
skip message, so unskipping it is the last step of whichever decision lands. The
resolver meanwhile resolves an anchor against either claimant, per #456, so
the collision does not manufacture false broken refs today.
Fixes #463