Close 17 of the parallel tracks from tracking #29 - #73
Merged
Conversation
GameSettings::default() pinned version 1 and its serde default fell back to 0, while every generated map, every self-play game and every arena match runs 115. A GameState that is never generated or loaded from JSON therefore played Aquarion-era rules: burn/clear-forest and polytaur costs, Boost's +0.5 attack, growth damage inheritance, push exhaustion, fungi max level and the lighthouse rules all resolved differently from the engine training exercises. No production or training path reaches that default - self_play, arena, mapgen and the server all pin the version explicitly, so nothing observable changes today - but any future default-state test of a version-gated rule would have quietly asserted a ruleset no real game plays. Both defaults now read one constant, version_sync::CURRENT_VERSION, and the nine scattered literal `< 115` / `>= 114` comparisons move onto the GameVersion enum, so version_sync is the single place a ruleset bump is made rather than a name with three cost helpers behind it. A new Legacy = 0 variant names the pre-versioning contract that scraper data and features.rs's `version <= 0` normalization scales depend on; that gate is untouched and still fires only on an explicit 0. tests/default_version.rs pins the default, the mapgen-versus-state agreement that was missing when the two drifted by 114 revisions, and the explicit-zero legacy contract. Closes #51
Three unrelated defects in the replay tooling, all of the same shape: a
second copy of something that was supposed to have one.
The no-Supabase branch of /replay/save wrote replays/{name}_{ts}.json while
the startup loader, import_replays and upload_replays all filter for
.replay.json. On a fresh clone, where .env is gitignored and so Supabase is
unreachable, every accepted and fully validated replay landed under a name
no consumer could see. The sibling local-only endpoint got the name right
but never created replays/ first. Both names, both sanitizer copies (which
disagreed on how dashes collapse) and all three consumer filters now derive
from src/replay/paths.rs, so a producer can no longer write a name its
readers ignore.
The Supabase duplicate check failed open in both copies: no status check,
and a non-array body fell straight through to the upload. An unreachable or
erroring Supabase therefore created a duplicate silently. Both now refuse
the upload unless a 2xx response carries an array, and say which of the
three failures it was. /replay/check's fail-open is deliberate and is left
alone - re-scraping costs a replay, not data.
unverify_all PATCHed verified:false onto every games row the moment it
started, and delete_all_replays emptied the storage bucket and the games
table behind a three-second sleep. The bucket holds the scarcest data in
the project, since each pro replay needs the Steam game physically
re-played. Both now print the resolved URL, bucket and counts, support
--dry-run, and refuse to act without a typed confirmation or --yes,
exiting 1 when there is no terminal to ask on.
Finally, the training-data writers held their own copy of the option-head
width. tests/parity_widths.rs ties NUM_MOVE_OPTIONS to train.py's pi_option
head, so a writer with a literal 192 would pass parity while emitting
stale-width files - the drift class behind the NUM_ACTION_TYPES 12-vs-11
trap (#3). replay/training.rs, recorder.rs and self_play.rs now read the
constant, the writer's own shape test asserts against it rather than the
literal, and tests/test_writer_widths.rs fails the build if a literal
comes back.
Behaviour notes: generated storage keys now collapse a run of separators
to a single dash consistently across both copies, and a name with no
alphanumerics yields the stem "replay" instead of an empty one. Replay
files already on disk under the old {name}_{ts}.json name stay orphaned;
no migration is included. No training behaviour changes - NUM_MOVE_OPTIONS
is 192 today, so every emitted tensor is byte-identical.
Closes #45
elo.py had no caller anywhere in the repo: no shell driver, no CI job, no scheduled step. The Elo the loop logged and the dashboard charted was ladder.py's per-reading chained win rate - one match against one anchor stacked onto that anchor's own number - so a single noisy reading moved the whole trajectory and nothing ever pooled the evidence the ladder had already collected. The gauge block now refits elo.py from ladder.json after every reading and writes elo_ratings.json, which /api/elo-ladder serves under a `ratings` key beside the readings it already returned. The refit is derived data, recomputable from ladder.json at any time, so unlike the reading itself a failure is reported and the run continues. The smoke asserts the file lands with greedy pinned at 0, because a non-fatal step that quietly stops running is exactly how elo.py got orphaned in the first place. The fit also pooled every reading regardless of the search budget it was taken at - the mixing ladder.py's plateau window is budget-keyed to avoid, where a 16-sim stint and a 64-sim stint chain as if they measured weights alone. A player in the fit is now (run, model, budget) on the same (mcts, gumbel_k, max_turns) key, and a ladder spanning budgets says so on stderr. Link readings stay untagged deliberately: a link match is what gives a frozen anchor its identity, and tagging it would strand every later reading taken at another budget in a disconnected component. For the same reason the audit row now records max_turns, which it was already played at, so an audit and its gauge stay one player instead of forking on a field one of them failed to write down. This changes what the gauge reports, not what it measures: no arena flag, no match, no anchor, no verdict and no training input moves. A rating previously printed by a hand-run elo.py will move where a ladder spans budgets. Closes #8
… Frozen undo Combat resolution decides every game the agent plays and every reward the search backs up, and it had no direct test coverage at all. Nothing in tests/ referenced attack_unit, calculate_combat or any of the combat move types; the only thing that ever reached them was the random-move undo fuzz, which compares JSON snapshots and so cannot assert a single damage number, and which will essentially never build a splash-with-index-shift or an already-frozen defender by chance. tests/attack_matrix.rs is a focused matrix over attack_unit on hand-built flat-Field states, so every expected number is the bare calculate_combat formula with a defense bonus of exactly 1.0: melee trade, melee kill plus move-in, ranged kill without move-in, the three retaliation gates (out of the defender's reach, in reach, attacker Surprise), a Stiff defender, a retaliation that kills the attacker and takes the early-return path, a splash kill that shifts the defender's index inside the tribe's unit vector, Persist, DoubleAttack, and Escape. Every case also asserts the undo round-trips the whole state. The splash case is the one the file exists for: deleting the defender re-find before removal makes it fail by removing an unrelated bystander instead, which is exactly the silent regression the PATCH comments in remove_unit warn about. tests/capture_elimination.rs covers capturing a tribe's last city: the elimination sweep, the killer/killed-turn bookkeeping, and the undo, which is the only exercise of capture_city's old_city_idx re-insert. The one behaviour change is the Frozen undo. attack_unit hand-rolled an insert/remove pair where the undo removed Frozen unconditionally, so attacking a defender that was already Frozen and then undoing stripped an effect the attack never added - a non-round-tripping undo that corrupts the search tree, the same failure mode as the freeze_area case in #19. It now uses try_add_effect, which no-ops when the effect is already present, exactly as the Poison call a few lines above already does. The forward behaviour is unchanged because a HashSet insert is idempotent. Two engine doubts surfaced while writing the matrix and were deliberately left alone rather than "fixed" to make a test pass. A melee Persist kill still ends the attacker's turn, because the move-in's step_unit sets attacked before the Persist branch runs and that branch only declines to set the flag; and the elimination sweep books no kills for the capturing tribe because remove_unit credits kills only when both killer_owner and killer_idx are Some. Both are pinned as current behaviour with a note. Closes #53
Undo drift does not panic, it lies: one move whose undo misses a field silently corrupts every MCTS statistic above the broken edge. The only systematic simulate/undo probe was #[ignore]d and no workflow anywhere passed --ignored, so outside a hand-launch it had never run. .github/workflows/undo_fuzz.yml now runs the probes nightly, on manual dispatch, and on pushes to main touching the engine paths they cover. It is a separate workflow rather than a job in smoke.yml because on.push.paths is workflow-wide, so folding it in would either fire the 90-minute training smoke on every src/actions change or leave the fuzz untriggered by its own sources; and because the smoke's release build is redirected to target/smoke-cargo at opt-level 1, so there is nothing to reuse there. The --ignored run is targeted at --test undo_integrity: the other ignored items in the tree are diagnostics and must not start running. The fuzz seeds rotate without making the nightly unreproducible. The start seed is github.run_number * 200 + 1, which is stable across re-runs of the same run, and it is echoed into the log before the fuzz starts. Each seed owns its own StdRng, so a failure replays exactly from the single-seed command the fuzz now prints, which echoes the run's own flags rather than the resolved arm. The missing arms: both probes now run adversarial mode on, descend inside a fogged clone_for_mcts view, and draw a distinct random tribe pair per seed. undo_integrity gained the four arms as ignored sweeps plus one cheap undo_arms_smoke on the normal gate, so a broken arm reds the PR instead of waiting a day. The adversarial switch is process-wide, so every test in that file holds the guard that used to live in adversarial_search.rs, now shared through tests/common/mod.rs. Order sensitivity: the fingerprint 5a45f95 added to the example is now src/state_fingerprint.rs and is used by both probes, so undo_integrity can see the IndexMap slot-order class at all for the first time. Two fields were still invisible and are now covered: _prediction's _villages/_terrain, and _sim_explored, which is #[serde(skip)] and so invisible to every JSON snapshot - exactly the shadow state the fogged arm exists to stress. Empty _sim_explored sets are skipped, because discover_tiles inserts via entry().or_default() while its undo removes only the indices, and comparing that strictly would red the first Harvest. Verified against re-broken engine code in a scratch worktree: dropping the _sim_explored undo, and reverting consume_resource's shift_insert, each red both probes with the offending field named. Clean otherwise over 498,776 simulated moves on the adversarial-plus-fogged arm alone. Closes #47
The project trains with fog of war on so the search cannot learn to cheat, and clone_for_mcts is meant to be the single choke point that guarantees it. It was not. obscure_fog blanked terrain, owner, roads, effects and _unit_owner_id on an unexplored tile but left capital_of, climate, ruling_city_coords, had_route and skin_type at their true values, so a tile the point of view has never seen still named its owner, its capital and its road history. Nothing leaked today, but only because every reader happened to gate on something else: the feature encoder skips unexplored tiles before it reaches the climate channel, the capital channel iterates surviving cities, analyze_expansion has an explicit explorer check, and the army capital bonus sits behind tile.owner != 0, which was already zeroed. The sharpest case was functions::is_city, which reads only ruling_city_coords and therefore returned true on a hidden enemy city centre - saved solely by is_enemy returning false for owner 0. That is a guarantee living in every reader remembering to gate, not in the state, so the next feature channel or evaluator term that reads capital_of or climate on a tile would have leaked the hidden enemy capital into the search silently. Behaviour is unchanged: no movegen, evaluator or feature term reads any of these five fields on a blanked tile, the feature tensor is byte-identical (hidden tiles are skipped before the climate channel), and the eval cache is keyed on a hash of those same feature bytes. No training behaviour changes, so no experiment registration is needed. skin_type is included alongside the four fields in the issue: it is the same residual-identity class, has no Rust reader at all, and leaving it would have pinned an incomplete invariant. The new test was confirmed red before the fix and green after, and its fow-disabled case proves the fixture really sets all five fields, so the assertions cannot pass vacuously. Closes #54
…CSV reader Two T3 leftovers, both cases of correct code nothing reached. scripts/backup_experiment_record.sh has staged, checksummed and verified snapshots since the T3 wave, but no caller anywhere in the repo - not the training loop, not CI, not a cron job. The entire experiment record therefore still lived on one disk, and the durability claim rested on a human remembering to type the command. The loop now calls it, on the checkpoint cadence and once more from the exit path. The cadence is the checkpoint cadence (override with POLYFISH_BACKUP_EVERY) and the call sits last in the iteration, so a snapshot holds the weights that iteration wrote together with the log row and gauge reading that grade them - weights and their metrics never land on the second disk out of step. The exit snapshot covers the rest: an aborted gauge, a failed self-play, a Ctrl-C, or just a window that has not closed yet, all of which would otherwise lose everything since the last checkpoint. A RECORD_DIRTY flag keeps the two from snapshotting the same unchanged record twice. The failure policy is deliberately non-fatal, the mirror of the fail-fatal gauge reading. Losing a campaign because a backup volume filled up or a mount went away is strictly worse than losing one snapshot, so a failure prints a BACKUP: line on stderr and the run continues; exit 3 (published, but an item looked suspect) counts as published. Silence is what made the missing caller invisible in the first place, so an unset POLYFISH_BACKUP_DIR now says so at startup. Two fixes in the backup script itself: .current_run joins the item list, so a mid-run snapshot records which run it was taken during, and a directory with no files in it no longer counts as a found item - an otherwise empty source dir published a 0-file snapshot, advanced LATEST onto it and called it complete. Separately, the header-driven CSV reader that fixed the dashboard's dropped columns had been copy-pasted verbatim into src/main.rs and src/bin/dashboard.rs, while the fixed-struct reader it replaced still sat in training_api.rs exposing 40 of the CSV's 63 columns. api_runs was a live consumer of it, and its dead api_training_metrics was one route away from regressing the dashboard back to the reported bug. There is now one reader, in training_api.rs, that both binaries route to; MetricRow, read_csv_rows and row_to_json are gone, and api_runs reads the same rows the dashboard does (treating a null cell as 0, as the old parse fallback did, so the runs list keeps its shape). Tested by tests/test_backup_record.py, which drives the backup script end to end (publish, checksums, hardlink reuse, torn CSV line, invalid JSON, empty source, usage) and then runs the loop's own snapshot_record under set -e against a failing backup to prove a dead backup target cannot end a campaign. The smoke exports POLYFISH_BACKUP_DIR and asserts a snapshot landed, which is the only place the exit-trap branch executes. training_api.rs gains cases for an unknown CSV column reaching the JSON, blank cells, and the run_started_at fallback. Closes #23
…train
The mod's serializer emits {uuid, turns, gameState} and no winner, and
TrainingCollector::finish hard-errored without replay.result, so every
captured game failed export-training at the trainingLabels stage. The
teacher path is the point of those captures - games_pro_* is the one
source of real plus/minus 1 outcome labels - so the whole seam was dead
for anything the mod produced, and for any historical result-less
replay.
finish() already receives the fully executed final Game, so the winner
is recoverable without touching the capture format. New
src/replay/outcome.rs::derive_result reads it: survival decides first,
because training plays Domination and most score outlives its owner -
tech, monuments, parks and exploration are never zeroed on death - and
score only breaks a genuine turn-limit terminal, and then only among
tribes still alive. That is the same rule ai::mcts_common's
compute_terminal_outcome applies in the tree, deliberately duplicated
rather than shared: the in-tree version answers per-player and can hand
0.0 to the tied-best living players while a living player below best
gets -1.0, which a single ReplayResult cannot express, and it sits on
the search path where a refactor would be a behaviour risk for no gain.
The derivation is gated on functions::is_game_over. A truncated replay
still fails, with a message naming the turn it stopped at and how many
tribes were standing. Without that gate a pro replay that simply ends
mid-game would silently become score-proxy teacher labels, which is
exactly the A2b extension (#38) this must not make. A derived winner is
always written into winner_player_id too, never left for
value_for_player's score fallback, because that fallback ranks over all
scores including dead tribes - the inversion the living-only rule exists
to prevent.
Derived labels are marked rather than silent. The synthesized result
carries reason "derived:elimination" / "derived:scoreAtLimit" /
"derived:mutualElimination" / "derived:scoreTieAtLimit", the dataset
manifest gains derivedResultSourceFiles listing exactly which inputs
were labelled that way, and import_replays prints DERIVED-RESULT per
file and counts them in its summary. A consumer can tell a derived
outcome from a captured one without re-reading the replay.
Nothing in the live training loop calls TrainingCollector, so this
changes no training behaviour today; the moment a campaign trains on the
teacher files it unlocks, that is an experiment and needs registering.
self_play's own winner computation is deliberately untouched: it still
maxes over all scores including the dead and never reports a draw, and
its result feeds value labels, so unifying it is a training-behaviour
change rather than part of this fix. The mod-side change the issue also
asks for is out of scope here - the payload it sends is the pre-timeline
initial state, so there are no final PlayerStates in it to read, and
there is no BepInEx toolchain in this repo.
Verified end to end through the binary, not just in unit tests: a
result-less replay whose final state is terminal exports one
games_pro_000001.safetensors with value label 1.0 for the surviving
tribe, derivedResultFiles: 1, and the file listed under
derivedResultSourceFiles; the same replay left non-terminal still fails
at stage trainingLabels with the new message.
Closes #42
Six follow-ups from the T1/T2 second pass, all CI and test only. The smoke's push-to-main path filter covered the two binaries and the shell/python drivers but not src/ai/**, src/game.rs, src/moves/**, Cargo.toml, Cargo.lock or requirements.txt - so a runtime-shaped break in the eval-server or network seam merged green and sat until the 05:00 nightly. It is covered now. No pull_request trigger was added; the build cost objection stands. The contract checker demanded only one training_log.py subcommand, so a restructure that dropped an invocation passed by yielding an empty flag set. It now requires all six the loop drives. The env-var seam - where the TRAIN_RUN_ID break sat - is guarded for ladder.py the way test_train already guards train.py: every os.environ read must be exported by the loop or the smoke, or be on an explicit allowlist. Renaming GAUGE_FREEZE_WR in a patched read fails the test, which is the point. The macOS tch-eval row installed torch and then only cargo check'd. It now runs examples/tch_parity.rs, advisory, after init_model.py - the first execution of a non-candle backend anywhere in CI. A sixth matrix row combines metal-eval and tch-eval purely so --all-targets stops silently skipping metal_parity, which carries required-features on both. Forward parity always ran on a fresh init_model.py checkpoint, where GroupNorm affines sit at identity and every bias is zero - so affine and bias mapping drift between the two implementations was numerically invisible, and none of _migrate_checkpoint's five branches ever ran. It now compares three checkpoints: the base, a seeded perturbation moving every affine off 1 and every bias off 0 (including MultiheadAttention's in_proj_bias, which does not end in .bias), and a synthesized legacy checkpoint put through the migration. Negative control: on the base checkpoint, dropping every affine and bias changes the outputs by exactly 0.0; on the perturbed fixture it moves them by 8.98, four orders above tolerance. The global clippy carve-outs are gone - the correctness gate no longer allows absurd_extreme_comparisons and never_loop tree-wide, which had left a hole shaped like the three most delicate MCTS files. Seven sites fired, one more than expected because the lib failed first and clippy stopped early. Each is now a statement-scoped allow with a reason rather than a hole in the gate. Closes #48
…lock Every *_INDEX table in features.rs is Enum::iter().enumerate() over declaration order in types.rs, so inserting a variant mid-enum - exactly what tracking a new Polytopia version invites - shifts every later channel. Nothing would have caught it. The existing tests only asserted block ordering, idx < COUNT and that the counts sum to NUM_CHANNELS; forward parity feeds a synthetic closed-form input that is identical on both sides under any permutation; and the smoke trains from a fresh init. The first shift would have made every trained checkpoint and every archived games_*.safetensors silently garbage, with a green build. It had already happened. TerrainType gained Wetland = 7 and Mangrove = 8 while TERRAIN_COUNT stayed at 8, so terrain_to_channel(Mangrove) returned channel 8 - which is CH_TILE_FROZEN, in the next block. Only states loaded from JSON (the mod, the reader, replays) can carry a Mangrove tile; mapgen never emits one, so no self-play data was affected and training behaviour is unchanged. channel_slots_are_stable pins all 98 enum slots and all 42 named flag/stat channels to literals, the way mapper.rs already pins the action head. no_enum_variant_escapes_its_block fails if any *_COUNT is outgrown again, and a const block asserts every block start at compile time. tests/feature_encoding_golden.rs holds a per-channel FNV-1a digest of state_to_cpu_features on a fixed mapgen seed, plus a separate fingerprint of the fixture map, so a mapgen re-roll and an encoding-semantics change report as two different failures rather than one ambiguous one. The single behaviour change is the clamp: a terrain past its block now folds onto the block's None slot instead of writing into CH_TILE_FROZEN. TERRAIN_COUNT cannot simply be widened - NUM_CHANNELS is baked into every checkpoint and train.py's pad_spatial only recovers channels appended at the end of the layout. Closes #46
The seeded map set has been paying for a variance reduction nobody collected. arena plays every seed twice with the sides swapped and writes one JSON per game carrying the seed, the swap and the winner, but every statistic the ladder stored read those games as 2N independent trials - the one thing the seeded design guarantees they are not. A map that hands a seat an easy start hands it to both configurations, and that cancellation was thrown away at the point the reading was summarised. ladder.py now buckets an arena --dump-stats-dir by seed, keeps the seeds that still have both halves, and scores each pair from the model's side. The estimate is the mean of the per-seed scores and its interval comes from their sample variance, so it is exactly as tight as the swap actually made it - no extra games. Every reading that dumped stats carries the result under `paired` (pair counts, the paired win rate and difference with their intervals, and rho, the within-seed correlation the swap left behind); the loop echoes one line beside the unpaired figure, and `ladder.py paired --stats-dir DIR` re-reads any retained dump without replaying its match. Recorded only. The freeze bar and the plateau rule are EXP-registered tests on the unpaired counts, so no verdict changes, and a test pins that a reading with a dump and one without give the same action, win rate, interval and Elo. required_games gains an optional rho, so the budget can be sized on a measured pairing instead of the unpaired worst case: the same evidence costs (1 + rho) x the games, exposed as `ladder.py power --rho`. The default is 0.0, so the ~571-game figure every registered bar was sized against is unchanged. GAUGE_GAMES is deliberately untouched - what to spend is the owner's call, and this only makes the bill computable. Closes #6
A replay captured under a ruleset Polyfish does not implement still loaded, validated and executed: validate_training_eligibility checked only map size, so a capture from a future or pre-105 version was re-derived under today's rules and every exported sample carried labels for a game that never happened. The failure report could not say so either - FileFailure recorded file, stage and message only, so a batch of failures could not be bucketed as version drift versus capture bugs. validate_training_eligibility now classifies initial_state.settings.version against a supported range declared in the replay module (105 up to the engine's CURRENT_VERSION, so the ceiling follows the ruleset the engine actually implements) and refuses anything outside it, naming the version and the range. import_replays --allow-version-drift downgrades that refusal to a WARN line plus a versionDriftFiles count, for a deliberate out-of-range import. FileFailure gains a version field, the summary gains failuresByVersion, and IllegalCommand/AmbiguousCommand carry the replay's game version in their message. This also lands the Rust half of the divergence check the issue asks for. metadata.sourceDiagnostics.endTurnCheckpoints is now a typed SourceCheckpoint, and DivergenceVerifier compares stars and unit count against it before each EndTurn command - before, not after, because Game::end_turn advances the active player and pays income in the same call. Those two hard-fail with a first-divergence report under a new sourceDivergence stage; score is reported only, since tribe.score is an incremental counter that provably drifts from a recomputed score. The verifier is inert on a replay carrying no checkpoints, which is every replay today: no source writes them yet, and the mod that would has to be ported to canonical schema v1 first. No training behaviour changes. import_replays has no shell or python driver, no replays are checked in, and validate_training_eligibility has no caller inside the training loop. Closes #44
The root package could not run its own documented entry point. telegram_agent.js requires @supabase/supabase-js, which was in neither package.json nor the lockfile, so a fresh clone plus npm install plus npm start died on MODULE_NOT_FOUND before reaching the Supabase/Telegram reporting path at all. The 142 dependency entries were a flattened transitive dump from an express/better-sqlite3 era that no root file imports. The two root consumers need exactly @supabase/supabase-js and dotenv, so that is what is declared now; the lockfile is regenerated from scratch (17 packages), main points at the daemon rather than the one-shot, both entry points get named scripts, engines records the Node 18 floor that the bare global fetch calls imply, and the repository/bugs/homepage URLs move off the HenBOMB fork to this repo. The opening book no longer forces moves. In the Zero path it shuffled the matching legal moves uniformly, played one, and fabricated the policy target to match: a one-hot in select_move_with_stats and a single MoveVisit carrying the whole iteration count in select_move_with_decomposed_visits, so a book turn taught the policy head a distribution no search ever produced. In the heuristic path it replaced every node's untried set, not just the root's, so a book hit pruned the tree several plies deep. book.rs itself admits it encodes no tribe-specific opening, and the mixing recommendation it was meant to serve already landed as the Gumbel prior blend. Both call sites are off the training path as the pipeline is configured: Gumbel is self_play's default backend and never consults the book, no shell or python driver passes --search-backend, and the anchor teacher is GreedyHeuristicAgent, which reads legal_moves directly. What changes is hand-run diagnostics (self_play or arena on the zero/heuristic backends) on game turns 0 and 1, since the book returns empty from turn 2 on. tests/test_book_not_forced.rs pins this behaviourally at a position where the book does fire; both cases were confirmed to fail with the forcing restored. mcts.rs is gone. MctsAgent was defined and re-exported but constructed nowhere, and it drew from the thread-local generator, so reviving it would have violated the seeded-agent convention every other agent now follows. Its three analysis structs move verbatim to heuristic_mcts.rs, their only consumer, field names untouched because they are the serde wire contract the UI's move-evaluation overlay reads. Anyone re-auditing adversarial sign handling will now find two sites rather than the three expert_pipeline_audit.md records: the deleted uct_select_child was one of them. brain.rs's _get_iterations had zero callers, and the inspect_seed and load_json binaries were referenced by nothing, with load_json self-documenting as a copy of validate_csv's fixup logic. Also implements the shuffle the candle trainer left as a TODO: it walked batches in a fixed order every epoch, which correlates each gradient step with the game a sample came from, since self-play writes whole games contiguously. That binary is not the trainer the pipeline runs, so this changes no training behaviour. Refs #27. Closes #57
…real loop polyfish-rs/run_training_loop.ps1 self-played and trained with no gauge, ladder, arena, anchor or plateau reference anywhere in its 249 lines, and it passed only 7 of the flags self_play now takes - no --anchor-frac, no --value-trust, no --decay-last-iter, no --gumbel-k, no --actors, no --eval-servers, no --gamemode - while hardcoding an Imperius mirror and calling a bare `python train.py` instead of the venv. It also lacked the #37 guard, so a bare Windows relaunch rewound every iteration-keyed schedule on a trained model. A Windows campaign therefore reproduced exactly the failure the M-series repairs fixed: training with no strength reading ever recorded, and nothing aborting because the gauge simply was not there. It was live, not dead code: start-hidden.vbs -> auto_train.ps1 -> run-training.bat -> run_training_loop.ps1. Porting it means re-implementing the gauge reading, the Wilson freeze gate, the link match, the tribe_audit rows, the plateau stop, the fail-fatal abort and the #37 guard in PowerShell, then teaching scripts/check_cli_contract.py a second lexer its bash one does not cover - and doing every future gauge repair twice. WSL2 runs the sh loop unchanged. So four files are deleted outright: polyfish-rs/run_training_loop.ps1 run-training.bat auto_train.ps1 start-hidden.vbs run-server.bat is deliberately kept - it launches the polyfish server and has no edge into the training chain. Git history preserves the four. One consequence, recorded rather than left to be rediscovered: run_training_loop.ps1 was the only writer anywhere in the repo to the Supabase training_metrics table, so telegram_agent.js's realtime listener now has nothing to subscribe to. run_analysis_now.js is unaffected - it reads polyfish-rs/training_log.csv directly. auto_train.sh pointed at ./run-server.sh under the comment "path to your training script", so the Linux babysitter has been restarting the simulator server and never training at all. It now runs polyfish-rs/run_training_loop.sh --resume from polyfish-rs/ (the loop does not cd, so cwd matters). --resume is mandatory, not decoration: the loop refuses a bare launch once model.safetensors and training_log.csv history both exist (#37), so an auto-restart without it would die at startup. The halt was `kill -9 "$TRAIN_PID"` on the wrapper alone, with the child-killing pkill commented out. SIGKILL skips run_training_loop.sh's EXIT trap, so every halt left finish-run unrun and the polyfish server the loop starts holding port 3000, which the next auto-restart then collided with. The loop now starts under setsid and the halt is a group TERM, a 15s grace, then a group KILL. Verified end to end against a stand-in loop: the group TERM runs the loop's EXIT trap and its grandchild is gone afterwards. The daily agy report block is deleted rather than repaired. It read session.log and training_log.csv "in the current directory" while both live in polyfish-rs/, read src/bin/self_play.rs at the wrong path, restated a 10-to-30-turn Tiny curriculum that self_play now emits on demand via --print-curriculum, and asked for a value-collapse verdict predating value_r2_holdout. A prompt that restates the curriculum inline is guaranteed to rot again - that is how this issue was born. The replacement is run_analysis_now.js, which hands over the actual CSV row and so needs no curriculum knowledge to stay correct. polyfish-rs/tests/test_auto_train.py pins all of it: the training target resolves to an existing run_training_loop.sh, --resume is passed, the halt is a process-group TERM and not a bare kill -9, there is no inline agy prompt, and none of the four deleted files is back. Each case was proved to fail when its property is violated. Closes #56
The imitation pipeline's correctness rests on from_move and matches_move being inverses across all eleven command types, and until now every command the replay suite executed was an EndTurn or a deliberately illegal Step. Ten of matches_move's arms had never run against a real position, so a single mismatched field would have surfaced only once the capture rig was repaired, tangled up with real capture bugs. tests/replay_round_trip.rs plays a real engine game with a network-free coverage-seeking driver (least-visited move bucket wins, EndTurn only when nothing else is legal), records it with ReplayRecorder, saves and reloads it through save_replay/load_replay, and re-executes it with ReplayExecutor. The assertion is per command rather than only on the final state: an observer requires the executor to resolve the exact move the driver played, which catches matches_move picking a different legal move that merely satisfies its predicate. The final states are then compared as JSON, with _messages cleared and the HashSet-backed arrays sorted, since neither carries ordering information. Three seeds across two map types cover Step, Attack, Capture, Build, Research, Summon, Ability, Reward, Harvest and EndTurn, and the run fails if any of them goes missing. Upgrade and Resign stay out of the required set: no engine move ever emits the serialized "upgrade" key that from_move reads, and generate_legal_moves never emits a ResignMove. The round trip found one live defect, which is why executor.rs changed. generate_legal_moves walks a tile once per city whose territory contains it, so a tile inside two of a player's territories yields the same Build or Harvest move twice, and the executor refused the command as ambiguous. Two matches that are the same move carry nothing to disambiguate, so the executor now collapses them and keeps the ambiguity error for genuinely distinct matches. Debug is the identity, so Summon and Upgrade, which share MoveType::Summon and serialize identically, still count as distinct. The duplicate movegen itself is untouched: deduplicating legal moves would change search priors and policy targets, which is a training behaviour change and belongs in a registered experiment. self_play now writes its high-score recap through save_replay instead of raw serde_json inside a swallow-all if-let, so every training iteration asserts that the recap its own writers produce is a loadable, validated replay. It creates replays/high_scores first, which it never did, so the write no longer fails silently in a tree that lacks the directory, which is exactly what the nightly smoke tree is. A validation failure warns rather than aborting, because games_*.safetensors is already on disk by that point. Closes #43
Two 129 MB copies of the same static UI existed, src/public and polyfish-ui/public/simulator, and main.rs mounted both at different URLs, so which simulator and which dashboard a contributor saw depended on whether they had ever run `npm run build` in polyfish-ui. It was worse than a stale copy. /simulator was nested from ../polyfish-ui/dist/simulator, and dist is gitignored, so on a fresh clone every /simulator/* URL was a hard 404, including README's documented dashboard URL; once dist was built it served the fork, which has no Elo ladder chart at all. Drift ran both ways. src/public/training.html was about 150 lines ahead (Elo ladder, value-label composition, decisive games, policy KL), while the fork was ahead on the replay-player rewrite and on three CodeRabbit correctness fixes src/public never received. One of those was live: both main.js and map.js inferred a move's type by testing `move.type` four times over in one ternary chain, so every move without an explicit moveType resolved to Research or 0. The real serde names are techType, structure and reward. src/public's replay player was dead code besides - it POSTed /replay/analyze, a route no router has ever had, and read data.state.history, which replay_playback_json never emits. The ported files are therefore taken verbatim from the fork, the copy proven against the live /replay/open and /replay/state contract, and training.html is kept from src/public. The one difference that was a judgment call rather than drift is the AI-step depth slider: the fork's 64 (range 16-1024) is adopted over src/public's 300 (100-1000), because that slider is an MCTS iteration budget and 64 is what self_play and `arena --mcts 64` actually search. /simulator now nests from ../src/public, the same bytes the root fallback serves. The prefix has to stay, because polyfish-ui/src/App.tsx iframes /simulator/index.html and /simulator/training.html and a matched nest never consults the router fallback, so repointing the ServeDir root is the whole fix. The four ServeDir paths move behind polyfish::web_static so the new test can assert on the same constants the server reads. Verified against a running server with polyfish-ui/dist absent: /, /index.html, /training.html, /simulator/index.html and /simulator/training.html all 200, the two dashboards and the two simulators are byte-identical, and a canonical replay round-trips through /replay/open and /replay/state forwards and backwards carrying every key the new replay.js reads. Closes #55
… refused one The canonical-schema refactor left the C# mod POSTing the pre-refactor payload shape while /replay/save parses the body only as a canonical Replay. Every capture session against the real Steam game therefore produced zero replays: the server answered HTTP 200 with an error body that the fire-and-forget POST never read, and the payload was gone. That is the entire real-game capture path, and the only source of pro/imitation replays. New src/replay/legacy.rs converts the payload rather than reshaping it. It walks the engine, so every segment's turn number and player id come from the engine instead of from Polytopia's timeline keys and the source's playerId-sorted player blocks, and a payload that converts always re-executes. The traps the shape carries are handled explicitly: currentPlayerTurnId is a 0-based seat index and is repaired from the first segment, Nature (255) is dropped from the tribes, startmatch/endmatch (moveType -1) and Resign (11) are dropped rather than translated because generate_legal_moves never emits Resign, StarFishing arrives as a build and becomes a capture, and the ruins and city-reward hints (_reward, _type, _revealedTiles) reach the canonical command so a non-deterministic reward still replays. Enum ids go through From<i32> and never Deserialize, so an id this build does not know degrades instead of failing the whole capture. The mod cannot snapshot the game before the game plays its own forced opening, so at most two leading commands the captured state already reflects are skipped, counted and named in metadata.sourceDiagnostics; past that bound an illegal opening still fails. Both save endpoints now take the raw body, so a body that is not even JSON reaches the handler, and anything refused is written to replays/rejected/<name>_<ts>.rejected.json with the reason beside it and the path in the response. import_replays gained convert-legacy, which is how a quarantined body or an archived capture gets back into canonical form. The regression fixture is a real 112 KB v114 capture recovered from git history; all 247 of its commands convert and replay command-for-command. It is a 14x14 game, so the test asserts it is NOT training-eligible: features::MAP_SIZE is 11, and the teachers/ imitation path stays empty for real games until the encoder handles other map sizes. polyfish-mod's four hardcoded /home/henry paths and its /tmp replay queue now read POLYFISH_SCRAPER_DATA and POLYFISH_REPLAY_QUEUE with the old literals as fallbacks. Nothing here compiles or lints C#, so that edit is unverified, and SaveReplaySync still does not inspect the response - the server-side quarantine is what makes a capture session survivable without rebuilding the mod. Closes #41
Seventeen implementation agents were each forbidden from editing the docs so their edits would not collide; this applies the deferred pass in one place, verified against the commits rather than the reports. CLAUDE.md gains the replay subsystem's five new traps (version gate, derived result, legacy conversion, divergence verifier, executor ambiguity rule), the obscure_fog and state_fingerprint invariants, the feature-channel layout pin, the paired-reading and joint-Elo additions to the strength gauge, and the two retirements - the PowerShell training chain and the polyfish-ui static-UI fork. Two agent claims were corrected against HEAD: original_mcts_zero.rs is still in the tree (only mcts.rs was deleted) and bin/load_json.rs no longer exists, so neither is cited as it was reported. expert_pipeline_audit.md keeps its finding text and gains an Aug 23 status subsection, updated status rows, and two new open items - the duplicate legal moves the replay round-trip found, and the mod capture path whose C# half cannot be verified here. M3's paired analysis and M5's Elo refit are recorded as landed but unmeasured; GAUGE_GAMES is still the open budget question. hypothesis_driven_improvements.md registers EXP_TEACH_002, EXP_TEACH_003 and EXP_LABEL_003, each with a hypothesis, a benchmark and a falsifier, and records the scope limit on EXP_SEARCH_001's belief state: obscure_fog hides the board but not the opponent's stars, tech or score. No result is claimed anywhere. The re-baseline reading still has not been taken, and every gauge-derived verdict stays provisional.
An adversarial review of this branch's 17 commits raised 38 findings; 33 were refuted. These are the four that survived. convert-legacy named its output from the input's basename alone, so under --recursive - which exists to flatten a tree - two captures called capture.json in different directories both wrote capture.replay.json. The second truncated the first and both printed CONVERTED, with "converted 2 of 2" reporting success. The stem now carries the path relative to the input root, and the naming lives in replay/paths.rs, which declares itself the one authority for replay names and which the converter was bypassing. A write failure now counts and continues like every other per-file failure instead of abandoning the batch. auto_train.sh restarted the loop on any exit at all. The loop's exit codes are its safety mechanism - it aborts nonzero on a failed gauge reading, a failed anchor snapshot or a failed link match - so the babysitter turned a deliberate fatal abort into a retry loop, and undid a plateau stop within a second of the gate deciding it. It now branches on the status. A plateau stop had no exit code to branch on, since it left the loop with a bare break and the script fell off the end at 0, indistinguishable from a finished iteration budget; it exits 3 now. Nothing else reads that status. paths.rs documented a `<stem>.error.txt` reason sidecar; the only writer produces `<stem>.rejected.txt`. Anyone grepping for the documented name after a refused capture finds nothing and concludes no reason was recorded. The name is now a function in paths.rs and main.rs calls it. attack_matrix.rs's header claimed every expected number derives from the bare calculate_combat formula with a defense bonus of exactly 1.0. That is false for the double-attack case: Phychi poisons on its first hit, and a poisoned defender takes the penalty twice, with the 1.0 terrain bonus rewritten to 0.5. A reader re-deriving 7.0 from the stated rule gets 8.0 and concludes combat regressed. The assertion was right; the header was over-general.
53 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the gauge-independent "parallel tracks" section of the tracking issue #29: 17 of its 22 open issues, one commit each.
Note the first commit (
e78929a, the cold-start guardrails) is pre-existing work fromchore/cold-start-guardrails, which had no PR of its own; this branch stacks on it.What landed
Capture pipeline - the supply chain that could not produce a single training sample from a real game.
/replay/saveendpoint rejects every capture, silently #41 - a Rust-side converter for the mod's pre-canonical payload, plus quarantine-to-disk so a refused capture is never lost. Fixture is a real 112 KB capture recovered from git history. The C# side cannot be compiled here (no dotnet/msbuild/mono), so the mod is unchanged.ReplayResultengine-side when a replay carries none. Survival decides first, score only breaks a genuine turn-limit terminal, and only among living tribes; gated onis_game_overso a truncated replay cannot quietly become score-proxy teacher labels (the A2b extension in A2b: the value label is built fromscore, but training plays Domination #38 this must not make).EndTurn.source_diagnosticsfor per-turn divergence checkpoints #44 - gate the replay's game version at training-eligibility time, and bucket import failures by version so drift is separable from capture bugs.Testing and CI
features.rsdid not) #46 - pin the feature-channel layout. This found a live bug:TerrainTypegainedMangrovewhileTERRAIN_COUNTstayed 8, soterrain_to_channel(Mangrove)returned a channel in the next block. Only JSON-loaded states can carry one, so no self-play data was affected;NUM_CHANNELSstays 142, so the Rust/Python sync holds.#[ignore]d and nothing anywhere passed--ignored), plus the adversarial, fogged-clone and mixed-tribe arms.Engine
GameSettings::default()pins version 1 (Aquarion-era rules) while training runs 115 — default-state tests silently assert a different ruleset #51 -GameSettings::default()pinned version 1 while every generated map is 115; both defaults now read one constant, and the scattered literal comparisons moved ontoGameVersion.attack_unit's kill/splash/retaliation matrix — the biggest untested surface inmoves/#53 - first direct coverage ofattack_unit's kill/splash/retaliation matrix, plus the Frozen-undo fix (it removed the effect unconditionally, so it did not round-trip when the defender was already Frozen).obscure_fog: hidden tiles keepcapital_of/climate/ruling_city_coords, enemy tribes keep stars/tech — a leak-in-waiting #54 -obscure_fognow clears the residual tile identity it was leaving behind. No live leak today; the point is that the guarantee should live in the state, not in every reader remembering to gate.Measurement - reporting changed, nothing about what the instrument measures.
Hygiene
run_training_loop.ps1trains with no gauge;auto_train.shbabysits the web server with a stale LLM report #56 - retire the Windows training chain, which trained with no gauge at all, and de-rotauto_train.sh.package.json, tracked runtime/personal files, dead scripts and binaries, dead search implementations #57 - leftovers only; everything PR chore: hygiene sweep for tracked runtime files and dead scripts #68 covers is deliberately excluded.Deliberately not done
#13 and #26 - every remaining item changes training behaviour and needs EXP pre-registration rather than a quiet landing. #20 and #25 - throughput work the repo's own rule says must be profiled first, and #25 is blocked on #18. #27 was already fixed; its only live remnants are in the #57 commit.
Verification
Gate (
cargo test --no-default-features --lib --tests --bin self_play) green at 77 suites, clippy exit 0, CLI contract OK, 175 python tests, parity widths pass.An adversarial review of the 17 commits raised 38 findings; 33 were refuted and 4 fixed in the last commit - including one real defect:
convert-legacy --recursivesilently overwrote outputs sharing a basename, andauto_train.shrestarted the loop on any exit, turning a fatal gauge abort into a retry loop and undoing a plateau stop.Two caveats. The nightly smoke was not run, and the CI workflow edits cannot be validated without CI running them - they are reviewable by eye only. Two commits delete a lot (#55's 2297 files, #56's Windows chain); both are authorized by their issues and recoverable from history, but worth an eye before merge.
🤖 Generated with Claude Code
https://claude.ai/code/session_017owkWmxKSz5pAPSmFNB1w7