Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 117 additions & 11 deletions crates/phase-ai/src/bin/ai_perf_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@
#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;

use std::collections::{BTreeMap, BTreeSet};
use std::fs::File;
use std::io::BufWriter;
use std::io::{BufWriter, Write};
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};

Expand All @@ -36,6 +37,7 @@ use phase_ai::duel_suite::perf::{
repro_margin_report, run_perf_suite, PerfReport, PERF_ACTION_CAP, PERF_BASE_SEED,
PERF_SAMPLE_COUNT,
};
use phase_ai::duel_suite::{find_matchup, resolve_deck_ref};

const DEFAULT_BASELINE: &str = "crates/phase-ai/baselines/perf-baseline.json";
const DEFAULT_CURRENT: &str = "target/ai-perf-gate-current.json";
Expand Down Expand Up @@ -214,16 +216,7 @@ fn run_parent_gate(args: &Args) {
let mut current = median_report(&samples);
// Stamp provenance the parent can compute without loading the DB.
current.git_sha = command_output("git", &["rev-parse", "--short=12", "HEAD"]);
let db_path = args.data_root.join("card-data.json");
current.card_data_hash = command_output(
"git",
&[
"hash-object",
db_path
.to_str()
.expect("card-data path must be valid UTF-8"),
],
);
current.card_data_hash = gate_card_data_hash(&args.data_root);

eprintln!(
"perf suite: seed={} action_cap={} sample_count={} scenarios={:?} wall_clock={}ms",
Expand Down Expand Up @@ -349,6 +342,119 @@ fn command_output(program: &str, args: &[&str]) -> Option<String> {
.filter(|s| !s.is_empty())
}

/// Provenance hash over ONLY the `card-data.json` entries this gate's scenarios
/// actually consume.
///
/// This was previously `git hash-object data/card-data.json` — the whole file,
/// ~35.6k cards. The three scenarios in [`default_scenarios`] draw from
/// committed, frozen decks (inline builders plus pinned snapshots) naming ~46.
/// `card-data.json` is a DERIVED artifact of both MTGJSON and the Oracle parser,
/// so every unrelated set release *and every parser change* moved the stamp.
/// `PerfCompareReport::card_data_changed` was therefore true on nearly every
/// run, which made it useless for the one judgement it exists to support:
/// telling a genuine cost-per-node regression (hashes equal) apart from a
/// card-data-driven trajectory shift (hashes differ).
///
/// Measured over five local card-data vintages spanning 2026-07-25..2026-08-04,
/// one pair differing only by an Oracle-parser change and not by MTGJSON: five
/// distinct whole-file hashes, exactly ONE distinct gate-subset hash.
///
/// Still a `git hash-object` blob SHA, so the field's format and meaning are
/// unchanged and no hashing dependency enters this crate. Re-serializing is
/// canonical: this workspace leaves serde_json's `preserve_order` off, so object
/// keys serialize sorted (documented at `engine/src/bin/set_check.rs`), and the
/// `BTreeMap`s below fix the order of everything above them. A card a deck names
/// but `card-data.json` lacks is simply absent from the subset — which still
/// moves the hash, since the key set shrinks.
///
/// Returns `None` on any failure, matching [`command_output`]'s convention:
/// `card_data_changed()` then reports false rather than inventing a delta. Every
/// failure path announces itself on stderr — an unstamped run must not read as a
/// clean one.
fn gate_card_data_hash(data_root: &Path) -> Option<String> {
// DB-free by construction: `resolve_deck_ref` expands inline builders and
// pinned snapshots without a `CardDatabase`, preserving this branch's
// documented never-loads-the-DB property.
let mut names = BTreeSet::new();
for id in default_scenarios() {
let Some(matchup) = find_matchup(id) else {
eprintln!("provenance: scenario {id:?} does not resolve — card-data hash unstamped");
return None;
};
for deck in [&matchup.p0, &matchup.p1] {
match resolve_deck_ref(deck) {
// The engine keys `card-data.json` by Rust `to_lowercase()`.
Ok(cards) => names.extend(cards.into_iter().map(|c| c.to_lowercase())),
Err(err) => {
eprintln!(
"provenance: deck {deck:?} failed to resolve ({err}) — card-data hash unstamped"
);
return None;
}
}
}
}

let db_path = data_root.join("card-data.json");
let db: BTreeMap<String, serde_json::Value> = match std::fs::read_to_string(&db_path)
.map_err(|e| e.to_string())
.and_then(|raw| serde_json::from_str(&raw).map_err(|e| e.to_string()))
{
Ok(db) => db,
Err(err) => {
eprintln!(
"provenance: could not read {} ({err}) — card-data hash unstamped",
db_path.display()
);
return None;
}
};
let subset: BTreeMap<&str, &serde_json::Value> = names
.iter()
.filter_map(|name| db.get(name).map(|entry| (name.as_str(), entry)))
.collect();

// `git hash-object` needs a path, so the canonical subset goes through a
// temp file. Removed on every exit path, including the failures below.
let tmp = std::env::temp_dir().join(format!("ai-perf-gate-cards-{}.json", std::process::id()));
let hash = match File::create(&tmp)
.map_err(|e| e.to_string())
.and_then(|file| {
let mut writer = BufWriter::new(file);
serde_json::to_writer(&mut writer, &subset).map_err(|e| e.to_string())?;
// `BufWriter` discards errors from its drop-time flush, so a failed
// final write would leave a TRUNCATED subset that `git hash-object`
// still hashes — yielding a well-formed provenance stamp for content
// that was never written. Flush explicitly and fail closed instead.
writer.flush().map_err(|e| e.to_string())
}) {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Ok(()) => match tmp.to_str() {
Some(path) => command_output("git", &["hash-object", path]),
None => {
eprintln!("provenance: temp path is not valid UTF-8 — card-data hash unstamped");
None
}
},
Err(err) => {
eprintln!(
"provenance: could not write the card subset ({err}) — card-data hash unstamped"
);
None
}
};
let _ = std::fs::remove_file(&tmp);

if hash.is_some() {
eprintln!(
"provenance: card-data hash covers {} of {} deck-named cards from scenarios {:?}",
subset.len(),
names.len(),
default_scenarios()
);
}
hash
}

fn write_report(report: &PerfReport, path: &Path) -> Result<(), std::io::Error> {
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
Expand Down
69 changes: 69 additions & 0 deletions crates/phase-ai/src/duel_suite/perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@
//! (`--refresh-baseline`), which prints the baseline-vs-current diff before
//! overwriting — never a blind widen.
//!
//! That stamp covers **only the card-data entries [`default_scenarios`]'s decks
//! actually name**, not the whole file (`ai_perf_gate::gate_card_data_hash`).
//! `card-data.json` is derived from both MTGJSON and the Oracle parser, so a
//! whole-file hash moved on every set release and every parser change while this
//! gate's frozen decks saw none of it — leaving the diagnostic true on nearly
//! every run, and therefore unable to make the distinction above. The workload is
//! fixed by construction (inline builders + pinned snapshots), so the narrow
//! stamp is the one that tracks this gate's real input.
//!
//! Wall-clock is recorded (`wall_clock_ms`) for human triage only; it is never
//! compared.

Expand Down Expand Up @@ -783,8 +792,68 @@ pub fn print_repro_margin(report: &ReproMarginReport) {
#[cfg(test)]
mod tests {
use super::*;
use crate::duel_suite::DeckRef;
use std::collections::BTreeSet;

/// The narrowed `card_data_hash` (`ai_perf_gate::gate_card_data_hash`) is only
/// meaningful because every gate scenario draws from a deck fixed at compile
/// time — an inline builder or a pinned snapshot. A scenario whose deck were
/// derived from the card pool would make the narrow stamp silently wrong: the
/// workload would move with the pool while the stamp reported "unchanged",
/// which is strictly worse than the whole-file hash it replaced.
///
/// This guards that premise at the point it can break — adding a scenario —
/// rather than at the hash, which cannot tell where its names came from.
#[test]
fn gate_scenarios_draw_only_from_decks_fixed_at_compile_time() {
let mut names = BTreeSet::new();
for id in default_scenarios() {
let matchup = crate::duel_suite::find_matchup(id)
.unwrap_or_else(|| panic!("gate scenario {id:?} must resolve to a matchup"));
for deck in [&matchup.p0, &matchup.p1] {
// Exhaustive and wildcard-free ON PURPOSE: the compiler is the
// census here, not the card count below. Both current variants
// are fixed at compile time — `Inline` is a Rust builder,
// `Snapshot` a committed, `frozen_date`-stamped file — and a
// future pool-derived variant would resolve perfectly well and
// land inside the band, so no runtime assertion can catch it.
// Adding a `DeckRef` variant must break THIS match, because
// `gate_card_data_hash`'s narrowing is unsound for any deck the
// card pool can move.
match deck {
DeckRef::Inline { .. } | DeckRef::Snapshot { .. } => {}
}
let cards = crate::duel_suite::resolve_deck_ref(deck).unwrap_or_else(|err| {
panic!("gate scenario {id:?} deck {deck:?} must resolve without a card pool: {err}")
});
assert!(
!cards.is_empty(),
"gate scenario {id:?} deck {deck:?} resolved to an EMPTY deck — the perf \
workload would be vacuous and the narrowed provenance stamp would cover \
nothing"
);
names.extend(cards.into_iter().map(|c| c.to_lowercase()));
}
}

// Non-vacuity: the stamp must actually cover cards. The upper bound is the
// load-bearing half — it is what fails if a scenario starts pulling a
// pool-sized deck, at which point narrowing the hash stops being sound.
// Measured at 46 distinct names for the three mirrors on 2026-08-04.
//
// The band is a const so the panic message cannot drift from the
// assertion — printing a hardcoded range here was wrong once already.
const BAND: std::ops::RangeInclusive<usize> = 20..=400;
assert!(
BAND.contains(&names.len()),
"gate scenarios name {} distinct cards, outside the {BAND:?} band this narrowing \
assumes. Too few means a scenario stopped resolving; too many means a deck is no \
longer a fixed list, and `gate_card_data_hash` must be re-justified before the \
band is widened",
names.len()
);
}

fn mk_report(counters: &[(&str, u64)]) -> PerfReport {
PerfReport {
schema_version: PERF_SCHEMA_VERSION,
Expand Down
Loading