Compare commits

...

2 commits

Author SHA1 Message Date
291d1e9798 Add CB-WP-0003: harden the loop from its own review
Some checks failed
ci / check (push) Failing after 3s
Covers what the immediate CI fixes did not. Framing finding: writing a
rule into InnerLoop did not prevent the next instance of the failure it
was written for; making it a CI step caught one on its first run. Four
instances of the harness-does-nothing class, zero prevented by prose.

Working hypothesis: a loop rule that cannot be executed is not a rule.
Each item is made executable or deleted, default delete.

Phase A makes rules executable: audit every InnerLoop rule as
executable/checkable/decorative, extend the positive-control gate beyond
benchmarks, and point adversarial review at measurement rather than only
research — all four real errors were in measurement and build config,
which the review never reads.

Phase B is session economics, from measurement: 592 turns, mean context
245k tokens, $0.245/turn in cache reads, zero batched tool calls, cache
reads 58% of cost against output at 12%. Cost is turns x context and
context grows with turns, so a long session is quadratic. Also replaces
the dead 8k/10k token budget, which was never enforced and which T08
exceeded by orders of magnitude with no signal.

Phase C removes or fixes the rest: resolve the chaos roll (never fired,
and at 1-in-10 will stay unevaluated), require retargeting to be an ADR
rather than something the implementer does in the commit that measures
the metric, give provisional items an owner and expiry, and strengthen
the coverage gate beyond tag-counting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-31 04:03:29 +02:00
72c594ee49 CI: enforce every gate; close the silent-skip holes
The gates existed; CI ran half of them and tolerated the failure case.

- cb-sim no longer has a "tolerable" non-zero exit. An unregistered game
  prefix is a failure, and a run in which nothing executed is a failure.
  Previously CI carried `|| test $? -eq 2`, so renaming a scenario prefix
  would have skipped every scenario while the pipeline stayed green.
  Verified with a negative control.
- CI now runs make coverage (AM-1) and make dep-weight (AM-4), both
  added after CI was written and neither enforced until now.
- dep-weight enforces its targets instead of only reporting them.
- CI lints the shipped-runtime configuration separately, so the feature
  split cannot rot unnoticed.
- Dropped the stale `make deps` target, which still measured the retired
  crate-count metric.

The positive-control rule is now executable: CI runs
`cargo bench -- --test`, which executes every benchmark once, so a
workload that stalls fails the build.

That step immediately found a fourth instance of the error class it was
written for. The committed replay benchmark was the broken version — an
earlier patch never applied, leaving a command sequence that omits
Resolve, so every round produced nothing and the log-building loop spun
forever. It had never run to completion; the reported AM-7 replay
numbers came from a probe test instead. Fixed, given the same positive
control as the round loop, and re-measured from the benchmark: 100k
events fold in 2.18ms (95% CI 2.14-2.23), against a 5s budget.

Evidence now reports confidence intervals rather than point estimates,
so the 3% regression rule in MetricsAndScenarios is enforceable.

The finding worth carrying: writing the positive-control rule into
InnerLoop v1.0 did not prevent the next instance. Making it a CI step
did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-31 04:02:33 +02:00
7 changed files with 404 additions and 70 deletions

View file

@ -14,5 +14,27 @@ jobs:
- run: rustup component add rustfmt clippy
- run: cargo fmt --all --check
- run: cargo clippy --workspace --all-targets -- -D warnings
# The shipped runtime is a distinct configuration (AM-4a); it must
# compile and lint on its own, or the feature split rots.
- run: cargo clippy -p games-ground --no-default-features -- -D warnings
- run: cargo test --workspace
- run: cargo run -q -p cb-sim -- scenarios/ground/*.yaml || test $? -eq 2
# No `|| test $? -eq 2`: cb-sim now fails on an unregistered game
# prefix and on a run that executed nothing. A silent skip is the
# failure this step exists to catch.
- run: cargo run -q -p cb-sim -- scenarios/ground/*.yaml
# AM-1: every numbered GR-rule is claimed by a scenario, and no
# scenario claims a rule the spec does not define.
- run: make coverage
# AM-4a/AM-4b: third-party source under audit, per configuration.
- run: make dep-weight
# InnerLoop v1.0 positive control, enforced rather than asserted in
# prose: --test runs every benchmark once, so a workload that
# stalls or produces the wrong event count fails the build instead
# of silently reporting throughput for work that never happened.
- run: cargo bench -p games-ground --bench synthetic -- --test

View file

@ -3,7 +3,7 @@
CARGO := cargo
.PHONY: check test sim bench deps loc all
.PHONY: check test sim bench bench-test coverage dep-weight loc all
## fmt + clippy (deny warnings) + HashMap deny-lint
check:
@ -24,13 +24,15 @@ coverage:
sim:
$(CARGO) run -q -p cb-sim -- scenarios/ground/*.yaml
## Criterion benches (AM-6/AM-7 wiring)
## Criterion benches (AM-6/AM-7)
bench:
$(CARGO) bench -p games-ground
## AM-4: transitive crate count (excludes dev/build deps)
deps:
@$(CARGO) tree --workspace -e normal --prefix none | sed 's/ (\*)//' | sort -u | grep -vE '^(cb-|games-|cb_|$$)' | tee /dev/stderr | wc -l
## InnerLoop positive control: run every bench once, no measurement.
## Fails if a workload stalls or produces the wrong event count.
bench-test:
$(CARGO) bench -p games-ground --bench synthetic -- --test
## AM-2/AM-3 input: source LOC per crate (excludes tests would need tokei)
loc:
@ -38,4 +40,4 @@ loc:
printf '%-28s %s\n' $$d "$$(find $$d/src -name '*.rs' | xargs cat | grep -vcE '^\s*(//|$$)')"; \
done
all: check test sim
all: check test sim coverage dep-weight bench-test

View file

@ -21,7 +21,7 @@ Machine: WSL2, Linux 6.18.33.2-microsoft-standard-WSL2, rustc 1.97.1,
| AM-4b dep weight, dev toolchain | ≤350,000 third-party lines | 317,021 (29 crates) | **met** |
| AM-6 throughput | ≥100,000 events/s | 1,651,400 events/s | **met, 16.5×** |
| AM-7 scaling | ≥0.9× at 20× workload | 1.08× | **met** |
| AM-7 replay | 100k events ≤5s | 4.13 ms | **met, 1,210×** |
| AM-7 replay | 100k events ≤5s | 2.18 ms (CI 2.142.23) | **met, 2,290×** |
| AM-8 determinism | zero divergence, 10 replays | 1 distinct hash / 10 runs | **met** |
| AM-8 lint | fmt + clippy clean | clean, `-D warnings` | **met** |
| AM-10 foreign types | zero `HashMap`/`HashSet` | 0 | **met** |
@ -45,12 +45,17 @@ breaks the test rather than silently rescaling the metric.
| 40,000 | 1,626,000 | 1.07× |
| 100,000 | 1,651,400 | 1.08× |
Replay — folding one growing event log back into state:
Replay — folding one growing event log back into state (Criterion
95% CI, low/median/high):
| Events | Time | Rate |
|---|---|---|
| 10,010 | 465 µs | 21.5M events/s |
| 100,007 | 4.13 ms | 24.2M events/s |
| Events | Time (median) | 95% CI | Rate |
|---|---|---|---|
| 10,010 | 184.8 µs | 180.9 189.3 µs | 54.2M events/s |
| 100,072 | 2.183 ms | 2.142 2.226 ms | 45.8M events/s |
**Correction (2026-07-31).** These originally read 465 µs / 4.13 ms and
were taken from the `replay_probe` **test**, not from the benchmark —
because the benchmark did not work. See §2a.
### The comparison against boardgame.io, stated carefully
@ -100,6 +105,30 @@ The benchmark now asserts the per-round event count on every round and
panics rather than measuring a stalled loop. The corrected figure is
**5.6× lower** than the bogus one.
### 2a. A fourth measurement error, found by enforcing the rule
The replay benchmark committed alongside this evidence was **the broken
version**. A `python3` patch that was supposed to replace its
log-building loop never applied, leaving a sequence that omits `Resolve`
— so `EndRound` was rejected, every round produced no events, and the
`while log.len() < target` loop spun forever. It was never run to
completion; the AM-7 replay numbers were taken from a separate probe
test instead, and the dead benchmark was committed and left hanging.
Found by adding `cargo bench -- --test` to CI, which runs every
benchmark once. That is the fourth instance of one error class in this
project — a harness that appears to work while doing no work — and the
**first one caught by a gate rather than by noticing**.
The replay loop now carries the positive control the round loop already
had: it asserts each round appended events and fails rather than
spinning. Corrected figures are in the table above; both configurations
still clear the AM-7 budget by three orders of magnitude.
The lesson recorded for the loop: writing the positive-control rule into
`specs/InnerLoop.md` did **not** prevent the next instance. Making it a
CI step did. Prose rules do not enforce themselves.
## 3. Determinism (AM-8)
- Every scenario runs twice per invocation with the same seed and fails

View file

@ -122,6 +122,44 @@ const FINAL_ROUND_EVENTS: usize = 12;
/// integers: (4 x 13 + 12) = 64.
const EVENTS_PER_5_ROUNDS: usize = 64;
/// Play one round, appending every applied event to `log`.
fn record_round(state: &mut GroundState, log: &mut Vec<games_ground::GroundEvent>) {
let mut run = |state: &mut GroundState, actor: Actor, cmd: &GroundCommand| {
if let Ok(produced) = state.validate(actor, cmd) {
for event in &produced {
state.fold(event);
log.push(event.clone());
}
}
};
for (seat, action, target) in [
(0u8, Action::Attack, Some(PlayerId(1))),
(2, Action::Support, Some(PlayerId(1))),
(1, Action::Ground, None),
] {
run(
state,
Actor::Player(PlayerId(seat)),
&GroundCommand::SelectAction {
action,
target,
problem: None,
},
);
}
run(state, Actor::System, &GroundCommand::Reveal);
run(
state,
Actor::Player(PlayerId(1)),
&GroundCommand::ChooseGroundMode {
mode: GroundMode::Gr,
choice: None,
},
);
run(state, Actor::System, &GroundCommand::Resolve);
run(state, Actor::System, &GroundCommand::EndRound);
}
fn bench_synthetic(c: &mut Criterion) {
// Events per round is fixed by the workload, so throughput can be
// reported in events/second — the AM-6 unit.
@ -146,55 +184,30 @@ fn bench_synthetic(c: &mut Criterion) {
// flat curve is partly by construction — this one is not, because
// the log here grows without bound.
let mut replay = c.benchmark_group("replay-ground-3p");
for &events in &[10_000usize, 100_000] {
replay.throughput(Throughput::Elements(events as u64));
replay.bench_function(format!("fold-{events}-events"), |b| {
// Build one log of `events` events, then measure folding it.
let mut source = setup(42);
let mut log = Vec::with_capacity(events);
while log.len() < events {
if source.outcome.is_some() {
source = setup(43);
}
let picks = [
(
PlayerId(0),
GroundCommand::SelectAction {
action: Action::Attack,
target: Some(PlayerId(1)),
problem: None,
},
),
(
PlayerId(1),
GroundCommand::SelectAction {
action: Action::Support,
target: Some(PlayerId(2)),
problem: None,
},
),
];
for (seat, cmd) in picks {
if let Ok(produced) = source.validate(Actor::Player(seat), &cmd) {
for e in &produced {
source.fold(e);
log.push(e.clone());
}
}
}
if let Ok(produced) = source.validate(Actor::System, &GroundCommand::Reveal) {
for e in &produced {
source.fold(e);
log.push(e.clone());
}
}
if let Ok(produced) = source.validate(Actor::System, &GroundCommand::EndRound) {
for e in &produced {
source.fold(e);
log.push(e.clone());
}
}
for &target_events in &[10_000usize, 100_000] {
// Build one log by playing real rounds, then measure folding it
// back. Must use the full command sequence: a shortened one
// stalls, because Reveal needs every seat's selection and
// EndRound is gated on Resolve.
let mut log = Vec::with_capacity(target_events);
let mut source = setup(42);
let mut games = 0u64;
while log.len() < target_events {
if source.outcome.is_some() {
games += 1;
source = setup(42 + games);
}
let before = log.len();
record_round(&mut source, &mut log);
// Positive control: a round that yields nothing means the
// workload stalled, and the loop above would spin forever.
assert!(
log.len() > before,
"replay workload stalled: a round produced no events"
);
}
replay.throughput(Throughput::Elements(log.len() as u64));
replay.bench_function(format!("fold-{target_events}-events"), |b| {
b.iter(|| {
let mut state = setup(42);
for event in &log {

View file

@ -1,7 +1,10 @@
//! cb-sim — scenario runner binary (GameKernel K17; precursor of `cb sim`).
//! Parses scenario files, dispatches each to its game by the `<game>/`
//! prefix of its `scenario` field, and executes it. Exit codes: 0 all
//! passed, 1 failures, 2 unknown game, 64 usage error.
//! passed, 1 any failure — including a scenario whose game prefix is not
//! registered, and a run in which nothing executed. There is deliberately
//! no "tolerable" non-zero exit: a silent skip is the failure mode this
//! binary exists to catch.
use cb_game_runtime::{scenario, RunOutcome, ScenarioFile};
use games_ground::GroundState;
@ -13,7 +16,6 @@ fn main() {
std::process::exit(64);
}
let mut unknown_game = false;
let mut failed = false;
let mut passed = 0usize;
let mut covered: Vec<String> = Vec::new();
@ -39,8 +41,14 @@ fn main() {
let outcome = match sc.scenario.split('/').next() {
Some("ground") => scenario::run::<GroundState>(&sc),
_ => {
println!("SKIP {} — no game registered for this prefix", sc.scenario);
unknown_game = true;
// A renamed or typo'd prefix would otherwise skip every
// scenario while the run still looked clean.
eprintln!(
"FAIL {} — no game registered for prefix {:?}",
sc.scenario,
sc.scenario.split('/').next().unwrap_or("")
);
failed = true;
continue;
}
};
@ -67,10 +75,14 @@ fn main() {
covered.dedup();
println!("{passed} passed, {} rules covered", covered.len());
// Positive control: a run that executed nothing must not pass. This
// is the same class of error as a benchmark timing rejected work.
if passed == 0 {
eprintln!("FAIL — no scenario executed; refusing to report success");
failed = true;
}
if failed {
std::process::exit(1);
}
if unknown_game {
std::process::exit(2);
}
}

View file

@ -32,6 +32,13 @@ CONFIGS = {
"dev-toolchain": [],
}
# AM-4a / AM-4b targets from specs/GameKernel.md §4. Breaching one fails
# the build: a gate that only reports is a suggestion.
TARGETS = {
"shipped-runtime": 250_000,
"dev-toolchain": 350_000,
}
def crates(extra_args):
"""Third-party crates in the normal (non-dev) dependency graph."""
@ -116,9 +123,12 @@ def main():
print(f" own source {own:>9,} lines")
for label in CONFIGS:
r = report[label]
limit = TARGETS[label]
mark = "ok " if r["third_party_loc"] <= limit else "FAIL"
print(
f" {label:<18}{r['crates']:>3} crates "
f"{r['third_party_loc']:>9,} lines third-party"
f"{r['third_party_loc']:>9,} lines third-party "
f"[{mark} target {limit:,}]"
)
delta = (
report["dev-toolchain"]["third_party_loc"]
@ -131,7 +141,18 @@ def main():
# shrink the total, so refuse to report rather than under-report.
print("\nERROR — source not found for:", ", ".join(missing), file=sys.stderr)
return 1
return 0
breached = [
(label, report[label]["third_party_loc"], limit)
for label, limit in TARGETS.items()
if report[label]["third_party_loc"] > limit
]
for label, actual, limit in breached:
print(
f"\nFAIL AM-4 — {label}: {actual:,} lines exceeds target {limit:,}",
file=sys.stderr,
)
return 1 if breached else 0
if __name__ == "__main__":

View file

@ -0,0 +1,235 @@
---
id: CB-WP-0003
title: "Harden the inner loop: executable rules, session economics, dead policy"
status: proposed
---
# Purpose
A rigorous review of the loop after its first full pass (CB-WP-0001)
found error modes the loop does not catch and policy that costs
something while delivering nothing. The CI fixes were taken immediately
(commit `72c594e`); this workplan covers the rest.
The finding that frames all of it:
> **Writing a rule into `specs/InnerLoop.md` did not prevent the next
> instance of the failure it was written for. Making it a CI step did.**
InnerLoop v1.0 added the positive-control rule after two measurement
errors. A third occurred immediately afterwards (`default-features`
silently ignored, build green, feature flag gating nothing), and a
fourth was found *only* when the rule became a CI step — the committed
replay benchmark had never run to completion. Four instances of one
error class; the prose rule prevented none; the executable rule caught
one on its first run.
**Working hypothesis for this workplan: a loop rule that cannot be
executed is not a rule.** Every item below is either made executable or
deleted.
## Phase A — Make the rules executable
## Task: Audit every InnerLoop rule for enforceability
```task
id: CB-WP-0003-T01
status: todo
priority: high
```
Go through `specs/InnerLoop.md` v1.0 rule by rule and classify each:
**executable** (a command fails when it is violated), **checkable**
(a human or agent can verify it in review, cheaply and objectively), or
**decorative** (neither). Record the table in
`history/YYMMDD-inner-loop-rule-audit.md`.
For each decorative rule, choose one of: make it executable, demote it
to guidance with that status stated, or delete it. The default is
delete — an unenforceable rule that reads like a requirement creates
false assurance, which is worse than silence.
Known candidates to start from: whole-file loadability (~400 lines) is
mechanically checkable and unchecked; "evidence or it didn't happen" is
executable via a committed-artifact check; the four implementation rules
added in v1.0 are currently prose.
## Task: Extend the positive-control gate beyond benchmarks
```task
id: CB-WP-0003-T02
status: todo
priority: high
```
`cargo bench -- --test` now covers benchmarks. The same error class
lives anywhere a harness can succeed while doing nothing. Extend
coverage to at least: the scenario runner (done — cb-sim fails on an
empty run), the coverage tool, `dep-weight`, and any future collector
including CB-WP-0002's.
Deliver a stated contract in `specs/InnerLoop.md`: **every tool that
reports a number states what it asserts to prove it did the work**, and
a CI step that fails when a reporting tool has no such assertion. Prefer
a mechanical check (e.g. each tool exposes `--self-test`) over a
convention nobody can verify.
## Task: Point adversarial review at measurement, not only research
```task
id: CB-WP-0003-T03
status: todo
priority: medium
```
The review step is specified for tier-L *research*. All four real errors
in this project were in *measurement and build configuration*, which the
review never touches. We adversarially review the artifact that is
cheapest to fix and leave unreviewed the one where errors actually
occur.
Revise InnerLoop §Step 2 so the review target follows the risk: for a
capability whose claim rests on numbers, the review reads the harness
and the evidence file, not only the survey. Keep it one round; state
explicitly what the reviewer must attempt (reproduce the number,
identify what the harness would report if the work silently stopped).
## Phase B — Session economics
## Task: Measure and specify session shape
```task
id: CB-WP-0003-T04
status: todo
priority: high
```
Measured from the CB-WP-0001 session: **592 assistant turns, mean
context 245,321 tokens, $0.245 per turn in cache reads alone**, and of
330 tool calls **none** were batched into a multi-call turn. Cache reads
were 58% of the $248.46 total; output was 12%.
Cost is therefore approximately `turns × mean_context`, and mean_context
grows with turns — a long session is quadratic. Running T01T09 in one
context cost ~$145 in cache reads; nine task-scoped sessions at ~40k
context each would plausibly cost ~$25.
Write `specs/SessionShape.md`: one task per session as the default,
what a fresh session must be able to load from committed artifacts to
start cold, when to compact versus start clean, and batching of
independent tool calls. Every claim in it carries the measurement it
rests on — this spec exists because the numbers were surprising, not
because the advice sounds sensible.
Depends on CB-WP-0002 for per-task attribution to validate the estimate;
the session-level numbers above are already sufficient to write the
policy.
## Task: Replace the dead token budget with a live cost budget
```task
id: CB-WP-0003-T05
status: todo
priority: medium
```
The global 8k soft / 10k hard per-task token budget was never
referenced or enforced during CB-WP-0001, and T08 exceeded it by orders
of magnitude with no signal. It is dead policy: it implies a control
that does not exist.
Replace it with a budget expressed in the unit that CB-WP-0002 makes
measurable, with a defined action on breach and a way to observe the
breach at the time it happens rather than in a retrospective. If no such
observation is possible, say so and delete the budget rather than
restating it.
## Phase C — Remove or fix the rest
## Task: Resolve the chaos roll
```task
id: CB-WP-0003-T06
status: todo
priority: low
```
The d10 tier roll never fired across the whole pass — roughly 0.2
expected firings across 23 tier decisions — so it is untested, and at
1-in-10 it will stay untested for many more passes while adding a step
to every decision.
Decide: raise the rate during a stated calibration period so the
mechanism produces evidence, or delete it. Keeping an unevaluated
mechanism at a rate that prevents its own evaluation is the one option
to reject.
## Task: Make retargeting a reviewed decision
```task
id: CB-WP-0003-T07
status: todo
priority: medium
```
AM-4's new targets were measured at 246,250 and set at 250,000 **in the
same commit, by the implementer, after seeing the number**. The
reasoning was recorded and is defensible, but the structure is exactly
what the loop exists to prevent.
Add to InnerLoop: a metric may not be retargeted in the commit that
measures it. A retarget is an ADR with the old target, the measurement
that motivated the change, and why the new target binds on future work
rather than merely passing present work. Apply retroactively to AM-4a
and AM-4b — either ratify them by ADR or change them.
## Task: Give provisional items an expiry
```task
id: CB-WP-0003-T08
status: todo
priority: low
```
Ten U-items plus GR-E02's "successes" are marked `provisional: true`
with no owner and no review date, so they can shape the kernel
indefinitely while looking handled.
Add an owner and a raised-on date to each provisional item, and make
`make coverage` report their age. Decide what happens when one goes
stale — the useful answer is probably that CI warns and the evidence
file must list them, not that the build breaks.
## Task: Strengthen the coverage gate beyond tag-counting
```task
id: CB-WP-0003-T09
status: todo
priority: low
```
`make coverage` compares rule IDs in the spec against `covers:` lists.
It proves no rule is unclaimed and no claimed rule is invented. It does
**not** prove a scenario exercises the rule it names, so 58/58 is weaker
evidence than it reads as.
Cheapest real strengthening to evaluate first: require every GR-id in a
`covers:` list to also appear in a doc comment in the aggregate, making
the spec→code→scenario chain mechanical rather than asserted. Consider
mutation-style checking (does removing the rule's code break the
scenario that claims it?) and cost it before adopting.
## Task: Retrospective
```task
id: CB-WP-0003-T10
status: todo
priority: low
```
Revise `specs/InnerLoop.md` to v1.1 from this pass. The question to
answer honestly: after making rules executable, did the *next* error
still slip through, and if so, what class was it? Four instances of the
harness-does-nothing class were found before a gate caught one. Record
whether the gate holds, and what the fifth error — of whatever class —
turns out to be.