LLP 0103 names its CLI and reporting sections; the usage-policy fail-safe refs cite a live doc - #461
Conversation
…safe refs cite a live doc (#457) Fourteen `@ref LLP 0103#cli` annotations, plus the prose in LLP 0111/0112 that describes them, pointed at an anchor LLP 0103 never defined in any of the three forms. The referent is unambiguous: LLP 0111 says "extends its CLI paragraph", and every code site glosses the `--private` / `--local-only` / `--sync` machine-local marking flags that paragraph mints. So the paragraph gets the name the refs already agree on, `{#cli}`, in the corpus's list-item form. One of the fifteen sites meant a different paragraph: `runIgnoreCheck`'s gloss ("`--check` names which source governs") is verbatim the Consequences bullet, not the CLI paragraph. That bullet gets `{#reporting}` and the ref is repointed, so both anchors land on the prose their glosses actually quote. `src/core/usage-policy/{local_only,matcher}.js` cited `LLP 0080#fail-safe`, a Superseded document. Its formal Superseded-by, LLP 0102, retires the in-login picker and carries no fail-safe content, so pointing there would be worse than leaving it. LLP 0080's own note says the machine-local list substrate lives on, and its `#fail-safe` section derives the rule from `LLP 0049#fail-safe` (Accepted, and already the citation on `src/core/usage-policy/format.js`). Both refs now cite that invariant as `[constrained-by]`, with the throw-not-empty mechanics credited to LLP 0080 in prose so the trail survives. Also unwraps the line-wrapped `@ref LLP 0011#interactive-walkthrough` quoted in llp/0135, whose trailing hyphen made a wrap artifact look like an anchor. Corrected ref-check (all three anchor forms, `{#slug}` on any line, one `-` per space): BROKEN 22 -> 6, superseded-target WARNING 4 -> 2. The remaining 6 are the placeholder examples inside the ref-check / ref-story skill docs and the remaining 2 WARNINGs are LLP 0101/0102 citing the docs they supersede, both correct as they stand. Co-Authored-By: Claude <noreply@anthropic.com>
…o LLP 0080 Review follow-ups on the LLP 0103 anchor split: - `runPolicyShow` (`policy.js`) carried the same "names the governing source (dotfile vs machine-local)" gloss that justified moving `runIgnoreCheck` to `LLP 0103#reporting`, but stayed on `#cli`. Split it the same way: `#reporting` for the source-and-class report, `#cli` for the store/resolver/lattice-unchanged claim its four sibling glosses make. - `LOCAL_ONLY_LIST_UNREADABLE_ERROR_KIND`'s one-line prose credit moves back to LLP 0080 §fail-safe. LLP 0049 §fail-safe states the invariant (suppress more, never expose more) and is the right `@ref` target, but it says nothing about throwing or about a structured error naming the file; the `error_kind` mechanic is LLP 0080's (spelled out in LLP 0081 T1). Annotation and JSDoc prose only. `npm test` at the 8-failure `leave-command.test.js` baseline; typecheck clean.
neutral
|
origin/master (f9b9667) |
b375535 |
|
|---|---|---|
| refs / files-with-refs / LLP numbers / LLP files | 1367 / 359 / 130 / 133 | 1367 / 359 / 130 / 133 |
| BROKEN (all forms) | 25 | 9 |
BROKEN (LLP NNNN form only) |
22 | 6 |
| WARNING (superseded/tombstoned target) | 4 | 2 |
| duplicate LLP numbers | 3 (0098, 0099, 0111) | 3 |
The PR's 22 -> 6 and 4 -> 2 reproduce exactly, and the small difference in the other
two numbers is fully explained, not a disagreement:
- My
refsis 1367, the PR's is 1364. The delta is exactly the 3 path-form refs
(@ref path/to/doc.md#anchor,@ref docs/vendor/spec.md#tokensx2) in
.claude/skills/ref-check/SKILL.md:24,96,115, which the PR's count excludes. - Same 3 refs explain BROKEN 25 vs 22 and 9 vs 6. They are the same class of
illustrative placeholder as the 6 the PR defers to item 4 - so the residue is
9 placeholder refs, not 6 (see finding N1 below).
The 6 remaining LLP-form BROKEN are exactly the ones the PR names
(ref-check/SKILL.md:88,114,122, ref-story/SKILL.md:101,116,135 ->
LLP 0042#anchor, LLP 0042#token-strategy x2, LLP 0074#focus-trap,
LLP 0074#focus-management, LLP 0007#layout). The 2 remaining WARNINGs are
llp/0101:21 -> LLP 0093 and llp/0102:18 -> LLP 0094, i.e. each decision citing the
document it supersedes. Correct as written; nothing to do.
All 15 LLP 0103#* sites resolve (14 #cli + 1 #reporting):
llp/0111:21, llp/0111:247, llp/0112:42, llp/0112:43,
clients.js:710,755,891,938,1008, clients.js:1074 (#reporting),
policy.js:33,218,254,291,338. The PR's 15th site (llp/0112:43) is real - the issue
counted 14 because it missed the T2 task line. llp/0112:40 also says
LLP 0103#cli in prose but is not an @ref, so it is correctly not counted.
Both LLP 0049#fail-safe repoints resolve, alongside the pre-existing
format.js:30. Zero @ref into LLP 0080#fail-safe remain.
llp/0135: on master the wrap produced anchor interactive- (BROKEN); at
b375535 line 960 resolves against LLP 0011's ## Interactive walkthrough
heading slug. Verified the unwrap is text-identical, only re-wrapped.
2. Judgement call: is {#cli} on the Decision bullet the real referent?
Verdict: yes for 14 of the 15, and adding the anchor was the right branch over
repointing - with one narrow correction, filed as finding A1 below.
Evidence for the anchor landing on the prose the glosses quote:
llp/0111:21is decisive and explicit:@ref LLP 0103#cli [constrained-by] - extends its CLI paragraph. The bullet atllp/0103:63-72is that paragraph.- The 5 remaining
clients.jssites gloss the marking flags the bullet mints. The
bullet says "Machine-local writes get flags on the same verb: the existing
--local-only, a new--private… and an explicit-sync marking"; the glosses at
clients.js:710(--private/--syncparsing),:755and:938(dispatch),
:891(the shared marking verb) and:1008("symmetric class-scoped removal", against
the bullet's "hyp unignoregrows symmetric removal") are direct hits. The
surrounding JSDoc prose atclients.js:705,751,934already writes
(LLP 0103 #cli)in plain text, so the author's intent is on record. - The 5
policy.jssites are a weaker but still correct fit, and here the PR body's
justification is inaccurate: it claims "all fivepolicy.jssites gloss the
--private/--local-only/--syncmachine-local marking flags". They do not -
four of them gloss "the store, resolver, and class lattice are unchanged; only the
verb spelling is new" (policy.js:33,218,291,338), and:338is about the version-2
class-per-entry store. That content sits in the Decision preamble and the class
bullets, not the CLI paragraph. The anchor is still right, for a reason the PR
body does not give: the{#cli}bullet is the only place in LLP 0103 that mentions
hyp policyat all - itsExtended-by: LLP 0110note (llp/0103:70-72) says
literally "the flag-on-ignore spelling this paragraph left open is retired in favor of
a class-neutralhyp policyverb; the class taxonomy above is unchanged", which is
what those four glosses paraphrase. Andllp/0111:247/llp/0112T2 both mandate
this exact citation ("the@ref LLP 0103#cliannotations … keep citing 0103 for the
store-and-classes rationale"), so repointing them elsewhere would falsify live plan
text. Conclusion: keep#cli; the PR body's reasoning for those five is wrong even
though its choice is right. Body is not editable by this review - noting it here. - Repointing (option b) was correctly rejected: 15 independent sites agreeing on a name,
one of them saying "extends its CLI paragraph", is about as unambiguous as intent gets. - **CLI** {#cli}:matches the corpus form exactly (- **Label** {#anchor}:), used by
LLP 0101, 0105, 0106, 0107, 0138 and others. No slug collision introduced: LLP 0103's
anchor set goes from{consequences, context, decision, llp-0103-…, the-surface-matrix-settled}
to that pluscliandreporting. No markdown link anywhere targets
0103-machine-local-policy-classes.decision.md#…, so nothing depended on the old
rendering.
3. Judgement call: LLP 0080#fail-safe -> LLP 0049#fail-safe
Verdict: right, and the relation downgrade to [constrained-by] is what makes it
right. Verified independently:
llp/0049-hypignore-usage-policy.spec.mdis**Status:** Accepted, and
### Fail-safe for unimplemented classes {#fail-safe}(line 96) states the invariant
the two code sites rest on: "the safe failure for a privacy control is 'suppress
more,' never 'record-and-export something the user flagged.'" R3 (line 185) restates it
normatively.- The derivation chain is in the corpus, not invented here:
llp/0080:82-84says the
empty-list reading is "forbidden by [LLP 0049 §fail-safe]". LLP 0049 is literally
0080's own cited authority for this rule. - The formal supersessor is genuinely unusable.
llp/0102is Accepted and contains no
fail-safe content;llp/0080's note scopes the supersession to the in-login picker.
Repointing to 0102 would have been the mis-file the PR describes. - I checked the alternative homes myself, not just on the PR's word:
llp/0069,
llp/0070,llp/0071are all Accepted and none has afail-safesection or
anchor, so 0080's supersede note ("the non-picker substrate … lives on under LLP
0069/0070/0071") does not in fact give the fail-safe mechanics a live home. A
corpus-wide grep forunparseable|unreadable|corruptfinds the rule restated in only
two other places, neither a candidate target:llp/0081:20(T1, a plan line that
itself cites "the LLP 0080 #fail-safe") andllp/0093:63(itself Superseded). So the
PR's claim that nothing else restates the rule holds, and the repoint-to-the-invariant
choice is correct. - On the "fix
Superseded-byinstead" option: the honest reading is that LLP 0080 is
correctly Superseded (the picker it designs really is gone) while one section of it
remains the only written record of a live mechanic. That is not aSuperseded-by
error, so correctingSuperseded-bywould not fix anything; the repo has no
"partially superseded" status to reach for. Crediting 0080 §fail-safe in JSDoc prose
while@ref-ing the live invariant is the right trade: the trail survives for a human
reader without a machine-visible ref into a superseded doc. It does mean the mechanics
are no longer reachable by tooling from code - worth a futurellp/0069-or-0071
§fail-safe section, but that is new doc work, not this PR's job.
Relation calibration is worth calling out as correct, since it is the subtle part:
0049 §fail-safe's literal mechanic is "unknown class token resolves to ignore", not
"throw". The code throws. [constrained-by] + the gloss "an uninterpretable privacy
signal resolves to 'suppress more', never to 'no exclusions'" claims exactly the
invariant and not the mechanic, which is accurate. [implements] here would have been
wrong.
4. Judgement call: the {#reporting} split
Verdict: correct, and under-applied by exactly one site (fixed below).
clients.js:1074's gloss - "--check names which source governs (dotfile vs
machine-local entry) and the entry's class" - is a near-verbatim quote of the
Consequences bullet at llp/0103:88-90 ("hyp ignore --check and hyp status
reporting must name which source governs (dotfile vs machine-local entry) or debugging
gets harder"). Nothing in the {#cli} bullet is about --check reporting at all;
--check is LLP 0049's flag, and the same function already carries
@ref LLP 0049#prospective-only. So the split is right, and the bullet rewrite is
content-identical - only a bold label and the marker were added.
Findings
A1 (minor, fixed) - runPolicyShow carries the same reporting gloss but stayed on #cli
src/core/commands/policy.js:254 read:
@ref LLP 0103#cli [constrained-by]: names the governing source (dotfile vs machine-local) and class; store/resolver unchanged
That first clause is the same {#reporting} bullet that justified moving
clients.js:1074, and policy show is hyp ignore --check's successor delegating to
the very function that now carries #reporting. Applying the PR's own rule to one of
the 15 sites and not the other silently mis-files it - and once it resolves, nothing
will ever flag it again. Split it the same way, which also makes the remaining four
policy.js glosses uniform:
@ref LLP 0103#reporting [constrained-by]: the report names which source governs (dotfile vs machine-local entry) and the class
@ref LLP 0103#cli [constrained-by]: the store, resolver, and class lattice are unchanged; only the verb spelling is new
A2 (minor, fixed) - the error_kind prose credit moved to a doc that does not contain it
src/core/usage-policy/local_only.js:18 was changed from (LLP 0080 #fail-safe) to
(LLP 0049 #fail-safe). LLP 0049 §fail-safe mentions no throw, no structured error and
no error_kind; LLP 0080 §fail-safe is where "the failure is a structured error naming
the file" is written, and llp/0081:20 (T1) names the literal string
local_only_list_unreadable as "the LLP 0080 #fail-safe". This is a bare prose mention,
not an @ref, so it never affected the WARNING count - the change bought nothing and
cost accuracy for a reader asking where that error_kind comes from. Reverted to
LLP 0080 #fail-safe, consistent with the 0080 credit the PR itself keeps in the class
JSDoc three lines below.
N1 (nit, not fixed - for the follow-up issue) - the placeholder residue is 9, not 6
The PR body's proposed item-2 follow-up text says "6 refs inside
.claude/skills/ref-check/SKILL.md (88, 114, 122) and ref-story/SKILL.md
(101, 116, 135)". The same two files also carry 3 path-form placeholders -
ref-check/SKILL.md:24 (path/to/doc.md#anchor) and :96, :115
(docs/vendor/spec.md#tokens) - which are broken for the same reason and would leave
/ref-check non-zero even after the 6 are handled. Issue #457 item 4 counted these
(it says 11 and lists all 9 line numbers). Worth folding into the follow-up issue so it
covers the whole residue.
N2 (nit, not fixed) - PR-body reasoning for the policy.js sites
Recorded under section 2 above: the body's stated ground for keeping #cli on the five
policy.js sites does not match what those glosses say. The conclusion is right; only
the argument needs the Extended-by-note framing. Flagged rather than changed, since
this review does not edit PR bodies.
Fixes pushed
Commit f0f603e, on top of b375535, pushed to fix/issue-457 only.
src/core/commands/policy.js | 3 ++-
src/core/usage-policy/local_only.js | 2 +-
Positively verified against the remote branch content (not just locally):
git show origin/fix/issue-457:src/core/usage-policy/local_only.js | sed -n 18p->
/** `error_kind` carried by {@link LocalOnlyListUnreadableError} (LLP 0080 #fail-safe). */git show origin/fix/issue-457:src/core/commands/policy.js | sed -n 254,255p-> the
#reporting+#clipair above.- Re-ran the checker at
f0f603e: refs 1368 (+1, the added ref), BROKEN 9 / 6
LLP-form, WARNING 2, unchanged;policy.js:254(#reporting) and:255(#cli)
both resolve, as do all otherLLP 0103#*andLLP 0049#fail-safesites.
Gates
npm test@b375535: 2846 pass / 8 fail / 1 skipped. The 8 are
test/core/leave-command.test.js(831, 832, 833, 835, 836, 837, 838, 839). I ran the
same suite onorigin/masterin the same worktree: byte-identical 8 failures,
2846 pass. Baseline confirmed, no regression. Same result after my fixes.npm run typecheck: clean atb375535and atf0f603e.- House style: no em dash (U+2014) on any added line, in the PR's diff or mine
(grepped the added lines of both diffs for U+2014: 0 hits each). The pre-existing em dashes in
clients.js(18) andmatcher.js(5) are untouched by both diffs - they are the
[relation] <U+2014> glossHINTs that issue Pre-existing @ref breakage found by a corrected ref-check: 14 refs to a nonexistent LLP 0103#cli anchor, 2 refs into a Superseded LLP, and 3 duplicate LLP numbers #457 item 4 defers. - No runtime behaviour change in either diff: two anchor names in one design doc,
one rewrap, and@ref/JSDoc prose. Consistent with there being no new unit test.
Held-PR overlap: none, verified
Ran git diff --name-only origin/master...origin/<branch> for every held branch and
grepped for the five files (plus src/core/commands/policy.js, which my fix adds):
| branch | PR | touches any of the six files? |
|---|---|---|
fix/issue-448 |
#449 | no |
fix/issue-442 |
#450 | no |
fix/issue-452 |
#456 | no |
fix/issue-453 |
#458 | no |
claude-desktop-consent-and-entrypoint-gate |
#422 | no |
integration/bounded-query-execution |
#221 | no |
fix/issue-459 |
(parallel) | no |
Zero file-level intersection, so no merge-tree reconciliation is needed. #456 does
touch .claude/skills/ref-check/SKILL.md - the document this PR's validation depends
on - but this PR does not edit it, so the dependency is one-way and not a conflict. The
practical consequence is only ordering: until #456 lands, a literal /ref-check run
still uses the heading-slugs-only §3 and will disagree with the numbers above.
Scope statement for #457
Confirmed the body is explicit that merging closes #457 while items 3 and 4 stay
unfixed: the section is headed "Merging this closes #457, and items 3 and 4 are NOT
fixed", followed by "They still need their own issues. Please file:" and three
enumerated follow-ups (duplicate LLP numbers 0098/0099/0111; the skill-doc placeholder
refs; the 459 em-dash HINTs). Clear enough that nothing is lost on close, subject to N1
above (the placeholder count should be 9). I did not touch items 3 or 4.
Verdict
The substance is sound: both judgement calls are right, both are right for defensible
reasons, and the numbers reproduce under an independent implementation of the corrected
algorithm. The two things worth fixing were consistency artefacts of the split itself
rather than errors of judgement, and both are pushed. Ready for a human, still draft -
not merged, not marked ready by this review.
Host state untouched: no hyp subcommand of any kind was run, and no
~/.claude/settings.json, ~/.codex/config.toml or ~/.config/systemd/ file was read
or written. Work was done in a detached throwaway worktree; /work/hypaware was never
switched.
…hor claim LLP 0103 never makes Two glosses that resolve but that their target's prose does not support. - `readListEntriesSync` (`matcher.js`) kept its "a corrupt list fails the resolve loudly" gloss verbatim when the target moved from LLP 0080 §fail-safe to LLP 0049 §fail-safe. LLP 0049 §fail-safe prescribes the opposite mechanic: an unimplemented class token *resolves* to `ignore` and logs a warning, it never throws. The same defect was corrected on `local_only.js` (gloss reworded to the pure invariant, throw mechanics credited to LLP 0080 in prose) but not here. Gloss now states the invariant LLP 0049 actually supplies; the block's own prose credits the throw-not-empty mechanic to LLP 0080 §fail-safe, matching `local_only.js`. - `runPolicyUnset` (`policy.js`) glossed `LLP 0103#cli` with "reuses the shared `isEqualOrDescendant` ancestor predicate". LLP 0103 mentions no ancestor or descendant predicate anywhere; that is LLP 0071's, already cited on `matchList`. Reworded to the symmetric removal `#cli` does mint. Annotation and JSDoc prose only. `npm test` 2846 pass / 8 fail, all 8 in `test/core/leave-command.test.js` and all 8 present on `origin/master`; typecheck clean; corrected ref-check unchanged at BROKEN 9, WARNING 2.
Neutral review, round 2 of 2 (FINAL) -
|
| site | anchor | what the gloss is about | verdict |
|---|---|---|---|
src/core/commands/clients.js:710 |
#cli |
--private/--sync flag parsing |
OK |
src/core/commands/clients.js:755 |
#cli |
flag dispatch to the marking verb | OK |
src/core/commands/clients.js:891 |
#cli |
shared marking verb behind the flags | OK |
src/core/commands/clients.js:938 |
#cli |
flag dispatch to the unmarking verb | OK (hyp unignore grows symmetric removal) |
src/core/commands/clients.js:1008 |
#cli |
symmetric class-scoped removal | OK |
src/core/commands/clients.js:1074 |
#reporting |
--check names the governing source |
OK (the PR's move) |
src/core/commands/policy.js:33 |
#cli |
store/resolver/lattice unchanged, verb spelling new | OK |
src/core/commands/policy.js:218 |
#cli |
same | OK |
src/core/commands/policy.js:254 |
#reporting |
the report names the governing source and class | OK (A1) |
src/core/commands/policy.js:255 |
#cli |
store/resolver/lattice unchanged | OK (A1) |
src/core/commands/policy.js:292 |
#cli |
"reuses the shared isEqualOrDescendant ancestor predicate" |
mis-filed, fixed in 6a9c6a3 |
src/core/commands/policy.js:339 |
#cli |
enumerates the version-2 class-per-entry store | nit, see below |
llp/0111:21, llp/0111:247, llp/0112:42, llp/0112:43 |
#cli |
the CLI paragraph / the store-and-classes rationale | OK |
Two points that settle the A1 question specifically:
- The five
policy.jssites are now genuinely uniform. Each carries exactly
one#cli"the model is unchanged, only the spelling is new" claim, andshow
carries#reportingon top because it is the only one of the five that
reports anything. Beforef0f603e,showwas the odd one out: it made the
source-naming claim under#cliwhile its four siblings made the
model-unchanged claim under the same anchor. - The
clients.js:1074vspolicy.js:254-255asymmetry is justified, not a
new defect.runIgnoreChecklegitimately loses#clientirely: the#cli
bullet never mentions--checkin any form.runPolicyShowlegitimately
keeps it: the "new verb spelling, same model" claim ispolicy.js's to make,
andllp/0111:21makes that exact claim against that exact anchor
("extends its CLI paragraph; store format, resolver, and class lattice
untouched"), which is independent corpus sanction for the wording round 1
copied onto:255.
2. Verdict on round 1's fix A2 (the prose credit back to LLP 0080)
Correct, and demonstrably so rather than merely conservative. The two
sections say different things, and the difference is exactly the mechanic:
llp/0049-hypignore-usage-policy.spec.md:96-109{#fail-safe}is about a
.hypignorenaming a class the running version does not implement. It
resolves toignoreand logs a warning. There is no throw, no
structured error, noerror_kind, and no file naming anywhere in it. What it
does supply is the invariant: "the safe failure for a privacy control is
'suppress more,' never 'record-and-export something the user flagged.'"llp/0080-local-only-dir-selection.design.md:79-90{#fail-safe}is where
the mechanic lives, verbatim: "the store throws, the resolver propagates, and
readRowsSincelets the error fail the partition read" and "the failure is a
structured error naming the file - never a silent state."llp/0081:20
T1 then spells the constant out: "unparseable => throw with
error_kind: 'local_only_list_unreadable'- the LLP 0080 #fail-safe".
So LOCAL_ONLY_LIST_UNREADABLE_ERROR_KIND genuinely belongs to LLP 0080, and
local_only.js:18 is right to say so. The @ref staying on LLP 0049#fail-safe
is also right: [constrained-by] an invariant is what that ref is, and it is the
citation format.js:30 already carries for the same polarity.
But A2 was incomplete. See finding B1.
3. Findings
B1 (medium, FIXED in 6a9c6a3) - the same A2 defect survived in matcher.js
src/core/usage-policy/matcher.js:215. The PR moved this ref's target from
LLP 0080#fail-safe to LLP 0049#fail-safe while keeping the gloss byte-for-byte:
- @ref LLP 0080#fail-safe [implements]: a corrupt list fails the resolve loudly, never silently to "no exclusions"
+ @ref LLP 0049#fail-safe [constrained-by]: a corrupt list fails the resolve loudly, never silently to "no exclusions"
"fails the resolve loudly" is not merely absent from LLP 0049 §fail-safe, it is
the opposite of what that section prescribes: 0049's fail-safe succeeds at
resolving (to ignore) and logs a warning. This is the identical defect round 1
caught on local_only.js - and note the PR itself had already reworded
local_only.js's gloss to the pure invariant ("resolves to 'suppress more',
never to 'no exclusions'"), which is why that one reads correctly. matcher.js
was the one where only the target moved.
The mechanic is credited to LLP 0080 nine lines earlier at matcher.js:170,
but that is matchList's JSDoc, a different construct; CLAUDE.md's attachment
rule means it does not carry to readListEntriesSync. Fixed the same way
local_only.js was: the gloss now states the invariant LLP 0049 supplies, and
the block's own prose credits the throw-not-empty mechanic to LLP 0080 §fail-safe.
B2 (low, FIXED in 6a9c6a3) - #cli glossed with a mechanic LLP 0103 never mentions
src/core/commands/policy.js:292 (runPolicyUnset) glossed LLP 0103#cli as
"reuses the shared isEqualOrDescendant ancestor predicate". grep -i 'ancestor\|descendant' llp/0103-machine-local-policy-classes.decision.md
returns nothing - the predicate is LLP 0071's, already cited correctly at
matcher.js:175 ("segment-aware equal-or-descendant list membership"). This was
invisible before the PR because the ref was BROKEN; making it resolve is what
exposed it. Reworded to the symmetric removal #cli actually mints. The
ancestor-walk fact itself is still stated in the function's prose at
policy.js:278-279, so nothing was lost.
N1 (nit, not fixed) - policy.js:339's gloss is about the store, not the CLI
"enumerates the version-2 class-per-entry store as-is; no format change" quotes
LLP 0103's Decision preamble (the version-2 JSON block, llp/0103:41-49),
not the #cli bullet. By the letter of the PR's rule it wants #decision, which
also resolves. Left alone on purpose: llp/0112:47 T6 explicitly instructs that
"the store-and-classes rationale keeps citing LLP 0103#cli", so changing it
would put the code at odds with an Accepted plan. Same tension applies mildly to
policy.js:33/:218/:255, whose "the store, resolver ... are unchanged" half
is not in the #cli bullet either - but llp/0111:21 makes that exact claim
against that exact anchor, so the corpus is self-consistent there. This is really
a corpus-level question (does #cli want renaming, or does LLP 0112 T6 want
updating) and belongs with #457's citation-form item, not here.
N2 (nit, not fixed) - "and the class" on the two #reporting glosses
clients.js:1074 says "and the entry's class"; policy.js:254 says "and the
class". The #reporting bullet says only "which source governs". Reporting the
class is squarely LLP 0103's point and both enclosing JSDoc blocks document it,
so this is a mild over-reach at worst.
N3 (nit, not fixed) - pre-existing em dash in a block this PR's ref now lives in
src/core/usage-policy/matcher.js:168 (throws — the same fail-safe) is a
CLAUDE.md house-style violation. Present on origin/master unchanged, so out of
scope; my edit is in the adjacent block and adds none. Flagging so it is not
mistaken for a regression.
4. My own ref-check totals, and the 6-vs-9 placeholder question
Ran my own implementation of the corrected §3 (index {#slug} on any line;
inline <a id=>/<a name=>; heading slugs lowercased with one - per
whitespace character and no collapsing, <a id>/<a name> tags stripped off
heading lines only). One calibration note for whoever lands #456: the slugger
must keep _. Strip it and 17 perfectly good refs go BROKEN, e.g.
LLP 0045#settings_file-is-home-relative-and-a-violation-is-loud against
llp/0045:404. The no-collapsing rule is also load-bearing and verifiable:
llp/0045:159 ## Part 2 — The attach handler (...) yields the double
hyphen in part-2--the-attach-handler-srccoreconfigaction_attachjs, which is
what action_attach.js:56 actually cites.
origin/master (f9b9667) |
f0f603e |
after 6a9c6a3 |
|
|---|---|---|---|
| refs / files / LLP docs | 1367 / 359 / 133 | 1368 / 359 / 133 | 1368 / 359 / 133 |
| BROKEN, total | 25 | 9 | 9 |
| - of which LLP-form | 22 | 6 | 6 |
| - of which path-form | 3 | 3 | 3 |
| WARNING | 4 | 2 | 2 |
| duplicate LLP numbers | 3 (0098, 0099, 0111) |
3 | 3 |
The placeholder residue is 9, not 6. The PR's 22 -> 6 is the LLP-form-only
tally; the true figure is 25 -> 9. The delta is exactly three path-form
placeholder refs in the same skill doc:
.claude/skills/ref-check/SKILL.md:24 @ref path/to/doc.md#anchor
.claude/skills/ref-check/SKILL.md:96 @ref docs/vendor/spec.md#tokens
.claude/skills/ref-check/SKILL.md:115 @ref docs/vendor/spec.md#tokens
And the difference does matter for the follow-up issue, for a reason beyond
arithmetic. Item 2 of #457's follow-up list asks for "an example-block marker or
an exclusion rule" so /ref-check can exit 0. A rule keyed on the LLP-number
form (LLP NNNN#anchor) silences 6 of the 9 and still leaves the tool exiting 1
forever. The rule has to cover the path form too, which is only visible once
you count all 9. Worth writing the number and the form breakdown into the issue.
The other independent confirmations of round 1's numbers: 1367 refs on master,
not 1364; WARNING 4 -> 2 reproduces exactly; and the 2 survivors
(llp/0101:21, llp/0102:18 citing the documents they supersede) are correct as
written.
5. Checks
npm run typecheck: clean.npm test: 2855 tests, 2846 pass, 8 fail, all 8 in
test/core/leave-command.test.js. Exactly the documentedorigin/master
baseline; no regression.- No em dash added (
git diff origin/master...6a9c6a3 | grep '^+' | grep U+2014
is empty). No semicolons added.§used for section references, matching the
existing convention inlocal_only.jsand the PR's own prose. - Corrected ref-check unchanged by
6a9c6a3(BROKEN 9, WARNING 2), as expected:
both fixes are gloss/prose text, neither moves a target. - Positively verified
6a9c6a3againstf0f603e:git diff f0f603e 6a9c6a3
touches exactlysrc/core/commands/policy.js(1 line) and
src/core/usage-policy/matcher.js(1 gloss line + 2 prose lines). Nothing else. - Anchor form audited:
- **Bold** {#slug}: proseis the corpus's established
list-item convention, used inllp/0049:154,llp/0101,llp/0105,
llp/0106,llp/0107,llp/0122:107,llp/0138. Both new anchors match it
exactly, and neither collides with an existing anchor inllp/0103. - Spot-checked the
llp/0135unwrap end to end:llp/0011:27
## Interactive walkthroughexists, the unwrapped ref resolves, and the
annotation the doc claims is "currently onrunPickerWalkthrough" really is
there, atsrc/core/cli/walkthrough.js:314.
6. For triage
Round cap is 2 and this was round 2, so everything below goes to a human rather
than a round 3. Nothing here blocks the PR.
- The follow-up issue's placeholder count is 9, not 6, and 3 of the 9 are
path-form (@ref path/to/doc.md#anchor). The exclusion rule item 2 asks for
must handle both citation forms or/ref-checkstill cannot exit 0. Evidence
in §4. policy.js:339(N1): the#clianchor carries glosses about the store
format becausellp/0112:47T6 tells it to. Either rename/re-scope the anchor
or update T6. Belongs with Pre-existing @ref breakage found by a corrected ref-check: 14 refs to a nonexistent LLP 0103#cli anchor, 2 refs into a Superseded LLP, and 3 duplicate LLP numbers #457's citation-form item.- The PR body's stated reason for the five
policy.jssites is still wrong
(round 1's nit, not re-litigated here): the body says all five "gloss the
--private/--local-only/--syncmachine-local marking flags", but four of
them gloss "store/resolver/lattice unchanged". The choice of#cliholds
anyway, for a different reason (llp/0111:21makes the same claim against the
same anchor;llp/0111:247andllp/0112:42-43,47mandate the citation).
Body deliberately not edited. - N2 and N3 above: cosmetic, no action needed unless a sweep is happening
anyway. N3 (matcher.js:168) is one of the 459 em-dash HINTs item 3 of the
follow-up list already covers.
Summary
A1 correct and complete; A2 correct on the fact but incomplete in scope, and the
missing half is now fixed. Two glosses that resolved but that their target's
prose contradicts or omits, both fixed in 6a9c6a3. Corrected ref-check
25 -> 9 BROKEN and 4 -> 2 WARNING at this head, with the settled placeholder
count of 9 (6 LLP-form + 3 path-form) for the follow-up issue. Baseline tests
and typecheck green. Held for human review as usual; not marking ready, not
merging.
Triage rung (LLP 0017) - review rounds exhausted, PR judged safe to shipRound cap (2) was reached at head Re-checked every unresolved finding from both review rounds against the
Classification: all five are documentation/annotation nits (PREFERENCE), Outcome: PR can merge safely. All residual findings are deferred rather Follow-up issueAn open follow-up already exists and covers every residual finding above, No new issue opened (idempotent - reusing #463 rather than creating a Triage marker will be appended to the PR body next, at head |
philcunliffe
added
the
neutral:approved
…n em dash (#463 items 2 and 3) (#468) * LLP corpus: every @ref gloss opens with a colon, never an em dash (#463 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> * Illustrative @refs are marked, so ref-check can reach exit 0 (#463 item 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> * The ignore marker only fires inside a comment, and suppression is gated 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> * A marker is a comment in its file's language and never a code sample 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> --------- Co-authored-by: neutral-reconciler <neutral@example.com> Co-authored-by: Claude <noreply@anthropic.com>
…639) (#651) * Ref hygiene: the tolerance list expires with the defect it forgives (#639) Issue #639 reported eleven broken `@ref` anchors. None of them are broken. Every anchor the issue names resolves on `origin/master`: LLP 0103 grew `{#cli}` and `{#reporting}` when PR #461 landed, and `0138#one-command`, `0138#one-materializer`, `0101#deadline`, `0101#no-release` and `0107#reversal` have been `{#slug}` markers on list items since the documents were written. A resolver that indexes only heading slugs reports all six of those as broken, which is the failure `ref-check`'s SKILL.md already warns is worth 47 references on this corpus. What did survive is the tolerance list in the hygiene gate. Its five entries, 16 forgiven occurrences, forgive nothing: PR #461 landed the repairs and the entries stayed behind, so the only remaining claim that `policy.js` cites a dead anchor was the list that exists to excuse it. Issue #639 read that claim back out. Prune the entries, and make the next one expire on its own: the spend ledger belongs to the caller now, and a tolerance nobody spends fails the suite instead of sitting there as evidence of a defect that healed. With the list empty the gate has no exceptions: 1901 annotations, zero broken. Fixes #639 Co-Authored-By: Claude <noreply@anthropic.com> * Ref hygiene staleness check: forgive no more than is still broken The staleness test only checked spent > 0, so a tolerance whose budget outran the number of genuinely broken refs at that identity passed silently, leaving unspent forgiveness that could absorb new breakage at the same file+target. Tighten it to spent < budget, and report both numbers in the failure message so the offending entry is directly editable. --------- Co-authored-by: test <test@example.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: test <test@test.com>
1 participant
What this fixes
Items 1 and 2 of #457. Items 3 and 4 are deliberately left alone (see below).
Item 1:
@ref LLP 0103#cli, 15 sites, anchor never definedThe issue counted 14; a full pass finds 15 (
llp/0112:43carries the ref too,on the T2 task line next to the T1 one the issue cited).
llp/0103defined noclianchor in any of the three forms.Chose (a), add the anchor, not (b), repoint. The referent is unambiguous:
llp/0111-hyp-policy-verb.design.md:21says outright@ref LLP 0103#cli [constrained-by] - extends its CLI paragraph.clients.jssites and all fivepolicy.jssites gloss the--private/--local-only/--syncmachine-local marking flags, which isexactly what that paragraph mints ("Machine-local writes get flags on the same
verb...", plus the
Extended-by: LLP 0110note thepolicyverb rests on).So the paragraph gets the name the refs already agree on,
- **CLI** {#cli}:, inthe list-item form 8 other docs in this corpus already use (LLP 0101, 0105, 0106,
0107, 0138, ...). One anchor, 14 refs fixed, intent preserved.
One of the fifteen meant a different paragraph.
runIgnoreCheck(
clients.js:1074) glosses "--checknames which source governs (dotfile vsmachine-local entry)", which is verbatim the Consequences bullet, not the CLI
paragraph. Naming that bullet
{#reporting}and repointing that one ref keepsthe rule the issue and CLAUDE.md both ask for: the anchor has to land on the
prose the gloss actually quotes. The other 14 are untouched, so the prose in
LLP 0111/0112 that talks about "the
@ref LLP 0103#cliannotations" stays true.Item 2: two refs into a Superseded LLP
src/core/usage-policy/local_only.jsandmatcher.jscitedLLP 0080#fail-safe. LLP 0080 isStatus: Superseded.Its formal
Superseded-byis LLP 0102 (skill-replaces-enrollment-picker),and repointing there mechanically would be wrong: 0102 retires the in-login
picker and contains no fail-safe content at all. A ref at the wrong live doc is
worse than one at a superseded doc.
LLP 0080's own supersede note scopes the supersession and says where the rest
lives: "The non-picker substrate (machine-local list, export-seam withholding,
durable CLI) lives on under LLP 0069/0070/0071." The corrupt-list rule is that
substrate, and 0080 §fail-safe derives it explicitly from
LLP 0049#fail-safe("forbidden by [LLP 0049 §fail-safe]") - Accepted, sameanchor name, and already the citation carried by
src/core/usage-policy/format.js:30for the same polarity. Both refs now cite itas
[constrained-by], with the throw-not-empty mechanics credited to LLP 0080§fail-safe in prose so the trail survives without a ref into a superseded doc.
Nothing else in the corpus restates the corrupt-list rule (checked
0049/0069/0070/0071/0081/0102/0103/0108), which is why it is a repoint to the
invariant rather than to a replacement section.
Bonus, explicitly allowed by scope
llp/0135:959quoted@ref LLP 0011#interactive-walkthroughacross a line wrap,so the trailing hyphen read as an anchor. Rewrapped.
LLP 0011does have an## Interactive walkthroughheading, so the unwrapped ref resolves.Ref-check before / after
Validated with the corrected §3 algorithm from #456 (all three anchor forms;
{#slug}indexed on any line, not just headings; heading slugs with one-per whitespace character and no collapsing;
<a>/</a>stripped off headinglines and nothing else). The version of §3 on
masteris still theheading-slugs-only one and reports hundreds of resolvable refs as broken, so it
was not used.
Over 1364 refs in 359 files, 133 LLP docs:
The 22 before: 15 x
LLP 0103#cli, 6 placeholder examples in the skill docs, 1wrap artifact in
llp/0135. The 6 after are exactly the skill-doc placeholders(
LLP 0042#token-strategyx2,LLP 0042#anchor,LLP 0074#focus-trap,LLP 0074#focus-management,LLP 0007#layout) - item 4, untouched. The 2remaining WARNINGs are
llp/0101:21andllp/0102:18citing the very documentsthey supersede, which is correct as written.
Per-site proof:
Checks
npm test: 2846 pass, 8 fail, all 8 intest/core/leave-command.test.jsand all 8 present on
origin/masterbefore this change. Baseline, noregression.
npm run typecheck: clean.one design doc. Which is why the gate here is the corrected ref-check rather
than a new unit test.
Held-PR overlap
None. Checked
git diff --stat origin/master...origin/<branch>forfix/issue-448(#449),fix/issue-442(#450),fix/issue-452(#456),fix/issue-453(#458),claude-desktop-consent-and-entrypoint-gate(#422), andintegration/bounded-query-execution(#221). None of them touch any of the fivefiles here (
llp/0103-machine-local-policy-classes.decision.md,llp/0135-install-experience-overhaul.design.md,src/core/commands/clients.js,src/core/usage-policy/local_only.js,src/core/usage-policy/matcher.js), so nomerge-treereconciliation is needed.Note the validation here depends on #456's corrected §3 wording but does not
edit that file, so the two do not conflict.
Merging this closes #457, and items 3 and 4 are NOT fixed
They still need their own issues. Please file:
0098(
inactive-not-unknown-dispatch-miss/scancolumn-where-pushdown),0099(
codex-attach-auth-route/dispatch-miss-repair-by-cause),0111(
hyp-policy-verb.design/report-cli.decision). Every bare@ref LLP 0111#surfaceis formally ambiguous, and a checker that keeps onlythe last file it walked reports refs aimed at the other claimant as broken.
Corpus-wide decision: renumber the later claimant and sweep its refs, or adopt
a filename-qualified citation form documented in LLP 0001. Not fixable in
passing.
/ref-checkcan never exit 0 on this repo - 6 refs inside.claude/skills/ref-check/SKILL.md(88, 114, 122) and.claude/skills/ref-story/SKILL.md(101, 116, 135) are illustrativeplaceholders (
LLP 0042#token-strategy,LLP 0074#focus-trap,LLP 0007#layout). Harmless as prose, fatal if anyone wires the skill intoCI. Needs an example-block marker or an exclusion rule.
@ref ... [relation] - glossHINTs using an em dash where LLP 0000specifies
:, which is also a CLAUDE.md house-style violation. Mechanical buta large diff, deliberately not swept here.
Fixes #457