From d35cae04608d628bff698c72cfa11df0bf22962b Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 16:42:43 -0700 Subject: [PATCH 01/11] Add the provenance manifest, with two required keys and no more 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. --- src/lib.rs | 1 + src/provenance/manifest.rs | 215 +++++++++++++++++++++++++++++++++++++ src/provenance/mod.rs | 103 ++++++++++++++++++ 3 files changed, 319 insertions(+) create mode 100644 src/provenance/manifest.rs create mode 100644 src/provenance/mod.rs diff --git a/src/lib.rs b/src/lib.rs index 148f2f5..63f618d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,6 +12,7 @@ pub mod lua; pub mod numbers; pub mod outcome; pub mod probe; +pub mod provenance; pub mod run; pub mod scaffold; pub mod spawn; diff --git a/src/provenance/manifest.rs b/src/provenance/manifest.rs new file mode 100644 index 0000000..d0c8271 --- /dev/null +++ b/src/provenance/manifest.rs @@ -0,0 +1,215 @@ +//! The provenance manifest: which Factorio version each fixture came from. +//! +//! Provenance lives beside the fixtures rather than inside them. Several +//! fixtures are verbatim copies of the game's own JSON and are asserted key for +//! key, so a metadata key added inside one would be data pollution rather than +//! annotation. +//! +//! The shape is copied from FactorioMapWebUI's `test/fixtures/PROVENANCE.json`, +//! which is the only version of this that has run long enough to be worth +//! copying. Measured 2026-08-17 across its 100 entries: every entry carries +//! exactly two keys, `factorioVersion` and `evidence`, and none of the richer +//! keys the design sketched appears even once. So those two are required and +//! everything else is carried through untouched. A checker demanding the +//! sketched shape would reject the only real manifest there is. + +use serde::Deserialize; +use std::collections::BTreeMap; +use std::path::Path; + +/// The file name, one per fixture directory. +pub const MANIFEST_NAME: &str = "PROVENANCE.json"; + +/// The version string meaning "nobody wrote it down". +pub const UNKNOWN: &str = "unknown"; + +#[derive(Debug, Clone, Deserialize)] +pub struct Manifest { + /// An array of strings, so a long explanation stays readable in a diff. + #[serde(rename = "_comment", default)] + pub comment: Vec, + + /// The ratchet: how many entries may say `unknown`. + /// + /// `None` means the manifest never declared one, which `check` reports as + /// its own finding rather than treating as zero. "We allow none" and "we + /// never decided" are different claims and should not look the same. + #[serde(rename = "maxUnknown", default)] + pub max_unknown: Option, + + /// Keyed by path relative to the manifest, with forward slashes on every + /// platform. Relative rather than bare filenames because this crate's own + /// fixture tree is two levels deep. MapWebUI's directory is flat, so bare + /// names were never tested against a tree. + /// + /// The value stays a `Value` rather than a struct with a flattened tail. + /// Only two keys are required, and every other key has to survive a round + /// trip untouched. (`#[serde(flatten)]` does work under this crate's + /// `arbitrary_precision` feature - measured 2026-08-17 - so this is a + /// choice about the data, not a workaround.) + pub fixtures: BTreeMap, + + /// Files in the tree that are deliberately not ground truth, each with its + /// reason. Naming one costs a sentence, exactly as `evidence` does. An + /// extension allowlist costs nothing, which is how eight captured map + /// exchange strings sat unrecorded in MapWebUI's fixture directory. + #[serde(rename = "notFixtures", default)] + pub not_fixtures: BTreeMap, +} + +/// Reads the manifest that belongs to `dir`. +pub fn load(dir: &Path) -> anyhow::Result { + let path = dir.join(MANIFEST_NAME); + let text = std::fs::read_to_string(&path) + .map_err(|e| anyhow::anyhow!("no provenance manifest at {}: {e}", path.display()))?; + serde_json::from_str(&text) + .map_err(|e| anyhow::anyhow!("{} is not a valid manifest: {e}", path.display())) +} + +/// `"2.1.14"` becomes `(2, 1, 14)`. `None` for anything else, `unknown` +/// included. +pub fn parse_triple(version: &str) -> Option<(u32, u32, u32)> { + let parts: Vec<&str> = version.split('.').collect(); + if parts.len() != 3 { + return None; + } + // std's integer parser accepts a leading plus and this must not, so the + // digits are checked before parsing rather than after. + if parts + .iter() + .any(|p| p.is_empty() || !p.bytes().all(|b| b.is_ascii_digit())) + { + return None; + } + Some(( + parts[0].parse().ok()?, + parts[1].parse().ok()?, + parts[2].parse().ok()?, + )) +} + +/// What is wrong with one entry, or `None` if it is well formed. +/// +/// `evidence` is checked for being present and non-blank, and nothing more. +/// The design wrote it as an enum of `stated`, `inferred` and `unknown` plus +/// free text. Measured across MapWebUI's 100 entries, the first word is +/// `stated` 48 times, `captured` 34, `RE-CAPTURED` 8, `inferred` 4, +/// `re-captured` 3, `UNDOCUMENTED` once, and twice it is just `the`. The grade +/// is a habit of phrasing, not a field, and enforcing it would reject 45 of the +/// 100. The grade that is real is `factorioVersion == "unknown"`, and that is +/// what the ratchet counts. +pub fn entry_problem(entry: &serde_json::Value) -> Option { + let Some(map) = entry.as_object() else { + return Some("entry is not an object".to_string()); + }; + match map.get("factorioVersion").and_then(|v| v.as_str()) { + None => return Some("factorioVersion is missing, or is not a string".to_string()), + Some(v) if v != UNKNOWN && parse_triple(v).is_none() => { + return Some(format!( + "factorioVersion {v:?} is neither \"a.b.c\" nor \"unknown\"" + )) + } + Some(_) => {} + } + match map.get("evidence").and_then(|v| v.as_str()) { + None => Some("evidence is missing, or is not a string".to_string()), + Some(e) if e.trim().is_empty() => Some("evidence is empty".to_string()), + Some(_) => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_a_manifest_with_only_the_two_required_keys() { + // This is the exact shape of every one of MapWebUI's 100 entries. + let text = r#"{ + "_comment": ["why this file exists"], + "maxUnknown": 1, + "fixtures": { + "a.json": { "factorioVersion": "2.1.14", "evidence": "stated" }, + "b.png": { "factorioVersion": "unknown", "evidence": "UNDOCUMENTED" } + }, + "notFixtures": { "README.md": "prose, not ground truth" } + }"#; + let m: Manifest = serde_json::from_str(text).expect("should parse"); + assert_eq!(m.comment.len(), 1); + assert_eq!(m.max_unknown, Some(1)); + assert_eq!(m.fixtures.len(), 2); + assert_eq!(m.not_fixtures.len(), 1); + } + + #[test] + fn keeps_keys_it_does_not_know_about() { + // The design sketched capturedBy, branch and targetVersionRange. No + // real entry uses them, so they are neither required nor stripped. + let text = r#"{ + "maxUnknown": 0, + "fixtures": { + "a.json": { + "factorioVersion": "2.1.14", + "evidence": "stated", + "branch": "experimental", + "capturedBy": "tools/oracle/probe-rail-placement.mjs" + } + } + }"#; + let m: Manifest = serde_json::from_str(text).unwrap(); + assert_eq!(m.fixtures["a.json"]["branch"], "experimental"); + assert!(entry_problem(&m.fixtures["a.json"]).is_none()); + } + + #[test] + fn an_absent_ratchet_is_not_the_same_as_zero() { + let text = r#"{ "fixtures": {} }"#; + let m: Manifest = serde_json::from_str(text).unwrap(); + assert_eq!(m.max_unknown, None); + } + + #[test] + fn a_well_formed_entry_has_no_problem() { + let entry = serde_json::json!({ "factorioVersion": "2.0.77", "evidence": "stated" }); + assert_eq!(entry_problem(&entry), None); + } + + #[test] + fn unknown_is_a_legal_version() { + let entry = + serde_json::json!({ "factorioVersion": "unknown", "evidence": "never recorded" }); + assert_eq!(entry_problem(&entry), None); + } + + #[test] + fn rejects_a_version_that_is_not_three_numbers_or_unknown() { + for bad in ["2.1", "2.1.14-rc1", "v2.1.14", "2.1.14 ", ""] { + let entry = serde_json::json!({ "factorioVersion": bad, "evidence": "x" }); + assert!( + entry_problem(&entry).is_some(), + "{bad:?} should be rejected" + ); + } + } + + #[test] + fn rejects_a_missing_or_empty_evidence() { + let missing = serde_json::json!({ "factorioVersion": "2.1.14" }); + assert!(entry_problem(&missing).is_some()); + let empty = serde_json::json!({ "factorioVersion": "2.1.14", "evidence": " " }); + assert!(entry_problem(&empty).is_some()); + } + + #[test] + fn rejects_an_entry_that_is_not_an_object() { + assert!(entry_problem(&serde_json::json!("2.1.14")).is_some()); + } + + #[test] + fn parses_a_version_into_parts() { + assert_eq!(parse_triple("2.1.14"), Some((2, 1, 14))); + assert_eq!(parse_triple("unknown"), None); + // std's u32 parser accepts a leading plus. A version does not. + assert_eq!(parse_triple("+2.1.14"), None); + } +} diff --git a/src/provenance/mod.rs b/src/provenance/mod.rs new file mode 100644 index 0000000..d1d9ef7 --- /dev/null +++ b/src/provenance/mod.rs @@ -0,0 +1,103 @@ +//! Recording which Factorio version each fixture was captured from, and +//! checking that the record stays honest. +//! +//! Two halves, and the split is the point. `check` needs no Factorio and +//! fails: it is the always-on test that a fixture cannot be committed without +//! saying where it came from. `report` needs a binary and never fails: a +//! fixture captured on 2.1.11 is not wrong because the binary moved on, so +//! deciding whether a gap matters is a human's job. + +// `check` arrives in Task 2 and `report` in Task 5. Each task adds its own +// line here, so the crate compiles at the end of every task rather than only +// at the end of the plan. +pub mod manifest; + +use std::path::Path; + +/// Every file under `root`, as a path relative to `root` with forward slashes, +/// sorted. +/// +/// Two exclusions, both deliberate: +/// +/// - The manifest itself. It is the record, not the record's subject. +/// - Anything whose name starts with a dot. `.DS_Store` appears in any +/// directory a Finder window has opened, so demanding an entry for it would +/// make this fail on a Mac and pass in CI. A check that fails only on the +/// machine that can fix it is worse than no check. +pub fn walk_fixtures(root: &Path) -> std::io::Result> { + let mut out = Vec::new(); + collect(root, root, &mut out)?; + out.sort(); + Ok(out) +} + +fn collect(root: &Path, dir: &Path, out: &mut Vec) -> std::io::Result<()> { + for entry in std::fs::read_dir(dir)? { + let entry = entry?; + if entry.file_name().to_string_lossy().starts_with('.') { + continue; + } + let path = entry.path(); + if path.is_dir() { + collect(root, &path, out)?; + continue; + } + let rel = path.strip_prefix(root).unwrap_or(&path); + // Joined by hand rather than by `Path::display`, so a Windows run and a + // macOS run produce the same key for the same file. A manifest is + // committed, and a backslash in it would be a permanent diff. + let key = rel + .components() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect::>() + .join("/"); + if key != manifest::MANIFEST_NAME { + out.push(key); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn write(root: &Path, rel: &str) { + let path = root.join(rel); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, "{}").unwrap(); + } + + #[test] + fn walks_a_tree_into_relative_forward_slash_paths() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(root, "a.json"); + write(root, "data/base/migrations/2.0.0.json"); + // A real fixture name, space included. + write( + root, + "data/base/migrations/1.2.0 stack inserter rename.json", + ); + let found = walk_fixtures(root).unwrap(); + assert_eq!( + found, + vec![ + "a.json".to_string(), + "data/base/migrations/1.2.0 stack inserter rename.json".to_string(), + "data/base/migrations/2.0.0.json".to_string(), + ] + ); + } + + #[test] + fn leaves_out_the_manifest_and_dotfiles() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(root, "a.json"); + write(root, manifest::MANIFEST_NAME); + write(root, ".DS_Store"); + assert_eq!(walk_fixtures(root).unwrap(), vec!["a.json".to_string()]); + } +} From b41b7beaff857229ce3e9ed20019e47231e15036 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 16:46:51 -0700 Subject: [PATCH 02/11] Check a fixture directory against its manifest 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. --- src/provenance/check.rs | 347 ++++++++++++++++++++++++++++++++++++++++ src/provenance/mod.rs | 1 + 2 files changed, 348 insertions(+) create mode 100644 src/provenance/check.rs diff --git a/src/provenance/check.rs b/src/provenance/check.rs new file mode 100644 index 0000000..40e9112 --- /dev/null +++ b/src/provenance/check.rs @@ -0,0 +1,347 @@ +//! The structural half of enforcement: coverage, dangling entries, +//! well-formedness, and the `unknown` ratchet. +//! +//! This needs no Factorio, which is the whole point. It answers one question - +//! does the record still describe the directory? - and a consumer can run it in +//! CI on a machine that has never had the game. +//! +//! Deliberately not here: whether a recorded version is old. That needs a +//! binary and it needs a human, so it lives in `report`. + +use crate::provenance::manifest::{entry_problem, Manifest, UNKNOWN}; +use std::collections::BTreeSet; +use std::path::Path; + +/// How the `unknown` count compares to what the manifest declared. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Ratchet { + /// Exactly the declared number. + Ok, + /// More than declared. A fixture was committed without recording where it + /// came from. + Exceeded { count: usize, max: usize }, + /// Fewer than declared. A gap was closed and the number was not lowered. + /// + /// This is stricter than the prior art on purpose. MapWebUI's test asserts + /// only `<=`, and its comment has said "lower it when one gets resolved" + /// for as long as the number has been 1. A cap that never has to fall is + /// not a ratchet. + Slack { count: usize, max: usize }, + /// The manifest never declared a number, which is not the same as + /// declaring zero. + Undeclared { count: usize }, +} + +/// One entry that is not well formed. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Malformed { + pub entry: String, + pub problem: String, +} + +/// Everything the check found. Empty lists and `Ratchet::Ok` mean a pass. +#[derive(Debug, Clone)] +pub struct CheckReport { + pub fixtures: usize, + pub not_fixtures: usize, + /// On disk, named by neither list. + pub missing: Vec, + /// Named by a list, not on disk. + pub dangling: Vec, + pub malformed: Vec, + /// Fixture entries recording `unknown`, which is what the ratchet counts. + pub unknown: Vec, + pub ratchet: Ratchet, +} + +impl CheckReport { + pub fn ok(&self) -> bool { + self.missing.is_empty() + && self.dangling.is_empty() + && self.malformed.is_empty() + && self.ratchet == Ratchet::Ok + } + + /// The JSON a consumer's own test runner reads. + pub fn to_json(&self, dir: &Path) -> serde_json::Value { + let (ratchet, max_unknown) = match &self.ratchet { + Ratchet::Ok => ("ok", serde_json::json!(self.unknown.len())), + Ratchet::Exceeded { max, .. } => ("exceeded", serde_json::json!(max)), + Ratchet::Slack { max, .. } => ("slack", serde_json::json!(max)), + Ratchet::Undeclared { .. } => ("undeclared", serde_json::Value::Null), + }; + serde_json::json!({ + "ok": self.ok(), + "dir": dir, + "fixtures": self.fixtures, + "notFixtures": self.not_fixtures, + "missing": self.missing, + "dangling": self.dangling, + "malformed": self.malformed + .iter() + .map(|m| serde_json::json!({ "entry": m.entry, "problem": m.problem })) + .collect::>(), + "unknown": self.unknown, + "maxUnknown": max_unknown, + "ratchet": ratchet, + }) + } + + /// Short lines for a human, printed to stderr when the check fails. The + /// JSON is the interface; this is the error message. + pub fn summary(&self) -> Vec { + let mut lines = Vec::new(); + for name in &self.missing { + lines.push(format!("no provenance entry: {name}")); + } + for name in &self.dangling { + lines.push(format!("entry names a file that is not there: {name}")); + } + for m in &self.malformed { + lines.push(format!("{}: {}", m.entry, m.problem)); + } + match &self.ratchet { + Ratchet::Ok => {} + Ratchet::Exceeded { count, max } => lines.push(format!( + "{count} entries record an unknown version, and the manifest allows {max}. \ + Re-capture against a known binary rather than raising maxUnknown." + )), + Ratchet::Slack { count, max } => lines.push(format!( + "the manifest allows {max} unknown entries and there are now {count}. \ + Lower maxUnknown to {count} so the number can only keep falling." + )), + Ratchet::Undeclared { count } => lines.push(format!( + "the manifest declares no maxUnknown. There are {count} unknown entries today, \ + so add \"maxUnknown\": {count} to lock that in." + )), + } + lines + } +} + +/// Compares a manifest against the files beside it. +/// +/// `on_disk` comes from `walk_fixtures`, so this stays pure and almost every +/// test needs no filesystem. +pub fn check(manifest: &Manifest, on_disk: &[String]) -> CheckReport { + let present: BTreeSet<&str> = on_disk.iter().map(String::as_str).collect(); + let mut malformed = Vec::new(); + + for (name, entry) in &manifest.fixtures { + if manifest.not_fixtures.contains_key(name) { + malformed.push(Malformed { + entry: name.clone(), + problem: "named as both a fixture and a not-fixture".to_string(), + }); + } + if let Some(problem) = entry_problem(entry) { + malformed.push(Malformed { + entry: name.clone(), + problem, + }); + } + } + + for (name, why) in &manifest.not_fixtures { + if why.trim().is_empty() { + malformed.push(Malformed { + entry: name.clone(), + problem: "a not-fixture must give a reason".to_string(), + }); + } + } + + let named: BTreeSet<&str> = manifest + .fixtures + .keys() + .chain(manifest.not_fixtures.keys()) + .map(String::as_str) + .collect(); + + let missing: Vec = present + .iter() + .filter(|name| !named.contains(*name)) + .map(|name| (*name).to_string()) + .collect(); + let dangling: Vec = named + .iter() + .filter(|name| !present.contains(*name)) + .map(|name| (*name).to_string()) + .collect(); + + let unknown: Vec = manifest + .fixtures + .iter() + .filter(|(_, entry)| entry.get("factorioVersion").and_then(|v| v.as_str()) == Some(UNKNOWN)) + .map(|(name, _)| name.clone()) + .collect(); + + let ratchet = match manifest.max_unknown { + None => Ratchet::Undeclared { + count: unknown.len(), + }, + Some(max) if unknown.len() > max => Ratchet::Exceeded { + count: unknown.len(), + max, + }, + Some(max) if unknown.len() < max => Ratchet::Slack { + count: unknown.len(), + max, + }, + Some(_) => Ratchet::Ok, + }; + + // Both sets iterate in key order, so every list is already sorted. + CheckReport { + fixtures: manifest.fixtures.len(), + not_fixtures: manifest.not_fixtures.len(), + missing, + dangling, + malformed, + unknown, + ratchet, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::provenance::manifest::Manifest; + + /// Builds a manifest from a compact description, so each test shows only + /// what it is about. + fn manifest( + max_unknown: &str, + fixtures: &[(&str, &str)], + not_fixtures: &[(&str, &str)], + ) -> Manifest { + let fixtures: serde_json::Map = fixtures + .iter() + .map(|(name, version)| { + ( + (*name).to_string(), + serde_json::json!({ "factorioVersion": version, "evidence": "stated" }), + ) + }) + .collect(); + let not_fixtures: serde_json::Map = not_fixtures + .iter() + .map(|(name, why)| ((*name).to_string(), serde_json::json!(why))) + .collect(); + let doc = serde_json::json!({ + "maxUnknown": serde_json::from_str::(max_unknown).unwrap(), + "fixtures": fixtures, + "notFixtures": not_fixtures, + }); + serde_json::from_value(doc).expect("test manifest should parse") + } + + #[test] + fn a_directory_that_matches_its_manifest_passes() { + let m = manifest("0", &[("a.json", "2.1.14")], &[("README.md", "prose")]); + let report = check(&m, &["README.md".to_string(), "a.json".to_string()]); + assert!(report.ok(), "{report:?}"); + assert_eq!(report.fixtures, 1); + assert_eq!(report.not_fixtures, 1); + } + + #[test] + fn a_file_named_by_neither_list_is_missing() { + let m = manifest("0", &[("a.json", "2.1.14")], &[]); + let report = check(&m, &["a.json".to_string(), "b.png".to_string()]); + assert_eq!(report.missing, vec!["b.png".to_string()]); + assert!(!report.ok()); + } + + #[test] + fn an_entry_with_no_file_is_dangling() { + let m = manifest("0", &[("a.json", "2.1.14"), ("gone.json", "2.1.14")], &[]); + let report = check(&m, &["a.json".to_string()]); + assert_eq!(report.dangling, vec!["gone.json".to_string()]); + assert!(!report.ok()); + } + + #[test] + fn a_not_fixture_with_no_file_is_dangling_too() { + let m = manifest("0", &[], &[("gone.md", "prose")]); + let report = check(&m, &[]); + assert_eq!(report.dangling, vec!["gone.md".to_string()]); + } + + #[test] + fn an_entry_that_is_not_well_formed_is_reported_by_name() { + let doc = serde_json::json!({ + "maxUnknown": 0, + "fixtures": { "a.json": { "factorioVersion": "2.1", "evidence": "stated" } }, + }); + let m: Manifest = serde_json::from_value(doc).unwrap(); + let report = check(&m, &["a.json".to_string()]); + assert_eq!(report.malformed.len(), 1); + assert_eq!(report.malformed[0].entry, "a.json"); + assert!(report.malformed[0].problem.contains("factorioVersion")); + } + + #[test] + fn a_not_fixture_must_say_why() { + let m = manifest("0", &[], &[("README.md", " ")]); + let report = check(&m, &["README.md".to_string()]); + assert_eq!(report.malformed.len(), 1); + assert!(report.malformed[0].problem.contains("reason")); + } + + #[test] + fn a_file_cannot_be_a_fixture_and_a_not_fixture_at_once() { + let m = manifest("0", &[("a.json", "2.1.14")], &[("a.json", "prose")]); + let report = check(&m, &["a.json".to_string()]); + assert_eq!(report.malformed.len(), 1); + assert!(report.malformed[0].problem.contains("both")); + } + + #[test] + fn the_ratchet_holds_when_the_count_is_exactly_the_declared_number() { + let m = manifest("1", &[("a.json", "unknown")], &[]); + let report = check(&m, &["a.json".to_string()]); + assert_eq!(report.ratchet, Ratchet::Ok); + assert_eq!(report.unknown, vec!["a.json".to_string()]); + assert!(report.ok()); + } + + #[test] + fn the_ratchet_fails_when_another_unknown_arrives() { + let m = manifest("1", &[("a.json", "unknown"), ("b.json", "unknown")], &[]); + let report = check(&m, &["a.json".to_string(), "b.json".to_string()]); + assert_eq!(report.ratchet, Ratchet::Exceeded { count: 2, max: 1 }); + assert!(!report.ok()); + } + + #[test] + fn the_ratchet_also_fails_when_a_gap_is_closed_and_the_number_is_not_lowered() { + // This is what makes it a ratchet rather than a cap. Without it the + // declared number never falls. + let m = manifest("1", &[("a.json", "2.1.14")], &[]); + let report = check(&m, &["a.json".to_string()]); + assert_eq!(report.ratchet, Ratchet::Slack { count: 0, max: 1 }); + assert!(!report.ok()); + } + + #[test] + fn a_manifest_with_no_ratchet_is_a_finding_of_its_own() { + let doc = serde_json::json!({ + "fixtures": { "a.json": { "factorioVersion": "unknown", "evidence": "x" } }, + }); + let m: Manifest = serde_json::from_value(doc).unwrap(); + let report = check(&m, &["a.json".to_string()]); + assert_eq!(report.ratchet, Ratchet::Undeclared { count: 1 }); + assert!(!report.ok()); + } + + #[test] + fn every_list_comes_back_sorted() { + let m = manifest("0", &[], &[]); + let report = check(&m, &["b.json".to_string(), "a.json".to_string()]); + assert_eq!( + report.missing, + vec!["a.json".to_string(), "b.json".to_string()] + ); + } +} diff --git a/src/provenance/mod.rs b/src/provenance/mod.rs index d1d9ef7..c136a35 100644 --- a/src/provenance/mod.rs +++ b/src/provenance/mod.rs @@ -10,6 +10,7 @@ // `check` arrives in Task 2 and `report` in Task 5. Each task adds its own // line here, so the crate compiles at the end of every task rather than only // at the end of the plan. +pub mod check; pub mod manifest; use std::path::Path; From 96d123ea000837db72e55694d5edcfa64f136dd5 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 16:52:19 -0700 Subject: [PATCH 03/11] Sort malformed by entry, and stop claiming it already was 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. --- src/provenance/check.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/provenance/check.rs b/src/provenance/check.rs index 40e9112..c8ebfb2 100644 --- a/src/provenance/check.rs +++ b/src/provenance/check.rs @@ -191,7 +191,15 @@ pub fn check(manifest: &Manifest, on_disk: &[String]) -> CheckReport { Some(_) => Ratchet::Ok, }; - // Both sets iterate in key order, so every list is already sorted. + // `missing`, `dangling` and `unknown` are already sorted: each is built + // from iterating a `BTreeMap`/`BTreeSet`, in one pass. `malformed` is not + // - it is built from two passes, fixtures then not-fixtures, so it is two + // sorted runs concatenated rather than one sorted list. Sorted here, + // stably, so that a name appearing in both passes (named as both a + // fixture and a not-fixture, and also malformed on its own terms) keeps + // its two problems in the order they were found. + malformed.sort_by(|a, b| a.entry.cmp(&b.entry)); + CheckReport { fixtures: manifest.fixtures.len(), not_fixtures: manifest.not_fixtures.len(), @@ -344,4 +352,22 @@ mod tests { vec!["a.json".to_string(), "b.json".to_string()] ); } + + #[test] + fn malformed_is_sorted_across_the_fixtures_and_not_fixtures_passes() { + // `malformed` is built in two passes - fixtures, then not-fixtures - + // so a name from the second pass that sorts before a name from the + // first pass would come back out of order without an explicit sort. + // `z.json` (a malformed fixture) sorts after `a.md` (a not-fixture + // with no reason), but is pushed first. + let doc = serde_json::json!({ + "maxUnknown": 0, + "fixtures": { "z.json": { "factorioVersion": "2.1", "evidence": "stated" } }, + "notFixtures": { "a.md": " " }, + }); + let m: Manifest = serde_json::from_value(doc).unwrap(); + let report = check(&m, &["z.json".to_string(), "a.md".to_string()]); + let names: Vec<&str> = report.malformed.iter().map(|e| e.entry.as_str()).collect(); + assert_eq!(names, vec!["a.md", "z.json"]); + } } From 534bb3fae0d9f082f9a9a2fa6d4fdcb04947c1b4 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 16:55:40 -0700 Subject: [PATCH 04/11] Add provenance check, which exits 1 on a finding 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. --- src/main.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/main.rs b/src/main.rs index f49ccf8..1b2384b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -57,6 +57,11 @@ enum Command { #[arg(long)] check: bool, }, + /// Check and report on fixture provenance + Provenance { + #[command(subcommand)] + action: ProvenanceAction, + }, } #[derive(Subcommand)] @@ -65,6 +70,16 @@ enum InstallsAction { List, } +#[derive(Subcommand)] +enum ProvenanceAction { + /// Check a fixture directory against its PROVENANCE.json. Needs no + /// Factorio, and exits 1 on any finding. + Check { + /// The fixture directory. Its manifest is the PROVENANCE.json inside it. + dir: PathBuf, + }, +} + fn main() -> anyhow::Result<()> { let cli = Cli::parse(); match cli.command { @@ -222,6 +237,27 @@ fn main() -> anyhow::Result<()> { println!("Wrote {}", out.display()); } } + Command::Provenance { + action: ProvenanceAction::Check { dir }, + } => { + let manifest = factorio_oracle::provenance::manifest::load(&dir)?; + let on_disk = factorio_oracle::provenance::walk_fixtures(&dir)?; + let report = factorio_oracle::provenance::check::check(&manifest, &on_disk); + + println!("{}", serde_json::to_string_pretty(&report.to_json(&dir))?); + + if !report.ok() { + // The JSON is the interface and the summary is the error + // message. A consumer's CI prints stderr on a failure and + // nothing else, so a bare exit code would say only that + // something is wrong. + eprintln!("{} provenance findings:", dir.display()); + for line in report.summary() { + eprintln!(" {line}"); + } + std::process::exit(1); + } + } } Ok(()) } From f555f72f1023fb6d46e2f835db2706389c5aacb5 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:00:01 -0700 Subject: [PATCH 05/11] Record where this crate's own 20 fixtures came from 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) Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82 --- tests/fixtures/PROVENANCE.json | 102 +++++++++++++++++++++++++++++++++ tests/provenance.rs | 38 ++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 tests/fixtures/PROVENANCE.json create mode 100644 tests/provenance.rs diff --git a/tests/fixtures/PROVENANCE.json b/tests/fixtures/PROVENANCE.json new file mode 100644 index 0000000..7de3c23 --- /dev/null +++ b/tests/fixtures/PROVENANCE.json @@ -0,0 +1,102 @@ +{ + "_comment": [ + "Which Factorio each fixture's ground truth was captured from.", + "This lives beside the fixtures rather than inside them because most of", + "them are verbatim copies of the game's own JSON, asserted byte for byte", + "by tests/acceptance.rs. A metadata key added inside one would be data", + "pollution, not annotation.", + "Every file in this tree is named, either here or in notFixtures. There is", + "no extension filter: FactorioMapWebUI's manifest globs .json and .png, and", + "eight captured map exchange strings have sat unrecorded in its fixture", + "directory as a result.", + "maxUnknown is a ratchet, not a cap. It fails if it is exceeded, and it", + "fails if the count drops below it and the number is not lowered.", + "An entry is a record of the moment a fixture was captured, not a live", + "claim. Never edit one to make it current, and never edit one to make a", + "test pass. A mismatch is a finding.", + "Enforced by tests/provenance.rs. Compare against an installed binary with", + "'factorio-oracle provenance report tests/fixtures'." + ], + "maxUnknown": 0, + "fixtures": { + "data-raw-slice.json": { + "factorioVersion": "2.1.14", + "evidence": "captured 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'factorio-oracle run' with a dump-data probe produced a 28 MB data-raw-dump.json, reduced here to the ten entity names FactorioTools asks for, taken from every prototype type that carries one, plus the key set of 'module'" + }, + "data/base/migrations/1.1.0.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/base/migrations/1.2.0 stack inserter rename.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/base/migrations/2.0.0-biter-egg.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/base/migrations/2.0.0-internal.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/base/migrations/2.0.0-internal2.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/base/migrations/2.0.0-internal3.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/base/migrations/2.0.0.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/base/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/aquilo-tilesets.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/biolab.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/cargo-bay-that-allows-inserters-to-remove-items.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/internal.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/jelly-yum-rename.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/ore-melting.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/shattered-planet.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/tree-seed.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "data/space-age/migrations/tungsten-belt-rename.json": { + "factorioVersion": "2.1.14", + "evidence": "copied verbatim 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam). 'diff -rq' against the install's own data/space-age/migrations reports no differing file; only the .lua and .txt siblings were left behind" + }, + "doc-html/runtime-api.json": { + "factorioVersion": "2.1.14", + "evidence": "captured 2026-08-17 from Factorio 2.1.14 (build 87180, mac-arm64, steam)'s own doc-html/runtime-api.json, reduced to the single 'direction' entry of the 'defines' array. The full file carries 1,554 entries across several megabytes" + }, + "expected-factorio-oracle-2.1.14.json": { + "factorioVersion": "2.1.14", + "evidence": "copied 2026-08-17 from FactorioTools' committed test/FactorioTools.Test/OilField/factorio-oracle.json, which tools/capture-factorio-oracle.sh produced against Factorio 2.1.14. tests/acceptance.rs rebuilds it byte for byte from the slice beside it, so this version claim is checked on every run rather than asserted once" + } + }, + "notFixtures": { + "factoriotools-trim-spec.json": "not ground truth. A caller's config, transcribed by hand from FactorioTools' tools/trim-factorio-oracle.py allowlists. Nothing in it comes from the game, so any captured-from version would be a false claim" + } +} diff --git a/tests/provenance.rs b/tests/provenance.rs new file mode 100644 index 0000000..2a292d3 --- /dev/null +++ b/tests/provenance.rs @@ -0,0 +1,38 @@ +//! Every fixture in this crate has to say which Factorio it came from. +//! +//! Always on, and it needs no game. That is the split the design asks for: a +//! fixture cannot be committed without stating where it came from, and a +//! deleted one cannot leave a dangling claim behind, and neither of those +//! questions needs the binary. Whether a recorded version is now old is a +//! different question, it needs a binary, and it needs a human, so it lives in +//! `provenance report` and never fails. + +use factorio_oracle::provenance::{check::check, manifest, walk_fixtures}; +use std::path::{Path, PathBuf}; + +fn fixtures_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures") +} + +#[test] +fn every_fixture_records_where_it_came_from() { + let dir = fixtures_dir(); + let manifest = manifest::load(&dir).expect("tests/fixtures/PROVENANCE.json should load"); + let on_disk = walk_fixtures(&dir).expect("tests/fixtures should walk"); + + // A walk that found nothing would pass every other assertion here, so the + // count is checked before the findings are. + assert!( + !on_disk.is_empty(), + "no fixtures found under {}", + dir.display() + ); + + let report = check(&manifest, &on_disk); + assert!( + report.ok(), + "provenance findings under {}:\n {}", + dir.display(), + report.summary().join("\n ") + ); +} From a9470f009c6dd524ce39c47aa0a7c34fe57559a7 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:07:33 -0700 Subject: [PATCH 06/11] Report fixture versions against an install, and never fail 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) Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82 --- src/install.rs | 64 +++++++++++ src/main.rs | 70 +++++++++--- src/provenance/mod.rs | 1 + src/provenance/report.rs | 228 +++++++++++++++++++++++++++++++++++++++ src/version.rs | 12 +++ 5 files changed, 363 insertions(+), 12 deletions(-) create mode 100644 src/provenance/report.rs diff --git a/src/install.rs b/src/install.rs index bd8aec8..1b60848 100644 --- a/src/install.rs +++ b/src/install.rs @@ -152,6 +152,35 @@ pub fn discover(home: &Path, env_bin: Option<&Path>) -> Vec { .collect() } +/// Whether a discovered install answers to `version`. +/// +/// An install whose binary would not run has no version, and is never picked: +/// every command here needs the version, either to stamp it or to build a mod +/// that declares it. +pub fn matches_version(found: &DiscoveredInstall, version: Option<&str>) -> bool { + match (version, &found.version) { + (Some(want), Some(got)) => got.triple() == want, + (None, Some(_)) => true, + _ => false, + } +} + +/// Picks one install. +/// +/// `factorio` wins over `FACTORIO_BIN`, and either is offered as an extra +/// candidate root rather than as the only one, which is what `run` has always +/// done. With no version given, the first install that reported one wins. +pub fn select( + home: &Path, + env_bin: Option<&Path>, + factorio: Option<&Path>, + version: Option<&str>, +) -> Option { + discover(home, factorio.or(env_bin)) + .into_iter() + .find(|d| matches_version(d, version)) +} + #[cfg(test)] mod tests { use super::*; @@ -322,4 +351,39 @@ mod tests { seen.dedup(); assert_eq!(seen.len(), roots.len(), "duplicate candidate in {roots:?}"); } + + fn discovered(version_line: Option<&str>) -> DiscoveredInstall { + DiscoveredInstall { + layout: InstallLayout { + root: PathBuf::from("/somewhere"), + binary: PathBuf::from("/somewhere/bin/x64/factorio"), + data_dir: PathBuf::from("/somewhere/data"), + doc_dir: PathBuf::from("/somewhere/doc-html"), + }, + version: version_line.and_then(crate::version::parse_version_line), + } + } + + #[test] + fn an_exact_version_is_what_matches() { + let found = discovered(Some("Version: 2.1.14 (build 87180, mac-arm64, steam)")); + assert!(matches_version(&found, Some("2.1.14"))); + assert!(!matches_version(&found, Some("2.1.13"))); + // major.minor is what a mod declares, not what selects an install. + assert!(!matches_version(&found, Some("2.1"))); + } + + #[test] + fn no_version_asked_for_takes_any_install_that_has_one() { + assert!(matches_version( + &discovered(Some("Version: 2.0.77 (build 84539, mac-arm64, full)")), + None + )); + } + + #[test] + fn an_install_whose_binary_will_not_run_is_never_picked() { + assert!(!matches_version(&discovered(None), None)); + assert!(!matches_version(&discovered(None), Some("2.1.14"))); + } } diff --git a/src/main.rs b/src/main.rs index 1b2384b..1cbe791 100644 --- a/src/main.rs +++ b/src/main.rs @@ -78,6 +78,18 @@ enum ProvenanceAction { /// The fixture directory. Its manifest is the PROVENANCE.json inside it. dir: PathBuf, }, + /// Compare each fixture's recorded version against an install. Always + /// exits 0, because deciding whether a version gap matters needs a human. + Report { + /// The fixture directory + dir: PathBuf, + /// Select an install by version, for example 2.0.77 + #[arg(long)] + version: Option, + /// Select an install by path + #[arg(long)] + factorio: Option, + }, } fn main() -> anyhow::Result<()> { @@ -98,7 +110,7 @@ fn main() -> anyhow::Result<()> { "binary": d.layout.binary, "dataDir": d.layout.data_dir, "docDir": d.layout.doc_dir, - "version": d.version.as_ref().map(|v| format!("{}.{}.{}", v.major, v.minor, v.patch)), + "version": d.version.as_ref().map(|v| v.triple()), "modFactorioVersion": d.version.as_ref().map(|v| v.major_minor()), "buildLine": d.version.as_ref().map(|v| v.line.clone()), }) @@ -122,17 +134,13 @@ fn main() -> anyhow::Result<()> { let spec: factorio_oracle::probe::ProbeSpec = serde_json::from_str(&std::fs::read_to_string(&probe)?)?; - let installs = install::discover(&home, factorio.as_deref().or(env_bin.as_deref())); - let chosen = installs - .into_iter() - .find(|d| match (&version, &d.version) { - (Some(want), Some(got)) => { - format!("{}.{}.{}", got.major, got.minor, got.patch) == *want - } - (None, Some(_)) => true, - _ => false, - }) - .ok_or_else(|| anyhow::anyhow!("no Factorio install matched"))?; + let chosen = install::select( + &home, + env_bin.as_deref(), + factorio.as_deref(), + version.as_deref(), + ) + .ok_or_else(|| anyhow::anyhow!("no Factorio install matched"))?; let work = match work_dir { Some(dir) => { @@ -258,6 +266,44 @@ fn main() -> anyhow::Result<()> { std::process::exit(1); } } + Command::Provenance { + action: + ProvenanceAction::Report { + dir, + version, + factorio, + }, + } => { + let manifest = factorio_oracle::provenance::manifest::load(&dir)?; + let home = PathBuf::from(std::env::var("HOME").unwrap_or_default()); + let env_bin = std::env::var_os("FACTORIO_BIN").map(PathBuf::from); + + // No install is an error, because there is nothing to compare + // against. Every comparison result is not: the whole point of this + // half is that a version gap is a finding for a human, not a + // failing build. + let chosen = install::select( + &home, + env_bin.as_deref(), + factorio.as_deref(), + version.as_deref(), + ) + .ok_or_else(|| { + anyhow::anyhow!( + "no Factorio install matched, so there is nothing to compare against" + ) + })?; + let found = chosen + .version + .expect("select filters to installs with a version"); + + print!( + "{}", + factorio_oracle::provenance::report::render( + &factorio_oracle::provenance::report::compare(&manifest, &found.triple()) + ) + ); + } } Ok(()) } diff --git a/src/provenance/mod.rs b/src/provenance/mod.rs index c136a35..6599ef1 100644 --- a/src/provenance/mod.rs +++ b/src/provenance/mod.rs @@ -12,6 +12,7 @@ // at the end of the plan. pub mod check; pub mod manifest; +pub mod report; use std::path::Path; diff --git a/src/provenance/report.rs b/src/provenance/report.rs new file mode 100644 index 0000000..c7cbe88 --- /dev/null +++ b/src/provenance/report.rs @@ -0,0 +1,228 @@ +//! The version comparison: which fixtures were captured on an older Factorio +//! than the one selected. +//! +//! This is a report, not a gate, and the caller always exits 0. A fixture +//! captured on 2.1.11 is not wrong because the binary moved on - it means that +//! ground truth has not been re-validated since, and whether the gap matters +//! depends on whether the subsystem changed. That is a human's call, so this +//! never gets a say in whether a build passes. + +use crate::provenance::manifest::{parse_triple, Manifest, UNKNOWN}; +use std::collections::BTreeMap; + +/// Where one recorded version stands against the selected binary. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Standing { + Current, + OlderThanBinary, + /// Captured on a newer game than the one selected, which happens whenever + /// an older install is selected on purpose. The prior art has no such case + /// and labels it "the binary is newer", which is backwards. + NewerThanBinary, +} + +/// Every fixture recording one version. +#[derive(Debug, Clone)] +pub struct Group { + pub version: String, + pub names: Vec, + pub standing: Standing, +} + +#[derive(Debug, Clone)] +pub struct VersionReport { + pub binary_version: String, + /// Oldest recorded version first. + pub groups: Vec, + /// Entries recording `unknown`, and entries whose version will not parse. + /// The second kind is `check`'s to reject; this one only has to avoid + /// losing it. + pub unknown: Vec, + pub stale: usize, +} + +pub fn compare(manifest: &Manifest, binary_version: &str) -> VersionReport { + let binary_key = parse_triple(binary_version); + let mut grouped: BTreeMap<(u32, u32, u32), (String, Vec)> = BTreeMap::new(); + let mut unknown = Vec::new(); + + for (name, entry) in &manifest.fixtures { + let recorded = entry + .get("factorioVersion") + .and_then(|v| v.as_str()) + .unwrap_or(UNKNOWN); + match parse_triple(recorded) { + Some(key) => grouped + .entry(key) + .or_insert_with(|| (recorded.to_string(), Vec::new())) + .1 + .push(name.clone()), + None => unknown.push(name.clone()), + } + } + + let mut stale = 0; + let groups: Vec = grouped + .into_iter() + .map(|(key, (version, names))| { + let standing = match binary_key { + Some(binary) if key < binary => { + stale += names.len(); + Standing::OlderThanBinary + } + Some(binary) if key > binary => Standing::NewerThanBinary, + _ => Standing::Current, + }; + Group { + version, + names, + standing, + } + }) + .collect(); + + VersionReport { + binary_version: binary_version.to_string(), + groups, + unknown, + stale, + } +} + +/// The text a human reads. Laid out like MapWebUI's `refs:sync --fixtures`, +/// because that output has been read enough times to be worth keeping. +pub fn render(report: &VersionReport) -> String { + let mut out = format!( + "Fixture ground truth vs the installed binary ({}):\n", + report.binary_version + ); + for group in &report.groups { + let mark = match group.standing { + Standing::Current => "current".to_string(), + Standing::OlderThanBinary => format!("{} is newer", report.binary_version), + Standing::NewerThanBinary => format!("newer than {}", report.binary_version), + }; + out.push_str(&format!( + " {:9} {:3} fixture(s) {}\n", + group.version, + group.names.len(), + mark + )); + } + if !report.unknown.is_empty() { + out.push_str(&format!( + " {:9} {:3} fixture(s) provenance never recorded\n", + "unknown", + report.unknown.len() + )); + for name in &report.unknown { + out.push_str(&format!(" {name}\n")); + } + } + if report.stale > 0 { + out.push_str(&format!( + "\n{} fixture(s) predate the installed binary. Not necessarily wrong -\n\ + re-capture only where the subsystem changed between those versions.\n", + report.stale + )); + } else { + out.push_str("\nNo fixture predates the installed binary.\n"); + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::provenance::manifest::Manifest; + + fn manifest(fixtures: &[(&str, &str)]) -> Manifest { + let fixtures: serde_json::Map = fixtures + .iter() + .map(|(name, version)| { + ( + (*name).to_string(), + serde_json::json!({ "factorioVersion": version, "evidence": "stated" }), + ) + }) + .collect(); + serde_json::from_value(serde_json::json!({ "fixtures": fixtures })) + .expect("test manifest should parse") + } + + #[test] + fn groups_by_recorded_version_oldest_first() { + let m = manifest(&[ + ("new.json", "2.1.14"), + ("old.json", "2.1.9"), + ("mid.json", "2.1.11"), + ]); + let r = compare(&m, "2.1.14"); + let versions: Vec<&str> = r.groups.iter().map(|g| g.version.as_str()).collect(); + assert_eq!(versions, vec!["2.1.9", "2.1.11", "2.1.14"]); + } + + #[test] + fn sorts_by_number_and_not_by_string() { + // "2.1.9" sorts after "2.1.14" as text, which is the bug this rules out. + let m = manifest(&[("a.json", "2.1.14"), ("b.json", "2.1.9")]); + let r = compare(&m, "2.1.14"); + assert_eq!(r.groups[0].version, "2.1.9"); + } + + #[test] + fn counts_only_the_fixtures_older_than_the_binary_as_stale() { + let m = manifest(&[ + ("a.json", "2.1.11"), + ("b.json", "2.1.11"), + ("c.json", "2.1.14"), + ]); + let r = compare(&m, "2.1.14"); + assert_eq!(r.stale, 2); + } + + #[test] + fn a_fixture_newer_than_the_binary_is_neither_current_nor_stale() { + // The prior art marks everything unequal as "the binary is newer", + // which is wrong for exactly this case: an older install selected + // deliberately, against fixtures captured on a newer one. + let m = manifest(&[("a.json", "2.1.14")]); + let r = compare(&m, "2.0.77"); + assert_eq!(r.groups[0].standing, Standing::NewerThanBinary); + assert_eq!(r.stale, 0); + } + + #[test] + fn unknown_entries_are_listed_apart_from_the_versions() { + let m = manifest(&[("a.json", "unknown"), ("b.json", "2.1.14")]); + let r = compare(&m, "2.1.14"); + assert_eq!(r.unknown, vec!["a.json".to_string()]); + assert_eq!(r.groups.len(), 1); + } + + #[test] + fn a_version_that_will_not_parse_is_treated_as_unknown_rather_than_dropped() { + // check() is what rejects a malformed entry. This one never fails, so + // it has to show the entry somewhere rather than lose it. + let m = manifest(&[("a.json", "2.1")]); + let r = compare(&m, "2.1.14"); + assert_eq!(r.unknown, vec!["a.json".to_string()]); + } + + #[test] + fn the_rendered_report_names_the_binary_and_every_group() { + let m = manifest(&[("a.json", "2.1.11"), ("b.json", "2.1.14")]); + let text = render(&compare(&m, "2.1.14")); + assert!(text.contains("2.1.14")); + assert!(text.contains("2.1.11")); + assert!(text.contains("1 fixture(s) predate the installed binary")); + } + + #[test] + fn a_clean_report_says_so_instead_of_printing_a_warning() { + let m = manifest(&[("a.json", "2.1.14")]); + let text = render(&compare(&m, "2.1.14")); + assert!(text.contains("No fixture predates the installed binary.")); + assert!(!text.contains("predate the installed binary. Not necessarily")); + } +} diff --git a/src/version.rs b/src/version.rs index dce9888..576c073 100644 --- a/src/version.rs +++ b/src/version.rs @@ -20,6 +20,12 @@ impl VersionInfo { pub fn major_minor(&self) -> String { format!("{}.{}", self.major, self.minor) } + + /// The `major.minor.patch` string, which is what a provenance entry + /// records and what `--version` selects on. + pub fn triple(&self) -> String { + format!("{}.{}.{}", self.major, self.minor, self.patch) + } } /// Parses the first line of `factorio --version`. @@ -68,6 +74,12 @@ mod tests { assert_eq!(info.major_minor(), "2.1"); } + #[test] + fn triple_is_what_a_provenance_entry_records() { + let info = parse_version_line("Version: 2.1.14 (build 87180, mac-arm64, steam)").unwrap(); + assert_eq!(info.triple(), "2.1.14"); + } + #[test] fn ignores_the_build_number_and_arch_digits() { // "84539" and the "64" in "mac-arm64" are digits too. Only the From 529d20df3aaad867016678d8b6faf2ea8eb3d9bd Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:12:33 -0700 Subject: [PATCH 07/11] Cross-check the provenance rules against another repo's manifest 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) Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82 --- tests/provenance.rs | 55 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/provenance.rs b/tests/provenance.rs index 2a292d3..66db8d7 100644 --- a/tests/provenance.rs +++ b/tests/provenance.rs @@ -36,3 +36,58 @@ fn every_fixture_records_where_it_came_from() { report.summary().join("\n ") ); } + +/// Runs this crate's check against a fixture directory from another repo. +/// +/// Skips itself unless `FACTORIO_ORACLE_PROVENANCE_DIR` names one, so CI and a +/// fresh clone stay green. It exists because Task 4's subject is 20 files this +/// crate also wrote the manifest for, which can only confirm what its author +/// already believed. FactorioMapWebUI's manifest has 100 entries, was written +/// by hand over months, and is enforced by a TypeScript test with rules of its +/// own. Agreement between the two is worth something; agreement with itself is +/// not. +/// +/// It asserts only the two claims both implementations make - no dangling +/// entry, and every entry well formed on the two required keys. Coverage and +/// the ratchet are printed rather than asserted, because another repo's +/// choices are not this crate's to gate, and because asserting a count would +/// break here the moment that repo adds a fixture. +/// +/// Run it with: +/// FACTORIO_ORACLE_PROVENANCE_DIR=../FactorioMapWebUI/test/fixtures \ +/// cargo test --test provenance -- --nocapture +#[test] +fn a_manifest_written_by_another_repo_agrees_with_this_check() { + let Some(dir) = std::env::var_os("FACTORIO_ORACLE_PROVENANCE_DIR").map(PathBuf::from) else { + eprintln!( + "skipping: set FACTORIO_ORACLE_PROVENANCE_DIR to a fixture directory to run this." + ); + return; + }; + + let manifest = manifest::load(&dir).expect("the named directory should hold a manifest"); + let on_disk = walk_fixtures(&dir).expect("the named directory should walk"); + let report = check(&manifest, &on_disk); + + eprintln!( + "{}: {} fixtures, {} not-fixtures, {} files on disk", + dir.display(), + report.fixtures, + report.not_fixtures, + on_disk.len() + ); + for line in report.summary() { + eprintln!(" {line}"); + } + + assert!( + report.dangling.is_empty(), + "entries naming files that are not there: {:?}", + report.dangling + ); + assert!( + report.malformed.is_empty(), + "entries that are not well formed: {:?}", + report.malformed + ); +} From eb74bd4d2fb82fc2cfae502072fc2ba67689437c Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:16:51 -0700 Subject: [PATCH 08/11] Document provenance, and record what measuring it corrected 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. --- CLAUDE.md | 34 ++++++++++++++++++++++++++++++++++ README.md | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 9532e98..cfe80e9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,6 +63,7 @@ tests** (3 in `tests/acceptance.rs`, 3 in `tests/real_game.rs`). | `run.rs` | Wiring the pure builders to disk and a spawner | | `numbers.rs` | Preserving the bits the game produced | | `trim/` | Cutting a full `data.raw` dump down to a consumer's slice | +| `provenance/` | Which Factorio each fixture came from, and whether that record is still honest | Five run modes, and **the success predicate differs per mode**: `dump-data` (no mod at all, the mod dir exists only to be empty), `create`, `interactive` @@ -124,6 +125,39 @@ code did. **A fake can only be wrong in the ways its author already considered.* - **`loadedMods` cannot come from the active-mods prelude**, because `script.active_mods` never reports `core` and the fixtures list it. Grep the game's stdout for `Loading mod `. +- **A real provenance manifest has two keys per entry, not eight.** Measured + 2026-08-17 across FactorioMapWebUI's 100 entries: every one carries + `factorioVersion` and `evidence`, and `factorioBuild`, `branch`, + `loadedMods`, `capturedOn`, `capturedBy` and `targetVersionRange` appear zero + times. The design sketched all eight. A checker requiring the sketch would + reject the only real manifest there is, so the other six are optional and + carried through untouched. +- **`evidence` is free text, and enforcing a grade would reject 45 of 100.** + First word across the same 100 entries: `stated` 48, `captured` 34, + `RE-CAPTURED` 8, `inferred` 4, `re-captured` 3, `UNDOCUMENTED` 1, and twice + it is just `the`. The grade that is real is `factorioVersion: "unknown"`, + which is a field, and that is what the ratchet counts. +- **An extension allowlist is how ground truth goes unrecorded.** MapWebUI's + provenance test globs `.json` and `.png`. Its fixture directory also holds 10 + `.txt` map exchange strings, and **8 of the 10 are read as ground truth** by + `decode.spec.ts`, `encode.spec.ts` and `jsonExport.spec.ts`. None has an + entry and none can get one while the glob decides. So this tool names every + file, and a deliberate non-fixture goes in `notFixtures` with a reason. +- **Manifest keys are relative paths, not bare filenames.** The design says + bare filenames, which is true of MapWebUI's flat directory and was never + tested against a tree. This crate's own `tests/fixtures/` is two levels deep. + The walk joins components with `/` by hand rather than using + `Path::display()`, so a Windows run and a macOS run produce the same + committed key. +- **The walk skips dotfiles, and that is load-bearing on a Mac.** `.DS_Store` + appears in any directory a Finder window has opened. Demanding an entry for + it would make the check fail on the machine that can fix it and pass in CI. +- **`#[serde(flatten)]` works under `arbitrary_precision`.** Measured + 2026-08-17: a struct with two named string fields plus a flattened + `BTreeMap` parsed a document holding both an integer and a + decimal, with no error. Recorded as a negative result so nobody spends an + afternoon ruling it out. Provenance entries stay a `Value` anyway, because + only two keys are required and the rest must round-trip untouched. ### Writing Lua for a probe diff --git a/README.md b/README.md index f408060..ad2b8f6 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,45 @@ Practical notes, measured 2026-08-17: never gate a capture: captures must stay reproducible offline and byte for byte. +## Provenance + +A fixture that does not say which Factorio produced it is a number with no +claim attached. Provenance records that, in a `PROVENANCE.json` beside the +fixtures rather than inside them: most fixtures are verbatim copies of the +game's own JSON, asserted byte for byte, so a metadata key added inside one +would be data pollution. + +```bash +# Structural check. No Factorio needed. Exits 1 on a finding. +factorio-oracle provenance check tests/fixtures + +# Version comparison. Needs an install. Always exits 0. +factorio-oracle provenance report tests/fixtures +``` + +Enforcement splits in two on purpose. `check` answers questions a machine can +settle - is every file recorded, does every entry name a file that exists, is +every entry well formed, and has the `unknown` count grown - so it runs in CI +with no game. `report` answers a question 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. + +Two required keys per entry, `factorioVersion` and `evidence`. Any other key is +carried through and never validated. `evidence` is free text; the grade that is +enforced is `factorioVersion: "unknown"`, and `maxUnknown` caps how many entries +may say it. That number is a ratchet, not a cap: the check fails if the count +rises above it and also if the count drops below it and the number is not +lowered. + +Every file in the tree needs an entry. A file that is deliberately not ground +truth goes in `notFixtures` with a reason. There is no extension filter, +because an ignore rule that costs nothing gets used without thinking. + +**A fixture's provenance is a record of the moment it was captured, not a live +claim.** Never edit one to make it current, and never edit one to make a test +pass. A mismatch is a finding. + ## Examples - [`examples/pumpjack-terminals`](examples/pumpjack-terminals) - a `create` probe From be9dd4edd5651293171b67be9714d12212e4f509 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:20:20 -0700 Subject: [PATCH 09/11] Fix a transcription error in the evidence-grade bullet 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. --- CLAUDE.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cfe80e9..f304e22 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,11 +132,13 @@ code did. **A fake can only be wrong in the ways its author already considered.* times. The design sketched all eight. A checker requiring the sketch would reject the only real manifest there is, so the other six are optional and carried through untouched. -- **`evidence` is free text, and enforcing a grade would reject 45 of 100.** +- **`evidence` is free text, and enforcing a grade would reject 48 of 100.** First word across the same 100 entries: `stated` 48, `captured` 34, `RE-CAPTURED` 8, `inferred` 4, `re-captured` 3, `UNDOCUMENTED` 1, and twice - it is just `the`. The grade that is real is `factorioVersion: "unknown"`, - which is a field, and that is what the ratchet counts. + it is just `the`. The design's enum was `stated | inferred | unknown`, which + accepts 52 of the 100 first words and rejects the other 48. The grade that + is real is `factorioVersion: "unknown"`, which is a field, and that is what + the ratchet counts. - **An extension allowlist is how ground truth goes unrecorded.** MapWebUI's provenance test globs `.json` and `.png`. Its fixture directory also holds 10 `.txt` map exchange strings, and **8 of the 10 are read as ground truth** by From d8cb62629c647a6d375975420e32ae61dece1f49 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:32:39 -0700 Subject: [PATCH 10/11] Apply final review fixes to the provenance branch 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) Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82 --- CLAUDE.md | 25 +++++++-- src/install.rs | 6 +- src/main.rs | 4 +- src/provenance/check.rs | 110 +++++++++++++++++++++++++++++++++++++ src/provenance/manifest.rs | 14 ++--- src/provenance/mod.rs | 59 ++++++++++++++++++-- src/provenance/report.rs | 26 +++++++++ 7 files changed, 222 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f304e22..ebdca53 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,8 +45,16 @@ cargo run -- run --probe dump-data.json --work-dir /tmp/w > /tmp/run.json cargo run -- trim --run /tmp/run.json --spec trim-spec.json --out fixture.json [--check] ``` -Test counts to expect: **113 unit tests**, plus **6 install-gated integration -tests** (3 in `tests/acceptance.rs`, 3 in `tests/real_game.rs`). +Test counts to expect: **149 unit tests**, plus **8 integration tests** split +across three files. `tests/acceptance.rs` has 3: two run offline against a +committed fixture, and one (`the_real_install_reproduces_it_too`) is +install-gated. `tests/provenance.rs` has 2: one always-on, and one gated on +the `FACTORIO_ORACLE_PROVENANCE_DIR` environment variable naming another +repo's fixture directory - not an install gate, so it skips even when +Factorio is present. `tests/real_game.rs` has 3, all install-gated. That is +**4 install-gated tests** in total. Without a real Factorio install they skip +rather than fail, so a green run on a machine with no game proves less than it +looks. Check which happened before trusting it. ## Layout @@ -151,9 +159,16 @@ code did. **A fake can only be wrong in the ways its author already considered.* The walk joins components with `/` by hand rather than using `Path::display()`, so a Windows run and a macOS run produce the same committed key. -- **The walk skips dotfiles, and that is load-bearing on a Mac.** `.DS_Store` - appears in any directory a Finder window has opened. Demanding an entry for - it would make the check fail on the machine that can fix it and pass in CI. +- **The walk skips dotfiles and dot-directories, plus two Windows names, and + that is load-bearing on both platforms.** `.DS_Store` appears in any + directory a Finder window has opened; demanding an entry for it would make + the check fail on the machine that can fix it and pass in CI. The skip + `continue`s before the directory branch, so a dot-prefixed name is skipped + whole even when it is a directory - a consumer keeping fixtures under + `.golden/` gets nothing recorded for that subtree, not an error. `Thumbs.db` + and `desktop.ini` (matched case-insensitively) are skipped for the same + reason on Windows: Explorer writes `Thumbs.db` into any folder of images it + has thumbnailed, and MapWebUI's fixture directory holds `.png` files. - **`#[serde(flatten)]` works under `arbitrary_precision`.** Measured 2026-08-17: a struct with two named string fields plus a flattened `BTreeMap` parsed a document holding both an integer and a diff --git a/src/install.rs b/src/install.rs index 1b60848..85c893d 100644 --- a/src/install.rs +++ b/src/install.rs @@ -170,10 +170,14 @@ pub fn matches_version(found: &DiscoveredInstall, version: Option<&str>) -> bool /// `factorio` wins over `FACTORIO_BIN`, and either is offered as an extra /// candidate root rather than as the only one, which is what `run` has always /// done. With no version given, the first install that reported one wins. +/// +/// Parameters are ordered `factorio` before `env_bin` deliberately, matching +/// the precedence documented above - so the signature itself shows which one +/// wins, rather than relying on a reader to check the body. pub fn select( home: &Path, - env_bin: Option<&Path>, factorio: Option<&Path>, + env_bin: Option<&Path>, version: Option<&str>, ) -> Option { discover(home, factorio.or(env_bin)) diff --git a/src/main.rs b/src/main.rs index 1cbe791..3e42b14 100644 --- a/src/main.rs +++ b/src/main.rs @@ -136,8 +136,8 @@ fn main() -> anyhow::Result<()> { let chosen = install::select( &home, - env_bin.as_deref(), factorio.as_deref(), + env_bin.as_deref(), version.as_deref(), ) .ok_or_else(|| anyhow::anyhow!("no Factorio install matched"))?; @@ -284,8 +284,8 @@ fn main() -> anyhow::Result<()> { // failing build. let chosen = install::select( &home, - env_bin.as_deref(), factorio.as_deref(), + env_bin.as_deref(), version.as_deref(), ) .ok_or_else(|| { diff --git a/src/provenance/check.rs b/src/provenance/check.rs index c8ebfb2..9902e16 100644 --- a/src/provenance/check.rs +++ b/src/provenance/check.rs @@ -370,4 +370,114 @@ mod tests { let names: Vec<&str> = report.malformed.iter().map(|e| e.entry.as_str()).collect(); assert_eq!(names, vec!["a.md", "z.json"]); } + + // `to_json` is the interface a consumer's own test runner reads, per its + // own doc comment - and nothing above this point called it. Renaming a + // key or swapping `count`/`max` in a message would break every consumer + // while every test above still passed, so these check the exact key set + // and the exact `ratchet` string for all four states. + + /// The exact key set `to_json` promises. A silent rename fails here + /// rather than only in a consumer's test runner. + fn assert_exact_keys(json: &serde_json::Value) { + let keys: BTreeSet<&str> = json + .as_object() + .expect("to_json should produce an object") + .keys() + .map(String::as_str) + .collect(); + let expected: BTreeSet<&str> = [ + "ok", + "dir", + "fixtures", + "notFixtures", + "missing", + "dangling", + "malformed", + "unknown", + "maxUnknown", + "ratchet", + ] + .into_iter() + .collect(); + assert_eq!(keys, expected, "to_json's key set changed"); + } + + #[test] + fn to_json_reports_the_ok_ratchet() { + let m = manifest("1", &[("a.json", "unknown")], &[]); + let report = check(&m, &["a.json".to_string()]); + let json = report.to_json(Path::new("fixtures")); + assert_exact_keys(&json); + assert_eq!(json["ok"], true); + assert_eq!(json["ratchet"], "ok"); + assert_eq!(json["maxUnknown"], 1); + assert_eq!(json["unknown"], serde_json::json!(["a.json"])); + } + + #[test] + fn to_json_reports_the_exceeded_ratchet() { + let m = manifest("0", &[("a.json", "unknown"), ("b.json", "unknown")], &[]); + let report = check(&m, &["a.json".to_string(), "b.json".to_string()]); + let json = report.to_json(Path::new("fixtures")); + assert_exact_keys(&json); + assert_eq!(json["ok"], false); + assert_eq!(json["ratchet"], "exceeded"); + // The Exceeded branch reports `max`, the declared cap, not `count`. + // Swapping them is exactly the silent break this test exists to catch. + assert_eq!(json["maxUnknown"], 0); + } + + #[test] + fn to_json_reports_the_slack_ratchet() { + let m = manifest("1", &[("a.json", "2.1.14")], &[]); + let report = check(&m, &["a.json".to_string()]); + let json = report.to_json(Path::new("fixtures")); + assert_exact_keys(&json); + assert_eq!(json["ok"], false); + assert_eq!(json["ratchet"], "slack"); + assert_eq!(json["maxUnknown"], 1); + assert_eq!(json["unknown"], serde_json::json!([])); + } + + #[test] + fn to_json_reports_the_undeclared_ratchet_with_a_null_max_unknown() { + let doc = serde_json::json!({ + "fixtures": { "a.json": { "factorioVersion": "unknown", "evidence": "x" } }, + }); + let m: Manifest = serde_json::from_value(doc).unwrap(); + let report = check(&m, &["a.json".to_string()]); + let json = report.to_json(Path::new("fixtures")); + assert_exact_keys(&json); + assert_eq!(json["ok"], false); + assert_eq!(json["ratchet"], "undeclared"); + assert_eq!(json["maxUnknown"], serde_json::Value::Null); + } + + #[test] + fn summary_names_a_dangling_entry_when_called_directly() { + // Elsewhere summary() is only ever called inside an assert! format + // argument, which Rust evaluates only on failure - so a green run + // never executed this method without a test that calls it directly. + let m = manifest("0", &[("a.json", "2.1.14"), ("gone.json", "2.1.14")], &[]); + let report = check(&m, &["a.json".to_string()]); + assert_eq!( + report.summary(), + vec!["entry names a file that is not there: gone.json".to_string()] + ); + } + + #[test] + fn summary_states_the_exceeded_ratchet_with_both_numbers() { + let m = manifest("1", &[("a.json", "unknown"), ("b.json", "unknown")], &[]); + let report = check(&m, &["a.json".to_string(), "b.json".to_string()]); + assert_eq!( + report.summary(), + vec![ + "2 entries record an unknown version, and the manifest allows 1. \ + Re-capture against a known binary rather than raising maxUnknown." + .to_string() + ] + ); + } } diff --git a/src/provenance/manifest.rs b/src/provenance/manifest.rs index d0c8271..d21d2f7 100644 --- a/src/provenance/manifest.rs +++ b/src/provenance/manifest.rs @@ -25,10 +25,6 @@ pub const UNKNOWN: &str = "unknown"; #[derive(Debug, Clone, Deserialize)] pub struct Manifest { - /// An array of strings, so a long explanation stays readable in a diff. - #[serde(rename = "_comment", default)] - pub comment: Vec, - /// The ratchet: how many entries may say `unknown`. /// /// `None` means the manifest never declared one, which `check` reports as @@ -94,10 +90,11 @@ pub fn parse_triple(version: &str) -> Option<(u32, u32, u32)> { /// The design wrote it as an enum of `stated`, `inferred` and `unknown` plus /// free text. Measured across MapWebUI's 100 entries, the first word is /// `stated` 48 times, `captured` 34, `RE-CAPTURED` 8, `inferred` 4, -/// `re-captured` 3, `UNDOCUMENTED` once, and twice it is just `the`. The grade -/// is a habit of phrasing, not a field, and enforcing it would reject 45 of the -/// 100. The grade that is real is `factorioVersion == "unknown"`, and that is -/// what the ratchet counts. +/// `re-captured` 3, `the` 2, `UNDOCUMENTED` 1 - which sums to 100. The design's +/// enum was `stated | inferred | unknown`, which accepts `stated` (48) plus +/// `inferred` (4), 52 of the 100 first words, and rejects the other 48. The +/// grade is a habit of phrasing, not a field. The grade that is real is +/// `factorioVersion == "unknown"`, and that is what the ratchet counts. pub fn entry_problem(entry: &serde_json::Value) -> Option { let Some(map) = entry.as_object() else { return Some("entry is not an object".to_string()); @@ -135,7 +132,6 @@ mod tests { "notFixtures": { "README.md": "prose, not ground truth" } }"#; let m: Manifest = serde_json::from_str(text).expect("should parse"); - assert_eq!(m.comment.len(), 1); assert_eq!(m.max_unknown, Some(1)); assert_eq!(m.fixtures.len(), 2); assert_eq!(m.not_fixtures.len(), 1); diff --git a/src/provenance/mod.rs b/src/provenance/mod.rs index 6599ef1..116a94c 100644 --- a/src/provenance/mod.rs +++ b/src/provenance/mod.rs @@ -16,16 +16,37 @@ pub mod report; use std::path::Path; +/// Files (and, since the check fires before the directory branch below, +/// directories too) that are operating-system detritus rather than something +/// any repo tracks. `Thumbs.db` and `desktop.ini` are Windows' equivalent of +/// `.DS_Store`: Explorer writes `Thumbs.db` into any folder of images it has +/// thumbnailed, which is exactly what a fixture directory full of `.png` +/// files invites. Compared case-insensitively because Windows filesystems are +/// case-insensitive, so `THUMBS.DB` is the same file. +/// +/// Deliberately just these three names, not an open-ended list: a skip that +/// costs nothing is how ground truth goes unrecorded, which is the exact +/// failure this whole feature exists to prevent. Widening this list is a +/// deliberate, reviewed decision each time, not a place to accumulate names. +const OS_DETRITUS: &[&str] = &["Thumbs.db", "desktop.ini"]; + +fn is_skipped(name: &str) -> bool { + name.starts_with('.') || OS_DETRITUS.iter().any(|d| name.eq_ignore_ascii_case(d)) +} + /// Every file under `root`, as a path relative to `root` with forward slashes, /// sorted. /// /// Two exclusions, both deliberate: /// /// - The manifest itself. It is the record, not the record's subject. -/// - Anything whose name starts with a dot. `.DS_Store` appears in any -/// directory a Finder window has opened, so demanding an entry for it would -/// make this fail on a Mac and pass in CI. A check that fails only on the -/// machine that can fix it is worse than no check. +/// - Operating-system detritus: dotfiles (and dot-directories - the `continue` +/// below fires before the directory branch, so a consumer's `.golden/` +/// fixtures are skipped too, not just `.DS_Store`), plus `Thumbs.db` and +/// `desktop.ini` on Windows. `.DS_Store` appears in any directory a Finder +/// window has opened, so demanding an entry for it would make this fail on +/// a Mac and pass in CI. A check that fails only on the machine that can fix +/// it is worse than no check. pub fn walk_fixtures(root: &Path) -> std::io::Result> { let mut out = Vec::new(); collect(root, root, &mut out)?; @@ -36,7 +57,7 @@ pub fn walk_fixtures(root: &Path) -> std::io::Result> { fn collect(root: &Path, dir: &Path, out: &mut Vec) -> std::io::Result<()> { for entry in std::fs::read_dir(dir)? { let entry = entry?; - if entry.file_name().to_string_lossy().starts_with('.') { + if is_skipped(&entry.file_name().to_string_lossy()) { continue; } let path = entry.path(); @@ -102,4 +123,32 @@ mod tests { write(root, ".DS_Store"); assert_eq!(walk_fixtures(root).unwrap(), vec!["a.json".to_string()]); } + + #[test] + fn leaves_out_windows_os_detritus_case_insensitively() { + // Thumbs.db and desktop.ini are Windows' equivalent of .DS_Store, and + // Windows filesystems are case-insensitive, so a differently-cased + // name has to be caught too. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(root, "a.json"); + write(root, "Thumbs.db"); + write(root, "THUMBS.DB"); + write(root, "desktop.ini"); + write(root, "Desktop.Ini"); + assert_eq!(walk_fixtures(root).unwrap(), vec!["a.json".to_string()]); + } + + #[test] + fn a_dot_directory_is_skipped_whole_not_just_the_dotfile() { + // The `continue` in `collect` fires before the directory branch, so a + // consumer keeping fixtures under `.golden/` gets nothing recorded + // for the whole subtree rather than an error - this is what proves + // that behaviour rather than just asserting it in a comment. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(root, "a.json"); + write(root, ".golden/b.json"); + assert_eq!(walk_fixtures(root).unwrap(), vec!["a.json".to_string()]); + } } diff --git a/src/provenance/report.rs b/src/provenance/report.rs index c7cbe88..bad0f72 100644 --- a/src/provenance/report.rs +++ b/src/provenance/report.rs @@ -218,6 +218,32 @@ mod tests { assert!(text.contains("1 fixture(s) predate the installed binary")); } + #[test] + fn an_older_fixture_says_the_binary_is_newer_not_the_other_way_round() { + // The two standing strings ("{binary} is newer" and "newer than + // {binary}") read almost identically and are easy for a human to + // swap, so each gets its own exact assertion rather than a shared + // substring check. + let m = manifest(&[("a.json", "2.1.11")]); + let text = render(&compare(&m, "2.1.14")); + assert!( + text.contains("2.1.14 is newer"), + "expected the binary-is-newer wording, got:\n{text}" + ); + assert!(!text.contains("newer than 2.1.14")); + } + + #[test] + fn a_newer_fixture_says_newer_than_the_binary() { + let m = manifest(&[("a.json", "2.1.14")]); + let text = render(&compare(&m, "2.0.77")); + assert!( + text.contains("newer than 2.0.77"), + "expected the newer-than-binary wording, got:\n{text}" + ); + assert!(!text.contains("2.0.77 is newer")); + } + #[test] fn a_clean_report_says_so_instead_of_printing_a_warning() { let m = manifest(&[("a.json", "2.1.14")]); From 40cb38510f96f857efcd52c5e0ba259c5fd526e0 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 17:34:30 -0700 Subject: [PATCH 11/11] Fix the unit test count FIX 2 itself left stale 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) Claude-Session: https://claude.ai/code/session_01Du2g8GxpozoXhtZFu8MF82 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index ebdca53..85e8ac5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,7 +45,7 @@ cargo run -- run --probe dump-data.json --work-dir /tmp/w > /tmp/run.json cargo run -- trim --run /tmp/run.json --spec trim-spec.json --out fixture.json [--check] ``` -Test counts to expect: **149 unit tests**, plus **8 integration tests** split +Test counts to expect: **159 unit tests**, plus **8 integration tests** split across three files. `tests/acceptance.rs` has 3: two run offline against a committed fixture, and one (`the_real_install_reproduces_it_too`) is install-gated. `tests/provenance.rs` has 2: one always-on, and one gated on