Make a history record required, once it is one record per change (#325)

.github/workflows/curation-history.yaml

@@ -1,14 +1,27 @@
 # Conventions for this directory: docs/WORKFLOW_CONVENTIONS.md
 name: Curation history
 
-# Two-part check, deliberately asymmetric (see history/README.md):
+# Two-part check, both BLOCKING since #325 (see history/README.md):
 #
-#   * VALIDITY is blocking. A malformed history record fails like any other
-#     validation error.
-#   * PRESENCE is advisory. If a trait record changes without a matching history
-#     record, this warns in the job summary and passes. A hard gate on provenance
-#     blocks legitimate work at inconvenient moments and trains people to route
-#     around it.
+#   * VALIDITY. A malformed history record fails like any other validation error,
+#     including one still carrying the `--details` TODO placeholder -- so
+#     scaffolding an empty record to satisfy the presence gate does not work.
+#   * PRESENCE. A PR that changes any .yaml under data/traits/ and adds no new
+#     history record fails. Spelled that way deliberately: the pathspec is
+#     `data/traits/*.yaml`, because a git `*` already crosses `/` and the `**/`
+#     form would MISS a trait added at the top level (#357 review).
+#
+# Presence was advisory until #325, on the reasoning that a hard gate trains
+# people to route around it. The measurement disagreed: of 134 commits that
+# modified trait records, 2 added a history record. Nobody routed around the gate
+# because there was no gate. What DID happen is that 275 trait records grew an
+# issue number hand-typed into a `changes` string -- the same provenance, in a
+# form nothing can query.
+#
+# The gate is only reasonable because #325 also fixed the granularity: ONE record
+# per change, not one per changed file. Under the old reading a 128-file migration
+# owed 128 near-identical records, and blocking on that would have been a fair
+# thing to route around.
 #
 # Validation uses the VENDORED schema at src/traitmech/schema/history.yaml, so
 # this job has no dependency on the private culturebotai-claw repo. Only the
@@ -47,6 +60,8 @@ jobs:
         with:
           fetch-depth: 0   # need the merge base to diff against
 
+      - uses: extractions/setup-just@53165ef7e734c5c07cb06b3c8e7b647c5aa16db3 # v4.0.0
+
       - name: Install uv
         uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0
         with:
@@ -69,31 +84,32 @@ jobs:
                 --schema src/traitmech/schema/history.yaml \
                 --target-class HistoryRecord
 
-      - name: Check for missing history records (advisory)
+      - name: Check for missing history records (blocking)
         if: github.event_name == 'pull_request'
         run: |
           set -euo pipefail
           git fetch --no-tags origin "${{ github.base_ref }}"
-          base="origin/${{ github.base_ref }}"
 
-          changed_traits=$(git diff --name-only "$base"...HEAD -- 'data/traits/**/*.yaml' | wc -l | tr -d ' ')
-          new_history=$(git diff --name-only --diff-filter=A "$base"...HEAD -- 'history/**/*.yaml' | wc -l | tr -d ' ')
+          # The RULE lives in scripts/audit_history_records.py, not here. Shell
+          # embedded in workflow YAML cannot be unit-tested and cannot be run
+          # locally before pushing, which is how this repo has repeatedly ended up
+          # with a gate nobody could exercise. The script takes --changed/--added
+          # so its rule is testable with no repo, network or PR at all.
+          #
+          # `|| status=$?` rather than a pipeline: under `set -euo pipefail` a
+          # failing `just ... | tee` aborts the step immediately, so the summary
+          # would never be written -- losing the remediation text on exactly the
+          # runs that need it.
+          status=0
+          just audit-history-records --base "origin/${{ github.base_ref }}" \
+            > out.txt 2>&1 || status=$?
+          cat out.txt
 
           {
             echo "## Curation history"
             echo
-            echo "- trait records changed: **$changed_traits**"
-            echo "- history records added: **$new_history**"
-            echo
-            if [ "$changed_traits" -gt 0 ] && [ "$new_history" -eq 0 ]; then
-              echo "> [!WARNING]"
-              echo "> This PR changes trait records but adds no history record."
-              echo "> Scaffold one with \`just new-history\` — see \`history/README.md\`."
-              echo ">"
-              echo "> Advisory only; this does not block the build."
-            elif [ "$changed_traits" -gt 0 ]; then
-              echo "Provenance recorded for this change."
-            else
-              echo "No trait records changed; nothing to record."
-            fi
+            echo '```'
+            cat out.txt
+            echo '```'
           } >> "$GITHUB_STEP_SUMMARY"
+          exit "$status"

history/README.md

@@ -1,8 +1,9 @@
 # Curation history
 
-Append-only provenance for curation sessions. One record per session per target,
-written once and **never edited afterwards**. Corrections go in a new record that
-references the old one in its `details`.
+Append-only provenance for curation sessions. **One record per change** — per
+target for hand curation, per *migration* for a bulk edit (see "One record per
+CHANGE" below). Written once and **never edited afterwards**; corrections go in a
+new record that references the old one in its `details`.
 
 ```
 history/<kind-dir>/<slug>/<TIMESTAMP>-<actor>-<shortid>.yaml
@@ -73,15 +74,74 @@ wall was so the next session does not rediscover it.
 `kind`: `record` · `schema` · `mapping` · `report` · `infrastructure` · `other`
 (`other` requires an explicit `--path`).
 
-## How strictly this is enforced
+## One record per CHANGE, not per file
+
+"One record per session per target" is the rule for **hand curation**, where the
+session and the target coincide: someone reasons about one trait and writes down
+what they concluded. The three records under `records/` are exactly that, and the
+`sulfur_globule` one is what a good record looks like.
+
+A **bulk change is a different animal** and the same rule read literally gives the
+wrong answer. #302 touched 128 trait records mechanically; #334 touched 15. Writing
+one record per file would produce 128 near-identical stubs and bury the handful of
+substantive records this directory exists for — destroying the signal in the name
+of provenance.
+
+So for a change that edits many records under one decision, write **one** record:
 
-Deliberately split:
+| the change is | `kind` | `path` |
+|---|---|---|
+| one trait, curated | `record` | that trait's YAML |
+| a migration driven by a script | `infrastructure` | the migration script |
+| a bulk change with no single script | `other` | the file that best explains it |
+
+Name the scope in `events[].details` — how many records, which issue, and what the
+selection rule was. The migration script is usually the honest target: it *is* the
+artifact that says what drove the change, and it is reviewable in a way that 128
+copies of the same sentence are not.
+
+The per-file `curation_history:` block still records what changed in each file.
+The two are not redundant: that block has no slot for the model, the tool, or the
+issue, and — because it hangs off an edit — **it cannot record a session that
+changed nothing.** An `AUDIT` that checked a trait and correctly found nothing
+wrong is invisible without a record here. That is what `outcome: no_change` is for.
+
+## How strictly this is enforced
 
-- **Presence is advisory.** CI warns when a trait record changes without a
-  matching history record. It does not block. A hard gate on provenance blocks
-  legitimate work at inconvenient moments and trains people to route around it.
-- **Validity is not.** If you write a record it must be schema-valid, and
-  `just validate-history` fails like any other validation error.
+- **Presence blocks** (#325). A PR that changes any `.yaml` under `data/traits/`
+  and adds no new history record fails CI.
+
+  This was advisory until #325, on the reasoning that a hard gate "trains people to
+  route around it". The measurement disagreed: of **134 commits** that modified
+  trait records, **2** added a history record — 1.5%. Nobody routed around the
+  gate, because there was no gate; the convention simply did not happen. Meanwhile
+  **275 trait records** carry an issue number hand-typed into a `changes` string,
+  which is the same information in a form nothing can query. An unenforced
+  convention here drifts exactly as #182, #184 and #215 drifted.
+
+  The cost is now one file per PR rather than one per changed record, which is what
+  makes the gate reasonable to impose at all — the granularity fix above had to come
+  first.
+
+- **Validity blocks too.** If you write a record it must be schema-valid, and
+  `just validate-history` fails like any other validation error. It also fails while
+  the `--details` TODO placeholder is unfilled, so scaffolding an empty record to
+  satisfy the presence gate does not work.
+
+## The vendored schema still states the old policy
+
+`src/traitmech/schema/history.yaml` describes presence as *advisory* and states
+"one record per session per target" unqualified. Both are superseded by #325 and
+**neither is edited here on purpose**: that file is vendored byte-identical from
+claw, which is private and unreachable from this repo's CI, so a one-copy edit
+would create drift that nothing detects — `src/traitmech/schema/history.yaml` is
+NOT in `scripts/check_vendored_sync.sh`'s checked set, which is the gap #209
+tracks. The canonical copy has to change in claw first and be re-vendored;
+tracked in #358.
+
+Until then this README and the `curation-history` workflow are the operative
+statements of the policy, and the schema's prose is stale by design rather than
+by neglect.
 
 ## Where the schema lives
 

justfile

@@ -167,6 +167,26 @@ audit-predicate-domains *args:
 audit-pr-checks *args:
     uv run python scripts/audit_pr_checks_present.py {{args}}
 
+# Fail a PR that changes trait records and records no provenance (#325).
+#
+# history/README.md describes a per-session record as the thing that captures
+# WHICH MODEL, USING WHICH TOOL, changed what, why, and under which issue. The
+# per-file `curation_history:` block has no slot for any of those, and because it
+# hangs off an edit it cannot record a session that changed NOTHING -- an AUDIT
+# that checked a trait and correctly found nothing wrong is invisible without a
+# record here.
+#
+# Presence was advisory until #325. Of the 134 commits that modified trait
+# records, 2 added a history record; meanwhile 275 records grew an issue number
+# hand-typed into a `changes` string, which is the same provenance in a form
+# nothing can query.
+#
+# ONE record per CHANGE, not one per changed file -- that granularity fix is what
+# makes this reasonable to block on. Needs a base ref to diff against, so it is
+# NOT in `qc`; it runs from curation-history.yaml on pull_request.
+audit-history-records *args:
+    uv run python scripts/audit_history_records.py {{args}}
+
 # The stronger companion to audit-pr-checks: not "did ANY check fire" but "did
 # every check that SHOULD have fired, fire" (#348).
 #
@@ -322,8 +342,10 @@ knowledge-gap-scan *args: (_require-claw "kg_microbe_kgscan")
 
 # ============== Curation history (append-only provenance) ==============
 # Records which model, using which tool, changed what, why, and under which
-# issue. One file per session per target under history/; never edited after
-# write. See history/README.md. Schema + scaffolder live in claw.
+# issue. ONE record per change under history/ -- per target for hand curation,
+# per migration for a bulk edit (#325) -- never edited after write. Required:
+# a PR that changes data/traits and adds no record fails CI. See
+# history/README.md. Schema + scaffolder live in claw.
 
 # Scaffold a history record. Prints the path as its last stdout line.
 #   just new-history --kind record --slug cellulolysis \

scripts/audit_history_records.py

@@ -0,0 +1,136 @@
+#!/usr/bin/env python3
+"""Fail a PR that changes trait records and records no provenance (#325).
+
+`history/README.md` describes a per-session record as the thing that captures
+*which model, using which tool, changed what, why, and under which issue*. The
+per-file ``curation_history:`` block captures what changed; it has no slot for
+the model, the tool, or the issue, and because it hangs off an edit it cannot
+record a session that changed **nothing** -- an ``AUDIT`` that checked a trait
+and correctly found nothing wrong is invisible without a record here.
+
+PRESENCE WAS ADVISORY UNTIL #325, on the reasoning that a hard gate "trains
+people to route around it". The measurement disagreed. Of the 134 commits that
+modified `data/traits/*.yaml`, **2** added a history record -- 1.5%. Nobody
+routed around the gate, because there was no gate; the convention simply did not
+happen. What did happen is that **275** trait records grew an issue number
+hand-typed into a `changes` string, which is the same provenance in a form
+nothing can query.
+
+THE GATE IS ONLY REASONABLE BECAUSE THE GRANULARITY WAS FIXED FIRST. Read
+literally, "one record per session per target" makes a 128-file migration owe 128
+near-identical records, which would bury the three substantive hand-written
+records the directory exists for -- destroying the signal in the name of
+provenance. Blocking on that would have been a fair thing to route around. One
+record per CHANGE costs one file per PR, and that is what this enforces.
+
+WHAT THIS DELIBERATELY DOES NOT CHECK. That the record is *about* the change. A
+record added for an unrelated reason satisfies it. Checking the correspondence
+would mean parsing intent, and the cheap proxies (does `target.path` name a
+changed file?) are wrong for the migration case, where the honest target is the
+script rather than any of the records it edited. The only guard against a
+contentless record is `validate-history`, whose schema pattern rejects the
+literal `TODO: replace this placeholder` prefix and nothing else -- ``--details
+'see PR'`` passes. That is the design, not an oversight: this gate asks whether
+provenance was recorded, not whether it was recorded well. Do not rely on it to
+judge substance.
+
+The rule is kept out of the workflow YAML so it is testable without a repo, a
+network, or a PR; ``main`` shells out to git and hands the file lists in.
+
+Usage:
+    just audit-history-records --base origin/main
+    python scripts/audit_history_records.py --changed a.yaml --added b.yaml
+"""
+from __future__ import annotations
+
+import argparse
+import subprocess
+import sys
+
+# NO `**/`. A git pathspec is not a shell glob: git's `*` already crosses `/`,
+# so `data/traits/**/*.yaml` still has to consume the literal slash in `**/` and
+# therefore requires AT LEAST ONE intervening directory. It misses a trait added
+# directly under data/traits/. The workflow's own trigger is
+# `paths: data/traits/**`, which is GitHub Actions semantics and DOES match that
+# file -- so the job would start and then clear the gate reporting "0 trait
+# records changed" (#357 review). `data/traits/*.yaml` is strictly more
+# inclusive and matches the same 477 files today.
+TRAIT_GLOB = "data/traits/*.yaml"
+HISTORY_GLOB = "history/*.yaml"
+
+
+def missing_record(changed_traits: list[str], added_history: list[str]) -> bool:
+    """True when trait records changed and no history record was added.
+
+    ``added_history`` must be files ADDED, not merely modified: editing an
+    existing record is explicitly not how corrections work here -- the README
+    says records are written once and never edited, and a correction goes in a
+    NEW record that references the old one. Counting modifications would accept
+    exactly the thing the append-only design forbids.
+    """
+    return bool(changed_traits) and not added_history
+
+
+def _git(args: list[str]) -> list[str]:
+    proc = subprocess.run(["git", *args], capture_output=True, text=True)
+    if proc.returncode != 0:
+        print(f"git {' '.join(args)} failed: {proc.stderr.strip()}", file=sys.stderr)
+        raise SystemExit(2)
+    return [ln for ln in proc.stdout.splitlines() if ln.strip()]
+
+
+def collect(base: str) -> tuple[list[str], list[str]]:
+    """(changed trait records, added history records) for base...HEAD."""
+    changed = _git(["diff", "--name-only", f"{base}...HEAD", "--", TRAIT_GLOB])
+    added = _git(["diff", "--name-only", "--diff-filter=A", f"{base}...HEAD",
+                  "--", HISTORY_GLOB])
+    return changed, added
+
+
+def main() -> int:
+    ap = argparse.ArgumentParser(description=__doc__)
+    ap.add_argument("--base", help="base ref to diff against, e.g. origin/main")
+    ap.add_argument("--changed", nargs="*", default=None,
+                    help="changed trait records, for testing offline")
+    ap.add_argument("--added", nargs="*", default=None,
+                    help="added history records, for testing offline")
+    args = ap.parse_args()
+
+    if args.changed is not None or args.added is not None:
+        changed, added = list(args.changed or []), list(args.added or [])
+    elif args.base:
+        changed, added = collect(args.base)
+    else:
+        ap.error("pass --base, or --changed/--added")
+
+    print("=== curation history ===", file=sys.stderr)
+    print(f"  trait records changed: {len(changed)}", file=sys.stderr)
+    print(f"  history records added: {len(added)}", file=sys.stderr)
+    for f in added:
+        print(f"    + {f}", file=sys.stderr)
+
+    if missing_record(changed, added):
+        print(
+            "\nThis PR changes trait records and adds no history record (#325).\n"
+            "Write ONE record for the whole change, not one per file. For a\n"
+            "migration the honest target is the script that drove it:\n\n"
+            "  just new-history --kind infrastructure \\\n"
+            "      --path scripts/<the migration script>.py \\\n"
+            "      --event EDIT --outcome changed \\\n"
+            "      --summary '<what the change did>' \\\n"
+            "      --model claude-opus-5 --agent-tool claude-code \\\n"
+            "      --issue https://github.com/CultureBotAI/TraitMech/issues/<n> \\\n"
+            "      --details '<how many records, which selection rule, how verified>'\n\n"
+            "See history/README.md. The per-file curation_history: block does not\n"
+            "cover this: it has no slot for the model, the tool, or the issue.",
+            file=sys.stderr)
+        return 1
+    if changed:
+        print("  provenance recorded for this change", file=sys.stderr)
+    else:
+        print("  no trait records changed; nothing to record", file=sys.stderr)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())