Skip to content

test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term - #1193

Merged
mforce merged 7 commits into
mainfrom
chore/correct-glossary-complete
Oct 10, 2026
Merged

mforce merged 7 commits into
mainfrom
chore/correct-glossary-complete

Conversation

@mforce

@mforce mforce commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Makes the in-app glossary complete against specs/product/GLOSSARY.md: every term needs an entry or a stated reason.

Part of #1186

What changes

helpGlossary.test.ts used to treat the in-app glossary as a "curated subset". It checked that every in-app entry names a real GLOSSARY.md term, but a new GLOSSARY.md term with no in-app entry passed. Three new tests:

  • Every term is covered. A term is a bold run that opens a paragraph (after a blank line or a heading), or an h3: 116 today. Each needs an in-app entry or a SPEC_ONLY row with a reason. On failure, the message lists the missing terms and says what to add.

  • SPEC_ONLY stays honest. A row whose term left GLOSSARY.md, or that now has an in-app entry, fails. So does an empty reason, or a bare TODO, TBD or FIXME.

  • NOT_YET is debt that only shrinks. 54 terms have no entry today. 8 have a real reason:

    The other 46 are user-facing and simply missing from the app: Flock, Customer, Payment, Steppers, Audit log, Break-glass reset (the Audit screen shows it) and so on. They are NOT_YET rows, and the test pins their count at 46. A count cannot stop a swap, where one row is removed and a new term deferred in its place. The term-by-term comparison with the PR's base therefore lives in the Docs impact CI check (ci: flag user-facing strings and glossary changes that ship without their docs #1194), which already reads both revisions with git. That check refuses a term deferred at head but not at the base.

The term rule and the rows live in web/src/routes/glossaryCoverage.ts, which has no imports, so the Docs impact check can load any revision of it. The test reads both from there; both directions of the check (entry to spec, spec to entry) use one extraction.

Adding the 46 entries in three locales is out of scope here, as with the async-effect sites in #1185.

web/AGENTS.md gets a "Glossary" rule describing the check.

Checks run locally

  • npx vitest run src/routes/helpGlossary.test.ts --maxWorkers=2: 11 passed (also at 9d63c5b1).
  • npm run typecheck and npm run lint pass.

Proof

Run locally at aa0f289a with npx vitest run src/routes/helpGlossary.test.ts --maxWorkers=2. Every mutation was reverted, and git status was clean afterwards.

Before this PR. origin/main's test file, with a brand-new GLOSSARY.md term (**Tray deposit (#9999)** — …) appended and no in-app entry: 8 passed. That is the gap.

1. The state #464 shipped: "Stock write-off" in GLOSSARY.md, no in-app entry.
Mutation: the StockWriteOff line removed from helpGlossary.ts (its catalog keys stay, so the catalog-parity test fails too).
1 file changed, 1 deletion(-)

     × carries every glossary term the en catalog has, and nothing else
     × has an in-app entry, or a SPEC_ONLY row with a reason, for every GLOSSARY.md term
⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯
AssertionError: expected [ 'glossaryAboveListTerm', …(65) ] to deeply equal [ 'glossaryAboveListTerm', …(66) ]
AssertionError: add an in-app glossary entry (helpGlossary.ts + en/es/tl), or a SPEC_ONLY row with a reason: expected [ 'stock write-off' ] to deeply equal []
      Tests  2 failed | 9 passed (11)

2. A brand-new GLOSSARY.md term with no entry.

     × has an in-app entry, or a SPEC_ONLY row with a reason, for every GLOSSARY.md term
⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
AssertionError: add an in-app glossary entry (helpGlossary.ts + en/es/tl), or a SPEC_ONLY row with a reason: expected [ 'tray deposit' ] to deeply equal []
      Tests  1 failed | 10 passed (11)

3. The same new term parked as NOT_YET.

     × only ever loses NOT_YET rows
⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
AssertionError: expected [ …(46) ] to have a length of 45 but got 46  (at aa0f289a; the pin is 46 since d9dcb88e)
      Tests  1 failed | 10 passed (11)

4. The same new term with a real reason.

      Tests  11 passed (11)

5. Reverted.

      Tests  11 passed (11)

#588 is not caught by this check, and could not be. #588 added no term. It added a paragraph to the existing Farm code entry in GLOSSARY.md (the ?farm= prefill and the remembered-codes picker), and the review finding was that the in-app glossaryFarmCodeDef still described only typing the code. That is drift in the content of an entry that already exists. An existence check cannot see it. It needs a diff check: a GLOSSARY.md paragraph changed for a term that has an in-app entry, with that entry's glossary<Key>Def unchanged in en.ts. That fits the CI diff script in #1186's second PR, and I have proposed it there.

Review round 1 fixes

Finding Fix Where
1. The NOT_YET count allowed a swap Term-by-term comparison with the PR's base; a newly deferred term fails, and no body line overrides it #1194 (6f0a26b3); data moved to glossaryCoverage.ts here (d9dcb88e)
2. A bold term right after a heading escaped A heading line also opens a paragraph; one shared glossaryTerms() serves both directions d9dcb88e
3. Break-glass reset and Money were misclassified Break-glass reset is NOT_YET (count 45 -> 46); Money's reason now names only what it covers d9dcb88e
4. A bare TODO counted as a reason Reasons that are empty, TODO, TBD or FIXME (with or without punctuation) fail d9dcb88e

9d63c5b1 updates web/AGENTS.md to say where the membership check lives.

Mutations, with npx vitest run src/routes/helpGlossary.test.ts --maxWorkers=2:

Mutation At aa0f289a At d9dcb88e
## Packaging then **Tray deposit** — … with no blank line between 11 passed 1 failed: completeness
New term with a SPEC_ONLY reason of "TODO" 11 passed 1 failed: the reason check
No mutation 11 passed 11 passed

Finding 1, end to end. Base = this PR's head 9d63c5b1. Head = a throwaway commit-tree commit on top of it that removes "flock": NOT_YET and adds a new GLOSSARY.md term Tray deposit as NOT_YET. The count stays at 46.

$ node .github/scripts/docs-impact.mjs --base 9d63c5b18ec0109c14403ad3fec509be465a2a60 --head e7123a622b66eb96f5749d9ffcfd08f29b0fa474   # as at 63aa4af6
docs-impact: nothing to flag.
exit=0

$ node .github/scripts/docs-impact.mjs --base 9d63c5b18ec0109c14403ad3fec509be465a2a60 --head e7123a622b66eb96f5749d9ffcfd08f29b0fa474   # at 6f0a26b3
"tray deposit" is newly deferred as NOT_YET in web/src/routes/glossaryCoverage.ts. Give it an in-app glossary entry, or a SPEC_ONLY row whose reason says why users never meet it.
exit=1

From #1194's round 1 (68c7f06f). A wrapped line that starts with an issue number, such as GLOSSARY.md's #729); Managers and below…, was treated as a heading. A bold word at the start of the next line then counted as a new term. Only ^#{1,6}(\s|$) is a heading now. Mutation: rewrap so the line after #729); starts with **Managers**. At 9d63c5b1: 1 failed, expected [ 'managers' ] to deeply equal []. At 68c7f06f: 11 passed.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 (Codex gpt-6-astra, head aa0f289a): 4 guard or doc findings, no product defect

  1. P2: the NOT_YET count allows swapping terms. Completing Flock and deferring a brand-new term keeps the count at 45, and all 11 tests pass. A count cannot enforce membership. Compare NOT_YET term names against the PR base, the way web/lint/check-suppressions.mjs does, and reject newly deferred terms. Until then, the PR's "can only shrink" claim is wrong.
  2. P2: a bold term right after a heading, with no blank line, escapes the completeness check. Recognise that boundary, or fail with an explicit formatting error. Keep term extraction shared between the two directions of the check.
  3. P2: one SPEC_ONLY reason is wrong. "Break-glass reset" appears on the Audit screen, so it is not operator-only. Move it to NOT_YET, or add its in-app entry. "Money" also appears in Reports and Help, so check that row too.
  4. P3: placeholder reasons pass. Reject a bare TODO/TBD/FIXME after trimming. Do not use a length heuristic.

Caught correctly: renamed terms, stale rows, empty reasons and punctuation.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

#1193 round 1: four test/doc-only findings (the NOT_YET count allows a swap; a term right after a heading escapes; break-glass reset and Money misclassified; a bare TODO counts as a reason).

All four are fixed. #1193 head: 9d63c5b1. #1194 head: 6f0a26b3.

  1. Swap. The rows moved into web/src/routes/glossaryCoverage.ts (d9dcb88e). It has no imports, so any revision of it loads directly. The Docs impact check in ci: flag user-facing strings and glossary changes that ship without their docs #1194 (6f0a26b3) compares the NOT_YET terms with the PR's base and refuses a newly deferred one, with no body-line override. It lives there because that check already reads both revisions with git on every PR, including body edits. A vitest test cannot see the base. End to end: base 9d63c5b1, head = a swap of flock for a new tray deposit with the count unchanged. The check exits 0 as at 63aa4af6 and 1 at 6f0a26b3. The PR's "only shrinks" claim and web/AGENTS.md now say where the check lives.
  2. Term after a heading (d9dcb88e). A heading line also opens a paragraph. One glossaryTerms() now serves both directions of the test. The same boundary is applied to the Docs impact term splitter in ci: flag user-facing strings and glossary changes that ship without their docs #1194. ## Packaging then **Tray deposit** — … passes 11/11 at aa0f289a and fails completeness at d9dcb88e.
  3. Classification (d9dcb88e). Break-glass reset is NOT_YET (pinned count 45 -> 46). Money's reason now says the entry is about storage, and that Help's export section covers what users meet.
  4. Placeholder reasons (d9dcb88e). An empty reason, or a bare TODO, TBD or FIXME, fails. "tray deposit": "TODO" passes 11/11 at aa0f289a and fails the reason check at d9dcb88e.

Also in #1194 (6f0a26b3): an unreadable --base/--head now exits 2, and a file missing at a revision is told apart from a git error. This is the same class as #1190's finding 4.

@mforce

mforce commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

Review round 2 (Codex gpt-6-astra, head 68c7f06f): no findings in #1193.

Every round 1 repro is closed: the NOT_YET swap (now ratcheted by #1194's docs-impact script), a term directly after a heading, the Break-glass reset and Money classification, and placeholder reasons. All 11 glossary tests pass. The two PRs stay correct whichever merges first. The one coupling gap found (renaming the module) belongs to #1194 and is being fixed there.

The review loop for #1193 stops here.

@mforce
mforce merged commit a11c18a into main Oct 10, 2026
21 checks passed
@mforce
mforce deleted the chore/correct-glossary-complete branch October 10, 2026 23:35
mforce added a commit that referenced this pull request Oct 10, 2026
Folds #806's OAuth audience and MCP tool-gate rules into src/AGENTS.md, and
ports the rules #1188, #1190, #1193 and #1199 added to the instruction files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant