From f1ffa14b8609f106f0406cb231bb2acdec1d5e88 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 05:47:51 +0000 Subject: [PATCH 1/2] workflow: stop parallel content cycles from colliding Four PRs (#165, #166, #168, #172) sat unmergeable at once. None of them had failing CI; they were blocked by the workflow itself, in three ways. 1. Every cycle re-verified 'the canons with the oldest last_confirmed'. That rule is deterministic, so parallel cycles selected the same files and the first PR to merge left the rest conflicting on precisely the date fields they came to refresh. generator/reverify.py hands out disjoint blocks instead: aging canons sorted oldest-first, cut into fixed blocks, each seed hashing to one. Two seeds get the same block or share nothing - never the partial overlap that conflicts on merge. --exclude takes the IDs open PRs already touch, turning 'unlikely to collide' into 'cannot collide'. 2. Nothing told a cycle to look at open PRs before choosing a target, so three sessions independently picked New Zealand off the same view of main. Step 2 of the workflow now claims a target against the open PR list first. 3. The duplicate check was 'rg your-slug data/canons/', which only catches an identical slug. The actual duplicates arrived under different slugs describing the same dead end. The check is now by topic - read the country's existing signatures and summaries, and for each planned canon name its nearest neighbour and why it differs. Partial overlap gets narrowed and cross-linked, or folded into the existing canon, rather than shipped as a competing page. Also documents the re-verification contract: re-read every source before bumping any date. A refreshed date on an unchecked canon tells every downstream agent the entry was confirmed when it was not. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ksw62KukVY7Ft7e8zZGhrp --- .claude/commands/add-canon.md | 103 +++++++++++++++++--- CLAUDE.md | 39 ++++++++ generator/reverify.py | 173 ++++++++++++++++++++++++++++++++++ tests/test_reverify.py | 150 +++++++++++++++++++++++++++++ 4 files changed, 454 insertions(+), 11 deletions(-) create mode 100644 generator/reverify.py create mode 100644 tests/test_reverify.py diff --git a/.claude/commands/add-canon.md b/.claude/commands/add-canon.md index e999687b..06c7a4db 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,24 @@ 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 and every canon ID appearing in an open PR as taken. Pick a +different country. Keep the list of touched canon IDs to hand - step 6 needs it. + +## 3. Pick a real coverage gap (never duplicate) Count what already exists, then choose an underserved slice: @@ -73,13 +91,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 +140,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 +166,41 @@ 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 slice 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, and exclude +whatever the open PRs from step 2 already touch: + +```bash +python -m generator.reverify --seed +python -m generator.reverify --seed --exclude id1,id2,id3 # IDs from step 2 +``` + +Blocks are disjoint by construction, so two cycles either get the same block or +share nothing - never a partial overlap. `--exclude` turns "unlikely to collide" +into "cannot collide", so pass it whenever any PR is open. + +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 +213,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 +223,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 +239,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 +252,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 +269,6 @@ 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 block + `python -m generator.reverify --seed ` assigns you. diff --git a/CLAUDE.md b/CLAUDE.md index c6d391bc..91bd934a 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 block 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 block (seed with the target country code) +python -m generator.reverify --seed nz +python -m generator.reverify --seed nz --exclude id1,id2 # IDs open PRs touch + # List all errors python -m generator.lookup --list @@ -179,6 +185,38 @@ 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 block instead: + +```bash +python -m generator.reverify --seed nz # seed = target country +python -m generator.reverify --seed nz --exclude id1,id2 # IDs open PRs touch +``` + +Aging canons are sorted oldest-first and cut into fixed blocks; a seed hashes to +one block. Two seeds therefore get either the same block or no shared files at +all - never a partial overlap, which is the shape that conflicts on merge. +Hashing makes a collision unlikely; `--exclude` makes it impossible, so pass the +canon IDs any open PR already touches. + +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 +299,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 block 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..f74fd92f --- /dev/null +++ b/generator/reverify.py @@ -0,0 +1,173 @@ +"""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. The aging canons are sorted +oldest-first, cut into fixed blocks, and each caller gets the block its ``seed`` +hashes to - so two sessions working on different countries claim different +files without coordinating. + +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 +""" + +import argparse +import hashlib +import json +import sys +from datetime import date +from pathlib import Path + +from generator.validate import AGING_THRESHOLD_DAYS, _canon_age_days + +DATA_DIR = Path(__file__).parent.parent / "data" / "canons" + +# How deep into the oldest-first ordering blocks are cut from. Everything in +# this window is past AGING_THRESHOLD_DAYS anyway, so a caller landing at the +# far end still re-verifies something genuinely due. A wider pool means more +# blocks and a smaller chance that two seeds collide, traded against reaching +# less urgent canons; 600 keeps the window inside the oldest two thirds of a +# ~1000-canon aging cohort while leaving ~200 blocks to spread across. +# +# Hashing only makes a collision unlikely, never impossible. `exclude` is what +# makes disjointness a guarantee - pass the IDs open PRs already touch. +DEFAULT_POOL = 600 +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 _block_index(seed: str, block_count: int) -> int: + """Map a seed to a block, stably across processes. + + ``hash()`` is salted per process, so it cannot be used here - two sessions + would disagree about which block a seed owns. + """ + digest = hashlib.sha256(seed.encode("utf-8")).hexdigest() + return int(digest, 16) % block_count + + +def select_targets( + seed: str, + count: int = DEFAULT_COUNT, + pool: int = DEFAULT_POOL, + exclude: set[str] | None = None, + data_dir: Path | None = None, + reference_date: date | None = None, +) -> list[dict]: + """Pick ``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 same seed always + returns the same canons; different seeds almost always return disjoint sets. + + ``exclude`` holds canon IDs already claimed elsewhere (an open PR, an + earlier batch). Blocks containing any of them are skipped, so a caller that + passes the IDs touched by open PRs never lands on a file someone else is + editing. Returns ``[]`` when nothing is aging, or when every block is + excluded. + """ + if count < 1: + raise ValueError("count must be at least 1") + + entries = load_aging_canons(data_dir=data_dir, reference_date=reference_date) + if not entries: + return [] + + window = entries[:pool] + blocks = [window[i:i + count] for i in range(0, len(window) - count + 1, count)] + if not blocks: + # Fewer aging canons than a full block: hand back what there is. + return window[:count] + + excluded = exclude or set() + start = _block_index(seed, len(blocks)) + for offset in range(len(blocks)): + block = blocks[(start + offset) % len(blocks)] + if not any(entry["id"] in excluded for entry in block): + return block + return [] + + +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("--pool", type=int, default=DEFAULT_POOL, + help=f"How deep into the oldest-first ordering to cut blocks from " + f"(default: {DEFAULT_POOL}).") + parser.add_argument("--exclude", default="", + help="Comma-separated canon IDs already claimed by an open PR.") + args = parser.parse_args() + + exclude = {s.strip() for s in args.exclude.split(",") if s.strip()} + targets = select_targets(args.seed, count=args.count, pool=args.pool, exclude=exclude) + + if not targets: + print("No aging canon block available for this seed.") + if exclude: + print("Every block overlapped an excluded ID - widen --pool or re-check open PRs.") + return 1 + + print(f"Re-verification block 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__": + sys.exit(main()) diff --git a/tests/test_reverify.py b/tests/test_reverify.py new file mode 100644 index 00000000..25182d3f --- /dev/null +++ b/tests/test_reverify.py @@ -0,0 +1,150 @@ +"""Tests for disjoint re-verification target selection.""" + +import json +from datetime import date + +import pytest + +from generator.reverify import load_aging_canons, 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", +] + + +@pytest.fixture +def corpus(tmp_path): + """300 canons past the aging threshold, plus fresh and undated ones. + + Sized to mirror production: the window yields 100 blocks, so the spread + assertions below exercise the same arithmetic the real corpus does. + """ + root = tmp_path / "canons" + for i in range(300): + # 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:03d}/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 + + +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) == 300 + 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 cycles may collide onto the same block - hashing makes that + unlikely, not impossible, which is what `exclude` is for. 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_seeds_spread_across_the_pool(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 + } + # 30 seeds over 100 blocks: uniform hashing predicts ~26 distinct. + assert len(distinct) >= 24, f"only {len(distinct)} distinct blocks for {len(SEEDS)} seeds" + + 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_push_selection_to_another_block(self, corpus): + 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 not {e["id"] for e in moved} & {e["id"] for e in block} + + 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_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_corpus_smaller_than_one_block_returns_what_exists(self, tmp_path): + root = tmp_path / "canons" + _write_canon(root, "python/only-one/py311-linux", "2026-02-01") + picked = select_targets("nz", data_dir=root, reference_date=REFERENCE) + assert [e["id"] for e in picked] == ["python/only-one/py311-linux"] + + def test_count_below_one_is_rejected(self, corpus): + with pytest.raises(ValueError): + select_targets("nz", count=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 len(picked) == 3 + assert all(e["path"].exists() for e in picked) + assert len({e["id"] for e in picked}) == 3 From f13431880ee1f616f7f0806cf1fc29043a47f16f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 06:57:07 +0000 Subject: [PATCH 2/2] reverify: bucket by canon ID, and never report an unchecked scan as clean Follow-up to the previous commit, from reviewing it. Selection cut the aging list into positional blocks. That made the central promise - two cycles never partially overlap - true only against a frozen corpus. Two cycles never see the same corpus: each branches from a different main, and any merge that ages canons in or refreshes them shifts every later block boundary, so two cycles could land on overlapping-but-unequal slices. Exactly the shape that conflicts on merge, and exactly what the module exists to prevent. Buckets are now a pure function of the canon ID, so a canon stays in its bucket no matter what else changed. Two seeds own the same bucket (identical picks, obvious at once) or share nothing. DEFAULT_BUCKETS has to stay constant for that agreement to hold, which is now documented at the constant and in both docs. Replaces the --pool knob with --buckets. Claim-scan fixes, all in the half the docs tell operators to trust: - A per-branch `git diff` failure was swallowed, so unrelated histories or a shallow CI clone produced ok=True with an empty set - the exact false clean bill of health the ClaimScan docstring forbids. Those branches are now collected and reported as INCOMPLETE, keeping the claims that were readable. - The short refname format renders refs/remotes/origin/HEAD as bare "origin", so the /HEAD guard never fired: HEAD was scanned as a branch, inflating the count and importing its canons as claims. Uses full refnames now. - An empty bucket was reported as "already claimed" whenever the scan had found anything, which with auto-exclude on is almost always. The two cases are now told apart by asking the corpus. - --count 0 / --buckets 0 ran the whole scan before dying on an uncaught ValueError; argparse rejects them up front. - The DEFAULT_BUCKETS comment claimed collisions among ~30 country seeds were unlikely. Across all seeds a collision is near-certain; the number that justifies the constant is per-concurrency (~2.3% for 3 at once). Tests: 32 for this module, including that no two seeds partially overlap across two *different* corpus states, that an unreadable branch marks the scan incomplete, and that origin/HEAD is not counted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ksw62KukVY7Ft7e8zZGhrp --- .claude/commands/add-canon.md | 40 +++-- CLAUDE.md | 41 +++-- generator/reverify.py | 322 ++++++++++++++++++++++++++++------ tests/test_reverify.py | 282 ++++++++++++++++++++++++++--- 4 files changed, 585 insertions(+), 100 deletions(-) diff --git a/.claude/commands/add-canon.md b/.claude/commands/add-canon.md index 06c7a4db..a33e2d01 100644 --- a/.claude/commands/add-canon.md +++ b/.claude/commands/add-canon.md @@ -46,8 +46,11 @@ List the open PRs and the files they touch **before** choosing anything: # mcp__github__pull_request_read (method: get_files) ``` -Treat every country and every canon ID appearing in an open PR as taken. Pick a -different country. Keep the list of touched canon IDs to hand - step 6 needs it. +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) @@ -166,7 +169,7 @@ 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. -## 6. Re-verify your assigned slice of aging canons +## 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, @@ -174,17 +177,31 @@ 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, and exclude -whatever the open PRs from step 2 already touch: +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 -python -m generator.reverify --seed --exclude id1,id2,id3 # IDs from step 2 ``` -Blocks are disjoint by construction, so two cycles either get the same block or -share nothing - never a partial overlap. `--exclude` turns "unlikely to collide" -into "cannot collide", so pass it whenever any PR is open. +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: @@ -270,5 +287,6 @@ which topic and why. 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 block - `python -m generator.reverify --seed ` assigns you. +- **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 91bd934a..e7d1e6e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,7 @@ 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 block of aging + 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 @@ -103,9 +103,9 @@ ruff check generator/ tests/ # Look up an error (CLI) python -m generator.lookup "error message" -# Get this cycle's re-verification block (seed with the target country code) -python -m generator.reverify --seed nz -python -m generator.reverify --seed nz --exclude id1,id2 # IDs open PRs touch +# 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 @@ -194,18 +194,35 @@ 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 block instead: +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 # IDs open PRs touch +python -m generator.reverify --seed nz --exclude id1,id2 # additional claims ``` -Aging canons are sorted oldest-first and cut into fixed blocks; a seed hashes to -one block. Two seeds therefore get either the same block or no shared files at -all - never a partial overlap, which is the shape that conflicts on merge. -Hashing makes a collision unlikely; `--exclude` makes it impossible, so pass the -canon IDs any open PR already touches. +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: @@ -299,6 +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 block selection (disjointness, determinism) +- `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 index f74fd92f..1c6d4f51 100644 --- a/generator/reverify.py +++ b/generator/reverify.py @@ -5,39 +5,56 @@ 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. The aging canons are sorted -oldest-first, cut into fixed blocks, and each caller gets the block its ``seed`` -hashes to - so two sessions working on different countries claim different -files without coordinating. +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 -DATA_DIR = Path(__file__).parent.parent / "data" / "canons" +REPO_ROOT = Path(__file__).parent.parent +DATA_DIR = REPO_ROOT / "data" / "canons" -# How deep into the oldest-first ordering blocks are cut from. Everything in -# this window is past AGING_THRESHOLD_DAYS anyway, so a caller landing at the -# far end still re-verifies something genuinely due. A wider pool means more -# blocks and a smaller chance that two seeds collide, traded against reaching -# less urgent canons; 600 keeps the window inside the oldest two thirds of a -# ~1000-canon aging cohort while leaving ~200 blocks to spread across. +# 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. # -# Hashing only makes a collision unlikely, never impossible. `exclude` is what -# makes disjointness a guarantee - pass the IDs open PRs already touch. -DEFAULT_POOL = 600 +# 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 @@ -76,56 +93,210 @@ def load_aging_canons( return entries -def _block_index(seed: str, block_count: int) -> int: - """Map a seed to a block, stably across processes. +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 block a seed owns. + would disagree about which bucket a seed owns. """ digest = hashlib.sha256(seed.encode("utf-8")).hexdigest() - return int(digest, 16) % block_count + 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, - pool: int = DEFAULT_POOL, exclude: set[str] | None = None, + buckets: int = DEFAULT_BUCKETS, data_dir: Path | None = None, reference_date: date | None = None, ) -> list[dict]: - """Pick ``count`` aging canons for the cycle identified by ``seed``. + """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 same seed always - returns the same canons; different seeds almost always return disjoint sets. - - ``exclude`` holds canon IDs already claimed elsewhere (an open PR, an - earlier batch). Blocks containing any of them are skipped, so a caller that - passes the IDs touched by open PRs never lands on a file someone else is - editing. Returns ``[]`` when nothing is aging, or when every block is - excluded. + ``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 [] - window = entries[:pool] - blocks = [window[i:i + count] for i in range(0, len(window) - count + 1, count)] - if not blocks: - # Fewer aging canons than a full block: hand back what there is. - return window[:count] - excluded = exclude or set() - start = _block_index(seed, len(blocks)) - for offset in range(len(blocks)): - block = blocks[(start + offset) % len(blocks)] - if not any(entry["id"] in excluded for entry in block): - return block - return [] + 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: @@ -140,23 +311,69 @@ def main() -> int: ) parser.add_argument("--count", type=int, default=DEFAULT_COUNT, help=f"How many canons to claim (default: {DEFAULT_COUNT}).") - parser.add_argument("--pool", type=int, default=DEFAULT_POOL, - help=f"How deep into the oldest-first ordering to cut blocks from " - f"(default: {DEFAULT_POOL}).") + 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 already claimed by an open PR.") + 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()} - targets = select_targets(args.seed, count=args.count, pool=args.pool, exclude=exclude) + 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: - print("No aging canon block available for this seed.") - if exclude: - print("Every block overlapped an excluded ID - widen --pool or re-check open PRs.") + 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"Re-verification block for seed {args.seed!r} ({len(targets)} canons):\n") + 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()) @@ -170,4 +387,9 @@ def main() -> int: if __name__ == "__main__": - sys.exit(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 index 25182d3f..27f8faad 100644 --- a/tests/test_reverify.py +++ b/tests/test_reverify.py @@ -1,11 +1,20 @@ """Tests for disjoint re-verification target selection.""" import json +import subprocess from datetime import date import pytest -from generator.reverify import load_aging_canons, select_targets +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) @@ -29,28 +38,32 @@ def _write_canon(root, canon_id, last_confirmed): ] -@pytest.fixture -def corpus(tmp_path): - """300 canons past the aging threshold, plus fresh and undated ones. - - Sized to mirror production: the window yields 100 blocks, so the spread - assertions below exercise the same arithmetic the real corpus does. - """ - root = tmp_path / "canons" - for i in range(300): +# 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:03d}/py311-linux", stamp) + _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) == 300 + assert len(entries) == CORPUS_SIZE assert "rust/fresh/rust1-linux" not in ids assert "go/undated/go1-linux" not in ids @@ -77,11 +90,10 @@ def test_same_seed_is_deterministic(self, corpus): def test_two_seeds_never_partially_overlap(self, corpus): """The invariant that actually prevents merge conflicts. - Two cycles may collide onto the same block - hashing makes that - unlikely, not impossible, which is what `exclude` is for. 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. + 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( @@ -94,21 +106,55 @@ def test_two_seeds_never_partially_overlap(self, corpus): shared = a & b assert shared in (frozenset(), a), f"{a_seed} partially overlaps {b_seed}: {shared}" - def test_seeds_spread_across_the_pool(self, corpus): + 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 } - # 30 seeds over 100 blocks: uniform hashing predicts ~26 distinct. - assert len(distinct) >= 24, f"only {len(distinct)} distinct blocks for {len(SEEDS)} 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_push_selection_to_another_block(self, corpus): + 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", @@ -117,7 +163,11 @@ def test_excluded_ids_push_selection_to_another_block(self, corpus): reference_date=REFERENCE, ) assert moved - assert not {e["id"] for e in moved} & {e["id"] for e in block} + 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)} @@ -125,26 +175,204 @@ def test_everything_excluded_returns_empty(self, corpus): "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_corpus_smaller_than_one_block_returns_what_exists(self, tmp_path): + 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") - picked = select_targets("nz", data_dir=root, reference_date=REFERENCE) - assert [e["id"] for e in picked] == ["python/only-one/py311-linux"] + 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 len(picked) == 3 + 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}) == 3 + 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