Record where each fixture came from, and check that the record stays honest - #8
Merged
Conversation
Copied from FactorioMapWebUI, whose manifest is the only one that has run long enough to be worth copying. Measured across its 100 entries: every one carries exactly factorioVersion and evidence, and none of the richer keys the design sketched appears once. So those two are required and every other key is carried through untouched. evidence stays free text. The design wrote it as an enum; the first word is 'stated' 48 times, 'captured' 34, 'RE-CAPTURED' 8 and twice just 'the', so requiring a grade token would reject 45 of the 100. The grade that is real is factorioVersion == unknown, which is what the ratchet counts. Keys are relative paths with forward slashes, because this crate's own fixture tree is two levels deep and MapWebUI's is flat.
Four findings, all offline: a file nothing records, an entry whose file is gone, an entry missing a required key, and the unknown ratchet. The ratchet fails in both directions. Failing when the count exceeds the declared number is the prior art. Failing when it drops below is not, and it is the half that makes the number fall - MapWebUI's cap has read 'lower it when one gets resolved' for as long as it has read 1. An undeclared ratchet is its own finding rather than an implied zero. 'We allow none' and 'we never decided' should not look the same, and the message names the number to write down.
check() builds malformed in two passes - fixtures, then not-fixtures - so it came back as two sorted runs concatenated rather than one sorted list. A manifest naming a malformed fixture "z.json" and a reasonless not-fixture "a.md" reported them in that order, not alphabetically. Sorted explicitly now, and the comment says why missing, dangling and unknown need no such step while malformed does. every_list_comes_back_sorted only ever exercised missing, so it never could have caught this; the new test picks a fixture name that sorts after the not-fixture name specifically so the two-pass concatenation would show through without the fix.
JSON to stdout so a consumer's test runner can read it, and the same findings as plain lines on stderr, because CI shows stderr on a failure and a bare exit code says only that something is wrong. No Factorio anywhere in this path. That is the requirement: a fixture cannot be committed without saying where it came from, and enforcing that must not need the game.
Nineteen captured or copied from Factorio 2.1.14 (build 87180, mac-arm64, steam), and one that is not ground truth at all: the FactorioTools trim spec is a caller's config, so any captured-from version would be a false claim. It is named in notFixtures with that reason rather than filtered out. maxUnknown starts at 0, which is the strongest form: every fixture here has a real version, so the ratchet is already at its floor. The migration copies are checkable rather than asserted - 'diff -rq' against the install's own directories reports no differing file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82
The other half of enforcement. A fixture captured on 2.1.11 is not wrong because the binary moved on, so this exits 0 on every comparison result and errors only when there is no install to compare against. One difference from the prior art: a fixture can be NEWER than the selected binary, which happens whenever an older install is chosen on purpose. MapWebUI's script labels every unequal version 'the binary is newer', which is backwards in exactly that case. install::select and VersionInfo::triple pull together the selection rule and the triple formatting that three call sites had each written out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82
Task 4's subject is 20 files this crate also wrote the manifest for, so it can only confirm what its author already believed. FactorioMapWebUI's manifest has 100 hand-written entries and a TypeScript test of its own. Asserts only what both implementations claim - no dangling entry, every entry well formed - and prints coverage rather than asserting it, since another repo's choices are not this crate's to gate and a count would break the moment that repo adds a fixture. Gated on FACTORIO_ORACLE_PROVENANCE_DIR, so CI and a fresh clone skip it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82
Four beliefs the design held that the data did not: a real entry has two keys and not eight, evidence is free text rather than a grade enum, an extension allowlist leaves captured ground truth unrecorded, and keys have to be relative paths because the flat directory the rule came from has no subdirectories. Also a negative result, so nobody rules it out twice: serde's flatten works fine under arbitrary_precision.
45 of 100 was wrong; the enum stated | inferred | unknown accepts stated (48) + inferred (4) = 52 of the 100 first words, so it rejects the other 48, not 45. Added the arithmetic inline so the number is checkable rather than asserted.
Consolidated fix wave from the whole-branch review, all applied together: - Fix the 45-vs-48 arithmetic error in entry_problem's doc comment and show the per-word counts so the number is checkable from the comment itself, matching CLAUDE.md's already-correct wording. - Rewrite CLAUDE.md's stale test-count paragraph (113 unit / 6 install-gated was wrong on both counts) with a real per-file breakdown: 149 unit tests (159 after this commit's new tests), and only 4 of the 8 integration tests are install-gated - tests/provenance.rs's second test is gated on FACTORIO_ORACLE_PROVENANCE_DIR, not an install. - Reorder install::select's parameters to (home, factorio, env_bin, version) so the signature matches its documented precedence instead of contradicting it. Behaviour-preserving: both main.rs call sites updated, body unchanged, and all 4 install-gated tests ran and passed against a real 2.1.14 install. - Add tests for CheckReport::to_json (all four Ratchet states, exact key names, null maxUnknown for Undeclared) and for summary() called directly, since neither was exercised by a passing run before. - Extend the fixture walk's skip list to Thumbs.db and desktop.ini (case-insensitive), Windows' equivalent of .DS_Store, with tests. - Fix CLAUDE.md's dotfile-skip bullet to say directories are skipped whole, not just dotfiles, and fold in the Windows names. - Drop Manifest::comment, an unread field with only a tautological test. - Add tests asserting render()'s two standing strings, so a future swap is caught. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82
The FIX 2 rewrite documented 149 unit tests - the count before that same fix wave added 10 new tests - instead of the 159 cargo test --all-targets actually reports. Corrected the one number; the rest of the paragraph (the 8-integration-test breakdown, the named install-gated test, the FACTORIO_ORACLE_PROVENANCE_DIR nuance, and the 4-install-gated-tests figure) was already accurate and is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82
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
1 participant
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.
Build-order step 6 from the shared oracle design: record which Factorio version each fixture was captured from, beside the fixtures rather than inside them, and enforce it.
Two commands, and the split is the point
checkanswers what a machine can settle: is every file recorded, does every entry name a file that exists, is every entry well formed, and has theunknowncount grown. It runs in CI on a machine that has never had the game.reportanswers what a machine cannot. A fixture captured on 2.1.11 is not wrong because the binary moved on, it just has not been re-validated since, and whether that matters depends on whether the subsystem changed. So it never fails a build.What measuring the prior art changed
The design sketched this from FactorioMapWebUI's
PROVENANCE.json. Reading the real file first disagreed with the sketch three times:factorioVersionandevidence.factorioBuild,branch,loadedMods,capturedOn,capturedByandtargetVersionRangeappear zero times. A checker built to the sketch would reject the only real manifest there is, so the other six are optional and carried through untouched.evidenceis free text. The design wrote it as an enum. First word across those entries:stated48,captured34,RE-CAPTURED8,inferred4,re-captured3,UNDOCUMENTED1, and twice justthe. The enum would reject 48 of 100. The grade that is a real field isfactorioVersion: "unknown", which is what the ratchet counts..jsonand.png. The same directory holds 10.txtmap exchange strings, and 8 of the 10 are read as ground truth bydecode.spec.ts,encode.spec.tsandjsonExport.spec.ts. None has an entry and none can get one while the glob decides. Filed as Fixture provenance misses 10 .txt files; 8 are read as ground truth FactorioMapWebUI#240.So this tool names every file. A deliberate non-fixture goes in
notFixtureswith a reason, which costs a sentence. An ignore rule that costs nothing gets used without thinking.Two deliberate departures
The ratchet fails in both directions. Too many
unknownentries fails, and so does a count below the declared number, with a message naming the lower value. The prior art asserts only<=, and its comment has read "lower it when one gets resolved" for as long as the number has read 1. A cap that never has to fall is not a ratchet.A fixture can be newer than the binary. That happens whenever an older install is selected on purpose. The prior art labels every unequal version "the binary is newer", which is backwards in exactly that case.
Dogfooding, and a cross-check
This repo's own 20 fixtures now have a manifest, enforced by an always-on test. Every evidence claim in it is checkable rather than asserted, and each was audited against the real install during review: all 16 migration
.jsoncopies are byte-identical to Factorio 2.1.14's own (diff -rqreports no differing file),expected-factorio-oracle-2.1.14.jsonis identical to FactorioTools' committed fixture, and the trim spec is correctly recorded as a hand-written caller config rather than captured data.maxUnknownstarts at 0, the strongest form: every fixture here has a real version, so the ratchet is already at its floor.There is also a test gated on
FACTORIO_ORACLE_PROVENANCE_DIRthat runs the same rules against another repo's manifest. Pointed at MapWebUI's 100 hand-written entries it reports 0 dangling and 0 malformed - a Rust implementation and a TypeScript one, written months apart, reaching the same verdict. It asserts only those two claims and prints the rest, so another repo adding a fixture cannot break this repo's CI.A pre-existing error this uncovered
CLAUDE.mdhas claimed 6 install-gated integration tests for as long as the file has existed. The real number is 4:tests/acceptance.rshas three tests but onlythe_real_install_reproduces_it_toocarries a skip guard, and the other two run offline against committed fixtures. Corrected here, along with the unit count.Testing
159 unit tests, 8 integration tests across three files, 167 total. All green. The 4 install-gated tests ran against a real Factorio 2.1.14 (build 87180, mac-arm64, steam) rather than skipping. CI has no Factorio, so they will skip there - which is exactly why the count line in
CLAUDE.mdmatters.Plan:
docs/superpowers/plans/2026-08-17-factorio-oracle-provenance.mdin FactorioTools, including a "Corrections found while executing this plan" section recording the eight things execution disproved.🤖 Generated with Claude Code
https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82