Repository navigation
Share the corpus builders' cache and file helpers - #690
Merged
Merged
Conversation
build_cldr_corpus.py, build_gnd_corpus.py and build_wikidata_corpus.py each carried their own corpus_lines and fields_lines, and the two that query a live service each their own cache_dir and check-before-cache, atomic-write fetch. corpus_build.py now holds them once; each builder appends its own directory to sys.path (the tests load builders by path) and imports it. GND's corpus_lines becomes the shape-aware one, which writes the same lines for its shape-less rows; its test's hand-built rows gain the shape key every real row has. All three corpora rebuild byte-identical (CLDR re-fetched from GitHub, GND and Wikidata from the cache, whose keys are unchanged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #690 +/- ##
=======================================
Coverage 99.03% 99.03%
=======================================
Files 46 46
Lines 4541 4541
=======================================
Hits 4497 4497
Misses 44 44 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review of #690: - write_corpus builds both files before writing either, so a refused row leaves the committed pair as it was (the old builders wrote the corpus first, and the docstring claimed 'always together'); - test_corpus_build.py loads each builder in a fresh -P interpreter (the test process's first builder had been finding the module for the other two), and pins the shape TypeError, write_corpus, and the atomic cache write by fault injection; - an untagged line sorts before the tagged ones of its text, so the order no longer rests on set iteration when shape 0 ties with None; - the sys.path comment said appending prevents shadowing; it prevents shadowing the stdlib, at the cost of an earlier corpus_build winning; load_tool's 'no import-time side effects' now names the one there is; the Wikidata cache path names its wikidata/ directory. All three corpora rebuild byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its negative control passed: with one text, the old key's tie between shape 0 and None happened to break the right way in set order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Second review of #690's fix: 'both or neither' held for a refused row but not for a write that fails partway (disk full, interrupt). Both files now go to .part files, removed again on failure, before either is renamed into place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is the follow-up to #689's
/simplifypass. Three labeled-corpus builders each carried their own copies of the same helpers:corpus_linesfields_linescache_dirtools/differential/corpus_build.pynow holds them once:cache_dir(name)parse_jsoncached_json(cache, key, label, fetch, check, refresh)corpus_linesfields_lineswrite_corpusEach builder keeps only what is its own: its query, its response predicate and its renderings.
tools/differential/imported a sibling before. Each builder appends its own directory tosys.pathbefore importing. That directory is usuallysys.path[0]when a builder runs as a script, but not under-P/PYTHONSAFEPATH, nor when the tests load a builder by path. It is appended rather than inserted, so it cannot shadow a stdlib or installed module. The trade-off: an earliercorpus_buildonsys.pathwould win, and none exists.corpus_linesbecomes the shape-aware one CLDR and Wikidata used. It writes the same lines for GND's rows, all of which carry"shape": None. Only the test's hand-built rows lacked the key, and they gain it. The shared function stays strict, so a builder that forgets the shape fails rather than emitting untagged lines.write_corpuswrites the pair through.partfiles. Both are built and written aside before either is renamed into place, and the.partfiles are removed on failure. A refused row (a duplicate rendering, a bad shape) or a failed write therefore leaves the committed pair as it was. Only a crash between the two renames could leave them out of step. The old builders wrote the corpus file first, then the fields file.0withNone, leaving that order to set iteration. No builder writes shape 0, so no output moves.corpus_build.py, and the Wikidata cache path names itswikidata/directory.Test plan
All three corpora rebuild byte-identical: CLDR re-fetched from GitHub, GND and Wikidata from the cache.
uv run pytest: 19,251 passedNew
tests/v2/test_corpus_build.pypins each helper's contract:-Pinterpreter. In the test process, the first builder loaded had been finding the module for the other two, so a builder that forgot its ownsys.pathstep still passed;TypeError;write_corpus: a refused row, and a write that fails partway, each leave the old pair and no.partfile;Each test was checked against a mutation of its code and fails.
uv run mypy,uv run ruff checkThree review rounds:
.partwrites now extend to a failed write.The deliberate behavior changes don't move output: the pair is never half-written, and the tie-break is deterministic.
🤖 Generated with Claude Code