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:")