Skip to content

Commit 225f3d6

Browse files
derek73claude
andauthored
Compare a local corpus against a base you name, not the shared snapshot (#888)
* Compare a local corpus against a base you name, not the shared snapshot snapshot.py --local SOURCE and local_agreement.py psc-individuals each compared against one file per corpus in the cache (snapshot.jsonl, agreement.jsonl), shared by every worktree and decided by whoever recorded last. On 2026-10-25 parallel sessions of #752's phase 2 read a sibling's re-record as their own moves; the silent case is a sibling's record from a branch holding the same change, which reads as no moves. --base PATH reads the base checkout's readings from a record kept by the digest of its nameparser/ files (snapshot.py's baselines/, and a new agreements/ folder for the score), recording one in a child process where none is kept, and never reads or writes the shared file. It prints the base's path, digest prefix and whether it was reused or recorded, and the head's digest, saying so when the two packages are byte-identical. It refuses a base that is the tree being read or holds no nameparser package; the child strips PYTHONPATH and refuses a package imported from anywhere but the base. --sample composes with it. Without --base both tools behave as before. BASELINES_KEPT goes from 4 to 10: four sessions with a base and a head each, plus one, at FEC's 41.7 MB a baseline about 420 MB for the largest corpus. The tests plant a change in a copied BASE tree, so a base read from the head's tree by mistake would show no move, and check a plain copy moves nothing, the shared files are left byte- and mtime-identical, a kept base is reused, and the refusals. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Publish a kept base record only once it is whole and its base unchanged Review of the --base commit. The agreement child wrote straight to the kept path through corpus_build.write_atomic's one `.part` name, so two sessions scoring the same base at once could rename each other's half written file into place, and a record cut on a line boundary passed read_record and was reused by every later session. It also published under the digest the parent took before the child started. The child now scores into a scratch folder of the run's own (snapshot.scratch, also used for the snapshot side's base), and the parent renames the record into place only after reading it back and taking the base's digest again. The record's header counts its entries and read_record refuses a record holding another number (a record written before the count passes). _prune sweeps a scratch folder a killed run left, once stale, and _reused reads a baseline another run pruned meanwhile as none kept rather than damaged. The PYTHONPATH test becomes the unit check it was: --tree's sys.path entry already outranks a decoy in the child, so no end-to-end run can show the strip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin what the second review found the fix tests did not Second review of d1ee456. Two of its tests passed against the pre-fix code: the changed-base test now asserts the kept path is empty when the after-check runs (it held the record before the fix), and the pruned-baseline test removes the file between the lookup and the open, where the old unlink raised. A header whose entry count is not the record's own, `null` included, is refused rather than raising while its message is formatted. Each of the four rows was run against 47bd62a and fails there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Record in AGENTS.md how a kept reading is published Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent f2e31d4 commit 225f3d6

7 files changed

Lines changed: 620 additions & 39 deletions

File tree

‎AGENTS.md‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

‎tests/v2/_differential_fixtures.py‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
"""
1010
import importlib.util
1111
import json
12+
import shutil
1213
import sys
1314
import time
1415
import urllib.request
1516
from pathlib import Path
1617
from types import ModuleType
18+
from typing import Any
1719

1820
import pytest
1921

@@ -97,13 +99,16 @@ def own_cache(monkeypatch: pytest.MonkeyPatch,
9799

98100

99101
def plant_corpus(monkeypatch: pytest.MonkeyPatch, corpora: ModuleType, source: str,
100-
records: list[dict[str, str]], date: str, unit: str) -> None:
102+
records: list[dict[str, str]], date: str, unit: str,
103+
detail: dict[str, Any] | None = None) -> None:
101104
"""`records` fetched as `source`'s local corpus into the cache, with no
102105
network: through a Source carrying the real one's credit and nothing
103-
else, so that none of the real source's renew or check applies."""
106+
else, so that none of the real source's renew or check applies in
107+
this process. A child process loads the corpus under the real
108+
Source, whose check reads `detail`, the meta.json detail it pins."""
104109
monkeypatch.setitem(corpora.SOURCES, source, corpora.Source(
105110
corpora.SOURCES[source].credit, unit,
106-
lambda: corpora.Fetched(list(records), date, {})))
111+
lambda: corpora.Fetched(list(records), date, dict(detail or {}))))
107112
corpora.fetch(source)
108113

109114

@@ -125,3 +130,25 @@ def urlopen(request: urllib.request.Request, timeout: int) -> _Response:
125130
return _Response(body)
126131
monkeypatch.setattr(urllib.request, "urlopen", urlopen)
127132
return fetched
133+
134+
135+
#: Appended to a copied package's __init__.py, a change every reading
136+
#: shows: each name is parsed with its words reversed.
137+
_PLANTED = """
138+
139+
_planted_parse = Parser.parse
140+
Parser.parse = lambda self, text: _planted_parse(self, " ".join(reversed(text.split())))
141+
"""
142+
143+
144+
def checkout_copy(at: Path, *, planted: bool = False) -> Path:
145+
"""A checkout at `at` holding a copy of this one's nameparser/
146+
package, and `planted`, a change every reading shows: a base tree
147+
for --base, which a child process reads, so that a check across a
148+
planted copy proves the base was read from its own tree."""
149+
shutil.copytree(_TOOLS.parents[1] / "nameparser", at / "nameparser",
150+
ignore=shutil.ignore_patterns("__pycache__"))
151+
if planted:
152+
with open(at / "nameparser" / "__init__.py", "a", encoding="utf-8") as f:
153+
f.write(_PLANTED)
154+
return at

‎tests/v2/test_local_agreement.py‎

Lines changed: 145 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
another fetch or another scorer, and a corpus damaged under the stream. The names are invented.
99
"""
1010
import hashlib
11+
import importlib
1112
import json
1213
import sys
1314
import unicodedata
@@ -17,7 +18,7 @@
1718

1819
from nameparser import Parser
1920

20-
from ._differential_fixtures import load_tool, own_cache, plant_corpus
21+
from ._differential_fixtures import checkout_copy, load_tool, own_cache, plant_corpus
2122

2223
agreement = load_tool("local_agreement")
2324
corpora = agreement.local_corpora
@@ -96,7 +97,10 @@ def test_a_title_box_holding_an_unlisted_word_the_parse_reads_otherwise_is_set_a
9697

9798
def _corpus(monkeypatch: pytest.MonkeyPatch, *records: dict[str, str]) -> None:
9899
"""A PSC corpus of `records` fetched into the cache, no network behind it."""
99-
plant_corpus(monkeypatch, corpora, _PSC, list(records), "2026-10-22", "filed names")
100+
# The sample pinned as the real source pins it: a --base child
101+
# loads the corpus under the real Source and its check.
102+
plant_corpus(monkeypatch, corpora, _PSC, list(records), "2026-10-22", "filed names",
103+
{"sample": f"1 in {importlib.import_module('local_psc').PSC_SAMPLE}"})
100104

101105

102106
_ALL = (_AGREES, _DOUBLE_SURNAME, _SUFFIX, _JOB_TITLE, _SHAPE_TITLE)
@@ -160,7 +164,7 @@ def test_a_record_of_another_fetch_is_refused(
160164
agreement.main([_PSC])
161165
path = _own_cache / "local" / _PSC / agreement.RECORD
162166
header = json.loads(path.read_text().splitlines()[0])
163-
assert set(header) == {"raw_sha256", "scorer", "data_date", "tree"}
167+
assert set(header) == {"raw_sha256", "scorer", "data_date", "tree", "entries"}
164168

165169

166170
def test_a_run_with_no_record_names_the_command_that_writes_one(
@@ -298,3 +302,141 @@ def test_a_tree_that_is_not_the_imported_package_is_refused(
298302
# The control: this checkout itself is the tree it imported.
299303
root = agreement.HERE.parents[1]
300304
assert agreement.main([_PSC, "--record", "--tree", str(root)]) == 0
305+
306+
307+
# -- --base ----------------------------------------------------------------------
308+
309+
def _kept(cache: Path) -> list[Path]:
310+
return sorted((cache / "local" / _PSC / agreement.KEPT).glob("*.jsonl"))
311+
312+
313+
def test_a_base_check_reports_a_change_planted_in_the_base_and_none_across_a_copy(
314+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path,
315+
capsys: pytest.CaptureFixture[str]) -> None:
316+
# The harness must be able to report a difference: a base scored with
317+
# the head's tree by mistake would show no move across a planted one.
318+
_corpus(monkeypatch, *_ALL)
319+
planted = checkout_copy(tmp_path / "planted", planted=True)
320+
assert agreement.main([_PSC, "--base", str(planted)]) == 1
321+
out = capsys.readouterr().out
322+
assert f"base: {planted.resolve()}, nameparser/ " in out and "scored it now" in out
323+
assert "progress: 3 records (" in out and "regression" not in out
324+
# The control: an unplanted copy is the head's package byte for byte.
325+
copy = checkout_copy(tmp_path / "copy")
326+
assert agreement.main([_PSC, "--base", str(copy)]) == 0
327+
out = capsys.readouterr().out
328+
assert "matches the base" in out and "byte-identical" in out
329+
assert len(_kept(_own_cache)) == 2
330+
331+
332+
def test_a_base_check_never_touches_the_shared_record(
333+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path) -> None:
334+
_corpus(monkeypatch, *_ALL)
335+
shared = _own_cache / "local" / _PSC / agreement.RECORD
336+
base = checkout_copy(tmp_path / "base", planted=True)
337+
assert agreement.main([_PSC, "--base", str(base)]) == 1
338+
assert not shared.exists()
339+
assert agreement.main([_PSC, "--record"]) == 0
340+
shared.write_text(shared.read_text().splitlines()[0] + "\n") # would read as moves
341+
before = (shared.stat().st_mtime_ns, shared.read_bytes())
342+
assert agreement.main([_PSC, "--base", str(checkout_copy(tmp_path / "copy"))]) == 0
343+
assert (shared.stat().st_mtime_ns, shared.read_bytes()) == before
344+
345+
346+
def test_a_kept_base_record_is_reused_and_a_damaged_one_scored_again(
347+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path,
348+
capsys: pytest.CaptureFixture[str]) -> None:
349+
_corpus(monkeypatch, *_ALL)
350+
base = checkout_copy(tmp_path / "base", planted=True)
351+
assert agreement.main([_PSC, "--base", str(base)]) == 1
352+
[kept] = _kept(_own_cache)
353+
recorded = kept.read_bytes()
354+
capsys.readouterr()
355+
real = agreement._score_base
356+
357+
def unscored(*args: object) -> None:
358+
raise AssertionError("scored the base again")
359+
monkeypatch.setattr(agreement, "_score_base", unscored)
360+
assert agreement.main([_PSC, "--base", str(base)]) == 1
361+
assert "reused the kept record" in capsys.readouterr().out
362+
# A record cut short on a line boundary still parses line by line;
363+
# its header's count is what refuses it.
364+
header = json.loads(recorded.decode().splitlines()[0])
365+
uncounted = json.dumps({**header, "entries": None}) + "\n" + "".join(
366+
recorded.decode().splitlines(keepends=True)[1:])
367+
for damage in ("not a record\n", "".join(recorded.decode().splitlines(keepends=True)[:-1]),
368+
uncounted):
369+
kept.write_text(damage)
370+
with pytest.raises(AssertionError, match="scored the base again"):
371+
agreement.main([_PSC, "--base", str(base)])
372+
assert "deleted the kept record" in capsys.readouterr().out
373+
monkeypatch.setattr(agreement, "_score_base", real)
374+
assert agreement.main([_PSC, "--base", str(base)]) == 1
375+
assert kept.read_bytes() == recorded
376+
assert not list(kept.parent.glob(".base-*")) # the child's scratch is gone
377+
monkeypatch.setattr(agreement, "_score_base", unscored)
378+
379+
380+
def test_a_base_that_changed_while_it_was_scored_keeps_no_record(
381+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path) -> None:
382+
_corpus(monkeypatch, *_ALL)
383+
base = checkout_copy(tmp_path / "base", planted=True)
384+
# The module base_entries imports by its bare name (local_agreement
385+
# put its folder on sys.path), not a second load by path.
386+
snapshot = importlib.import_module("snapshot")
387+
answers = ["a" * 64, "b" * 64]
388+
published: list[bool] = []
389+
390+
def digest(root: Path) -> str:
391+
if not answers:
392+
return "c" * 64
393+
if len(answers) == 1: # the check after the score: nothing kept yet
394+
published.append(bool(_kept(_own_cache)))
395+
return answers.pop(0)
396+
monkeypatch.setattr(snapshot, "_source_digest", digest)
397+
with pytest.raises(SystemExit, match="changed while it was scored"):
398+
agreement.main([_PSC, "--base", str(base)])
399+
# Written to the kept path before the check, a sibling could have
400+
# reused it as the base it was keyed for in between.
401+
assert published == [False]
402+
assert not _kept(_own_cache)
403+
assert not list((_own_cache / "local" / _PSC / agreement.KEPT).glob(".base-*"))
404+
405+
406+
def test_a_base_that_is_the_tree_read_or_no_checkout_is_refused(
407+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path) -> None:
408+
_corpus(monkeypatch, *_ALL)
409+
with pytest.raises(SystemExit, match="is the tree being read"):
410+
agreement.main([_PSC, "--base", str(agreement.HERE.parents[1])])
411+
with pytest.raises(SystemExit, match="no nameparser package"):
412+
agreement.main([_PSC, "--base", str(tmp_path)])
413+
assert not _kept(_own_cache)
414+
415+
416+
def test_a_base_whose_import_fails_is_refused_with_the_childs_output(
417+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path) -> None:
418+
_corpus(monkeypatch, *_ALL)
419+
base = checkout_copy(tmp_path / "base")
420+
with open(base / "nameparser" / "__init__.py", "a", encoding="utf-8") as f:
421+
f.write("\nraise ImportError('planted')\n")
422+
with pytest.raises(SystemExit, match="(?s)scoring the base .* failed.*planted"):
423+
agreement.main([_PSC, "--base", str(base)])
424+
assert not _kept(_own_cache)
425+
426+
427+
def test_a_base_check_refuses_to_record(
428+
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None:
429+
_corpus(monkeypatch, *_ALL)
430+
for extra in (["--record"], ["--out", "x.jsonl"]):
431+
with pytest.raises(SystemExit) as raised:
432+
agreement.main([_PSC, "--base", ".", *extra])
433+
assert raised.value.code == 2 and "--base compares" in capsys.readouterr().err
434+
435+
436+
def test_record_writes_where_out_says_and_leaves_the_shared_record(
437+
monkeypatch: pytest.MonkeyPatch, _own_cache: Path, tmp_path: Path) -> None:
438+
_corpus(monkeypatch, *_ALL)
439+
out = tmp_path / "elsewhere.jsonl"
440+
assert agreement.main([_PSC, "--record", "--out", str(out)]) == 0
441+
assert agreement.read_record(out, corpora.load(_PSC))
442+
assert not (_own_cache / "local" / _PSC / agreement.RECORD).exists()

0 commit comments

Comments
 (0)