CB-WP-0048 T02/T03/T04: Configuration replaces Variant in the state
Some checks failed
ci / check (push) Has been cancelled
Some checks failed
ci / check (push) Has been cancelled
The three-armed enum could not express a configuration carrying two
modules. It does now: --variant h1 expands to problem_stress.flat_any_open
AND attack_relief.self_soothe_ge4, and scoped_plus_attack_soothe plays.
Legacy names alias forever via serde(alias="variant") plus a
scalar-or-map deserialiser. serde(default) alone would have been a silent
migration bug -- every H1/H2 recording would have come back as baseline.
All 26 scenarios pass unchanged; none pins a state hash.
Three things shipped broken first, all caught by gates rather than by
reading:
1. `profile` was in the state hash. with_config(select("ground-darvo-r0"))
sets profile=Some("baseline") where setup alone leaves None, so two
states at THE SAME POINT IN ASPECT SPACE hashed differently and a
replay bundle stopped reproducing its own initial state. Now
serde(skip): a hash covers what determines play. This was the open
judgement from T01 and it did not survive contact with the replay path.
2. A YAML parse inside the event loop. rules() -> resolve() -> catalog()
re-parsed catalog.yaml per rule check; AM-6 fell to 9,345 events/s
against a 100,000 target. OnceLock, and aspect validation moved to
where a configuration is BUILT.
Then I nearly optimised a phantom: 470k still looked like a 3x
regression against the "~1.7M on bnt-lap001" reference in the gate's
own message. Making rules() free measured 491k -- this machine's
ceiling. Before optimising against a reference, measure the ceiling
with the suspect code removed.
3. The refusal did not fire on the path a player takes.
`--module problem_deal.pressure_deck` played a full baseline game and
reported success, because with_config is a builder and fell back to
the printed rules -- the silent no-op ADR-0022 exists to refuse. Every
unit test of resolve() passed. The helper was tested and the driver
was not, which is CB-WP-0033's finding verbatim. The new test asserts
refusal BY NAME and that no game was played.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
704b99975b
commit
48a7bce04d
12 changed files with 538 additions and 67 deletions
|
|
@ -113,8 +113,21 @@ pub struct Catalog {
|
|||
/// says so by name rather than returning an empty aspect list.
|
||||
pub const SCHEMA: u32 = 2;
|
||||
|
||||
/// Parse the vendored catalog.
|
||||
pub fn catalog() -> Result<Catalog, String> {
|
||||
/// Parse the vendored catalog. **Parsed once.**
|
||||
///
|
||||
/// This returned an owned `Catalog` and parsed the YAML on every call —
|
||||
/// and `GroundState::rules()` calls it per rule check, inside the event
|
||||
/// loop. AM-6 throughput fell from ~1.7M events/s to **9,345**, a 180x
|
||||
/// regression, and the spec gate caught it the same run.
|
||||
///
|
||||
/// The catalog is a compile-time constant (`include_str!`), so parsing
|
||||
/// it more than once was never doing anything but work.
|
||||
pub fn catalog() -> Result<&'static Catalog, String> {
|
||||
static PARSED: std::sync::OnceLock<Result<Catalog, String>> = std::sync::OnceLock::new();
|
||||
PARSED.get_or_init(parse).as_ref().map_err(|e| e.clone())
|
||||
}
|
||||
|
||||
fn parse() -> Result<Catalog, String> {
|
||||
let f: CatalogFile =
|
||||
serde_yaml::from_str(CATALOG_YAML).map_err(|e| format!("catalog.yaml: {e}"))?;
|
||||
if f.schema_version != SCHEMA {
|
||||
|
|
|
|||
|
|
@ -36,20 +36,82 @@ use std::collections::BTreeMap;
|
|||
/// *resolved*: every aspect the catalog declares has an entry, defaults
|
||||
/// filled in, so a recording states the whole point in aspect space
|
||||
/// rather than the delta from an assumed origin.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
|
||||
pub struct Configuration {
|
||||
/// The content package (ADR-0011), e.g. `ground-darvo-r0`.
|
||||
pub baseline: String,
|
||||
/// aspect id → module id, resolved.
|
||||
pub modules: BTreeMap<String, String>,
|
||||
/// The profile that was asked for, if one was. **Kept for the
|
||||
/// account, never consulted for behaviour** — two runs with the same
|
||||
/// modules play identically whether or not a profile named them, and
|
||||
/// a reader still wants to know what was requested.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
/// The profile that was asked for, if one was.
|
||||
///
|
||||
/// **Kept for the account, and deliberately OUT of the state.** Two
|
||||
/// runs with the same modules play identically whether or not a
|
||||
/// profile named them, so a label must not change the state hash.
|
||||
///
|
||||
/// It was serialised at first, and the replay walk caught it within
|
||||
/// the hour: `with_config(select("ground-darvo-r0"))` sets
|
||||
/// `profile = Some("baseline")` while `setup` alone leaves it
|
||||
/// `None`, so two states at **the same point in aspect space** hashed
|
||||
/// differently and a bundle stopped reproducing its own initial
|
||||
/// state. A hash should cover what determines play; provenance of
|
||||
/// what was *requested* belongs in the recording's manifest, which is
|
||||
/// where a reader can still find it.
|
||||
#[serde(skip)]
|
||||
pub profile: Option<String>,
|
||||
}
|
||||
|
||||
/// **Reads a legacy name as well as a configuration** (ADR-0022 D2).
|
||||
///
|
||||
/// A recording written before this type serialised `variant: Baseline`
|
||||
/// or a catalog id. Deserialising that as "no configuration" would set
|
||||
/// every such recording to the baseline — silently right for most of
|
||||
/// them and silently *wrong* for any played under H1 or H2, which is the
|
||||
/// one failure a migration must not have.
|
||||
///
|
||||
/// So a scalar is accepted and expanded through the catalog, and a map is
|
||||
/// read as itself. Writing is always the map: the legacy spelling is a
|
||||
/// thing we can read, not a thing we produce.
|
||||
impl<'de> Deserialize<'de> for Configuration {
|
||||
fn deserialize<D: serde::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
|
||||
#[derive(Deserialize)]
|
||||
#[serde(untagged)]
|
||||
enum Wire {
|
||||
Named(String),
|
||||
Full {
|
||||
baseline: String,
|
||||
modules: BTreeMap<String, String>,
|
||||
},
|
||||
}
|
||||
match Wire::deserialize(d)? {
|
||||
Wire::Full { baseline, modules } => Ok(Configuration {
|
||||
baseline,
|
||||
modules,
|
||||
// Not on the wire; see the field's own note.
|
||||
profile: None,
|
||||
}),
|
||||
Wire::Named(name) => Configuration::select_legacy(&name)
|
||||
.map_err(|e| serde::de::Error::custom(format!("configuration {name:?}: {e}"))),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Configuration {
|
||||
/// A legacy `variant` value, as an enum name or a catalog id.
|
||||
///
|
||||
/// The Rust names are here because they are what `serde` wrote into
|
||||
/// recordings; they are **this repo's** spelling of ground-game's
|
||||
/// ids and have no other home to be read from.
|
||||
pub fn select_legacy(name: &str) -> Result<Self, String> {
|
||||
let id = match name {
|
||||
"Baseline" => "ground-darvo-r0",
|
||||
"H1ProblemStress" => "h1-problem-stress",
|
||||
"H2ScopedProblemStress" => "h2-scoped-problem-stress",
|
||||
other => other,
|
||||
};
|
||||
Configuration::select(id)
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for Configuration {
|
||||
/// The printed game: every aspect at its default module.
|
||||
///
|
||||
|
|
@ -119,7 +181,7 @@ impl Configuration {
|
|||
let mut claimed: BTreeMap<String, String> = BTreeMap::new();
|
||||
for id in modules {
|
||||
let Some(aspect) = cat.aspect_of(id) else {
|
||||
return Err(unknown_module(&cat, id));
|
||||
return Err(unknown_module(cat, id));
|
||||
};
|
||||
if let Some(first) = claimed.get(aspect) {
|
||||
return Err(format!(
|
||||
|
|
@ -130,11 +192,13 @@ impl Configuration {
|
|||
claimed.insert(aspect.to_string(), id.clone());
|
||||
chosen.insert(aspect.to_string(), id.clone());
|
||||
}
|
||||
Ok(Configuration {
|
||||
let c = Configuration {
|
||||
baseline: baseline.to_string(),
|
||||
modules: chosen,
|
||||
profile: None,
|
||||
})
|
||||
};
|
||||
c.validate()?;
|
||||
Ok(c)
|
||||
}
|
||||
|
||||
/// Build from a named profile.
|
||||
|
|
@ -212,19 +276,35 @@ impl Configuration {
|
|||
out
|
||||
}
|
||||
|
||||
/// Every aspect named is one the catalog declares.
|
||||
///
|
||||
/// Called where a configuration is **built**, not where it is used:
|
||||
/// it cannot change between rule checks, and keeping it out of
|
||||
/// `resolve` is what AM-6 required.
|
||||
pub fn validate(&self) -> Result<(), String> {
|
||||
let cat = crate::catalog::catalog()?;
|
||||
for aspect in self.modules.keys() {
|
||||
if cat.aspect(aspect).is_none() {
|
||||
return Err(format!("{aspect:?} is not an aspect the catalog declares"));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// **Identity → behaviour.** The boundary in ADR-0022 D1.
|
||||
///
|
||||
/// Refuses, by name and with the reason, any module the catalog has
|
||||
/// and this kernel cannot run.
|
||||
pub fn resolve(&self) -> Result<Rules, String> {
|
||||
let cat = crate::catalog::catalog()?;
|
||||
let mut rules = Rules::default();
|
||||
for (aspect, id) in &self.modules {
|
||||
// An aspect the catalog does not declare is not something to
|
||||
// guess about.
|
||||
if cat.aspect(aspect).is_none() {
|
||||
return Err(format!("{aspect:?} is not an aspect the catalog declares"));
|
||||
}
|
||||
for id in self.modules.values() {
|
||||
// **No catalog lookup in this loop.** `resolve` runs per rule
|
||||
// check inside the event loop, and validating that each
|
||||
// aspect is declared meant a linear scan of the catalog per
|
||||
// call. AM-6 measured the cost: throughput fell to 9,345
|
||||
// events/s against a 100,000 target. Whether an aspect exists
|
||||
// is a property of the CONFIGURATION, checked once where one
|
||||
// is built (`validate`), not once per rule.
|
||||
match id.as_str() {
|
||||
"problem_stress.none" => rules.problem_stress = ProblemStress::None,
|
||||
"problem_stress.flat_any_open" => rules.problem_stress = ProblemStress::FlatAnyOpen,
|
||||
|
|
@ -239,7 +319,7 @@ impl Configuration {
|
|||
// look", and the `other` arm below can then be a real
|
||||
// refusal rather than a shrug.
|
||||
"end_condition.fixed_rounds_5" | "problem_deal.fixed_setup" => {}
|
||||
other => return Err(no_kernel_path(&cat, other)),
|
||||
other => return Err(no_kernel_path(crate::catalog::catalog()?, other)),
|
||||
}
|
||||
}
|
||||
Ok(rules)
|
||||
|
|
|
|||
|
|
@ -211,8 +211,20 @@ pub struct GroundState {
|
|||
/// diverge silently, and the recording is what every other artifact
|
||||
/// rests on. `#[serde(default)]` so every scenario written before
|
||||
/// variants existed still loads, as baseline — which is what it was.
|
||||
#[serde(default)]
|
||||
pub variant: Variant,
|
||||
/// Which point in aspect space this game is played at
|
||||
/// ([ADR-0022](../../../decisions/ADR-0022-a-configuration-is-a-point-in-aspect-space.md)).
|
||||
///
|
||||
/// **Was `variant: Variant`, a three-armed enum** — which could not
|
||||
/// express `problem_stress.scoped` × `attack_relief.self_soothe_ge4`,
|
||||
/// a profile the catalog ships. Not "unimplemented": unrepresentable.
|
||||
///
|
||||
/// `alias = "variant"` plus `Configuration`'s scalar-or-map
|
||||
/// deserialiser means a recording written before this loads by its
|
||||
/// old name and old spelling. **`serde(default)` alone would have
|
||||
/// been a silent migration bug**: every recording played under H1 or
|
||||
/// H2 would have come back as the baseline.
|
||||
#[serde(default, alias = "variant")]
|
||||
pub config: crate::config::Configuration,
|
||||
/// Which of the edition's four Scenarios is on the table
|
||||
/// (CB-WP-0047).
|
||||
///
|
||||
|
|
@ -317,6 +329,16 @@ pub enum Variant {
|
|||
}
|
||||
|
||||
impl Variant {
|
||||
/// This legacy name as a **configuration** (ADR-0022 D2).
|
||||
///
|
||||
/// The enum survives as a spelling the maintainer types and every
|
||||
/// recording carries; it is no longer what the kernel branches on.
|
||||
/// Expansion goes through the catalog's own `legacy_experiment_id`,
|
||||
/// so ground-game's mapping has one home.
|
||||
pub fn configuration(self) -> crate::config::Configuration {
|
||||
crate::config::Configuration::select(self.id()).unwrap_or_default()
|
||||
}
|
||||
|
||||
/// The catalog's `variant_id`, which is how ground-game names these.
|
||||
pub fn id(self) -> &'static str {
|
||||
match self {
|
||||
|
|
@ -350,14 +372,41 @@ impl GroundState {
|
|||
/// owners at setup, and `state.variant = v` would leave them
|
||||
/// unassigned — a silently wrong game rather than a failing one.
|
||||
/// Every driver path goes through here.
|
||||
pub fn with_variant(mut self, variant: Variant) -> Self {
|
||||
self.variant = variant;
|
||||
if variant == Variant::H2ScopedProblemStress {
|
||||
self.apply_h2_setup();
|
||||
pub fn with_variant(self, variant: Variant) -> Self {
|
||||
self.with_config(variant.configuration())
|
||||
}
|
||||
|
||||
/// Select the configuration, and apply whatever its modules need at
|
||||
/// setup (ADR-0022 D3).
|
||||
///
|
||||
/// **A builder rather than a bare field write.**
|
||||
/// `problem_stress.scoped` assigns Problem owners at setup, and
|
||||
/// `state.config = c` would leave them unassigned — a silently wrong
|
||||
/// game rather than a failing one. That defect had three call sites
|
||||
/// under the old enum; with N aspects it is N times as likely, so
|
||||
/// every driver path goes through here.
|
||||
pub fn with_config(mut self, config: crate::config::Configuration) -> Self {
|
||||
let rules = config.resolve();
|
||||
self.config = config;
|
||||
if let Ok(r) = rules {
|
||||
if r.problem_stress == crate::config::ProblemStress::Scoped {
|
||||
self.apply_h2_setup();
|
||||
}
|
||||
}
|
||||
self
|
||||
}
|
||||
|
||||
/// What this game's configuration means for the kernel.
|
||||
///
|
||||
/// **Falls back to the printed rules** when the configuration names
|
||||
/// something this kernel cannot run. `setup` and `with_config` are
|
||||
/// where that is refused loudly; by the time a rule is being applied
|
||||
/// the game is already in progress, and a mid-game panic would turn
|
||||
/// a selection error into a crash at an unrelated moment.
|
||||
fn rules(&self) -> crate::config::Rules {
|
||||
self.config.resolve().unwrap_or_default()
|
||||
}
|
||||
|
||||
/// H2-SCOPE and H2-OWN, applied to the dealt board.
|
||||
///
|
||||
/// Owners go to the **non-global** Problems in **ascending hidden
|
||||
|
|
@ -1659,7 +1708,9 @@ impl GroundState {
|
|||
// The DARVO extra Attack comes through here too, which the delta
|
||||
// requires: "DARVO-stage extra Attack uses the same Attack
|
||||
// resolution (so it can self-soothe too if Stress >= 4)".
|
||||
if self.variant == Variant::H1ProblemStress && attacker_stress_before >= 4 {
|
||||
if self.rules().attack_relief == crate::config::AttackRelief::SelfSootheGe4
|
||||
&& attacker_stress_before >= 4
|
||||
{
|
||||
let stress = self.stress_after(attacker, -1);
|
||||
events.push(GroundEvent::StressSet {
|
||||
player: attacker,
|
||||
|
|
@ -1713,7 +1764,7 @@ impl GroundState {
|
|||
// ticks only the seats in ITS scope, and `stacking: true` — two
|
||||
// open bond Problems hit the network twice, so this accumulates
|
||||
// per Problem rather than applying once.
|
||||
if self.variant == Variant::H2ScopedProblemStress {
|
||||
if self.rules().problem_stress == crate::config::ProblemStress::Scoped {
|
||||
let mut ticks: BTreeMap<PlayerId, i16> = BTreeMap::new();
|
||||
for problem in self.problems.values() {
|
||||
if problem.claimed_by.is_some() {
|
||||
|
|
@ -1751,7 +1802,7 @@ impl GroundState {
|
|||
}
|
||||
}
|
||||
|
||||
if self.variant == Variant::H1ProblemStress
|
||||
if self.rules().problem_stress == crate::config::ProblemStress::FlatAnyOpen
|
||||
&& self.problems.values().any(|p| p.claimed_by.is_none())
|
||||
{
|
||||
for seat in self.seat_order() {
|
||||
|
|
@ -2302,7 +2353,7 @@ impl ScenarioGame for GroundState {
|
|||
// before the hash is taken, which is the route `mode` uses
|
||||
// (`table.rs`) — so a recorded session replays under the
|
||||
// variant it was played under.
|
||||
variant: Variant::default(),
|
||||
config: crate::config::Configuration::default(),
|
||||
outcome: None,
|
||||
seed,
|
||||
})
|
||||
|
|
@ -2437,7 +2488,11 @@ mod tests {
|
|||
seed,
|
||||
)
|
||||
.expect("setup");
|
||||
assert_eq!(untouched.variant, Variant::Baseline, "the default moved");
|
||||
assert_eq!(
|
||||
untouched.config,
|
||||
crate::config::Configuration::default(),
|
||||
"the default moved"
|
||||
);
|
||||
assert_eq!(
|
||||
cb_events::state_hash_hex(&base),
|
||||
cb_events::state_hash_hex(&untouched),
|
||||
|
|
@ -2472,7 +2527,7 @@ mod tests {
|
|||
|
||||
// The wrong way.
|
||||
let mut bare = base.clone();
|
||||
bare.variant = Variant::H2ScopedProblemStress;
|
||||
bare.config = Variant::H2ScopedProblemStress.configuration();
|
||||
assert!(
|
||||
bare.problems.values().all(|p| p.scope.is_none()),
|
||||
"a bare write should leave scopes unassigned — if it does not, \
|
||||
|
|
@ -3501,7 +3556,7 @@ mod tests {
|
|||
support_responses: BTreeMap::new(),
|
||||
darvo_targets: BTreeMap::new(),
|
||||
mode: ScoringMode::SharedGround,
|
||||
variant: Variant::Baseline,
|
||||
config: crate::config::Configuration::default(),
|
||||
scenario: default_scenario(),
|
||||
outcome: None,
|
||||
seed: 0,
|
||||
|
|
|
|||
|
|
@ -46,8 +46,15 @@ pub struct GroundView {
|
|||
/// **The page had no way to say this**, so a player could not tell
|
||||
/// baseline from H1 from H2 by looking — and reported a variant
|
||||
/// change as "no changes" after playing the default.
|
||||
#[serde(default)]
|
||||
pub variant: crate::Variant,
|
||||
/// Which point in aspect space is being played (ADR-0022).
|
||||
///
|
||||
/// **The page had no way to say this**, so a player could not tell
|
||||
/// baseline from H1 from H2 by looking — and reported a variant
|
||||
/// change as "no changes" after playing the default (CB-WP-0044).
|
||||
/// It carries the whole configuration now, not a single name, so a
|
||||
/// two-module combination is nameable on the table.
|
||||
#[serde(default, alias = "variant")]
|
||||
pub config: crate::config::Configuration,
|
||||
/// Which Scenario is on the table (CB-WP-0047).
|
||||
///
|
||||
/// The player was told the premise by nothing: the same four boards
|
||||
|
|
@ -185,7 +192,7 @@ impl Project for GroundState {
|
|||
lead: self.lead,
|
||||
step: self.step,
|
||||
mode: self.mode,
|
||||
variant: self.variant,
|
||||
config: self.config.clone(),
|
||||
scenario: self.scenario.clone(),
|
||||
players: self
|
||||
.players
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue