make trials reported "positions unreachable: 4, target 0 — the recording exists but the position moved". All four were false. A recording holds one hash, the final state, and reachability asked whether the note's hash was in that file — so a mid-game note could never match, and a post-game note from any but the last game could not either. Instance 8 of the ADR-0018 family: vary only WHEN a note was written and the answer flips, with nothing having moved. The root cause was not the metric. play again reused state belonging to a game: it overwrote the previous game's recording (data loss), never cleared the journal (game 2's log opened with game 1's commands), and so a note's command index pointed into a recording without those commands. Fixing reachability alone would have gone green while a session still destroyed its own evidence. A note now binds by (game, after) — an index into the recording's own commands list, which a reader can replay to. The hash keeps a job as the integrity check at the end of a game, where it can actually fail. Game 1 keeps the path it was given, so GameDesign §5's documented invocation is unchanged; later games get -2, -3 and nothing is overwritten. Legacy 5-column logs stay readable and are reported as legacy, never as orphans — an unsubstantiated orphan claim is the defect being fixed. All three fixes mutation-proven, including at the call site via a real two-game session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
4ddff3b17c
commit
edab11c0e4
10 changed files with 909 additions and 41 deletions
|
|
@ -41,19 +41,28 @@ pub struct Server {
|
|||
/// to — a partial write leaves a file that neither a human nor
|
||||
/// `make trials` can read.
|
||||
notes: RefCell<Vec<TrialNote>>,
|
||||
/// Which game of the session is being played, 1-based (ADR-0019 D1).
|
||||
/// The journal and the recording are per game; the notes and the
|
||||
/// tally are per session, and conflating the two is what made a
|
||||
/// second game overwrite the first one's evidence.
|
||||
game: std::cell::Cell<usize>,
|
||||
}
|
||||
|
||||
/// One thing a player said, and where they said it (ADR-0014 D3).
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct TrialNote {
|
||||
pub n: usize,
|
||||
/// Which game of the session, 1-based (ADR-0019 D3).
|
||||
pub game: usize,
|
||||
/// Commands played when this was written (CB-WP-0032), so the page
|
||||
/// can put the comment where it was made. **Held in memory only.**
|
||||
/// It is deliberately NOT a column in the trial log: `trials.py`
|
||||
/// skips any row that is not five cells, silently, so a sixth column
|
||||
/// would make `make trials` report zero notes for a file full of
|
||||
/// them — the exact failure its own docstring says it exists to
|
||||
/// prevent.
|
||||
/// can put the comment where it was made — and, since ADR-0019 D3,
|
||||
/// so a reader can reach the position: it is an index into the
|
||||
/// recording's own `commands:` list.
|
||||
///
|
||||
/// **CB-WP-0032 kept this out of the file** because `trials.py`
|
||||
/// silently skipped any row that was not five cells, so a new column
|
||||
/// would have emptied every log at once. That parser now raises and
|
||||
/// accepts both widths, which is what made this column safe to add.
|
||||
pub after: usize,
|
||||
pub round: u8,
|
||||
pub step: String,
|
||||
|
|
@ -61,6 +70,31 @@ pub struct TrialNote {
|
|||
pub text: String,
|
||||
}
|
||||
|
||||
/// Where game `n`'s recording goes (ADR-0019 D2).
|
||||
///
|
||||
/// **Game 1 keeps the path it was given.** GameDesign §5 documents that
|
||||
/// exact invocation as the trial protocol, and a scheme that renamed
|
||||
/// every file would have made the spec wrong for the common case on the
|
||||
/// day it landed. Games 2, 3 … get `-2`, `-3` before the extension.
|
||||
///
|
||||
/// Nothing is overwritten: a recording is evidence, and `play again`
|
||||
/// deleting the previous game's evidence is a data-loss defect on its own.
|
||||
pub fn game_path(base: &std::path::Path, n: usize) -> std::path::PathBuf {
|
||||
if n <= 1 {
|
||||
return base.to_path_buf();
|
||||
}
|
||||
let stem = base
|
||||
.file_stem()
|
||||
.map(|s| s.to_string_lossy().into_owned())
|
||||
.unwrap_or_default();
|
||||
let mut name = format!("{stem}-{n}");
|
||||
if let Some(ext) = base.extension() {
|
||||
name.push('.');
|
||||
name.push_str(&ext.to_string_lossy());
|
||||
}
|
||||
base.with_file_name(name)
|
||||
}
|
||||
|
||||
/// Write the trial log: readable Markdown with one machine-readable block.
|
||||
///
|
||||
/// **Reusing `FindingRegister.md`'s idiom deliberately** (ADR-0014 D2) —
|
||||
|
|
@ -78,7 +112,14 @@ pub fn write_trial_log(path: &std::path::Path, notes: &[TrialNote]) -> Result<()
|
|||
register finding, by a human, with the wording chosen then.\n\n",
|
||||
);
|
||||
s.push_str("<!-- trial-log:begin -->\n\n");
|
||||
s.push_str("| n | round | step | state_hash | comment |\n|---|---|---|---|---|\n");
|
||||
// ADR-0019 D3/D4: `game` and `after` are the binding a reader can act
|
||||
// on -- replay `after` commands of game `game` and you are standing
|
||||
// where the player stood. `state_hash` stays as an integrity check at
|
||||
// the end of a game, where it is the one thing the recording carries.
|
||||
s.push_str(
|
||||
"| n | game | after | round | step | state_hash | comment |\n\
|
||||
|---|---|---|---|---|---|---|\n",
|
||||
);
|
||||
for note in notes {
|
||||
// Pipes and newlines would break the table, so they are replaced
|
||||
// rather than escaped: the note is prose, and a reader losing a
|
||||
|
|
@ -86,8 +127,8 @@ pub fn write_trial_log(path: &std::path::Path, notes: &[TrialNote]) -> Result<()
|
|||
let text = note.text.replace(['\n', '\r'], " ").replace('|', "/");
|
||||
let _ = writeln!(
|
||||
s,
|
||||
"| {} | {} | {} | {} | {} |",
|
||||
note.n, note.round, note.step, note.state_hash, text
|
||||
"| {} | {} | {} | {} | {} | {} | {} |",
|
||||
note.n, note.game, note.after, note.round, note.step, note.state_hash, text
|
||||
);
|
||||
}
|
||||
s.push_str("\n<!-- trial-log:end -->\n");
|
||||
|
|
@ -119,6 +160,7 @@ impl Server {
|
|||
meta: RefCell::new(Vec::new()),
|
||||
trial: None,
|
||||
notes: RefCell::new(Vec::new()),
|
||||
game: std::cell::Cell::new(1),
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -155,10 +197,12 @@ impl Server {
|
|||
};
|
||||
let hash = cb_events::state_hash_hex(state);
|
||||
let after = self.journal.borrow().len();
|
||||
let game = self.game.get();
|
||||
let mut log = self.notes.borrow_mut();
|
||||
let n = log.len() + 1;
|
||||
log.push(TrialNote {
|
||||
n,
|
||||
game,
|
||||
after,
|
||||
round: state.round,
|
||||
step: step.to_string(),
|
||||
|
|
@ -168,6 +212,26 @@ impl Server {
|
|||
write_trial_log(path, &log)
|
||||
}
|
||||
|
||||
/// Begin the next game (ADR-0019 D1).
|
||||
///
|
||||
/// **Clearing the journal is the point.** It is shared with the
|
||||
/// driver and nothing ever emptied it, so a second game's log opened
|
||||
/// with the first game's commands still in it — and a note's `after`,
|
||||
/// counted against that journal, indexed into a recording that did
|
||||
/// not contain those commands.
|
||||
///
|
||||
/// The notes are NOT cleared: the trial log is the session's record
|
||||
/// (ADR-0019 D4). Only the log *view* is per game.
|
||||
pub fn next_game(&self) {
|
||||
self.journal.borrow_mut().clear();
|
||||
self.game.set(self.game.get() + 1);
|
||||
}
|
||||
|
||||
/// Which game is being played, 1-based.
|
||||
pub fn game(&self) -> usize {
|
||||
self.game.get()
|
||||
}
|
||||
|
||||
/// Set what the meta panel shows about the session (CB-WP-0027 T02).
|
||||
///
|
||||
/// Called by the driver between games, so the running tally is visible
|
||||
|
|
@ -185,9 +249,14 @@ impl Server {
|
|||
|
||||
/// The player's own comments, positioned for the log (CB-WP-0032).
|
||||
fn log_notes(&self) -> Vec<cb_render_html::doc::LogNote> {
|
||||
let game = self.game.get();
|
||||
self.notes
|
||||
.borrow()
|
||||
.iter()
|
||||
// THIS game's comments. A note from game 1 placed by its
|
||||
// command index into game 2's log would sit against a move
|
||||
// nobody made (ADR-0019 D1).
|
||||
.filter(|n| n.game == game)
|
||||
.map(|n| cb_render_html::doc::LogNote {
|
||||
after: n.after,
|
||||
round: n.round,
|
||||
|
|
@ -994,6 +1063,120 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// ADR-0019 D2. Game 1 keeps the path it was given.
|
||||
#[test]
|
||||
fn only_later_games_get_a_suffixed_recording() {
|
||||
let base = std::path::Path::new("trials/2026-08-07-x.yaml");
|
||||
// GameDesign §5 documents this exact invocation; renaming it
|
||||
// would have made the spec wrong for the common case.
|
||||
assert_eq!(game_path(base, 1), base);
|
||||
assert_eq!(
|
||||
game_path(base, 2),
|
||||
std::path::Path::new("trials/2026-08-07-x-2.yaml")
|
||||
);
|
||||
assert_eq!(
|
||||
game_path(base, 3),
|
||||
std::path::Path::new("trials/2026-08-07-x-3.yaml")
|
||||
);
|
||||
// Distinct paths are the whole point: nothing may be overwritten.
|
||||
assert_ne!(game_path(base, 2), game_path(base, 3));
|
||||
// And the suffix must not COMPOUND. The driver derives every
|
||||
// game's path from the ORIGINAL, because deriving it from the
|
||||
// previous game's would put game 3 in `x-2-3.yaml`.
|
||||
assert_eq!(
|
||||
game_path(&game_path(base, 2), 3),
|
||||
std::path::Path::new("trials/2026-08-07-x-2-3.yaml"),
|
||||
"this is the shape the driver must avoid, and does by keeping the base"
|
||||
);
|
||||
}
|
||||
|
||||
/// ADR-0019 D1. The journal belongs to a GAME.
|
||||
///
|
||||
/// **Nothing ever cleared it.** It is shared with the driver and
|
||||
/// lives on the `Server`, which outlives every game, so a second
|
||||
/// game's log opened with the first game's commands still in it — and
|
||||
/// a note's `after`, counted against that journal, indexed into a
|
||||
/// recording that never contained those commands.
|
||||
#[test]
|
||||
fn a_second_game_does_not_inherit_the_first_ones_log() {
|
||||
let server = Server::bind(0).expect("bind");
|
||||
let j = server.journal();
|
||||
j.borrow_mut().push(games_ground::bot::Applied {
|
||||
actor: cb_kernel::Actor::System,
|
||||
command: GroundCommand::EndRound,
|
||||
events: vec![],
|
||||
});
|
||||
assert_eq!(server.game(), 1);
|
||||
assert_eq!(server.journal().borrow().len(), 1);
|
||||
|
||||
server.next_game();
|
||||
|
||||
assert_eq!(server.game(), 2);
|
||||
assert!(
|
||||
server.journal().borrow().is_empty(),
|
||||
"game 2 opened holding game 1's commands"
|
||||
);
|
||||
}
|
||||
|
||||
/// A comment belongs to the game it was made in (ADR-0019 D1).
|
||||
///
|
||||
/// Placed by command index into a LATER game's log, a note from game 1
|
||||
/// would sit against a move nobody made — a position stated
|
||||
/// confidently and wrong, which is the family ADR-0018 names.
|
||||
#[test]
|
||||
fn a_comment_from_an_earlier_game_is_not_shown_in_a_later_one() {
|
||||
let dir = std::env::temp_dir().join(format!("cb-g-{}", std::process::id()));
|
||||
let server = Server::bind(0)
|
||||
.expect("bind")
|
||||
.with_trial(Some(dir.join("t.md")));
|
||||
let st = state();
|
||||
|
||||
server
|
||||
.record_note(
|
||||
&st,
|
||||
"Select",
|
||||
&Note {
|
||||
text: "in game one".into(),
|
||||
},
|
||||
)
|
||||
.expect("note 1");
|
||||
assert_eq!(server.log_notes().len(), 1, "its own game shows it");
|
||||
|
||||
server.next_game();
|
||||
assert!(
|
||||
server.log_notes().is_empty(),
|
||||
"game 1's comment leaked into game 2's log"
|
||||
);
|
||||
|
||||
server
|
||||
.record_note(
|
||||
&st,
|
||||
"Select",
|
||||
&Note {
|
||||
text: "in game two".into(),
|
||||
},
|
||||
)
|
||||
.expect("note 2");
|
||||
let shown = server.log_notes();
|
||||
assert_eq!(shown.len(), 1);
|
||||
assert_eq!(shown[0].text, "in game two");
|
||||
|
||||
// But the trial log keeps BOTH: it is the session's record, and
|
||||
// the game column is what tells them apart (ADR-0019 D4).
|
||||
let written = std::fs::read_to_string(dir.join("t.md")).expect("trial log");
|
||||
assert!(written.contains("in game one"), "the session lost a note");
|
||||
assert!(written.contains("in game two"));
|
||||
assert!(
|
||||
written.contains("| 1 | 1 |"),
|
||||
"game column missing: {written}"
|
||||
);
|
||||
assert!(
|
||||
written.contains("| 2 | 2 |"),
|
||||
"second note not in game 2: {written}"
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// **CB-WP-0020 T05.** "play again" must deal a second game, not just
|
||||
/// answer politely and exit — the browser could not start a second
|
||||
/// game without going back to a terminal, which made it a strictly
|
||||
|
|
@ -1002,6 +1185,13 @@ mod tests {
|
|||
fn play_again_deals_a_second_game() {
|
||||
let buf = std::sync::Arc::new(std::sync::Mutex::new(Vec::new()));
|
||||
let out = SharedOut(buf.clone());
|
||||
// ADR-0019 D2: a real recording, so this covers the CALL SITE.
|
||||
// The unit test proves `game_path` names files apart; only this
|
||||
// proves the driver uses it, which is where the data loss was.
|
||||
let dir = std::env::temp_dir().join(format!("cb-again-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).expect("tmp");
|
||||
let rec = dir.join("s.yaml");
|
||||
let rec_for_thread = rec.clone();
|
||||
let game = std::thread::spawn(move || {
|
||||
crate::table::play(
|
||||
&crate::table::Config {
|
||||
|
|
@ -1010,7 +1200,7 @@ mod tests {
|
|||
human_seats: vec![0],
|
||||
bot: "random".into(),
|
||||
replay_dir: None,
|
||||
record: None,
|
||||
record: Some(rec_for_thread),
|
||||
serve: Some(0),
|
||||
trial: None,
|
||||
mode: games_ground::ScoringMode::SharedGround,
|
||||
|
|
@ -1052,6 +1242,19 @@ mod tests {
|
|||
.collect();
|
||||
assert!(hashes.len() >= 2, "only {} game(s) finished", hashes.len());
|
||||
assert_ne!(hashes[0], hashes[1], "play again re-dealt the same game");
|
||||
|
||||
// ADR-0019 D2. BOTH recordings survive. `play again` used to
|
||||
// write game 2 over game 1's file, destroying the evidence a
|
||||
// trial exists to produce -- and every note bound to it.
|
||||
let second = dir.join("s-2.yaml");
|
||||
assert!(rec.exists(), "game 1's recording is gone");
|
||||
assert!(second.exists(), "game 2 was never recorded separately");
|
||||
assert_ne!(
|
||||
std::fs::read_to_string(&rec).expect("g1"),
|
||||
std::fs::read_to_string(&second).expect("g2"),
|
||||
"the two recordings hold the same game"
|
||||
);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// A game that ended badly must say so rather than draw a table for a
|
||||
|
|
|
|||
|
|
@ -315,6 +315,10 @@ pub fn play<R: BufRead, W: Write>(config: &Config, input: R, out: W) -> Result<S
|
|||
// point — it is declared here, beside the listener and the seed,
|
||||
// because those are the other two things that survive `play again`.
|
||||
let mut tally = MatchTally::default();
|
||||
// The path the caller gave us, kept UNCHANGED. Deriving the next
|
||||
// game's path from the previous game's would compound the suffix --
|
||||
// game 3 would land in `x-2-3.yaml`.
|
||||
let record_base = config.record.clone();
|
||||
loop {
|
||||
// CB-WP-0027 T02: the panel shows the series *during* the next
|
||||
// game, not only after it.
|
||||
|
|
@ -326,6 +330,19 @@ pub fn play<R: BufRead, W: Write>(config: &Config, input: R, out: W) -> Result<S
|
|||
return Ok(summary);
|
||||
}
|
||||
cfg.seed = cfg.seed.wrapping_add(1);
|
||||
// ADR-0019 D1. The journal and the recording belong to a GAME;
|
||||
// the notes and the tally belong to the session. Nothing advanced
|
||||
// the first pair, so the second game opened with the first one's
|
||||
// log still in it and then overwrote its recording.
|
||||
if let Some(s) = &server {
|
||||
s.next_game();
|
||||
}
|
||||
// ADR-0019 D2. The server holds which game this is; asking it
|
||||
// keeps ONE counter rather than a second copy here that could
|
||||
// disagree with the one the notes are stamped with.
|
||||
if let (Some(s), Some(base)) = (&server, &record_base) {
|
||||
cfg.record = Some(crate::hotseat::game_path(base, s.game()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue