[mforce]

Adds a CI check that a PR adding user-facing strings also touches the docs that explain them, and that a changed glossary term keeps its in-app definition in step.

Closes #1186

(The first half of #1186, the glossary completeness test, is #1193.)

What it checks

.github/scripts/docs-impact.mjs --base <ref> [--head <ref>] [--body-file <file>], run from the repository root. Without --head it reads the working tree, so the CI command also works locally: node .github/scripts/docs-impact.mjs --base origin/main --body-file body.md.

1. New strings with no docs change (7564bb1e). The PR adds keys outside help to web/src/i18n/en.ts, and changes neither specs/product/GLOSSARY.md nor any help.* key. The failure names the new keys.

2. Glossary drift (63aa4af6, feat(web): prefill the farm code from ?farm= and remember it (#535) #588's shape). A GLOSSARY.md term's text changes, and that term has an in-app entry, but its help.glossary<Key>Def in en.ts stays the same. When a term has several in-app entries (Daily entry lifecycle has three), every unchanged definition is reported. A term's text runs to the next term or heading, so a paragraph added under it counts. A brand-new term, or a term with no in-app entry, is not drift; test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term #1193 covers those.

3. Newly deferred glossary terms (6f0a26b3, from test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term #1193's review). The check loads web/src/routes/glossaryCoverage.ts (test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term #1193) at base and head and refuses a term that is NOT_YET at head but not at the base. A count cannot tell a removal from a swap. No body line overrides this; a SPEC_ONLY row with a real reason is the way. Until test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term #1193 merges, the file is missing at the base and this check skips. That is the only skip: once the base has the module, a head without it fails, so a rename cannot retire the check.

Either one passes when the PR body has a line Docs-impact: none — <reason> (an en dash, - or -- also work, followed by a space and a reason). The maintainer reads that line at merge, so only a visible line counts. A quoted line (> Docs-impact: …), a line inside a code fence or an HTML comment, and a line with no reason do not.

A --base or --head that does not resolve to a commit exits 2. A file that is missing at a revision is told apart from a git error, so a bad ref cannot skip a check. A bold term straight after a heading opens a section, the same rule as #1193's glossaryCoverage.ts.

en.ts and helpGlossary.ts have no imports, so the script loads them as TypeScript from a temporary copy of each revision. It does not parse them with regular expressions.

Why a separate workflow, not a step in ci.yml

Fixing the PR body has to re-run this check, so it needs the edited pull-request type. That type is added to .github/workflows/docs-impact.yml, a one-job workflow, and ci.yml is unchanged:

• With edited added to ci.yml, every body or title edit would start a full CI run.
• Gating each ci.yml job with if: github.event.action != 'edited' is worse. An edited-event run then skips those jobs on the same head commit, and the skipped results replace the earlier ones. A red test job could read as skipped after a typo fix in the body.

The workflow also has concurrency with cancel-in-progress, so a burst of body edits leaves one run. The PR body reaches the script through env, never interpolated into shell.

AGENTS.md's "Keep documentation in sync" rule now names the workflow and the local command.

Checks run locally

• node --test .github/scripts/docs-impact.test.mjs: 14 passed at 9326ef83.
• node .github/scripts/docs-impact.mjs --base origin/main: nothing to flag on this branch.

Proof

Run locally at 63aa4af6 against real history. The #588 states are throwaway commits made with git commit-tree from a temporary index, so no branch, index or worktree was touched.

1. #588: GLOSSARY.md "Farm code" gains its prefill paragraph; glossaryFarmCodeDef unchanged.
Base = origin/main with #588's paragraph ("On the SPA sign-in page the field is prefilled…") removed from GLOSSARY.md; head = origin/main. The diff is the paragraph #588 added, as it reads today (later PRs extended it).
1 file changed, 13 insertions(+)

$ node .github/scripts/docs-impact.mjs --base e4f115952b2648a2a3fcce0e6cbf4c19138d8bb6 --head 31348001d12898017757e0b6fa055eff946ce94a
specs/product/GLOSSARY.md changed "farm code", but its in-app definition did not: help.glossaryFarmCodeDef (and es/tl).
exit=1

Green once the in-app definition changes with it:

$ node .github/scripts/docs-impact.mjs --base e4f115952b2648a2a3fcce0e6cbf4c19138d8bb6 --head abf3667e7a971848902d18c1205137f23581959c
docs-impact: nothing to flag.
exit=0

2. New strings with neither docs change: #667 (feat(web): expenses filters by a date range…).
Squash commit f13858f6, base = its parent.

$ node .github/scripts/docs-impact.mjs --base f13858f6^ --head f13858f6
New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.
  Update specs/product/GLOSSARY.md and the in-app Help (en/es/tl `help.*`), or add `Docs-impact: none — <reason>` to the PR body.
exit=1

Green with the escape line in the body:

$ node .github/scripts/docs-impact.mjs --base f13858f6^ --head f13858f6 --body-file /tmp/body1194.md

**Before the drift commit.** The script as at `7564bb1e` (new-strings check only), run on scenario 1's base and head, prints `docs-impact: nothing to flag.` and exits 0. So `63aa4af6` is what makes #588 red.

**#697 is not red, and should not be under this rule.** The research report says #697 changed `en.ts` with neither GLOSSARY.md nor Help. At `aad41633` it did change Help: it added `help.expensesClearingFilters` and `help.auditClearingFilters`. CodeRabbit's finding there was that **GLOSSARY.md** was not updated. Catching that needs a stricter rule: new strings require a GLOSSARY.md change **and** a Help change. I measured both rules on the last 250 commits to `main` that touch `en.ts`. 86 of them add non-help keys:

| Rule | Would have fired on |
|---|---|
| As built: neither GLOSSARY.md nor `help.*` changed | 24 of 86 (28%), e.g. #667, #666, #781, #883 |
| Stricter: GLOSSARY.md required as well | 29 of 86 (34%), adding #697's shape plus #463, #465, #899, #927 |

The stricter rule is a one-line change in `newStringFindings`. I left it out because it is not what #1186 specifies, and it makes the escape line routine for UI-copy PRs.

**4. Newly deferred term (`6f0a26b3`).** Base = #1193's head `9d63c5b1`. Head = a throwaway commit on top of it that removes `"flock": NOT_YET` and adds a new GLOSSARY.md term `Tray deposit` as `NOT_YET`, so the count is unchanged.

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

$ node .github/scripts/docs-impact.mjs --base 9d63c5b --head e7123a622b66eb96f5749d9ffcfd08f29b0fa474 # at 6f0a26b
"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

## Review round 1 fixes (`a076cc76`)

| Finding | Fix |
|---|---|
| 1. A waiver inside a code fence or an HTML comment counted | `waiver(body)` drops fenced blocks (backtick or tilde, closed or not) and HTML comments (closed or not) before matching |
| 2. `Docs-impact: none --` or `---` with no reason passed | The separator must be followed by a space or tab, then a non-blank reason |
| 3. `#729);` at the start of a wrapped line read as a heading and cut the Low-stock floor section | Only `^#{1,6}(\s\|$)` is a heading. #1193's `glossaryCoverage.ts` had the same split and is fixed in `68c7f06f` |
| 4. One updated definition excused stale siblings | Every unchanged definition of a changed term is reported |

Also: the checkout in `docs-impact.yml` sets `persist-credentials: false`, because the check imports catalog files from the PR.

Each case run on real history, with the script as at `6f0a26b3` and as now. The two GLOSSARY.md cases are throwaway `commit-tree` commits on `origin/main`.

| Case | Exit as at `6f0a26b3` | Exit now | Output now |
|---|---|---|---|
| #667, waiver inside a code fence | 0 | 1 | `New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.` |
| #667, waiver inside an HTML comment | 0 | 1 | `New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.` |
| #667, `Docs-impact: none --` with no reason | 0 | 1 | `New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.` |
| #667, `Docs-impact: none ---` with no reason | 0 | 1 | `New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.` |
| #667, a visible waiver with a reason (control) | 0 | 0 | `docs-impact: flagged, and the PR body says why not:` |
| Low-stock floor: `Available < floor` -> `<= floor` below the wrapped `#729);` line, definition unchanged | 0 | 1 | `specs/product/GLOSSARY.md changed "low-stock floor", but these in-app definitions did not: help.glossaryLowStockFloorDef (and es/tl).` |
| Daily entry lifecycle changed; only `glossaryLockedEntryDef` updated | 0 | 1 | `specs/product/GLOSSARY.md changed "daily entry lifecycle", but these in-app definitions did not: help.glossaryAdjustEntryDef, help.glossaryVoidEntryDef (and es/tl).` |

**Merge enforcement is a maintainer setting.** The review found that `Docs impact` is not among `main`'s required checks, so today a red result does not block a merge. Making it a gate means adding the `Docs impact` check to the `main` ruleset (and the release ruleset, if wanted).

## Review round 2 fixes (`9326ef83`)

| Finding | Fix |
|---|---|
| 1. Renaming `glossaryCoverage.ts` switched off the `NOT_YET` ratchet for good | A base that has the module and a head that does not now fails, telling the PR to update `COVERAGE` in `docs-impact.mjs`. Only a base without the module (the bootstrap) skips |
| 2. A fence line with an info string closed the code block early | Fences are tracked line by line. A closing fence must use the opening character, be at least as long, and have only spaces or tabs after it (CommonMark). A backtick fence whose info string contains a backtick opens nothing |

Run on real history, with the script as at `a076cc76` and as now:

| Case | Exit as at `a076cc76` | Exit now | First line now |
|---|---|---|---|
| #667, waiver inside a `markdown` fence that shows a nested `text` fence line (same length) | 0 | 1 | `New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.` |
| #667, the same with `~~~markdown` / `~~~text` | 0 | 1 | `New user-facing strings with no Help or glossary change: expenses.fromLabel, expenses.toLabel, expenses.periodTotalLabel, expenses.noExpensesMatch.` |
| #667, a visible waiver after a closed fence (control) | 0 | 0 | `docs-impact: flagged, and the PR body says why not:` |
| #1193's head, then the module renamed and `flock` swapped for a new `tray deposit` NOT_YET row | 0 | 1 | `web/src/routes/glossaryCoverage.ts exists at the base but not at head. If it moved, update COVERAGE in .github/scripts/docs-impact.mjs in the same PR.` |
| #1193's head against `origin/main` (bootstrap: the base has no module) | 0 | 0 | `docs-impact: nothing to flag.` |

## Known limits (not fixed, by the maintainer's decision)

- **A literal `<!--` inside code hides a valid waiver.** Comments are stripped before code context is read. So a body that shows `<!--` in inline code or a fenced HTML example, followed by a visible waiver, still fails. This fails safe: the waiver is ignored, and the PR has to move the example or the line.
- **A waiver inside a multi-line inline-code span counts.** A backtick, a line break, the waiver line at column 0, another line break and a closing backtick render as one code span, but the scanner reads the middle line. This needs a deliberately unusual wrap. A single-line inline-code example is rejected.

## Note: the term rule exists twice until #1193 merges

`termSections` here and `glossaryTerms` in #1193's `web/src/routes/glossaryCoverage.ts` use the same rule (a bold run after a blank line or a real heading, or an h3). #1193 is not on `main` yet, so this script cannot import it. Once both are merged, `docs-impact.mjs` should import the rule from `glossaryCoverage.ts` and drop its copy.

[mforce]

Review round 1 (Codex gpt-6-astra, head 63aa4af6): 4 realistic findings

1. P2: an escape line inside a code fence or HTML comment counts. Against SPA: Expenses filters by a single month, where every sibling screen filters by a date range #667, both a fenced and a hidden Docs-impact: none — … waive the check. A hidden comment also defeats "the maintainer can read the reason at merge". Fix: strip fenced code and HTML comments before matching, and test whole bodies.
2. P2: Docs-impact: none -- (or ---) passes with no reason, because the regex backtracks and consumes a hyphen as the reason. Fix: require whitespace after the whole separator, then a non-blank reason.
3. P2: #729); at the start of a wrapped line in GLOSSARY.md:309 is read as a heading, which truncates the Low-stock floor section. Changing Available < floor passes silently. Fix: match a real Markdown heading (/^#{1,6}(\s|$)/).
4. P2: one updated definition excuses stale siblings. Daily entry lifecycle maps to three entries. Updating only LockedEntryDef exempts a stale VoidEntryDef. Fix: report every unchanged definition of a changed term.

Security: the workflow uses pull_request with contents: read, and the body is passed through env, so there is no injection. Importing the PR's catalog runs PR code with the read-only checkout token persisted. That is acceptable for a public repo; optional hardening is persist-credentials: false.

Maintainer setting: "Docs impact" is not in main's required checks, so a red result does not block a merge today.

[mforce]

#1194 round 1: four realistic gaps (a waiver inside a code fence or an HTML comment counts; none -- with no reason passes; #729); read as a heading; one updated definition excuses stale siblings), plus optional persist-credentials: false.

All four are fixed in a076cc76. #1194 head: a076cc76. Each case was run on real history with the script as at 6f0a26b3 and as now:

Case	Before	Now
#667 with the waiver in a code fence	0	1
#667 with the waiver in an HTML comment	0	1
#667 with Docs-impact: none -- / --- and no reason	0 / 0	1 / 1
#667 with a visible waiver and a reason (control)	0	0
Low-stock floor: Available < floor -> <= floor below the #729); line	0	1, names help.glossaryLowStockFloorDef
Daily entry lifecycle changed, only glossaryLockedEntryDef updated	0	1, names help.glossaryAdjustEntryDef and help.glossaryVoidEntryDef

1. waiver(body) drops fenced blocks (backtick or tilde, closed or not) and HTML comments (closed or not), then looks for the line. The self-tests use whole bodies.
2. The separator must be followed by a space or tab, then a non-blank reason. none -- reason still passes.
3. Only ^#{1,6}(\s|$) counts as a heading. glossaryCoverage.ts in test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term #1193 had the same split: a line starting with a bold word right after #729); was read as a new term. Fixed there in 68c7f06f, with the same before/after check (1 failure naming managers, then 11/11).
4. Every unchanged definition of a changed term is reported.

docs-impact.yml's checkout now sets persist-credentials: false.

Self-tests: 13/13. The required-check gap you noted (Docs impact is not among main's required checks) is in the PR body as a maintainer setting.

[mforce]

Review round 2 (Codex gpt-6-astra, #1193 68c7f06f + #1194 a076cc76): 3 realistic findings in #1194, 1 contrived; #1193 clean

Every round 1 repro on both PRs is closed when both PRs are in place.

1. P2: renaming glossaryCoverage.ts switches the NOT_YET ratchet off for good. Update the test import, and all 11 glossary tests pass while Docs impact says "nothing to flag". Fix: when the base has the module, a missing head module fails. Keep only the missing-base bootstrap.
2. P2: a fence line with an info string ends a code block early, so a waiver inside the rest of the block counts. Fix: a closing fence must use the opening character, be at least as long, and carry no info string (CommonMark).
3. P2, fails safe: a literal <!-- inside code hides a real waiver, because comment stripping runs before code detection. This blocks a valid waiver; it does not let one through.
4. Contrived: a multiline inline-code span containing a waiver line counts.

The CI base fetch works with persist-credentials: false, as the CI log confirms. Docs impact is still not a required check on main.

[mforce]

Round 2 (#1193 + #1194): fix 2, note 2, ship, no round 3.

Done. #1194 head: 9326ef83.

1. Rename (9326ef83). A base that has glossaryCoverage.ts and a head that does not now fails, and the message tells the PR to update COVERAGE in docs-impact.mjs. Only a base without the module (the bootstrap) skips. Base 68c7f06f, head = the module renamed plus flock swapped for a new tray deposit NOT_YET row: exit 0 as at a076cc76, 1 now. test(web): require an in-app glossary entry or a reasoned exclusion for every GLOSSARY.md term #1193's head against origin/main (the bootstrap) still exits 0. New self-test: base present, head missing.
2. Fence closing (9326ef83). Fences are tracked line by line under CommonMark's rule: same character, at least as long, only spaces or tabs after it. A backtick fence whose info string contains a backtick opens nothing. Against SPA: Expenses filters by a single month, where every sibling screen filters by a date range #667, the nested markdown/text body exits 0 as at a076cc76 and 1 now, with backticks and with tildes. A visible waiver after a closed fence still passes. New self-tests: the info-string case for both characters, a shorter closing fence, the other character, and a longer closing fence with trailing spaces.

3 and 4 are listed in the body under "Known limits". The term-rule dedup is a note in the body, because #1193 is not on main yet. Self-tests: 14/14.

[mforce]

Round 2 fixes verified at 9326ef83 (coordinator check; no round 3, as the maintainer decided)

• The delta is docs-impact.mjs (+20/−5) plus 14 self-test lines. node --test .github/scripts/docs-impact.test.mjs passes 14/14.
• Rename guard: when the base has glossaryCoverage.ts and the head does not, the check now fails, naming COVERAGE in the script. Only a base without the module skips.
• Run against real SPA: Expenses filters by a single month, where every sibling screen filters by a date range #667:
  • a waiver inside a fenced block → exit 1;
  • a visible waiver with a reason → exit 0;
  • Docs-impact: none -- → exit 1.
• Findings 3 (a literal <!-- in code hides a valid waiver, which fails safe) and 4 (a multiline inline-code span, contrived) are listed in the body as known limits.

The review loop stops here.