From 81e326829b09906445d26c7fab8560d15e42e68d Mon Sep 17 00:00:00 2001 From: null Date: Mon, 17 Aug 2026 23:50:21 -0500 Subject: [PATCH] fix(tools): doc-triggers matched the glob but not the trigger's verb DOC_TRUST_MAP.md declares `Governs: docs/**`, the broadest glob in the tree, while its Review trigger is one of the narrowest -- any doc added, deleted or moved. Matching on the glob alone fired it on every edit to every document, forever, and correctly by the only rule the tool had. Touching one script fired three documents and exactly one of them applied. A prompt that always fires is one people stop reading, and it takes the true positives with it. This tool exits 0 by design -- it is a prompt, not a gate -- which makes it more vulnerable to that, not less, because nothing forces the reading. Documents now declare the kinds of change their trigger names, in an optional `Fires on:` header field, read against git's own status letter. Absent, empty or unparseable means every kind, so nothing changes for the six other path-governing documents and a document is only ever quietened by somebody writing the line deliberately. ## Why declared rather than read out of the trigger prose The obvious first cut is to look for added/deleted/moved with no changed/change to. Tried against the seven path-governing documents here, it misclassifies the one it exists to fix: DOC_TRUST_MAP.md's trigger ends "any change to which doc owns a subject", so it reads as a change-verb. That clause is about which document owns a subject, not about a file being edited, and nothing lexical separates it from architecture/README.md's "any change to a module boundary or a data shape", which genuinely does mean modification. Guessing at English is silent in the expensive direction: a document wrongly read as existence-only stops being prompted for and goes quietly stale, which is the failure this whole tool exists to prevent. So the narrowing is declared or it does not happen. ## Also changed_paths now carries a status letter per path, from --name-status for --staged and --range and from the porcelain columns for the working tree. Paths named on the command line have no diff to read, so the kind is inferred: absent from disk is a deletion, present but untracked is an addition, otherwise a modification. Documents that govern a path in the change but do not fire on its kind are named in their own short block rather than dropped, because a reader who saw nothing would have to guess whether they had been considered. The no-match message now distinguishes "nothing governs these paths" from "governed, but not this kind of change" -- the second is a declaration somebody wrote, not an unclaimed area. Verified: modifying a script fires TOOLS.md and architecture/README.md and not DOC_TRUST_MAP.md; adding, deleting and moving a document under docs/ each still fire it; modifying a document fires nothing; an unknown word warns and fires on everything; an empty or absent field fires on everything. closes #20 Co-Authored-By: Claude Opus 5 (1M context) --- docs/DOC_TRUST_MAP.md | 23 ++++ docs/architecture/README.md | 2 +- docs/architecture/scripts/doc-triggers.py | 161 ++++++++++++++++++++-- 3 files changed, 173 insertions(+), 13 deletions(-) diff --git a/docs/DOC_TRUST_MAP.md b/docs/DOC_TRUST_MAP.md index 4bcf952..0770d4f 100644 --- a/docs/DOC_TRUST_MAP.md +++ b/docs/DOC_TRUST_MAP.md @@ -6,6 +6,7 @@ Owner: Last reviewed: Governs: docs/** Review trigger: Any doc added, deleted or moved; any change to which doc owns a subject +Fires on: added, deleted, moved ``` > Write this file **last**, once the rest exists, and describe what is actually @@ -87,6 +88,7 @@ Owner: Last reviewed: Governs: Review trigger: +Fires on: ``` **Exactly those four status words.** They are not a suggestion: a checker reads @@ -106,6 +108,27 @@ the first sixteen lines of the file, and one carrying `Status` without `Review trigger` is reported as incomplete — that combination looks finished and is not. +**Fires on** is optional, and only for the case where `Governs:` is much broader +than the trigger. `Governs:` says *where* a document is authoritative; the +trigger says *which kinds of change* to that place should bring somebody back, +and `scripts/doc-triggers.py` can only read the first. This file is the extreme +of the gap: it governs `docs/**`, the broadest glob here, while its trigger is +one of the narrowest — *any doc added, deleted or moved*. Without the +declaration it fires on every edit to every document forever, and is right to by +the only rule the tool has. A prompt that always fires is one people stop +reading, and it takes the true positives with it. + +The words are `added`, `deleted`, `moved` and `changed`, comma-separated. **Omit +the line unless it is genuinely needed** — absent means fire on every kind, which +is what almost every document wants. It is deliberately declared rather than +inferred from the trigger prose: the obvious heuristic — existence verbs and no +change verb — was tried against the documents here and misread this one, whose +trigger ends "any change to which doc owns a subject". That clause is about which +document owns a subject, not about a file being edited, and nothing mechanical +separates it from a trigger that does mean modification. A wrong guess is silent +in the expensive direction, so an unreadable or missing declaration fires on +everything and only a deliberate line ever quietens a document. + ## Two markers: does this apply, and who earned it This tree is copied into every project, and some of what it carries will not diff --git a/docs/architecture/README.md b/docs/architecture/README.md index e005dc0..9d9ee54 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -52,7 +52,7 @@ can stop you, and where to start in a fresh clone. | `scripts/controls.sh` | which operational controls this project actually has, each row saying **how** it is known: measured, declared, n/a, or unknown. An unknown is never rendered as absent — "I could not tell" and "it is not there" send people to different places. | | `scripts/dev.sh` | bring the local stack up | | `scripts/scaffold.sh` | lay out a new project in this shape | -| `scripts/doc-triggers.py` | which documents a **pending** change fires, read from their `Governs:` headers. The `Review trigger` on each document names the change that should send somebody back to it; this is the check that asks before the commit rather than after | +| `scripts/doc-triggers.py` | which documents a **pending** change fires, read from their `Governs:` headers and narrowed by the optional `Fires on:` — the kinds of change (added, deleted, moved, changed) a document's trigger actually names, so one governing `docs/**` for existence changes alone does not fire on every edit. The `Review trigger` on each document names the change that should send somebody back to it; this is the check that asks before the commit rather than after | | `scripts/prove-guard.sh` | breaks the thing a guard protects, requires the guard to go red, restores the file from a trap. `GUARDS.md` §1 written out as a command, including the count — one failing test reported on six lines is not six failures | | `scripts/commit-mine.sh` | commits only the paths you name, by pathspec, after the secret scan. For a tree something else is also writing: what anyone else has staged is reported and left exactly as it was | | `scripts/doc-claims.sh` | every file a document names must exist, and (`--covers`) every file that exists is named — the second is the one that catches a list missing rows | diff --git a/docs/architecture/scripts/doc-triggers.py b/docs/architecture/scripts/doc-triggers.py index ba4fc2d..459fe9b 100755 --- a/docs/architecture/scripts/doc-triggers.py +++ b/docs/architecture/scripts/doc-triggers.py @@ -27,6 +27,21 @@ Exit status is always 0. This is a prompt, not a gate: a trigger asks a human whether the prose is still true, and a check that failed the build for that would be bumped past rather than read. +## Matching the trigger's verb, not only its glob + +A document is fired when a changed path matches its `Governs:` **and** the kind +of change matches its `Fires on:`. That second field is optional and almost +never needed; it exists for the case where `Governs:` is far broader than the +trigger. `DOC_TRUST_MAP.md` is the extreme — it governs `docs/**` while its +trigger is *any doc added, deleted or moved* — so on the glob alone it fired on +every edit to every document, forever, and correctly by the only rule there was. +A prompt that always fires is one people stop reading, and this one exits 0 by +design, so nothing forces the reading. + +The kinds are `added`, `deleted`, `moved` and `changed`, read from git's own +status letter. Absent, empty or unparseable means every kind, which is the +behaviour every document without the line still has. + ## Two things it deliberately does not do **It does not read `Exempt:` declarations.** Those mark a *required* document as @@ -100,7 +115,9 @@ DOCS = ROOT / "docs" # at the wrap and silently under-reports, which for this tool means quietly # failing to name a document that should have been updated. So fields are # assembled line by line instead. -FIELD_START = re.compile(r"^(Status|Owner|Last reviewed|Governs|Review trigger):\s*(.*)$") +FIELD_START = re.compile( + r"^(Status|Owner|Last reviewed|Governs|Review trigger|Fires on):\s*(.*)$" +) HEADER_LINES = 16 @@ -156,6 +173,65 @@ def matches(path: str, glob: str) -> bool: return fnmatch.fnmatch(path, glob) +# `Governs:` says *where* a document is authoritative; `Fires on:` says which +# kinds of change to that place its `Review trigger` actually names. The two come +# apart badly at the extreme: `DOC_TRUST_MAP.md` governs `docs/**`, the broadest +# glob in the tree, while its trigger is one of the narrowest — *any doc added, +# deleted or moved*. Matching on the glob alone fires it on every edit to every +# document forever, and a prompt that always fires is one people stop reading, +# which takes the true positives with it. +# +# Why a declared field rather than reading the trigger prose. The obvious first +# cut — look for `added`/`deleted`/`moved` and no `changed`/`change to` — was +# tried against the seven path-governing documents here and misclassified the one +# it exists to fix. `DOC_TRUST_MAP.md`'s trigger ends "any change to which doc +# owns a subject", so it reads as a change-verb; the clause is about which +# document owns a subject, not about a file being edited. Nothing lexical +# separates it from `architecture/README.md`'s "any change to a module boundary +# or a data shape", which genuinely does mean modification. Guessing at English +# and getting it wrong here is silent in the expensive direction: the document +# stops being prompted for and goes quietly stale. +# +# So the narrowing is declared or it does not happen. Absent, unparseable, or +# empty means fire on everything, which is the old behaviour — a document is only +# ever quietened by someone writing the line deliberately. +KIND_LETTERS = { + "added": {"A", "C"}, + "deleted": {"D"}, + "moved": {"R"}, + "changed": {"M", "T"}, +} +ALL_KINDS = {letter for letters in KIND_LETTERS.values() for letter in letters} +KIND_OF = {letter: word for word, letters in KIND_LETTERS.items() for letter in letters} + + +def fires_on(header: dict[str, str]) -> tuple[set[str], list[str]]: + """The status letters a document accepts, and any words not understood. + + Returns every letter when nothing is declared or the declaration cannot be + read, so the failure mode of a typo is a document that is prompted for too + often rather than one that is silently dropped. + """ + raw = header.get("Fires on", "").strip() + if not raw: + return ALL_KINDS, [] + + words = [w.strip().lower().rstrip(".") for w in re.split(r"[,;]|\band\b", raw)] + words = [w for w in words if w] + + letters: set[str] = set() + unknown: list[str] = [] + for word in words: + if word in KIND_LETTERS: + letters |= KIND_LETTERS[word] + else: + unknown.append(word) + + if unknown or not letters: + return ALL_KINDS, unknown or ["(empty)"] + return letters, [] + + def _git(*args: str) -> str: result = subprocess.run( ["git", *args], cwd=ROOT, capture_output=True, text=True, check=False @@ -163,28 +239,62 @@ def _git(*args: str) -> str: return result.stdout -def changed_paths(argv: list[str]) -> tuple[list[str], str]: +def _name_status(*args: str) -> list[tuple[str, str]]: + """(letter, path) from a `--name-status` listing. + + A rename arrives as `R100oldnew`, so the path is taken from the + last field: the new name governs, as it did when only names were read. + """ + pairs = [] + for line in _git(*args, "--name-status").splitlines(): + if not line.strip(): + continue + fields = line.split("\t") + if len(fields) < 2 or not fields[0].strip(): + continue + pairs.append((fields[0].strip()[0].upper(), fields[-1].strip())) + return pairs + + +def _status_of_named(path: str) -> str: + """The kind of change a path named on the command line represents. + + There is no diff to read here, so it is inferred: gone from disk is a + deletion, present but untracked is an addition, and anything else is a + modification — the usual reason to ask about a path by name. + """ + if not (ROOT / path).exists(): + return "D" + return "M" if _git("ls-files", "--", path).strip() else "A" + + +def changed_paths(argv: list[str]) -> tuple[list[tuple[str, str]], str]: if argv and argv[0] == "--staged": - return [p for p in _git("diff", "--cached", "--name-only").splitlines() if p], "staged" + return _name_status("diff", "--cached"), "staged" if argv and argv[0] == "--range": if len(argv) < 2: sys.exit("doc-triggers: --range needs a revision range") - return [p for p in _git("diff", "--name-only", argv[1]).splitlines() if p], f"range {argv[1]}" + return _name_status("diff", argv[1]), f"range {argv[1]}" if argv: - return list(argv), "named paths" + return [(_status_of_named(a), a) for a in argv], "named paths" # Untracked files are included on purpose: a brand-new module is the case # most likely to need a document and least likely to be remembered, and it # is invisible to `git diff`. - paths = [] + pairs = [] for line in _git("status", "--porcelain").splitlines(): if not line.strip(): continue + index, worktree = line[0], line[1] path = line[3:].strip() if " -> " in path: # a rename; the new name governs path = path.split(" -> ", 1)[1] - paths.append(path.strip('"')) - return paths, "working tree" + if "?" in (index, worktree): + letter = "A" # untracked: a file that is new + else: + letter = (index if index != " " else worktree).upper() + pairs.append((letter, path.strip('"'))) + return pairs, "working tree" def main() -> int: @@ -199,8 +309,9 @@ def main() -> int: print(f"doc-triggers: {len(paths)} path(s) from the {source}\n") - fired: list[tuple[str, list[str], str]] = [] + fired: list[tuple[str, list[tuple[str, str]], str]] = [] subject_only: list[str] = [] + wrong_kind: list[tuple[str, str]] = [] for doc in sorted(DOCS.rglob("*.md")): rel = str(doc.relative_to(ROOT)) @@ -215,14 +326,28 @@ def main() -> int: subject_only.append(rel) continue - hits = sorted({p for p in paths for g in path_globs if matches(p, g)}) + letters, unknown = fires_on(header) + if unknown: + print( + f"doc-triggers: {rel} declares 'Fires on: " + f"{header.get('Fires on', '')}' — {', '.join(unknown)} not " + f"understood, so it fires on everything.\n" + ) + + matched = {(s, p) for s, p in paths for g in path_globs if matches(p, g)} + hits = sorted({(s, p) for s, p in matched if s in letters}, key=lambda x: x[1]) if hits: fired.append((rel, hits, header.get("Review trigger", "(none stated)"))) + elif matched: + # Governed, and deliberately not prompted for: the paths changed in a + # way this document's trigger does not name. Said out loud, because a + # reader who saw nothing would have to guess whether it was checked. + wrong_kind.append((rel, header.get("Fires on", "").strip())) for rel, hits, trigger in fired: print(f"\033[1m{rel}\033[0m") - for hit in hits[:6]: - print(f" {hit}") + for letter, hit in hits[:6]: + print(f" {KIND_OF.get(letter, letter.lower()):>7} {hit}") if len(hits) > 6: print(f" … and {len(hits) - 6} more") print(f" trigger: {trigger}\n") @@ -231,11 +356,23 @@ def main() -> int: print(f"{len(fired)} document(s) govern something in this change.") print("Read each trigger and decide — the rule is to update them in the") print("SAME commit as the code, not afterwards.") + elif wrong_kind: + # Distinct from matching nothing, and worth separating: a path here *is* + # governed, and the reason nothing fired is a declaration somebody wrote, + # not an area no document claims. + print("Nothing fired. The paths in this change are governed, but only by") + print("documents whose Fires on declaration excludes this kind of change.") else: print("No document's Governs matched these paths. Worth a second look if") print("this change added a module, a migration, or a new boundary — an") print("unmatched path can also mean no document claims that area yet.") + if wrong_kind: + print("\nGovern a path in this change but do not fire on this kind of") + print("change, by their own Fires on declaration:") + for rel, declared in wrong_kind: + print(f" {rel} — fires on {declared}") + if subject_only: print("\nNot checked here — these govern a subject rather than paths, so") print("no change can fire them mechanically. Judge them yourself:")