-
Notifications
You must be signed in to change notification settings - Fork 2
feat(docs-hygiene): add container-position pattern forms to rename-references #1335
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.
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
Changes from all commits
ceeb0b9
595fff5
a5686a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,10 +156,160 @@ invokes?\s+`?/<other-skill>\s+<old>\b | |
| - **False-positives:** rare — `<old>.` followed by a word char is specific. Sentence-end prose (`verify. Then…`) is excluded because `[\w-]+` requires a word char immediately after the dot (the space after the dot breaks it). | ||
| - **Coupled-rename note:** dot-form is one face of coupled-sibling renames. When a skill renames, ALSO enumerate its internal mode names and content-file basenames that changed in lockstep (`quality` mode, `context/quality.md`) and sweep EACH as its own rename pair — they carry no primary token, so a sweep keyed only on `<old>` never reaches them. See SKILL.md "Gotchas" coupled-rename entry. | ||
|
|
||
| ## Container-position forms (13–15) | ||
|
|
||
| Forms 1–12 assume `<old>` is a skill/mode identifier. When the renamed thing is a | ||
| **container** — a plugin, a marketplace entry, anything a user names as an argument or | ||
| titles a document after — three positions carry it that none of the earlier forms reach. | ||
| Each is high-precision because the SURROUNDING SYNTAX proves the token is a proper name, | ||
| not a verb. | ||
|
|
||
| Why this matters more than coverage: when `<old>` is also an English verb *in the consuming | ||
| codebase*, Form 2 cannot separate the two senses at any triage setting. Measured on the | ||
| `re-anchor` → `discipline` rename, over the plugin's own tree: Form 2 matched **134** lines; | ||
| Forms 13–15 matched **9** — the 8 real defects plus one frozen CHANGELOG-history line the | ||
| existing "Frozen historical records" rule already excludes. See `triage.md` | ||
| "Verb-sense collision the blocklist cannot serve". | ||
|
|
||
| ## Form 13: Command-argument position | ||
|
|
||
| ```regex | ||
| (^|[^\w/])/plugins?\s+(install|uninstall|configure|enable|disable|update|add|remove)\s+`?<old>\b | ||
| \b<old>@[\w.-]+ | ||
| ``` | ||
|
|
||
| - **Triage default:** Certain | ||
| - **Catches:** `<old>` as the ARGUMENT to a management command rather than as the command | ||
| itself — `/plugin install <old>@marketplace`, `/plugin configure <old>`, | ||
| `/plugin enable <old>` — plus the `<old>@<marketplace>` qualified-id form wherever it | ||
| appears (settings examples, install snippets, `enabledPlugins` / `pluginConfigs` keys). | ||
| - **Why Form 1 misses it:** Form 1 anchors on `/<old>`. Here the slash belongs to `plugin`, | ||
| and `<old>` sits one-to-several words downstream with no slash of its own. | ||
| - **Why the leading `[^\w/]` alternation:** keeps `.../plugin install x` (a path) from | ||
| matching while still allowing a line start, a space, or a backtick before the slash. | ||
| - **Optional backtick before `<old>`:** these appear inside inline code spans constantly | ||
| (`` `/plugin configure <old>` ``); without `` `? `` the pattern misses the most common | ||
| rendering. | ||
| - **False-positives:** rare. The enclosing management verb is what supplies the | ||
| disambiguation bare-token position lacks — prose does not accidentally say | ||
| "/plugin configure" before an English verb. | ||
| - **Severity note:** these are FUNCTIONAL breaks, not cosmetic. A reader following | ||
| `/plugin install <old>@marketplace` gets `plugin-not-found`. Rank them above title hits | ||
| when reporting. | ||
|
|
||
| ## Form 14: Document title / declared name | ||
|
|
||
| ```regex | ||
| ^#{1,6}\s+`?<old>`?\s*$ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a lowercase container name is capitalized in a heading, such as the documented Useful? React with 👍 / 👎. |
||
| ^(name|title):\s*("<old>"|'<old>'|<old>)\s*$ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a plugin/package container rename where Useful? React with 👍 / 👎. |
||
| ``` | ||
|
|
||
| - **Triage default:** Certain | ||
| - **Catches:** an ATX heading whose ENTIRE content is the renamed token — the README H1 that | ||
| names the thing — and frontmatter `name:` / `title:` declaring it. | ||
| - **Why the `$` anchor is load-bearing:** it is what makes this Certain rather than | ||
| ambiguous. A heading that merely *contains* the token (`## How re-anchor works`) may well | ||
| be verb usage and belongs in Form 2's ambiguous bucket; a heading that IS the token can | ||
| only be naming it. | ||
| - **Quote handling:** the alternation accepts a bare, double-quoted, or single-quoted value | ||
| and requires the quotes to PAIR — `"<old>"` and `'<old>'`, never `"<old>'`. A naive | ||
| `["']?<old>["']?` would match the mismatched form, which is not valid YAML. | ||
| - **False-positives — real, and the reason for the scope rule below.** "A heading that IS the | ||
| token can only be naming it" holds when the token is coined or hyphenated. It FAILS when the | ||
| container has an ordinary-word name: renaming a `testing` plugin matches this repository's own | ||
| `README.md:86` (`### Testing`, a marketplace category heading), and renaming an `architecture` | ||
| plugin matches `plugins/miro/README.md:39` (`## Architecture`, an unrelated design section). | ||
| Both were verified against the tree. Under precedence, a false Certain here is worse than a | ||
| Form 2 hit, because it DISCARDS the safer classification. | ||
| - **Scope rule (required):** rate a title match Certain only when the file is plausibly | ||
| container-owned — the container's own README/SKILL/manifest, or a path under its directory. | ||
| A heading match in a file the container does not own is **Ambiguous**, whatever the token | ||
| looks like. When the token is a common English word, demote every title match to Ambiguous | ||
| regardless of path. | ||
| - **Note:** a plugin/skill README H1 is the landing surface every consumer sees first, and | ||
| it is the single most-missed reference in practice — the rename moves the directory, so | ||
| the path-form patterns all pass, and nothing looks at line 1. That is why the form exists; | ||
| the scope rule is what keeps it from over-reaching to every document in the tree. | ||
|
|
||
| ## Form 15: Possessive and appositive container reference | ||
|
|
||
| ```regex | ||
| `?\b<old>\b`?'s\b | ||
| \bthe `?<old>`? (plugin|skill|marketplace entry|package|module)\b | ||
| ``` | ||
|
|
||
| - **Triage default:** Certain | ||
| - **Catches:** prose where `<old>` stands in for the CONTAINER — "Report `<old>`'s effective | ||
| configuration", "the `<old>` plugin ships…". | ||
| - **Inline-code wrapping is the common case, not the exception:** in markdown the token is | ||
| usually a code span, so the literal `<old>'s` sequence never appears — it is | ||
| `` `<old>` `` followed by `'s`. The optional backticks are what make this form fire on | ||
| real documentation; without them it silently misses its own motivating example. Form 13 | ||
| carries the same allowance for the same reason. | ||
| - **Why it is Certain even when `<old>` is a blocklisted verb:** English verbs do not take | ||
| the possessive clitic, and a noun-class appositive (`the X plugin`) forces the naming | ||
| reading. Both shapes are grammatically incompatible with the verb sense, so this is safe | ||
| where Form 2 is not. | ||
| - **False-positives:** a token that is a noun in ordinary use ("the review plugin" vs a | ||
| review) can still collide; when `<old>` is a common NOUN rather than a verb, demote this | ||
| form to ambiguous. | ||
| - **Extend the appositive noun class** to whatever the consuming repository calls its | ||
| containers. | ||
|
|
||
| ## Phase 0 — pre-sweep blocklist load | ||
|
|
||
| Before running any pattern, load the English-verb blocklist from `triage.md`. Any bare-token (Form 2) or chain-context match (Forms 4, 5, 6, 9) where the token is in the blocklist is forced into ambiguous bucket regardless of regex precision. | ||
|
|
||
| **Precedence: a container-position match wins its line outright.** Forms 13–15 are strictly | ||
| more specific than Form 2 — every line they match, Form 2 also matches. Without precedence the | ||
| new forms would only ADD hits, leaving the Form 2 flood they exist to avoid fully intact. | ||
|
|
||
| Deduplicate by `(file, line)` AFTER the sweep and BEFORE triage: | ||
|
|
||
| 1. A `(file, line)` matched by any of Forms 13–15 is attributed to that form and enters the | ||
| **Certain** bucket. Drop the Form 2 (and any chain-form) match for that same line — it is | ||
| the same reference seen through a weaker lens, not a second finding. | ||
|
Comment on lines
+269
to
+271
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Form 14 matches a title outside a container-owned document or matches a common-word name, its new scope rule says to demote it to Ambiguous, but this precedence step still unconditionally sends every Form 13–15 match to Certain and drops the safer Form 2/chain classification. Fresh evidence after the earlier title-scope finding is this unconditional instruction, which conflicts directly with the added remedy and can still auto-apply unrelated headings; preserve Form 14's computed bucket while deduplicating. Useful? React with 👍 / 👎. |
||
| 2. Only lines Forms 13–15 did NOT match fall through to Form 2's blocklist rule above. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CONFIRMED — you are right, and my previous reply overclaimed. Fixed in I wrote that precedence turned the 126 lines into "ordinary verb uses that were never candidates." Nothing in the precedence rule made that true. Deduplicating by Added container-rename mode (
Mode works where the static blocklist cannot because it is a property of the rename, not of whether anyone remembered to list the token in advance — which was the original complaint in Fixture with mode + precedence: 8 Certain findings, 126 reported-not-proposed, 0 confirmation prompts, against Form 2 unaided at 134. |
||
|
|
||
| The Phase 0 rule is therefore scoped to what actually reaches Form 2: it forces a | ||
| blocklisted token's bare-token matches ambiguous, and container-position matches never | ||
| become bare-token matches. Report the deduplication in the audit output — "N Form-2 hits | ||
| superseded by container-position matches" — so a reader can see the suppression happened | ||
| rather than inferring it from a smaller number. | ||
|
|
||
| **Precedence alone is NOT sufficient — it only resolves lines the container forms also | ||
| matched.** On the measured fixture that is 8 lines out of Form 2's 134. The other 126 are | ||
| ordinary verb uses that no container form touches, so they fall through to Form 2 and, when | ||
| the token is absent from the static blocklist, take its **Certain** default. Deduplicating | ||
| overlaps does nothing for them. | ||
|
|
||
| ## Phase 0b — container-rename mode | ||
|
|
||
| Declare the sweep's MODE at Phase 0, from what is being renamed: | ||
|
|
||
| - **Identifier rename** (a skill, a mode, a dotted ID) — every form applies as before. Nothing | ||
| below changes. | ||
| - **Container rename** (a plugin, a marketplace entry, a package) — the thing being renamed is | ||
| a proper name, so a bare-token occurrence is EVIDENCE OF NOTHING: it is as likely to be the | ||
| word used ordinarily as the container referenced. In this mode: | ||
| 1. Forms 13–15 (plus Forms 1 and 3, which are already position-anchored) produce the | ||
| **Certain** bucket. | ||
| 2. Form 2's residue — every bare-token line NOT matched by a position-anchored form — is | ||
| **excluded from Certain entirely**, regardless of blocklist membership. Report it as a | ||
| single aggregate count ("126 bare-token occurrences not in container position, not | ||
| proposed"), never as per-match prompts. | ||
| 3. Surface the residue only if the user explicitly asks to widen (`--include-bare-token`), | ||
| and then as Ambiguous, never Certain. | ||
|
Comment on lines
+301
to
+302
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a user follows this instruction and passes Useful? React with 👍 / 👎. |
||
|
|
||
| Container renames are exactly the case where bare-token position carries no signal, so | ||
| spending the user's attention on it is a cost with no corresponding catch. The static | ||
| blocklist is irrelevant here — mode is a property of what is being renamed, not of whether | ||
| someone remembered to list the token. | ||
|
|
||
| **With mode + precedence together**, the measured fixture resolves as: 8 Certain | ||
| container-position findings, 126 bare-token occurrences reported as an aggregate and not | ||
| proposed, and 0 confirmation prompts — against Form 2's unaided 134. | ||
|
|
||
| ## Phase 6 — pattern library evolution | ||
|
|
||
| When the skill's re-sweep finds a NEW syntactic form not covered above: | ||
|
|
@@ -168,6 +318,13 @@ When the skill's re-sweep finds a NEW syntactic form not covered above: | |
| 2. Document the pattern in this file with all 5 fields (form name, regex, triage default, example, false-positives) | ||
| 3. Re-run sweep with extended pattern library | ||
|
|
||
| **Validate a new form on BOTH axes before adding it.** Recall alone is not evidence — Form 2 | ||
| already has perfect recall on every form here and is still unusable when the token is a verb. | ||
| Measure the candidate against a real fixture: the reference commit that FIXED the missed | ||
| references (its removed lines are the defect set) for recall, and the whole pre-fix tree for | ||
| precision, reporting the new form's hit count beside bare-token Form 2's on that same tree. | ||
| A form that does not beat Form 2 on precision is not carrying its weight. | ||
|
|
||
| ## Cross-platform note | ||
|
|
||
| All patterns are ripgrep-compatible (PCRE2 subset). Invoke via the Grep tool, NOT raw shell — Grep handles cross-platform path quoting and is faster than spawning `rg`. If a shell fallback is unavoidable, use `git grep -nE` scoped to tracked files or `rg` from the repo root with the Auto-exclusions applied — never `grep -P` (Perl regex doesn't exist on macOS BSD grep). Do not use lookbehinds — Form 1 uses `\B` instead. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the renamed container has a generic name, this pattern promotes every exact heading with that word to Certain even when the heading names a concept or catalog category rather than the container. For example, renaming the
testingplugin in this repository also matchesREADME.md:86(### Testing), the marketplace category heading, while renamingarchitecturematches the unrelated Architecture section inplugins/miro/README.md; precedence then discards the safer Form-2 classification and these become auto-apply candidates. Constrain this form to known container-owned title files or demote headings without container context.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CONFIRMED — verified both examples against this repository before fixing. Fixed in
a5686a1b.And both
testingandarchitectureare real plugin names inmarketplace.json, so the collision is live, not hypothetical.Form 14's stated premise — "a heading that IS the token can only be naming it" — holds for a coined or hyphenated name like
re-anchorand fails for an ordinary-word one. I generalized from the fixture, which had only the former.Your point about precedence making it worse is the sharp end of it, and I want to name it explicitly: without precedence a bad title match is one Certain hit competing with a Form 2 hit on the same line. With precedence it DISCARDS the safer Form 2 classification, so the false positive is laundered into an auto-apply candidate. The mechanism I added in the prior commit is what raised the stakes on this form's over-reach.
Scope rule added to Form 14:
The false-positives field now carries both verified examples with their line numbers rather than the "effectively none" I originally wrote, which was wrong. Eval 11 uses your
testingcase directly.