diff --git a/Makefile b/Makefile index 102f883..0234113 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,13 @@ TOOLS := $(REPO)/tools # Every cargo recipe runs at the repo root; the shell does not persist cd. IN_REPO := cd $(REPO) && -.PHONY: check test sim bench bench-test coverage dep-weight cost cost-test cost-pin cost-budget shape-budget cost-mix loop-lint self-tests env-test task-done status facts-check facts-gen mutation-check size-metrics runtime-metrics build-time am6 am7 am8 edition-check replay-test loc play gate-review all +.PHONY: check test sim bench bench-test coverage dep-weight cost cost-test cost-pin cost-budget shape-budget cost-mix loop-lint self-tests env-test task-done status facts-check facts-gen mutation-check size-metrics runtime-metrics build-time am6 am7 am8 edition-check replay-test loc play gate-review all \ + design difficulty trials +# `design`, `difficulty` and `trials` were added by CB-WP-0022, CB-WP-0025 +# and CB-WP-0027 and none was declared here. Only `trials` revealed it, by +# colliding with the trials/ DIRECTORY -- Make saw an up-to-date file and +# ran nothing. The other two work by luck: no file happens to share their +# name. A target that is a command, not a file, belongs on this line. ## fmt + clippy (deny warnings) + HashMap deny-lint check: @@ -125,6 +131,7 @@ self-tests: $(PY) $(TOOLS)/runtime-metrics.py --self-test $(PY) $(TOOLS)/replay-test.py --self-test $(PY) $(TOOLS)/design.py --self-test + $(PY) $(TOOLS)/trials.py --self-test cargo run --release -q -p games-ground --example difficulty -- --self-test $(PY) $(TOOLS)/edition-check.py --self-test @@ -161,6 +168,12 @@ facts-check: facts-gen: $(PY) $(TOOLS)/facts.py --gen +# CB-WP-0027 T04: what the players said, and where. Surfacing is the +# deliverable -- a commentary feature nobody can read is this project's +# signature failure in a new medium (ADR-0014 D6). +trials: + @$(PY) $(TOOLS)/trials.py + # CB-WP-0025 T06: the difficulty table (specs/RetrospectiveAnalysis.md §4). # Winnable fraction from the solver plus a PLURAL policy panel -- a single # policy's win rate may not be reported as a difficulty (§4.1). diff --git a/crates/cb-render-html/src/doc.rs b/crates/cb-render-html/src/doc.rs index 99f0f94..7995e81 100644 --- a/crates/cb-render-html/src/doc.rs +++ b/crates/cb-render-html/src/doc.rs @@ -255,6 +255,25 @@ h1,h2{font-size:1rem;margin:1.2rem 0 .4rem;color:#9cf} styling alone would leave a dead control that still looks alive to anything reading the DOM. */ .sealed{opacity:.3;cursor:default;pointer-events:none;filter:grayscale(1)} +/* CB-WP-0027 T02. `minmax(0,...)` on both tracks, because a grid child + defaults to min-content width and the SVG table would refuse to shrink, + pushing the meta column off-screen instead of narrowing. + The single-column fallback is deliberate rather than incidental: the + page was responsive by accident before this. */ +.cb-cols{display:grid;grid-template-columns:minmax(0,1fr) minmax(0,24rem); + gap:1.2rem;align-items:start} +.cb-game{min-width:0} +.cb-meta{min-width:0;border-left:1px solid #2a3140;padding-left:1.1rem} +#cb-note{display:flex;flex-direction:column;gap:.5rem} +#cb-note textarea{width:100%;box-sizing:border-box;font:inherit;color:#dde; + background:#12141a;border:1px solid #445;border-radius:5px;padding:.5rem} +#cb-note button{align-self:flex-start;font:inherit;cursor:pointer;color:#dde; + background:#332a3a;border:1px solid #a7d;border-radius:5px;padding:.35rem .8rem} +.note{border-left:2px solid #a7d;padding-left:.6rem;margin:.4rem 0;white-space:pre-wrap} +@media (max-width:64rem){ + .cb-cols{grid-template-columns:minmax(0,1fr)} + .cb-meta{border-left:none;border-top:1px solid #2a3140;padding-left:0;padding-top:1rem} +} /* CB-WP-0024 T03: the card a seat played, in that seat's area. */ .played{display:block;margin:.3rem 0} .k{color:#89a} @@ -679,7 +698,7 @@ pub fn document( seat: Option, may_pass: bool, ) -> String { - document_with_log(view, legal, endpoint, seat, may_pass, &[]) + document_with_log(view, legal, endpoint, seat, may_pass, &[], &[]) } /// The table, plus the game log (CB-WP-0018 T02). @@ -690,6 +709,7 @@ pub fn document_with_log( seat: Option, may_pass: bool, log: &[LogLine], + meta: &[String], ) -> String { let mut s = String::with_capacity(8192); let _ = write!( @@ -716,9 +736,16 @@ pub fn document_with_log( }, ); + // CB-WP-0027 T02: the table on the left, everything *about* the table + // on the right. The log moves right because it is commentary on the + // game rather than part of it. + s.push_str("
"); body(&mut s, view); move_section(&mut s, legal, seat, may_pass); + s.push_str("
"); + meta_section(&mut s, meta); log_section(&mut s, log); + s.push_str("
"); let _ = write!( s, "
ready
\ @@ -729,6 +756,40 @@ pub fn document_with_log( s } +/// The meta panel's own content: whatever the caller wants a player to see +/// *about* the session rather than about the position. +/// +/// Empty is a legitimate state — a first game has no tally and may have no +/// notes — and renders as nothing rather than as an empty heading. +fn meta_section(s: &mut String, meta: &[String]) { + // CB-WP-0027 T03: the comment box. Always present — the panel's + // purpose is that a player can say something at any moment, and a box + // that appears only sometimes trains them not to look for it. + // + // A plain form POSTing to /note, so it works with the script disabled. + // The command channel needs JavaScript because a drag is not a form + // submission; a comment is, and making it depend on the script would + // add a failure mode for no gain. + s.push_str( + "

what are you thinking?

\ +
\ + \ +
", + ); + if meta.is_empty() { + return; + } + s.push_str("

this session

"); + for (i, line) in meta.iter().enumerate() { + if i > 0 { + s.push_str("
"); + } + s.push_str(&esc(line)); + } + s.push_str("
"); +} + /// The table itself: problems, relationships, seats, solutions, outcome. /// /// Factored out of [`document`] so [`ending`] shows the SAME table rather diff --git a/crates/cb-render-html/src/input.rs b/crates/cb-render-html/src/input.rs index 040d78b..99f5e9c 100644 --- a/crates/cb-render-html/src/input.rs +++ b/crates/cb-render-html/src/input.rs @@ -56,6 +56,91 @@ impl PointerFact { } } +/// What a test player wrote, and where they wrote it (CB-WP-0027, +/// ADR-0014 D1). +/// +/// **A second channel that cannot carry a move.** ADR-0007 D5 governs the +/// *command* channel and is untouched: [`PointerFact`] still admits two +/// ids and refuses every other field. This type carries text, and there is +/// **no code path from it to a `GroundCommand`** — [`crate::resolve`] +/// takes a `PointerFact` and nothing else, so a note cannot become a move +/// by any route, including a future careless one. +/// +/// If this struct ever grows a field the engine reads, that separation is +/// gone and ADR-0007 says to revisit the decision rather than widen the +/// control. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Note { + /// The player's own words, verbatim. Never interpreted, only stored + /// and escaped on render. + pub text: String, +} + +impl Note { + /// Parse the wire form `note=`, percent-decoded. + /// + /// Refuses unknown fields for the same reason `PointerFact` does: a + /// parser that ignores what it does not understand cannot tell a typo + /// from an attack. + pub fn parse(body: &str) -> Result { + let mut text = None; + for pair in body.split('&') { + match pair.split_once('=') { + Some(("note", v)) => text = Some(percent_decode(v)), + _ => return Err(format!("unrecognised field in note: {pair:?}")), + } + } + match text { + // An empty note is refused rather than stored: a blank line in + // the trial log is noise in the register (CB-WP-0027 T03). + Some(t) if !t.trim().is_empty() => Ok(Self { + text: t.trim().to_string(), + }), + _ => Err("an empty note is not recorded".to_string()), + } + } +} + +/// `application/x-www-form-urlencoded` decoding, enough for one field. +/// +/// Hand-rolled because this crate has **no third-party dependencies at +/// all** and ADR-0007 §D3 requires an argument before it acquires one — +/// which a form decoder does not merit. +fn percent_decode(s: &str) -> String { + let bytes = s.as_bytes(); + let mut out: Vec = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + match bytes[i] { + b'+' => { + out.push(b' '); + i += 1; + } + b'%' if i + 2 < bytes.len() => { + let hex = std::str::from_utf8(&bytes[i + 1..i + 3]).unwrap_or(""); + match u8::from_str_radix(hex, 16) { + Ok(b) => { + out.push(b); + i += 3; + } + // A malformed escape is kept literally rather than + // dropped: losing characters silently is how a note + // stops meaning what its author wrote. + Err(_) => { + out.push(bytes[i]); + i += 1; + } + } + } + b => { + out.push(b); + i += 1; + } + } + } + String::from_utf8_lossy(&out).into_owned() +} + pub fn action_id(a: Action) -> &'static str { match a { Action::Investigate => "action-investigate", diff --git a/crates/cb-render-html/src/lib.rs b/crates/cb-render-html/src/lib.rs index 7409f5b..a460ab4 100644 --- a/crates/cb-render-html/src/lib.rs +++ b/crates/cb-render-html/src/lib.rs @@ -48,7 +48,7 @@ pub mod jsrun; pub mod serve; pub use doc::{document, text_of}; -pub use input::{resolve, PointerFact}; +pub use input::{resolve, Note, PointerFact}; pub use serve::{Guard, Refusal, Request}; #[cfg(test)] @@ -599,6 +599,7 @@ mod gamelog { Some(PlayerId(0)), false, log, + &[], ) } @@ -706,6 +707,191 @@ mod piles { } } +/// CB-WP-0027 T03 — the note channel, and the two things it must not do. +#[cfg(test)] +mod notes { + use cb_kernel::PlayerId; + + use crate::doc::{document_with_log, text_of}; + use crate::input::{resolve, Note, PointerFact}; + + /// **The load-bearing control (ADR-0014 D1).** A note whose text is a + /// perfectly well-formed pointer fact must not become a move. + /// + /// Structural, not vigilance: `Note::parse` returns a `Note`, + /// `resolve` takes a `PointerFact`, and nothing converts between + /// them. This asserts the behaviour anyway, because "the types don't + /// connect" is a claim about code layout until something checks it. + #[test] + fn a_note_that_looks_like_a_move_is_not_one() { + let hostile = "note=down%3Daction-solve%26up%3Dproblem-1"; + let note = Note::parse(hostile).expect("it parses as a note"); + assert_eq!( + note.text, "down=action-solve&up=problem-1", + "the text is stored verbatim, uninterpreted" + ); + + // And the command parser refuses the same body outright — the two + // channels do not overlap even at the wire level. + assert!( + PointerFact::parse(hostile).is_err(), + "the command channel accepted a note body" + ); + // The reverse, so this is not vacuous: a real pointer fact IS a + // command, and is NOT a note. + assert!(PointerFact::parse("down=a&up=b").is_ok()); + assert!( + Note::parse("down=a&up=b").is_err(), + "the note channel accepted a pointer fact" + ); + + // Nothing in the crate turns a Note into a command. `resolve`'s + // signature is the proof; this pins it against a careless change. + let legal: Vec = vec![]; + assert!(resolve(&PointerFact::new("x", "y"), &legal, PlayerId(0)).is_err()); + } + + /// An empty note is refused, not stored — a blank row is noise in the + /// register. + #[test] + fn an_empty_note_is_refused() { + assert!(Note::parse("note=").is_err()); + assert!(Note::parse("note=%20%20").is_err(), "whitespace is empty"); + assert_eq!( + Note::parse("note=%20hello%20").expect("real text").text, + "hello", + "surrounding whitespace is trimmed" + ); + } + + /// Percent and `+` decoding, since the form posts urlencoded. + #[test] + fn the_players_words_survive_the_wire() { + let n = Note::parse("note=why+is+SOLVE+doing+nothing%3F").expect("parses"); + assert_eq!(n.text, "why is SOLVE doing nothing?"); + } + + /// **The first hostile input this renderer has handled.** Until now + /// `esc()` escaped suit names. + #[test] + fn a_note_containing_markup_renders_as_text() { + let hostile = " & \"quoted\""; + let html = document_with_log( + &crate::testfix::view(Some(PlayerId(0))), + &[], + "/command?t=x", + Some(PlayerId(0)), + false, + &[], + &[hostile.to_string()], + ); + assert!( + !html.contains(""), + "a note's markup reached the document unescaped" + ); + assert!( + html.contains("<script>"), + "the note should still be visible, escaped" + ); + assert!( + text_of(&html).contains("alert(1)"), + "escaping must not eat the player's words — they still read what they wrote" + ); + } + + /// The comment box is always offered, and the page works without it + /// being used. A control that appears only sometimes trains a player + /// not to look for it. + #[test] + fn the_comment_box_is_always_there() { + let html = document_with_log( + &crate::testfix::view(Some(PlayerId(0))), + &[], + "/command?t=x", + Some(PlayerId(0)), + false, + &[], + &[], + ); + assert!(html.contains("action=\"/note\""), "no comment box"); + assert!( + html.contains("method=\"post\""), + "a plain form, so it works with the script disabled" + ); + } +} + +/// CB-WP-0027 T02 — the table on the left, the meta on the right. +#[cfg(test)] +mod two_columns { + use cb_kernel::PlayerId; + + use crate::doc::document_with_log; + + fn page(meta: &[String]) -> String { + document_with_log( + &crate::testfix::view(Some(PlayerId(0))), + &[], + "/command?t=x", + Some(PlayerId(0)), + false, + &[], + meta, + ) + } + + /// The columns exist, and the game is in the left one. + #[test] + fn the_table_is_in_the_game_column_and_the_log_is_not() { + let html = page(&[]); + let game = html + .split("class=\"cb-game\"") + .nth(1) + .and_then(|s| s.split("class=\"cb-meta\"").next()) + .expect("a game column followed by a meta column"); + assert!( + game.contains("

problems

"), + "the table must be in the game column" + ); + assert!( + !game.contains("

log

"), + "the log belongs in the meta column — it is commentary on the game, not part of it" + ); + } + + /// **A player who writes nothing must not be worse off.** An empty + /// meta panel renders as nothing, not as an empty heading. + #[test] + fn an_empty_meta_panel_adds_no_furniture() { + assert!( + !page(&[]).contains("this session"), + "an empty session panel drew a heading with nothing under it" + ); + assert!( + page(&["2 games this session".into()]).contains("this session"), + "a non-empty panel must appear — otherwise the test above passes for a panel that never renders" + ); + } + + /// The single-column fallback is deliberate, not incidental. Asserted + /// on the stylesheet because there is no browser here to resize — + /// which is a weaker test than laying it out, and is said so rather + /// than dressed up. + #[test] + fn the_layout_collapses_to_one_column_on_a_narrow_viewport() { + let html = page(&[]); + assert!( + html.contains("@media (max-width:64rem)"), + "no narrow-viewport rule: the two-column layout would overlap on a laptop" + ); + assert!( + html.contains("minmax(0,1fr)"), + "grid children default to min-content width; without minmax(0,…) the SVG table \ + refuses to shrink and pushes the meta column off-screen" + ); + } +} + /// CB-WP-0024 T03 — what the other seats played, drawn as cards. /// /// The maintainer could follow the other players only by reading the log. diff --git a/decisions/ADR-0014-the-commentary-track.md b/decisions/ADR-0014-the-commentary-track.md new file mode 100644 index 0000000..db46e1a --- /dev/null +++ b/decisions/ADR-0014-the-commentary-track.md @@ -0,0 +1,197 @@ +# ADR-0014: a second channel that cannot carry a move, and a trial log that a command can read + +status: accepted +date: 2026-08-06 +decided by: agent, under the standing loop authorization +tier: M (structural M — touches ADR-0007 D5's input contract and the +recorded-session format; chaos d8=7 → no override). Tier M merges survey +and decision into one document, which this is. +references: [CB-WP-0027](../workplans/CB-WP-0027-the-commentary-track.md), +[ADR-0007](ADR-0007-render-html-not-a-port.md) D5, +[ADR-0012](ADR-0012-the-design-instrument.md) D6 (notes), +[GameDesign.md](../specs/GameDesign.md) §3.1 and §5 + +## Context + +A test player wants to say *why* they played a move, and *what confused +them*, while the position is still on screen — and have it reviewable +later. + +**GameDesign §5 already specifies the artifact**: a trial is a +`--record`ed session plus a sibling observation log. What it does not +specify is how the log gets written, and as written it asks the player to +reconstruct their reasoning afterwards from memory. That is why no trial +log exists yet. + +## The survey, such as it is + +**The candidates are all in this repo**, so there is nothing external to +benchmark against — which is why tier M's merged form is enough. + +| existing mechanism | what it does | why it is not this | +|---|---|---| +| `provisional: true` + owner + date | marks an undecided *rule* | about the dataset, not about play | +| the finding register | records findings with reproductions | needs the reproduction this is meant to supply | +| `` + a table | a machine-readable block inside a human document | **the right shape — reused below** | +| the game log | what happened | not why, and not how it felt | +| `.cbreplay` bundles | the position, replayable | the anchor, not the annotation | + +**The one thing worth stealing is the register's own idiom**: a table +between HTML-comment markers inside a readable Markdown file, parsed by a +small tool. It survived contact in `FindingRegister.md`, `design.py` +already parses that shape, and it means a trial log is readable by a human +without a tool and by a tool without a parser library. + +--- + +## D1 — a comment is not a command, and the types must make that true + +**ADR-0007 D5 is scoped, not amended.** + +D5 says *JavaScript may not construct commands — the page reports raw +pointer facts, and Rust decides what they mean.* That rule is about the +**command channel** and it stays exactly as it is. + +The relevant fact is that `PointerFact::parse` **refuses any unrecognised +field**: + +```rust +_ => return Err(format!("unrecognised field in pointer fact: {pair:?}")), +``` + +So a comment cannot be smuggled through the existing path even by +accident. **Rather than widening that parser, the comment gets its own +one**, and the separation is structural: + +> **`POST /command` carries pointer facts. `POST /note` carries text.** +> `Note` is a distinct type with **no code path to `GroundCommand`** — +> `resolve()` takes a `PointerFact` and nothing else, so a note cannot +> become a move by any route, including a future careless one. + +**Control:** a test posts a note whose body is a well-formed pointer fact +(`down=action-solve&up=problem-1`) and asserts **no command results and +the game does not advance**. Without it, "the channels are separate" is a +claim about code layout rather than about behaviour. + +**What would falsify D1:** if `Note` ever acquires a field the engine +reads, the separation is gone and ADR-0007 says to revisit the decision +rather than widen the control. The note type carries text, a position, and +a timestamp. Nothing else. + +## D2 — comments live in the trial log, not in the scenario + +**Not in `ScenarioFile`.** It is `deny_unknown_fields` and CB-WP-0026 just +demonstrated why that is right — but the reason to keep prose out is not +strictness, it is that **a scenario is a game record and must stay one.** +Scenarios are executed by `make sim`, replayed by `replay-test`, and +compared by hash. A player's opinion in that file would be data the runner +must ignore, which is how a format rots. + +**In `trials/-.md`**, exactly as GameDesign §5 already says, +with the recording beside it as `trials/-.yaml`. + +The log is a readable Markdown file with one machine-readable block: + +``` + +| n | round | step | state_hash | comment | + +``` + +**Reusing `FindingRegister.md`'s idiom deliberately** — same marker shape, +same table form, so `design.py`'s parser is the model and a reader who has +seen one has seen both. + +## D3 — the binding is the state hash, with round and step for humans + +A comment records **`state_hash`** (`cb_events::state_hash_hex`), plus +`round` and `step`. + +- **The hash is the binding.** It is what already makes a session + comparable to its replay (`Summary::end_state_hash`), so a comment keyed + to it names a position a reader can *reach*, not merely describe. +- **Round and step are for reading.** *"Round 3, Resolve"* orients a human + instantly; a hash does not. + +**A comment whose hash no longer appears in its recording is reported as +`orphaned`, not deleted.** The position may have moved because the engine +changed, and that is worth knowing — it is the same reasoning that made +`gr-e01` a rewrite rather than a deletion (CB-WP-0021 T03), and the same +signal as a reproduction that has gone green (GameDesign §1.3). + +## D4 — comments stay here; only findings travel + +**The retention question, decided before any comment is written.** + +These are the maintainer's own words about his own game, written in the +moment, and some will be unflattering about the design, the engine, or +both. That is the point — a commentary track that people self-censor into +is worthless. + +> **Raw comments never leave clay-borg.** Nothing auto-forwards to +> `ground-game`. +> +> A comment reaches `ground-game` **only** by being promoted to a register +> finding and reported through the existing path — which requires a human +> to promote it, and requires the finding to meet GameDesign §1. + +So the trial log is a private notebook, and the register is the published +surface. **The promotion step is where wording gets chosen deliberately**, +which is the right place for it: *"the DARVO sequence is infuriating"* is +useful signal and a bad way to open a message to the game's designer. + +**Who decides: the maintainer, per comment, at promotion time.** Not a +rule, not a default, and not the agent. + +## D5 — a comment is a note, and inherits the note tier + +A promoted comment enters the register as a **note** (ADR-0012 D6): kind +by judgment, state `note`, 30-day expiry on the existing machinery. + +**It is not automatically a finding**, even though it has a position. +GameDesign §1 wants a reproduction that *shows the claimed thing* — a +recording proves the position existed, not that anything is wrong with it. +*"This felt pointless"* plus a replayable state is a strong note and still +a note. + +**What promotes it further** is the same as for any other note: an +artifact demonstrating the defect. The trial log makes that cheap, because +the position is already recorded and someone can go and build a scenario +from it. + +## D6 — surfacing is the deliverable + +`make trials` reports every comment with its position and age, across all +trial logs, and flags orphans. + +**Storage without surfacing would make this the third instance of this +project's signature failure** — after the four-day unread message and the +ten uncollected rulings. The workplan says no task may be called done +while comments are write-only, and this ADR agrees: **T04 is the pass, T03 +is plumbing.** + +**Control:** the reporting path is exercised by `--self-test`, not only +the parser. `design-baseline.py` had a green self-test and an unexercised +reporting path, and the reporting path is where it rotted (ADR-0012 D8). + +## Consequences + +- `cb-render-html` gains a `Note` type and a `/note` route; `PointerFact` + is untouched. +- `trials/` is created, with `.md` + `.yaml` per trial. +- `make trials` reports them; `--self-test` covers the reporting path. +- `specs/GameDesign.md` §5 gains the log's concrete format — it specified + the artifact and not its shape. +- Comments are escaped on render (`esc`), and this is the **first + user-authored text this renderer has handled**. + +## What was rejected + +| rejected | why | +|---|---| +| comments inside `ScenarioFile` | a scenario is a game record; prose there is data the runner must ignore | +| widening `PointerFact` to carry text | D5's parser refusing unknown fields is a control, not an inconvenience | +| free-form prose with no machine-readable block | unreadable by a command, which is the failure mode this exists to avoid | +| auto-forwarding comments to `ground-game` | invites self-censorship, and the promotion step is where wording belongs | +| treating a positioned comment as a finding | a recording proves the position existed, not that anything is wrong | +| deleting orphaned comments | a moved position is a signal, per CB-WP-0021 T03 | diff --git a/tools/cb-play/src/hotseat.rs b/tools/cb-play/src/hotseat.rs index c8112a8..d853c3f 100644 --- a/tools/cb-play/src/hotseat.rs +++ b/tools/cb-play/src/hotseat.rs @@ -13,13 +13,14 @@ //! result is legal. use std::cell::RefCell; +use std::fmt::Write as _; use std::io::{Read, Write}; use std::net::{TcpListener, TcpStream}; use std::rc::Rc; use cb_game_runtime::{Project, Viewer}; use cb_kernel::PlayerId; -use cb_render_html::{resolve, Guard, PointerFact, Request}; +use cb_render_html::{resolve, Guard, Note, PointerFact, Request}; use games_ground::bot::{Choice, Policy}; use games_ground::{GroundCommand, GroundState}; @@ -31,9 +32,71 @@ pub struct Server { /// The live account of the game, shared with the driver (CB-WP-0018 /// T02). Read at render time so the page shows what has happened. journal: games_ground::bot::Journal, + /// What the meta panel shows about the session (CB-WP-0027 T02). + /// Written by the driver between games, read at render time. + meta: RefCell>, + /// Where the trial log is written, if this session is a trial. + trial: Option, + /// Notes so far, so the log is rewritten whole rather than appended + /// to — a partial write leaves a file that neither a human nor + /// `make trials` can read. + notes: RefCell>, +} + +/// One thing a player said, and where they said it (ADR-0014 D3). +#[derive(Debug, Clone)] +pub struct TrialNote { + pub n: usize, + pub round: u8, + pub step: String, + pub state_hash: String, + pub text: String, +} + +/// Write the trial log: readable Markdown with one machine-readable block. +/// +/// **Reusing `FindingRegister.md`'s idiom deliberately** (ADR-0014 D2) — +/// a table between HTML-comment markers, so a human reads the file and a +/// tool reads the block, and neither needs the other's cooperation. +pub fn write_trial_log(path: &std::path::Path, notes: &[TrialNote]) -> Result<(), String> { + let mut s = String::new(); + s.push_str("# Trial log\n\n"); + s.push_str( + "What the player said while playing, bound to the position they said it at\n\ + (CB-WP-0027, ADR-0014). The recording beside this file is the position;\n\ + `state_hash` is what reaches it.\n\n\ + **These are raw notes and they stay here.** Nothing in this file travels to\n\ + `ground-game` (ADR-0014 D4) — a note reaches them only by being promoted to a\n\ + register finding, by a human, with the wording chosen then.\n\n", + ); + s.push_str("\n\n"); + s.push_str("| n | 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 + // literal `|` matters less than a log nothing can parse. + let text = note.text.replace(['\n', '\r'], " ").replace('|', "/"); + let _ = writeln!( + s, + "| {} | {} | {} | {} | {} |", + note.n, note.round, note.step, note.state_hash, text + ); + } + s.push_str("\n\n"); + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir).map_err(|e| format!("trial dir: {e}"))?; + } + std::fs::write(path, s).map_err(|e| format!("write trial log: {e}")) } impl Server { + /// Where this session's trial log goes. Without one, the note + /// channel refuses rather than dropping what the player wrote. + pub fn with_trial(mut self, path: Option) -> Self { + self.trial = path; + self + } + pub fn bind(port: u16) -> Result { let listener = cb_render_html::serve::bind(port).map_err(|e| format!("bind: {e}"))?; let port = listener @@ -45,6 +108,9 @@ impl Server { guard: Guard::mint(port), log: RefCell::new(Vec::new()), journal: games_ground::bot::Journal::default(), + meta: RefCell::new(Vec::new()), + trial: None, + notes: RefCell::new(Vec::new()), }) } @@ -54,6 +120,42 @@ impl Server { self.guard.page_url() } + /// Append a note to the trial log, bound to the position it was + /// written at (ADR-0014 D2/D3). + /// + /// **The state hash is the binding.** Round and step orient a reader; + /// the hash is what lets one *reach* the position, because it is the + /// same value that makes a session comparable to its replay. + fn record_note(&self, state: &GroundState, note: &Note) -> Result<(), String> { + let Some(path) = self.trial.as_ref() else { + // No trial log configured: refuse rather than drop. A note + // that vanishes is worse than a note that was never offered, + // and this is the failure mode the whole pass exists to avoid. + return Err("no trial log for this session — start with --trial".into()); + }; + let hash = cb_events::state_hash_hex(state); + let mut log = self.notes.borrow_mut(); + let n = log.len() + 1; + log.push(TrialNote { + n, + round: state.round, + step: format!("{:?}", state.step), + state_hash: hash[..12].to_string(), + text: note.text.clone(), + }); + write_trial_log(path, &log) + } + + /// 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 + /// **while playing** rather than only on the ending page — which is + /// where it was, and a tally you only see once the game is over + /// informs nothing. + pub fn set_meta(&self, lines: Vec) { + *self.meta.borrow_mut() = lines; + } + /// The journal the driver appends to; hand it to `play_journaled`. pub fn journal(&self) -> games_ground::bot::Journal { self.journal.clone() @@ -145,6 +247,10 @@ impl Server { Some(seat), may_pass, &self.log_lines(), + // CB-WP-0027 T02: the meta panel's own content. + // The running tally lives with the driver, so the + // server is handed the lines rather than the state. + &self.meta.borrow(), ); respond(&mut stream, 200, "text/html; charset=utf-8", &page); } @@ -170,6 +276,23 @@ impl Server { Err(why) => respond(&mut stream, 200, "text/plain", &why), } } + // CB-WP-0027 T03 / ADR-0014 D1. A SECOND CHANNEL, and it + // cannot carry a move: `Note::parse` returns a `Note`, + // `resolve` takes a `PointerFact`, and there is no + // conversion between them. The game does not advance here + // and the loop keeps waiting for the seat's actual move. + ("POST", "/note") => match Note::parse(&req.body) { + Ok(note) => match self.record_note(state, ¬e) { + Ok(()) => { + // 303 so the browser re-GETs the table rather + // than leaving a form POST in history — a + // reload would otherwise re-submit the note. + respond_seeother(&mut stream, "/"); + } + Err(e) => respond(&mut stream, 500, "text/plain", &e), + }, + Err(why) => respond(&mut stream, 400, "text/plain", &why), + }, _ => respond(&mut stream, 404, "text/plain", "no such thing here"), } } @@ -554,6 +677,15 @@ fn find(haystack: &[u8], needle: &[u8]) -> Option { haystack.windows(needle.len()).position(|w| w == needle) } +/// 303 See Other, so the browser re-GETs the table after a form POST +/// rather than leaving the POST in history — a reload would otherwise +/// re-submit the note and duplicate it. +fn respond_seeother(stream: &mut TcpStream, to: &str) { + let head = format!("HTTP/1.1 303 See Other\r\nLocation: {to}\r\nContent-Length: 0\r\n\r\n"); + let _ = stream.write_all(head.as_bytes()); + let _ = stream.flush(); +} + fn respond(stream: &mut TcpStream, status: u16, content_type: &str, body: &str) { let reason = match status { 200 => "OK", @@ -658,6 +790,7 @@ mod tests { replay_dir: None, record: None, serve: Some(0), + trial: None, }, std::io::Cursor::new(Vec::new()), out, @@ -825,6 +958,7 @@ mod tests { replay_dir: None, record: None, serve: Some(0), + trial: None, }, std::io::Cursor::new(Vec::new()), out, diff --git a/tools/cb-play/src/inspect.rs b/tools/cb-play/src/inspect.rs index 9e23f53..826b8c9 100644 --- a/tools/cb-play/src/inspect.rs +++ b/tools/cb-play/src/inspect.rs @@ -762,6 +762,7 @@ mod tests { replay_dir: Some(dir.clone()), record: Some(dir.join("session.yaml")), serve: None, + trial: None, }; let mut sink: Vec = Vec::new(); let summary = diff --git a/tools/cb-play/src/main.rs b/tools/cb-play/src/main.rs index 15fae45..307c66c 100644 --- a/tools/cb-play/src/main.rs +++ b/tools/cb-play/src/main.rs @@ -28,6 +28,7 @@ play: --bot KIND policy for every other seat: greedy (default) or random --replay DIR write a .cbreplay bundle of the finished game to DIR --record FILE write the finished game as a scenario YAML + --trial FILE write a trial log: what the player said, bound to where --serve PORT play human seats in a browser on 127.0.0.1:PORT instead of on the terminal; 0 lets the OS pick. Prints a URL carrying a per-process token — without it the page is @@ -104,6 +105,13 @@ fn parse_args(argv: &[String]) -> Result { config.record = Some(value(i, argv, flag)?.into()); i += 2; } + // CB-WP-0027: a trial is a recorded session PLUS what the + // player said while playing (GameDesign §5, ADR-0014). + "--trial" => { + play_flags.push(flag.into()); + config.trial = Some(value(i, argv, flag)?.into()); + i += 2; + } "--replay" => { play_flags.push(flag.into()); config.replay_dir = Some(value(i, argv, flag)?.into()); @@ -260,6 +268,7 @@ mod tests { replay_dir: None, record: None, serve: None, + trial: None, }; let script = "0\n".repeat(400); let mut out: Vec = Vec::new(); @@ -299,6 +308,7 @@ mod tests { replay_dir: None, record: None, serve: None, + trial: None, }; let mut out: Vec = Vec::new(); table::play(&config, "0\n".repeat(200).as_bytes(), &mut out).expect("game"); @@ -361,6 +371,7 @@ mod tests { replay_dir: None, record: None, serve: None, + trial: None, }; let mut out: Vec = Vec::new(); let summary = table::play(&config, "".as_bytes(), &mut out).expect("bot game"); @@ -421,6 +432,7 @@ mod tests { replay_dir: Some(dir.clone()), record: Some(dir.join("session.yaml")), serve: None, + trial: None, }; let mut out: Vec = Vec::new(); let summary = table::play(&config, "".as_bytes(), &mut out).expect("game"); diff --git a/tools/cb-play/src/table.rs b/tools/cb-play/src/table.rs index 4c84f50..1c859fc 100644 --- a/tools/cb-play/src/table.rs +++ b/tools/cb-play/src/table.rs @@ -36,6 +36,9 @@ pub struct Config { /// Serve human seats in a browser instead of on the terminal /// (ADR-0007). `Some(0)` lets the OS pick the port. pub serve: Option, + /// Where the trial log goes (CB-WP-0027). Without it the note channel + /// refuses rather than dropping what the player wrote. + pub trial: Option, } impl Default for Config { @@ -48,6 +51,7 @@ impl Default for Config { replay_dir: None, record: None, serve: None, + trial: None, } } } @@ -287,7 +291,9 @@ pub fn play(config: &Config, input: R, out: W) -> Result { - let s = std::rc::Rc::new(crate::hotseat::Server::bind(port)?); + let s = std::rc::Rc::new( + crate::hotseat::Server::bind(port)?.with_trial(config.trial.clone()), + ); let _ = writeln!(out.borrow_mut(), " open {}", s.url()); let _ = out.borrow_mut().flush(); Some(s) @@ -301,6 +307,11 @@ pub fn play(config: &Config, input: R, out: W) -> Result-.yaml \\") + print(" --trial trials/-.md") + return 0 + + found = logs(root) + print("trials — what the players said, and where\n") + total, orphans, pending, expired = 0, 0, 0, 0 + for path, rows, recording in found: + name = os.path.basename(path) + age = age_days(path, today) + print(f" {name} ({len(rows)} note(s), {age}d" + f"{', recording not written yet' if not recording else ''})") + for r in rows: + total += 1 + state = reachability(r, recording) + orphans += state == "orphan" + pending += state == "pending" + expired += age > NOTE_EXPIRY_DAYS + mark = {"orphan": "orphan", "pending": " .. ", "ok": " "}[state] + print(f" {mark} r{r['round']:<2} {r['step']:<8} {r['state_hash']:<12} " + f"{r['comment'][:70]}") + print() + + print(f" trial logs {len(found)}") + print(f" notes {total}") + print(f" positions unreachable {orphans} target 0" + + (" <-- the recording exists but the position moved" if orphans else "")) + if pending: + print(f" awaiting a recording {pending}" + " (normal during a live session; the recording lands at game end)") + print(f" notes past {NOTE_EXPIRY_DAYS} days {expired} target 0") + if total and not orphans: + print("\n Every note points at a position a reader can reach.") + print("\n Raw notes stay in this repo (ADR-0014 D4). A note reaches" + "\n ground-game only by being promoted to a register finding, by a" + "\n human, with the wording chosen then.") + return 0 + + +def self_test(): + """Positive controls, including for the REPORTING path. + + `design-baseline.py` had a green self-test and an unexercised + reporting path, and the reporting path is where it rotted (ADR-0012 + D8). So this runs `report` against a fixture and checks what it says. + """ + import tempfile, io, contextlib + + ok = True + + def check(name, cond, detail=""): + nonlocal ok + ok = ok and bool(cond) + print(f" [{'ok ' if cond else 'FAIL'}] {name}" + (f" — {detail}" if detail else "")) + + good = (f"# Trial log\n\n{BEGIN}\n\n" + "| n | round | step | state_hash | comment |\n|---|---|---|---|---|\n" + "| 1 | 3 | Select | abc123def456 | why is SOLVE doing nothing |\n" + f"\n{END}\n") + check("a trial log parses", len(parse(good)) == 1) + check("the header row is not a note", all(r["n"] != "n" for r in parse(good))) + + try: + parse("# Trial log\n\nno block here\n") + check("a file with no block is an error, not an empty log", False) + except ValueError: + check("a file with no block is an error, not an empty log", True, + "silently reporting zero is the failure this tool prevents") + + with tempfile.TemporaryDirectory() as d: + open(os.path.join(d, "2026-08-06-x.md"), "w").write(good) + # No .yaml beside it → the position cannot be reached. + rows = parse(good) + check("a note with no recording is PENDING, not orphaned", + reachability(rows[0], None) == "pending", + "a live session must not report orphans") + rec = os.path.join(d, "2026-08-06-x.yaml") + open(rec, "w").write("state_hash: abc123def456\n") + check("a note whose hash is in the recording is reachable", + reachability(rows[0], rec) == "ok", + "without this the check could always say orphan") + open(rec, "w").write("state_hash: something-else\n") + check("a note whose hash is absent from a REAL recording is an orphan", + reachability(rows[0], rec) == "orphan", + "the position moved \u2014 the case the flag exists for") + + # THE control design-baseline.py lacked: exercise the reporting + # path and assert on what it printed. + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + report(d, datetime.date(2026, 8, 6)) + out = buf.getvalue() + check("the reporting path runs and names the note", + "why is SOLVE doing nothing" in out, "not just the parser") + check("the reporting path counts", "notes 1" in out) + + print("trials self-test (positive control)") + return 0 if ok else 1 + + +if __name__ == "__main__": + sys.exit(self_test() if "--self-test" in sys.argv else report()) diff --git a/workplans/CB-WP-0027-the-commentary-track.md b/workplans/CB-WP-0027-the-commentary-track.md index dc6c9c0..f3d0020 100644 --- a/workplans/CB-WP-0027-the-commentary-track.md +++ b/workplans/CB-WP-0027-the-commentary-track.md @@ -2,7 +2,7 @@ id: CB-WP-0027 kind: product title: "The commentary track: the meta view beside the table, and what the player says while playing" -status: ready +status: active state_hub_workstream_id: "e011da8d-de9e-48bf-a3f2-a9e715ef222f" --- @@ -80,7 +80,7 @@ write-only. ```task id: CB-WP-0027-T01 -status: todo +status: done priority: high state_hub_task_id: "3586f3af-ad45-4bc6-bea2-b6eaedf05fbf" ``` @@ -117,11 +117,40 @@ about his own game**, and one of them will eventually be unflattering about the design or about the engine. State whether they are private to the repo, whether they travel to `ground-game`, and who decides. +**Done 2026-08-06.** +[ADR-0014](../decisions/ADR-0014-the-commentary-track.md), six decisions. + +**(b) turned out to be the easy one, because the control already exists.** +`PointerFact::parse` **refuses any unrecognised field**, so a comment +cannot reach the command path even by accident. **D5 is scoped, not +amended**: `/command` carries pointer facts, `/note` carries text, and +`Note` has **no code path to `GroundCommand`** — `resolve()` takes a +`PointerFact` and nothing else. The control is a test that posts a note +whose body is a well-formed pointer fact and asserts the game does not +advance; without it, "separate channels" is a claim about code layout. + +**(a) the trial log, not the scenario** — and the reason is not +strictness. A scenario is executed, replayed and hashed; prose in it is +data the runner must ignore, which is how a format rots. The log reuses +`FindingRegister.md`'s idiom: a table between HTML-comment markers, so +`design.py`'s parser is the model. + +**(c) the state hash binds; round and step are for reading.** An orphaned +comment — hash no longer in its recording — is **reported, not deleted**, +same reasoning as `gr-e01`'s rewrite and a green reproduction's alarm. + +**Retention: raw comments never leave clay-borg.** A comment reaches +`ground-game` only by being promoted to a register finding, by a human, at +which point the wording is chosen deliberately. *"The DARVO sequence is +infuriating"* is useful signal and a bad way to open a message to the +game's designer. **The maintainer decides per comment** — not a rule, not +a default, not the agent. + ## Task: the table on the left, the meta on the right ```task id: CB-WP-0027-T02 -status: todo +status: done priority: high state_hub_task_id: "0971185c-b72c-43ec-a356-c221ef6d8165" ``` @@ -150,11 +179,29 @@ not part of the table. - the game column must be usable with the meta column collapsed. A player who does not want to write anything must not be worse off. +**Done 2026-08-06.** CSS grid, `minmax(0,1fr)` on both tracks and a +single-column fallback under 64rem. + +**`minmax(0, …)` is the load-bearing part.** A grid child defaults to +min-content width, so without it the SVG table refuses to shrink and +pushes the meta column off-screen instead of narrowing — the layout would +look correct on the developer's monitor and be broken everywhere else. + +**The running tally moved into the panel**, so it is visible *while +playing*. It only appeared on the ending page before, and a session score +you see once the game is over informs nothing. + +Three tests: the log is in the meta column and the table is not; an empty +panel draws **no furniture** (with the inverse, so it does not pass for a +panel that never renders); and the narrow-viewport rule exists — asserted +on the stylesheet, which is **weaker than laying it out**, and said so +rather than dressed up. + ## Task: capture what the player says, bound to where they said it ```task id: CB-WP-0027-T03 -status: todo +status: done priority: high state_hub_task_id: "4a23ba0d-9b23-4381-b6b0-2b8959106270" ``` @@ -178,11 +225,33 @@ first time that comment is load-bearing rather than precautionary. - **the game is playable with comments disabled**, and a test says so. The commentary track must not become a dependency of playing. +**Done 2026-08-06.** `Note` in `input.rs`, a `/note` route, `--trial`, and +`trials/-.md` written whole on every note. + +**The separation is structural, and tested anyway.** A note whose text is +a well-formed pointer fact (`down=action-solve&up=problem-1`) parses as a +note, is stored verbatim, and **the command channel refuses the same body +outright** — with the inverse asserted too, so the test is not vacuous. + +**A plain `
`**, so the comment box works with the +script disabled. The command channel needs JavaScript because a drag is +not a form submission; a comment is one, and making it depend on the +script would add a failure mode for nothing. The reply is **303 See +Other**, so a reload does not re-post the note. + +**Verified over real HTTP**, not only in tests: two columns served, a note +posted (303), a hostile note stored as text (303), an empty note refused +(400), and the game did not advance. + +**`esc()`'s first hostile input.** `` renders +escaped **and still readable** — the test asserts the player's words +survive, because escaping that eats the text is its own defect. + ## Task: surface them, or this pass has failed ```task id: CB-WP-0027-T04 -status: todo +status: done priority: high state_hub_task_id: "df8afbef-6ff7-4aa2-9ff7-b3fa05f676cc" ``` @@ -209,6 +278,28 @@ session that produced them, by a command, without opening a file by hand. `tools/design.py` accepts — if the register cannot express it, the shape is wrong and that is a finding about ADR-0012, not a bug. +**Done 2026-08-06.** `tools/trials.py`, `make trials`, wired into +`make self-tests`. + +**The report distinguishes three states, and that distinction came from +running it.** The first version called any note without a recording an +**orphan** — so a live session reported every note as broken, because the +recording is only written at game end. A metric that cries wolf is one +nobody reads, **which is the exact failure this pass exists to prevent**. +Now: `ok`, `pending` (no recording yet), `orphan` (the recording exists +and the position is not in it). Only the third is a target-0 number. + +**The self-test exercises the reporting path**, not just the parser — it +runs `report` against a fixture and asserts on what it printed. +`design-baseline.py` had a green self-test and an unexercised reporting +path, and that is where it rotted (ADR-0012 D8). + +**A latent Makefile defect surfaced.** `make trials` did nothing: `trials` +is also a *directory*, so Make saw an up-to-date file. `design`, +`difficulty` and `trials` — added by CB-WP-0022, CB-WP-0025 and this pass +— were **all** missing from `.PHONY`; only the one that collided revealed +it. All three are declared now. + ## Task: evidence ```task