diff --git a/.claude/commands/add-canon.md b/.claude/commands/add-canon.md index e999687b..a33e2d01 100644 --- a/.claude/commands/add-canon.md +++ b/.claude/commands/add-canon.md @@ -13,7 +13,8 @@ the PR is **merged** (or you have reported a hard blocker). Argument (optional): `$ARGUMENTS` may name a target, e.g. `visa jp`, `banking br`, `emergency pl`, or a domain like `legal`. If empty, pick the gap -yourself in step 2. +yourself in step 3 — but run step 2 either way, so you do not collide with a +cycle already in flight. --- @@ -31,7 +32,27 @@ Prefer a descriptive name once you know the topic, e.g. `canon/visa-jp-work-permit`. Confirm `git config user.email` is `yujinhong3@gmail.com` before committing. -## 2. Pick a real coverage gap (never duplicate) +## 2. Claim a target no open PR is already working on + +`main` is not the whole picture. Other cycles may be running right now with +branches already pushed, and their canons will not show up in any count of +`data/canons/`. Three separate PRs once authored New Zealand coverage at the +same time because each of them looked only at `main`. + +List the open PRs and the files they touch **before** choosing anything: + +```bash +# mcp__github__list_pull_requests (state: open), then for each PR: +# mcp__github__pull_request_read (method: get_files) +``` + +Treat every country appearing in an open PR as taken, and pick a different one. + +Step 6 excludes the canon IDs those branches touch on its own, by reading the +pushed branches. It cannot see a PR from a fork, so note the touched IDs of any +fork PR here and pass them along explicitly. + +## 3. Pick a real coverage gap (never duplicate) Count what already exists, then choose an underserved slice: @@ -73,13 +94,39 @@ Selection rules: - The country code **must already be in** `SUPPORTED_COUNTRIES` (`generator/country_canon_template.py`). If you want a new country, add it there in the same PR. -- Grep before writing so you do not re-author an existing slug: - `rg -l "your-slug" data/canons/` + +Then check for duplicates **by topic, not by slug**. A different slug describing +the same dead end is still a duplicate, and the site is penalised for it. Read +what the country already has before writing: + +```bash +python - <<'PY' +import json, pathlib +CC = "nz" # your target country +for f in sorted(pathlib.Path("data/canons").rglob(f"{CC}.json")): + c = json.loads(f.read_text()) + print(c["id"]) + print(" ", c["error"]["signature"]) + print(" ", c["verdict"]["summary"][:160]) +PY +``` + +For each canon you intend to write, name the existing entry it is closest to and +say in one line why yours is a different dead end. If you cannot, drop it. Real +examples of duplicates that got caught only at merge time: an IRD-number canon +about bank interest when `banking/rwt-non-declaration-rate/nz` already covered +it, and a border-declaration canon under `legal/` when +`food-safety/undeclared-biosecurity-goods/nz` already covered it. + +When the overlap is partial - your canon has one genuinely new angle and the +rest restates an existing entry - do not ship a competing page. Either narrow +yours to the new angle alone and cross-link the two, or fold the new angle into +the existing canon as an extra `dead_ends[]` / `workarounds[]` entry. Pick **3–5 canons** for this cycle. Fewer is fine; more than 5 makes review and sourcing quality slip. -## 3. Research from primary sources +## 4. Research from primary sources For each canon, find real, current, citable sources before writing a word: @@ -96,7 +143,7 @@ For each canon, find real, current, citable sources before writing a word: A good canon captures something an AI would confidently get **wrong**: a non-obvious dead end, not a fact anyone can restate. -## 4. Author the canons +## 5. Author the canons Use the scaffold: @@ -122,7 +169,55 @@ Follow `docs/country-canon-guide.md` and the schema in `CLAUDE.md`. Non-negotiab - `error.regex` must be a valid, ReDoS-safe pattern (no `(a+)+`, no `(a|b)+`) that matches how someone would phrase the problem. -## 5. Validate — must be clean before committing +## 6. Re-verify your assigned bucket of aging canons + +Roughly a thousand canons sit past the 180-day aging threshold, so each cycle +refreshes a few. Do **not** pick "the oldest three" - that rule is deterministic, +every parallel cycle lands on the same files, and whichever PR merges first +leaves the rest conflicting on exactly the date fields they came to update. That +is what stalled PRs #165, #166, #168, and #172. + +Ask for your slice instead. Seed it with your target country code: + +```bash +git fetch origin --prune # so the claim scan sees current branches +python -m generator.reverify --seed +``` + +Each canon sits in a bucket fixed by hashing its ID, and your seed owns one +bucket. Two cycles get the same bucket (identical picks) or share nothing - +a partial overlap, the thing that conflicts on merge, cannot happen. The command +also excludes canons other pushed branches already touch. Fetch first, or that +scan reads a stale view of the remote. + +**Read the `Claim scan:` line it prints.** `INCOMPLETE` means some or all +branches went unchecked and another branch may own your picks - resolve that +before editing anything. Add `--exclude id1,id2` for what the scan cannot see: +a PR from a fork, or canons you will touch later in this same cycle. + +The scan only sees branches already pushed, and you pick before you push. If +another cycle started at the same time, re-run this command just before +committing; if your picks are now claimed, take the new ones instead. + +If your bucket comes back empty or fully claimed, vary the seed (`nz` -> +`nz-2`). Do not raise `--buckets` - that re-shuffles every canon and breaks the +agreement with every other cycle. + +Then actually re-verify, in this order: + +1. Open every `sources[]` URL on the canon and read it. +2. If a URL moved, update it to the new canonical location. If the claim no + longer holds, fix the claim - a wrong canon is worse than a stale one. +3. Only then bump `last_confirmed`, `verdict.last_updated`, and + `metadata.last_verification`. + +Never bump a date you did not earn by re-reading the source. A refreshed date on +an unchecked canon is a silent lie to every agent that reads it. + +If a source contradicts the canon, say so explicitly in the PR body - that +finding is worth more than the new pages. + +## 7. Validate — must be clean before committing ```bash ruff check generator/ tests/ @@ -135,7 +230,7 @@ python -m generator.validate --site-only Fix every failure. Do not commit red. If the build emits new warnings for your files, resolve them too. -## 6. Commit and push +## 8. Commit and push ```bash git add data/ generator/ @@ -145,7 +240,7 @@ git push -u origin On network failure only, retry up to 4 times with backoff (2s, 4s, 8s, 16s). -## 7. Open the PR +## 9. Open the PR Use `mcp__github__create_pull_request` against `main`. Body should state: @@ -161,7 +256,7 @@ End the body with the attribution footer: _Generated by [Claude Code](https://claude.ai/code)_ ``` -## 8. Drive to green, then merge +## 10. Drive to green, then merge 1. Read CI status with `mcp__github__pull_request_read` (`get_status`). 2. If checks are still running, wait with the `Monitor` tool (never a foreground @@ -174,7 +269,7 @@ _Generated by [Claude Code](https://claude.ai/code)_ 5. If CI is red for a reason that also fails on `main` (pre-existing breakage), say so explicitly in the PR and stop — that one is not yours to force. -## 9. Report +## 11. Report Finish with a short summary: canons added (IDs), the PR number and merge state, and the new total canon count. If anything was skipped for lack of sources, say @@ -191,3 +286,7 @@ which topic and why. - Duplicate or near-duplicate pages actively hurt indexing. If the only thing you can produce this cycle is a rewording of an existing canon, produce nothing and report that instead. +- **Never** bump `last_confirmed` on a canon whose sources you did not re-read. +- **Never** re-verify by "oldest first" — always take the bucket + `python -m generator.reverify --seed ` assigns you, after + `git fetch origin --prune`. diff --git a/CLAUDE.md b/CLAUDE.md index c6d391bc..e7d1e6e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,6 +51,8 @@ generator/ lookup.py # Programmatic error lookup SDK (lookup, lookup_all, search, batch_lookup) ping_search_engines.py # Search engine ping on deploy pipeline.py # Unified pipeline: validate → generate → build → test + reverify.py # Assigns each content cycle a disjoint bucket of aging + # canons to re-verify (see 'Re-verifying Aging Canons') schema.py # ErrorCanon JSON Schema (ERRORCANON_SCHEMA) submit_indexnow.py # IndexNow submission on deploy templates/ # Jinja2 HTML templates @@ -101,6 +103,10 @@ ruff check generator/ tests/ # Look up an error (CLI) python -m generator.lookup "error message" +# Get this cycle's re-verification bucket (seed with the target country code) +git fetch origin --prune && python -m generator.reverify --seed nz +python -m generator.reverify --seed nz --exclude id1,id2 # additional claims + # List all errors python -m generator.lookup --list @@ -179,6 +185,55 @@ sourcing standards and confidence calibration. Quick reference: `generator/country_canon_template.py` gates new country codes - add there first before writing canons for a new country. +## Re-verifying Aging Canons + +The validator warns at 180 days since `last_confirmed` and errors at 365, so a +large cohort is permanently due for re-checking. Each content PR refreshes a +few of them. + +**Do not pick "the oldest N".** That rule is deterministic, so every parallel +cycle selects the same files and all but the first PR to merge is left +conflicting on the very date fields it came to update. Ask `generator.reverify` +for a bucket instead: + +```bash +git fetch origin --prune # refresh the claim scan +python -m generator.reverify --seed nz # seed = target country +python -m generator.reverify --seed nz --exclude id1,id2 # additional claims +``` + +Every canon belongs to a bucket determined solely by hashing its **ID**; a seed +hashes to one bucket and the cycle takes the oldest canons in it. Two seeds +therefore own the same bucket (identical picks - obvious immediately) or share +nothing at all. A partial overlap, the shape that conflicts on merge, cannot +occur. + +Bucketing by ID rather than by position in the aging list is the point. Two +cycles never see the same list - each branches from a different `main` - so +positional blocks would shift their boundaries whenever a merge aged canons in +or refreshed them, and two cycles would land on overlapping-but-unequal slices. +A canon's ID does not move. This is why `DEFAULT_BUCKETS` must stay constant: +changing it re-shuffles every canon, which is only safe with no content PR open. + +The command also excludes every canon touched by another pushed branch +(`claimed_canon_ids`) and always says which state it is in - excluded N, clean, +or **incomplete**. A scan that could not read every branch reports as such +rather than as a clean bill of health. That scan is conservative by design: it +does not try to tell a merged branch from a live one, because squash-merging +severs ancestry and leaves no reliable local signal. It also only sees *pushed* +branches, so re-run it before committing rather than trusting one check at the +start. + +Re-verification means re-reading the sources, in this order: + +1. Open every `sources[]` URL and read it. +2. Update moved URLs; fix the claim if it no longer holds. +3. Only then bump `last_confirmed`, `verdict.last_updated`, and + `metadata.last_verification`. + +A refreshed date on a canon nobody re-read is worse than a stale one - it tells +every downstream agent the entry was confirmed when it was not. + ## Security When editing canon JSON files or templates, follow these rules: @@ -261,5 +316,6 @@ Tests are in `tests/` using pytest. Key test files: - `test_build.py` - Site builder unit tests - `test_build_integration.py` - Integration tests for full site build - `test_pipeline.py` - Pipeline tests +- `test_reverify.py` - Re-verification bucket selection (disjointness, determinism) Shared fixtures in `conftest.py`: `valid_canon` (deep copy of a valid canon) and `make_canon` (factory with overrides). diff --git a/generator/reverify.py b/generator/reverify.py new file mode 100644 index 00000000..1c6d4f51 --- /dev/null +++ b/generator/reverify.py @@ -0,0 +1,395 @@ +"""Pick which aging canons a content cycle should re-verify. + +Every content PR refreshes a few of the oldest canons alongside its new pages. +When each session picks "the N canons with the oldest ``last_confirmed``", they +all pick the *same* files, and whichever PR merges first leaves the rest +conflicting on the very date fields they came to update. + +This module hands out disjoint slices instead. Every canon belongs to a bucket +determined solely by hashing its ID; a cycle's ``seed`` hashes to one bucket and +it takes the oldest canons in there. + +Bucketing by ID rather than by position in the aging list is the whole trick. +Two cycles never see the same list - each branches from a different ``main``, +and any merge that ages canons in or refreshes them would shift every later +boundary. Position-based blocks would then overlap *in part* between two cycles, +which is precisely the shape that conflicts on merge. A canon's ID does not move. + +Two seeds therefore either own the same bucket (identical picks - visible at +once, and harmless) or share nothing at all. The CLI additionally excludes what +other pushed branches already touch (see :func:`claimed_canon_ids`), on by +default: a guarantee nobody has to remember beats one that needs the right flag. + +CLI:: + + python -m generator.reverify --seed nz + python -m generator.reverify --seed visa-br --count 3 + python -m generator.reverify --seed nz --exclude docker/foo/bar,rust/baz/qux + python -m generator.reverify --seed nz --no-auto-exclude +""" + +import argparse +import hashlib +import json +import subprocess +import sys +from datetime import date +from pathlib import Path +from typing import NamedTuple + +from generator.validate import AGING_THRESHOLD_DAYS, _canon_age_days + +REPO_ROOT = Path(__file__).parent.parent +DATA_DIR = REPO_ROOT / "data" / "canons" + +# Number of buckets the aging canons are spread over. MUST stay constant: it is +# what makes a canon's bucket independent of corpus state, so two cycles reading +# different `main`s still agree on who owns what. Changing it re-shuffles every +# canon and is only safe when no content PR is open. +# +# 128 buckets over a ~1000-canon aging cohort leaves ~8 per bucket, comfortably +# more than a cycle claims. What matters for collisions is how many cycles run +# *concurrently*, not how many seeds exist: 3 at once collide ~2.3% of the time, +# 4 at once ~4.6%. (Across all ~30 country seeds some pair inevitably shares a +# bucket - that is fine, since those cycles do not run together.) And a +# collision is not a conflict: both cycles get the identical set, visible +# immediately rather than at merge time. +DEFAULT_BUCKETS = 128 +DEFAULT_COUNT = 3 + + +def load_aging_canons( + data_dir: Path | None = None, + reference_date: date | None = None, + threshold_days: int = AGING_THRESHOLD_DAYS, +) -> list[dict]: + """Return aging canons, oldest ``last_confirmed`` first. + + Each entry is ``{"id", "path", "age_days", "last_confirmed"}``. Canons with + a missing or unparseable ``last_confirmed`` are skipped - they are reported + separately by the validator and are not re-verification targets. Ties break + on ``id`` so the ordering is stable across machines and runs. + """ + root = data_dir or DATA_DIR + entries = [] + for f in sorted(root.rglob("*.json")): + try: + data = json.loads(f.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + continue + age = _canon_age_days(data, reference_date) + if age is None or age <= threshold_days: + continue + canon_id = data.get("id") + if not canon_id: + continue + entries.append({ + "id": canon_id, + "path": f, + "age_days": age, + "last_confirmed": data["error"]["last_confirmed"], + }) + entries.sort(key=lambda e: (-e["age_days"], e["id"])) + return entries + + +def _git(*args: str, repo_root: Path | None = None) -> str | None: + """Run a read-only git command. Returns None if git or the ref is unavailable.""" + try: + proc = subprocess.run( + ["git", *args], + cwd=repo_root or REPO_ROOT, + capture_output=True, + text=True, + timeout=30, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + return proc.stdout if proc.returncode == 0 else None + + +def _canon_id_from_path(path: str) -> str | None: + """Derive a canon ID from its path, for blobs that cannot be read. + + Handles both layouts: ``{domain}/{slug}/{env}.json`` and the flat + ``{domain}/{slug}_{env}.json``. + """ + parts = Path(path).with_suffix("").parts + if len(parts) >= 5 and parts[:2] == ("data", "canons"): + return "/".join(parts[2:5]) + if len(parts) == 4 and parts[:2] == ("data", "canons"): + slug_env = parts[3] + if "_" in slug_env: + slug, _, env = slug_env.rpartition("_") + return f"{parts[2]}/{slug}/{env}" + return None + + +class ClaimScan(NamedTuple): + """Result of scanning pushed branches for canons they already touch. + + ``ok`` distinguishes "scanned, found nothing" from "could not scan". They + are the same empty set but very different facts: the first means the + bucket is genuinely free, the second means nothing was checked. Collapsing + would let a shallow clone or a typo'd base ref look exactly like a clean + bill of health. + """ + + ids: frozenset[str] + branches: int + ok: bool + reason: str | None = None + + +def claimed_canon_ids( + base_ref: str = "origin/main", + repo_root: Path | None = None, + skip_branch: str | None = None, +) -> ClaimScan: + """Canon IDs touched by pushed branches other than ``base_ref``. + + Deliberately conservative: it does not try to tell a merged branch from a + live one. The repo squash-merges, which severs ancestry, so ``git + merge-base --is-ancestor`` reports long-merged branches as unmerged and + there is no reliable local signal to replace it. Over-excluding is cheap - + the caller takes the next canon in its bucket - while under-excluding + reintroduces exactly the merge conflicts this module exists to prevent. + + Only sees branches that have been **pushed**. Two cycles that start close + together can both pick before either pushes, so re-run the scan before + committing rather than trusting a single check at the start. + + Never raises: an unusable repo comes back as ``ok=False`` with a reason, so + the caller can degrade to hash-only selection *and say so*. + """ + if not _git("rev-parse", "--git-dir", repo_root=repo_root): + return ClaimScan(frozenset(), 0, False, "not a git repository, or git unavailable") + base_commit = _git("rev-parse", "--verify", "--quiet", f"{base_ref}^{{commit}}", + repo_root=repo_root) + if base_commit is None: + return ClaimScan(frozenset(), 0, False, f"base ref {base_ref!r} not found") + + # Full refnames, not %(refname:short): the short form renders + # refs/remotes/origin/HEAD as bare "origin", which no HEAD test can catch. + refs_out = _git("for-each-ref", "--format=%(refname)", "refs/remotes/", + repo_root=repo_root) + if refs_out is None: + return ClaimScan(frozenset(), 0, False, "could not list remote refs") + + claimed: set[str] = set() + scanned = 0 + unreadable: list[str] = [] + for refname in refs_out.split(): + if refname.endswith("/HEAD"): + continue + ref = refname.removeprefix("refs/remotes/") + if ref == base_ref: + continue + # ref is "/"; compare the branch part. + if skip_branch and ref.partition("/")[2] == skip_branch: + continue + scanned += 1 + paths = _git("diff", "--name-only", f"{base_ref}...{ref}", "--", "data/canons", + repo_root=repo_root) + if paths is None: + # Unrelated histories, a shallow clone with no merge base, a + # corrupt ref. Whatever the cause, this branch went unchecked and + # the scan must not go on to report itself clean. + unreadable.append(ref) + continue + if not paths: + continue + for path in paths.splitlines(): + path = path.strip() + if not path.endswith(".json"): + continue + blob = _git("show", f"{ref}:{path}", repo_root=repo_root) + canon_id = None + if blob: + try: + canon_id = json.loads(blob).get("id") + except json.JSONDecodeError: + canon_id = None + claimed.add(canon_id or _canon_id_from_path(path) or path) + + if unreadable: + # Keep the IDs that were readable - they are still real claims - but + # report the scan as incomplete so the caller does not read it as clean. + listed = ", ".join(sorted(unreadable)[:3]) + more = f" (+{len(unreadable) - 3} more)" if len(unreadable) > 3 else "" + return ClaimScan(frozenset(claimed), scanned, False, + f"could not diff {len(unreadable)} branch(es): {listed}{more}") + return ClaimScan(frozenset(claimed), scanned, True) + + +def current_branch(repo_root: Path | None = None) -> str | None: + """Name of the checked-out branch, or None when detached or git is absent.""" + out = _git("rev-parse", "--abbrev-ref", "HEAD", repo_root=repo_root) + name = (out or "").strip() + return name if name and name != "HEAD" else None + + +def seed_bucket(seed: str, buckets: int = DEFAULT_BUCKETS) -> int: + """Map a cycle's seed to the bucket it owns, stably across processes. + + ``hash()`` is salted per process, so it cannot be used here - two sessions + would disagree about which bucket a seed owns. + """ + digest = hashlib.sha256(seed.encode("utf-8")).hexdigest() + return int(digest, 16) % buckets + + +def canon_bucket(canon_id: str, buckets: int = DEFAULT_BUCKETS) -> int: + """Which bucket a canon belongs to. A pure function of its ID. + + Bucket membership deliberately does not depend on the corpus. Cutting the + aging list into positional blocks would have been simpler, but two cycles + never see the same list - each branches from a different `main` - and any + merge that ages in or refreshes canons shifts every later boundary. Two + cycles would then get blocks that overlap in part, which is exactly the + shape that conflicts on merge. Hashing the ID keeps a canon in the same + bucket no matter what else changed. + """ + digest = hashlib.sha256(canon_id.encode("utf-8")).hexdigest() + return int(digest, 16) % buckets + + +def select_targets( + seed: str, + count: int = DEFAULT_COUNT, + exclude: set[str] | None = None, + buckets: int = DEFAULT_BUCKETS, + data_dir: Path | None = None, + reference_date: date | None = None, +) -> list[dict]: + """Pick up to ``count`` aging canons for the cycle identified by ``seed``. + + ``seed`` should identify this cycle - the target country code, or + ``domain-cc`` when a country gets more than one cycle. The seed hashes to + one bucket, and the cycle takes the oldest unclaimed canons in it. + + Two seeds that hash to different buckets can never share a canon, whatever + state either corpus is in, because bucket membership depends only on the + canon ID. Two seeds that hash to the *same* bucket get the same picks, not + a partial overlap - so a collision is visible and harmless rather than a + silent conflict. + + ``exclude`` holds canon IDs already claimed elsewhere; they are skipped + within the bucket, so the cycle stays in its own lane instead of wandering + into someone else's. Returns ``[]`` when nothing is aging, or when the + bucket holds nothing unclaimed - in which case vary the seed + (``nz`` -> ``nz-2``) rather than widening the search. + """ + if count < 1: + raise ValueError("count must be at least 1") + if buckets < 1: + raise ValueError("buckets must be at least 1") + + entries = load_aging_canons(data_dir=data_dir, reference_date=reference_date) + if not entries: + return [] + + excluded = exclude or set() + target = seed_bucket(seed, buckets) + # `entries` is already oldest-first, so this takes the most overdue members. + return [ + e for e in entries + if canon_bucket(e["id"], buckets) == target and e["id"] not in excluded + ][:count] + + +def main() -> int: + """CLI: print the canons this cycle should re-verify.""" + parser = argparse.ArgumentParser( + description="Pick which aging canons this content cycle should re-verify.", + ) + parser.add_argument( + "--seed", + required=True, + help="Identifier for this cycle, e.g. the target country code ('nz') or 'domain-cc'.", + ) + parser.add_argument("--count", type=int, default=DEFAULT_COUNT, + help=f"How many canons to claim (default: {DEFAULT_COUNT}).") + parser.add_argument("--buckets", type=int, default=DEFAULT_BUCKETS, + help=f"Bucket count (default: {DEFAULT_BUCKETS}). Changing this " + f"re-shuffles every canon - leave it alone unless no PR is open.") + parser.add_argument("--exclude", default="", + help="Comma-separated canon IDs to treat as already claimed.") + parser.add_argument("--base-ref", default="origin/main", + help="Ref the other branches are compared against (default: origin/main).") + parser.add_argument("--no-auto-exclude", action="store_true", + help="Do not read claimed IDs from other pushed branches.") + args = parser.parse_args() + + if args.count < 1: + parser.error("--count must be at least 1") + if args.buckets < 1: + parser.error("--buckets must be at least 1") + + exclude = {s.strip() for s in args.exclude.split(",") if s.strip()} + scan: ClaimScan | None = None + if not args.no_auto_exclude: + scan = claimed_canon_ids(base_ref=args.base_ref, skip_branch=current_branch()) + exclude |= set(scan.ids) + + # Report the scan before the result: an unavailable scan changes how much + # the result below can be trusted, and must not be buried under it. + if scan is None: + print("Claim scan: OFF (--no-auto-exclude). Selection is hash-only.") + elif not scan.ok: + print(f"Claim scan: INCOMPLETE - {scan.reason}.") + if scan.ids: + print(f" Applied the {len(scan.ids)} claim(s) it did read, but the rest went " + f"unchecked.") + else: + print(" Nothing was checked; selection is hash-only.") + print(" Another branch may already own these canons. Run `git fetch origin --prune`, " + "or pass claims with --exclude.") + elif scan.ids: + print(f"Claim scan: excluded {len(scan.ids)} canon(s) claimed by " + f"{scan.branches} other pushed branch(es).") + else: + print(f"Claim scan: clean - none of the {scan.branches} other pushed branch(es) " + f"touch data/canons.") + + targets = select_targets(args.seed, count=args.count, buckets=args.buckets, exclude=exclude) + + if not targets: + aging = load_aging_canons() + if not aging: + print("\nNothing is past the aging threshold - no re-verification needed.") + return 1 + # Distinguish "bucket is empty" from "bucket is fully claimed" by + # asking the corpus, not by whether `exclude` happens to be non-empty - + # with the claim scan on by default it almost always is. + owned = [e for e in aging if canon_bucket(e["id"], args.buckets) == seed_bucket( + args.seed, args.buckets)] + if not owned: + print(f"\nBucket for seed {args.seed!r} is empty ({len(aging)} aging canons over " + f"{args.buckets} buckets). Vary the seed, e.g. {args.seed}-2.") + else: + print(f"\nAll {len(owned)} canon(s) in the bucket for seed {args.seed!r} are " + f"already claimed. Vary the seed, e.g. {args.seed}-2.") + return 1 + + print(f"\nRe-verification bucket for seed {args.seed!r} ({len(targets)} canons):\n") + for entry in targets: + try: + rel = entry["path"].relative_to(Path.cwd()) + except ValueError: + rel = entry["path"] + print(f" {entry['id']}") + print(f" file: {rel}") + print(f" last_confirmed: {entry['last_confirmed']} ({entry['age_days']} days ago)") + print("\nRe-fetch every source URL on these before touching any date field.") + return 0 + + +if __name__ == "__main__": + try: + sys.exit(main()) + except BrokenPipeError: + # Output was piped into something that closed early (`| head`). + sys.stderr.close() + sys.exit(0) diff --git a/tests/test_reverify.py b/tests/test_reverify.py new file mode 100644 index 00000000..27f8faad --- /dev/null +++ b/tests/test_reverify.py @@ -0,0 +1,378 @@ +"""Tests for disjoint re-verification target selection.""" + +import json +import subprocess +from datetime import date + +import pytest + +from generator.reverify import ( + _canon_id_from_path, + canon_bucket, + claimed_canon_ids, + current_branch, + load_aging_canons, + seed_bucket, + select_targets, +) + +REFERENCE = date(2026, 8, 18) + + +def _write_canon(root, canon_id, last_confirmed): + """Write a minimal canon just rich enough for age calculation.""" + domain, slug, env = canon_id.split("/") + path = root / domain / slug + path.mkdir(parents=True, exist_ok=True) + (path / f"{env}.json").write_text( + json.dumps({"id": canon_id, "error": {"last_confirmed": last_confirmed}}), + encoding="utf-8", + ) + + +# Countries the project already ships canons for - a realistic spread of seeds. +SEEDS = [ + "nz", "za", "my", "ph", "ke", "et", "jp", "kr", "us", "de", + "uk", "fr", "cn", "hk", "tw", "th", "in", "vn", "id", "sg", + "sa", "ae", "tr", "il", "ru", "br", "mx", "au", "ca", "pl", +] + + +# Mirrors production: ~1000 aging canons over DEFAULT_BUCKETS is ~8 per bucket, +# so bucket occupancy and hash spread behave as they do against the real corpus. +CORPUS_SIZE = 1024 + + +def _build_corpus(root, size=CORPUS_SIZE): + for i in range(size): + # Deliberately interleave two dates so ordering has ties to break. + stamp = "2026-02-01" if i % 2 == 0 else "2026-02-11" + _write_canon(root, f"python/aging-{i:04d}/py311-linux", stamp) + _write_canon(root, "rust/fresh/rust1-linux", "2026-08-17") + _write_canon(root, "go/undated/go1-linux", "not-a-date") + return root + + +@pytest.fixture(scope="module") +def corpus(tmp_path_factory): + """Aging canons, plus a fresh and an undated one. Read-only, so shared.""" + return _build_corpus(tmp_path_factory.mktemp("corpus") / "canons") + + +class TestLoadAgingCanons: + def test_only_aging_canons_are_returned(self, corpus): + entries = load_aging_canons(data_dir=corpus, reference_date=REFERENCE) + ids = {e["id"] for e in entries} + assert len(entries) == CORPUS_SIZE + assert "rust/fresh/rust1-linux" not in ids + assert "go/undated/go1-linux" not in ids + + def test_ordered_oldest_first_with_stable_tiebreak(self, corpus): + entries = load_aging_canons(data_dir=corpus, reference_date=REFERENCE) + ages = [e["age_days"] for e in entries] + assert ages == sorted(ages, reverse=True) + # Within one last_confirmed date, ties break on id, not filesystem order. + oldest = [e["id"] for e in entries if e["last_confirmed"] == "2026-02-01"] + assert oldest == sorted(oldest) + + def test_empty_corpus_returns_nothing(self, tmp_path): + (tmp_path / "canons").mkdir() + assert load_aging_canons(data_dir=tmp_path / "canons", reference_date=REFERENCE) == [] + + +class TestSelectTargets: + def test_same_seed_is_deterministic(self, corpus): + first = select_targets("nz", data_dir=corpus, reference_date=REFERENCE) + second = select_targets("nz", data_dir=corpus, reference_date=REFERENCE) + assert [e["id"] for e in first] == [e["id"] for e in second] + assert len(first) == 3 + + def test_two_seeds_never_partially_overlap(self, corpus): + """The invariant that actually prevents merge conflicts. + + Two seeds may collide onto the same bucket, in which case they get the + *identical* set - obvious immediately. What must never happen is a + partial overlap, where two PRs share some files and not others: that is + the shape that conflicts on merge while looking like independent work. + """ + picks = { + s: frozenset( + e["id"] for e in select_targets(s, data_dir=corpus, reference_date=REFERENCE) + ) + for s in SEEDS + } + for a_seed, a in picks.items(): + for b_seed, b in picks.items(): + shared = a & b + assert shared in (frozenset(), a), f"{a_seed} partially overlaps {b_seed}: {shared}" + + def test_no_partial_overlap_across_differing_corpora(self, corpus, tmp_path): + """Two cycles never see the same corpus - each branches from its own main. + + Position-based blocks shift their boundaries whenever a merge ages + canons in or refreshes them, letting two cycles land on overlapping-but- + not-equal slices. Bucketing by canon ID has to survive that. + """ + # What a later `main` looks like: some canons refreshed out of the aging + # set, others newly aged in, at offsets that would move every boundary. + grown = _build_corpus(tmp_path / "grown") + for i in range(0, CORPUS_SIZE, 7): + _write_canon(grown, f"python/aging-{i:04d}/py311-linux", "2026-08-17") + for i in range(CORPUS_SIZE, CORPUS_SIZE + 40): + _write_canon(grown, f"python/newly-aged-{i:04d}/py311-linux", "2026-02-05") + + before = { + s: {e["id"] for e in select_targets(s, data_dir=corpus, reference_date=REFERENCE)} + for s in SEEDS + } + after = { + s: {e["id"] for e in select_targets(s, data_dir=grown, reference_date=REFERENCE)} + for s in SEEDS + } + assert before != after, "corpora are too similar to prove anything" + for a_seed, a in before.items(): + for b_seed, b in after.items(): + if seed_bucket(a_seed) == seed_bucket(b_seed): + continue # same bucket: same lane, by design + assert not a & b, f"{a_seed}@before overlaps {b_seed}@after: {a & b}" + + def test_seeds_spread_across_buckets(self, corpus): + """Hashing must actually spread; a degenerate mapping would be useless.""" + distinct = { + frozenset(e["id"] for e in select_targets(s, data_dir=corpus, reference_date=REFERENCE)) + for s in SEEDS + } + assert len(distinct) >= 24, f"only {len(distinct)} distinct buckets for {len(SEEDS)} seeds" + + def test_bucket_membership_depends_only_on_the_id(self): + assert canon_bucket("python/foo/py311-linux") == canon_bucket("python/foo/py311-linux") + assert canon_bucket("a/b/c", buckets=16) < 16 + + def test_selection_stays_inside_the_aging_pool(self, corpus): + for seed in ("nz", "za", "visa-br"): + for entry in select_targets(seed, data_dir=corpus, reference_date=REFERENCE): + assert entry["age_days"] > 180 + + def test_excluded_ids_are_skipped_within_the_bucket(self, corpus): + """A claimed canon is stepped over; the cycle stays in its own lane.""" + block = select_targets("nz", data_dir=corpus, reference_date=REFERENCE) + moved = select_targets( + "nz", + exclude={block[0]["id"]}, + data_dir=corpus, + reference_date=REFERENCE, + ) + assert moved + assert block[0]["id"] not in {e["id"] for e in moved} + # Still the same bucket, so the survivors carry over rather than jumping. + assert {e["id"] for e in block[1:]} <= {e["id"] for e in moved} + bucket = seed_bucket("nz") + assert all(canon_bucket(e["id"]) == bucket for e in moved) + + def test_everything_excluded_returns_empty(self, corpus): + every_id = {e["id"] for e in load_aging_canons(data_dir=corpus, reference_date=REFERENCE)} + assert select_targets( + "nz", exclude=every_id, data_dir=corpus, reference_date=REFERENCE + ) == [] + + def test_exclusion_applies_to_a_corpus_smaller_than_one_bucket(self, tmp_path): + """Regression: the short-corpus path used to return before excluding.""" + root = tmp_path / "canons" + _write_canon(root, "python/only-one/py311-linux", "2026-02-01") + _write_canon(root, "python/only-two/py311-linux", "2026-02-01") + assert select_targets( + "nz", + exclude={"python/only-one/py311-linux", "python/only-two/py311-linux"}, + data_dir=root, + reference_date=REFERENCE, + ) == [] + + def test_no_aging_canons_returns_empty(self, tmp_path): + root = tmp_path / "canons" + _write_canon(root, "rust/fresh/rust1-linux", "2026-08-17") + assert select_targets("nz", data_dir=root, reference_date=REFERENCE) == [] + + def test_single_canon_corpus_is_returned_only_to_its_own_bucket(self, tmp_path): + root = tmp_path / "canons" + _write_canon(root, "python/only-one/py311-linux", "2026-02-01") + owner = canon_bucket("python/only-one/py311-linux") + for s in SEEDS: + picked = [e["id"] for e in select_targets(s, data_dir=root, reference_date=REFERENCE)] + expected = ["python/only-one/py311-linux"] if seed_bucket(s) == owner else [] + assert picked == expected + + def test_count_below_one_is_rejected(self, corpus): + with pytest.raises(ValueError): + select_targets("nz", count=0, data_dir=corpus, reference_date=REFERENCE) + + def test_bucket_count_below_one_is_rejected(self, corpus): + with pytest.raises(ValueError): + select_targets("nz", buckets=0, data_dir=corpus, reference_date=REFERENCE) + + +class TestRealCorpus: + def test_selection_works_against_the_shipped_dataset(self): + """Guards against the shipped corpus drifting out of the pool assumptions.""" + picked = select_targets("nz") + assert picked, "no aging canon fell in the 'nz' bucket" + assert len(picked) <= 3 + assert all(e["path"].exists() for e in picked) + assert len({e["id"] for e in picked}) == len(picked) + assert all(canon_bucket(e["id"]) == seed_bucket("nz") for e in picked) + + +class TestCanonIdFromPath: + def test_directory_layout(self): + assert _canon_id_from_path( + "data/canons/python/some-error/py311-linux.json" + ) == "python/some-error/py311-linux" + + def test_flat_file_layout(self): + assert _canon_id_from_path( + "data/canons/go/too-many-open-files_go1-linux.json" + ) == "go/too-many-open-files/go1-linux" + + def test_flat_file_layout_keeps_underscores_in_slug(self): + assert _canon_id_from_path( + "data/canons/go/a_b_c_go1-linux.json" + ) == "go/a_b_c/go1-linux" + + def test_unrecognised_paths_return_none(self): + assert _canon_id_from_path("README.md") is None + assert _canon_id_from_path("data/canons/orphan.json") is None + + +def _run(repo, *args): + subprocess.run(["git", *args], cwd=repo, check=True, capture_output=True, text=True) + + +@pytest.fixture +def git_repo(tmp_path): + """A repo with a base ref and one other branch that touches two canons.""" + repo = tmp_path / "repo" + (repo / "data" / "canons" / "python" / "base").mkdir(parents=True) + _run(repo.parent, "init", "-q", "-b", "main", str(repo)) + _run(repo, "config", "user.email", "t@example.com") + _run(repo, "config", "user.name", "t") + + (repo / "data/canons/python/base/py311-linux.json").write_text( + json.dumps({"id": "python/base/py311-linux"}) + ) + _run(repo, "add", "-A") + _run(repo, "commit", "-qm", "base") + _run(repo, "update-ref", "refs/remotes/origin/main", "HEAD") + + _run(repo, "checkout", "-q", "-b", "feature") + (repo / "data/canons/python/claimed").mkdir(parents=True) + (repo / "data/canons/python/claimed/py311-linux.json").write_text( + json.dumps({"id": "python/claimed/py311-linux"}) + ) + # Flat-file layout, and a non-canon file that must be ignored. + (repo / "data/canons/go/flat_go1-linux.json").parent.mkdir(parents=True, exist_ok=True) + (repo / "data/canons/go/flat_go1-linux.json").write_text( + json.dumps({"id": "go/flat/go1-linux"}) + ) + (repo / "README.md").write_text("not a canon") + _run(repo, "add", "-A") + _run(repo, "commit", "-qm", "feature") + _run(repo, "update-ref", "refs/remotes/origin/feature", "HEAD") + _run(repo, "checkout", "-q", "main") + return repo + + +class TestClaimedCanonIds: + def test_collects_ids_touched_by_other_branches(self, git_repo): + scan = claimed_canon_ids(repo_root=git_repo) + assert scan.ok + assert scan.branches == 1 + assert set(scan.ids) == {"python/claimed/py311-linux", "go/flat/go1-linux"} + + def test_base_ref_itself_is_not_claimed(self, git_repo): + assert "python/base/py311-linux" not in claimed_canon_ids(repo_root=git_repo).ids + + def test_skip_branch_drops_your_own_claim(self, git_repo): + scan = claimed_canon_ids(repo_root=git_repo, skip_branch="feature") + assert scan.ok and scan.ids == frozenset() and scan.branches == 0 + + def test_outside_a_git_repo_reports_failure_not_a_clean_scan(self, tmp_path): + """A scan that could not run must never read as "nothing is claimed".""" + plain = tmp_path / "plain" + plain.mkdir() + scan = claimed_canon_ids(repo_root=plain) + assert scan.ids == frozenset() + assert scan.ok is False + assert scan.reason + + def test_missing_base_ref_reports_failure(self, git_repo): + scan = claimed_canon_ids(base_ref="origin/nope", repo_root=git_repo) + assert scan.ok is False + assert "origin/nope" in scan.reason + + def test_claimed_ids_actually_steer_selection(self, corpus, git_repo): + """The two halves compose: what git reports is what select_targets skips.""" + scan = claimed_canon_ids(repo_root=git_repo) + assert scan.ok and scan.ids + + # Seed whichever bucket owns a real claimed canon, then confirm that + # feeding the scan's IDs in as `exclude` removes it from the picks. + claimed_id = sorted(scan.ids)[0] + root = corpus.parent / "composed" + _write_canon(root, claimed_id, "2026-02-01") + _write_canon(root, "python/spare-a/py311-linux", "2026-02-01") + + # buckets=1 puts everything in one lane, so the only thing that can + # remove the claimed canon from the picks is the exclusion itself. + without = [e["id"] for e in select_targets( + "nz", buckets=1, data_dir=root, reference_date=REFERENCE)] + assert claimed_id in without + + with_scan = [e["id"] for e in select_targets( + "nz", buckets=1, exclude=set(scan.ids), data_dir=root, reference_date=REFERENCE)] + assert claimed_id not in with_scan + assert "python/spare-a/py311-linux" in with_scan + + +class TestCurrentBranch: + def test_reports_checked_out_branch(self, git_repo): + assert current_branch(repo_root=git_repo) == "main" + + def test_detached_head_reports_none(self, git_repo): + _run(git_repo, "checkout", "-q", "--detach", "HEAD") + assert current_branch(repo_root=git_repo) is None + + def test_outside_a_git_repo_reports_none(self, tmp_path): + plain = tmp_path / "plain2" + plain.mkdir() + assert current_branch(repo_root=plain) is None + + +class TestClaimScanDegradation: + """A scan that could not check everything must never read as clean.""" + + def test_unreadable_branch_marks_the_scan_incomplete(self, git_repo): + # An orphan branch has no merge base with main, so `git diff a...b` + # fails - the same shape a shallow CI clone produces. + _run(git_repo, "checkout", "-q", "--orphan", "orphan") + _run(git_repo, "rm", "-rqf", ".") + (git_repo / "data/canons/python/orphan").mkdir(parents=True) + (git_repo / "data/canons/python/orphan/py311-linux.json").write_text( + json.dumps({"id": "python/orphan/py311-linux"}) + ) + _run(git_repo, "add", "-A") + _run(git_repo, "commit", "-qm", "orphan") + _run(git_repo, "update-ref", "refs/remotes/origin/orphan", "HEAD") + _run(git_repo, "checkout", "-q", "main") + + scan = claimed_canon_ids(repo_root=git_repo) + assert scan.ok is False + assert "origin/orphan" in scan.reason + # The branches it *could* read are still reported, not thrown away. + assert "python/claimed/py311-linux" in scan.ids + + def test_origin_head_is_not_counted_as_a_branch(self, git_repo): + before = claimed_canon_ids(repo_root=git_repo) + _run(git_repo, "symbolic-ref", "refs/remotes/origin/HEAD", + "refs/remotes/origin/main") + after = claimed_canon_ids(repo_root=git_repo) + assert after.branches == before.branches + assert after.ids == before.ids