Corridor control, visited-cell trail, and request-loop hardening - #31
Merged
Conversation
…ength from start to target position
Fix the golang concurrency problem Dimensions and mazedata safe update
dmigwi
force-pushed
the
optimize-path-branching-controls
branch
from
August 1, 2026 18:55
5ecf901 to
3f28c92
Compare
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.
Summary
Reworks how maze generation controls branching, adds a visible trail behind the player,
tightens the agent-api request loop's failure handling, replaces the agent ranking metric with
one that compares across maze sizes, and fixes two data races in the Go runtime loop. Go runtime
1.0.1 → 1.1.1; browser SPA2.1.0 → 2.2.0.Browser storage version goes
4.1 → 4.2, so saved progress is discarded on first load.Keys are namespaced
tapoo.v<version>.<mode>.<suffix>andclearStaleStorageVersionsprunesevery entry from an older version out of both
localStorageandsessionStorageat startup.The bump is required rather than cosmetic: the win-summary records changed shape when request
counts were replaced by traversal speed, so a 4.1 entry would restore fields that no longer
exist.
Maze generation: branching control that actually works
The previous profile exposed three knobs —
SoftCorridorLimit,HardCorridorLimitandPreferTurnPercent— none of which moved junction density; only the grid row count did.They are replaced by two:
MaxCorridorLengthLeastNeighborsBiasNeighbour count, unlike corridor length or turn direction, directly predicts whether a cell
gets orphaned and retroactively turned into a junction by a later branch — so it is the lever
that genuinely controls junction density.
Higher bias also means the solution path covers more of the maze, since a tree's longest path
is a larger share of its cells the less it branches. That trade-off — straighter route, longer
walk — is documented on the type.
Proving the knob is the cause
The profile is a pure function of area, so nothing in the shipped configuration can hold the
grid fixed while moving a knob. The first version of these tests compared a 10x7 maze at bias
100 against a 40x40 at bias 0 and credited the difference to the bias — but area, shape and
bias all moved at once, and the test would have passed unchanged if the bias did nothing.
Two measurement seams fix that, unused by gameplay:
Dimensions.GenerateMazeWithProfile(Go)and a third
profileOverrideargument togenerateMaze(TS).GenerateMazedelegates to theformer, so the derived-profile path is byte-for-byte the same.
Grid shape is a second uncontrolled variable, and a real one: at full bias a 28x10 grid produces
about 38% more junctions than a 20x14 of the same area, while across skew 1.4 to 5.7 the effect
at bias 0 is 1.2%. It is concentrated at exactly the end this measurement cares about, so the
sweep runs on a square 20x20 grid with the corridor cap
GetNavigationProfilederives forthat area rather than a round number — which also makes it directly comparable to the
area400_20x20case of the benchmark.With grid and shape both pinned and only the bias moving, junction density falls from 0.0990
per cell at bias 0 to 0.0086 at bias 100 — 40 junctions per maze down to 3, an 11x reduction.
The curve is a Bernoulli mixture, which is what the implementation implies:
chooseNextCellrolls once per decision point, so by the law of total expectation the outcome is a convex
combination of the two pure policies.
Near-linear — not inverse, and not exponential (R^2 = 0.78). The small
p(1-p)term reflectsthat consecutive decisions are not independent. The same fit on the earlier 20x14 grid gave
0.1004 / 0.0082 / 0.0132 at R^2 = 0.9997: coefficients shift with the grid, the structure does
not, so the mixture reading is a property of the mechanism rather than of one measurement.
Dead-end density tracks junctions almost exactly (0.105 → 0.014), which follows from the
spanning-tree degree identity: suppressing branch points converts both into corridor, which is
the property that makes long agent move batches viable.
Tests are split to match.
TestLeastNeighborsBiasCutsJunctionDensityisolates the knob at afixed grid,
TestJunctionDensityFollowsBiasMixtureholds the shape of the curve, and thepre-existing area-based test is renamed
TestJunctionDensityRisesWithMazeAreaand kept for thederived-profile wiring players actually get, without claiming attribution.
One consequence for review: at bias 100 the early levels are junction-free, so the maze is a
single corridor spanning every cell and the solution path is ~98% of the score budget. That is a
deliberate on-ramp — no wrong turn exists to punish a lightweight model — but it means early
levels exercise instruction-following rather than navigation. Later levels invert it: the path
falls to ~50% of budget while junctions rise past 150, so slack grows and so does the cost of
spending it wrongly.
Visited-cell trail
Cells the player has left are marked with
░, rendered in the player colour in both theterminal and the browser.
passageGlyphderives its padding fromcellPathWidthso a markedcell stays exactly as wide as the blank it replaced, keeping row rendering aligned.
Agent request loop
requestPredictionWithAbortreturns a typedAgentPredictionFailurecarrying a diagnostic instead of invoking callbacks, so all gameconsequences (score decay, disabling a seat) live in one place in
agent-api.ts.now
malformed-response; a handler throwing staysnetwork-error.naming them; a second such round in a row ends the turn. Duplicates mixed with genuinely new
calls no longer discard the whole round.
sent name-only — a definition-less entry reads to a model as a newly declared tool it knows
nothing about.
openMovesresolves every open exitto the neighbouring cell and whether it is already visited, so the model no longer derives
adjacency from coordinates itself.
level's first request, which repeat verbatim every round.
agent/config.tsis split. The wire-format group — response parsing, tool-callmarshalling, log previews — moves to
agent/protocol.ts, leavingconfig.tsmeaning onething: validation of the agent records a human configures.
The tool-gathering loop's request count is bounded by the tool inventory plus the
repeat-duplicate rule, so the removed round-count ceiling is not needed; the derivation is
documented at the loop.
Agent ranking: traversal speed
batchEfficiencyRankchanges fromuniqueCells / requeststouniqueCells / decayUnits—progress per decay unit spent. Request counts scale with maze size, so they could never be
compared across levels; decay units are charged per turn at a fixed rate, so the ratio is
comparable. Only a cell's first visit counts as progress, which keeps backtracking cheap but not
free and closes the batched-oscillation exploit an
appliedMovesnumerator would have had.The win summary compares traversal speeds instead of request counts, showing rank and value
together (
3.12 (Trailblazer) — …);lastWinRequestCountandbestWinRequestCountare gone.Where no previous record exists, any result is by definition a new record, so the impossible
matchedBest/behindBestbranches are removed on both the TypeScript and Go sides. DroppinglastWinRequestCountandbestWinRequestCountis what forces the storage version bump noted inthe summary — a restored 4.1 record would carry fields the new summary cannot read.
agentRequestCountis renamedturnCount: it counted turns, not requests — verified against areal log at 183 HTTP requests against 86 counted — and the
agentprefix is dropped becauseStateis shared by both control modes.Data races in the Go runtime loop
Two races existed between the keyboard goroutine and the event loop. Both are fixed and were
verified by mutation — reverting either lock makes the race detector fire again.
The maze grid. The event loop passed the same
[][]stringstraight to the renderer whilethe keyboard goroutine wrote visited markers into it. A new
RuntimeMazeowns the grid; allaccess goes through it:
advanceRenderUIinitLevelRoundInforeplaceGridPlayerPositionMarking the cell and moving the player now happen under one lock, so a frame can no longer
show the trail and the player disagreeing.
The player position.
Dimensions.StartPositionwas rewritten on every move by the keyboardgoroutine and read every frame by the renderer.
StartPositionandFinalPositionare nowstatic level endpoints; the live position lives on
RuntimeMazeasplayerLivePos. Therenderer receives a per-frame copy of
Dimensionscarrying the live position, sorenderLiveSceneis unchanged and the shared struct is never written.Movement bounds moved onto
RuntimeMazetoo, so the keyboard goroutine dereferences noDimensionsfield at all — which also closes the level-reload race, where the loop's*val = *nextConfigoverlapped the goroutine's bounds check.Dimensionsis now effectively immutable for the life of a level.TypeScript strict mode and state invariants
"strict": trueintsconfig.json.strictNullCheckscarried all 50 violations; the otherseven flags had none. This caught a real bug:
get_prediction_rulescalledgetNavigationProfile(state.mazeDimensions)outside the null guard that existed to protect it,which would have thrown and disabled the agent seat. Also fixed two closure-narrowing errors in
agent.tsand typed four test fixtures.Beyond the compiler flag:
canProceedStatusis the single base check. Every compound status predicate is nowwritten as that set plus or minus what makes it differ, so the shared membership is stated
once.
state.canResumeis deleted: all ten of its writes were equal tocanProceedStatus(state.status), so a resumable status could disagree with the flag.hasActiveRoundStateis exported as a type predicate narrowing to a newActiveRoundState, replacing repeated six-field null checks inbuildRoundSnapshotandresolvePlayerMove.stateInvariantErrorno longer throws in the render path.renderStateruns on the blinkcadence and nothing in
game.tscatches, so a violation reached the global handler andreplaced the game with placeholder art — turning a recoverable inconsistency into a lost
round, worst during unattended agent runs. It now logs once per distinct violation, beside the
gameplay that caused it. The redundant check in
storage.tsis removed:buildRoundSnapshotalready guards the same fields, and storage cannot log without an import cycle.
TypeScript never verifies a type predicate's body against its
x is Tclaim, so these areenforced at runtime instead — see Verification.
Other fixes
(Space paused the round mid-edit).
paused.Restore now only forces a pause when the mode is interactive and the round was running;
otherwise the persisted status is kept intact.
fillno longer recolours all terminal UI text: it keyed off "is this traversable", and everybanner and status string begins with a space, so the whole UI — including overlay lines
carrying their own colour — rendered in the player colour.
crypto.getRandomValueswith rejection sampling instead ofMath.random.countPresentNeighborsavoids a per-candidate allocation on every generation step.generateMazeAreais exported so the benchmark can derive a case's level from the productionfunction instead of restating its seed and step.
Branching benchmarks in CI
make ci-benchruns the same sweep in both languages and prints them as one report. It runs onpush to master and on pull requests touching generation, after
test, and takes about eightseconds at 200 mazes per case.
One sweep, and every row is a state the game can produce. An earlier version had a second
benchmark that pinned a grid and swept the bias independently. That was removed: area and bias
both follow from the level, so varying them separately reports combinations no round can reach,
and a number that cannot occur in production cannot be used to judge production. The attribution
it provided is already asserted by
TestJunctionDensityFollowsBiasMixture, and that arrangementis counterfactual by necessity — which is exactly why it belongs in the test suite as a one-time
assertion rather than in CI as a metric reported forever.
The remaining sweep reports rather than asserts: generation is random, so any single maze proves
nothing and a threshold on a random mean is either too loose to catch drift or too tight to
avoid flaking. Each case prints mean, stddev, p5, p95 and dead-end density with its knobs beside
the outcome, plus the level it belongs to and whether the shape is the one a roomy viewport
receives. Both of those are answered by each port's own production functions —
GenerateMazeAreaandGetMazeDimensions— rather than restated in the report, so a change toeither shows up instead of being silently agreed with.
The one thing asserted is cross-implementation parity. Running both ports only earns the CI
time if a divergence between them is caught, and unlike a random mean, divergence is a bug — it
would mean the Go CLI and the browser SPA are playing different games at the same level. The
report computes, per case, the gap between the two junction densities in standard errors of
their difference, and fails the build above z 3.5.
That threshold accounts for the run making 23 comparisons rather than one: at z 2 a case has a
1-in-22 chance of flagging by noise, so something would flag in two runs out of three. At 3.5 it
is about one run in 94. A resolution floor sits underneath it — both ports report to four
significant digits, so a gap narrower than that rounding may be an artefact of printing — but it
never binds at the iteration counts CI uses. An earlier fixed floor justified as "too small to
change how a maze plays" was removed: the two ports run the same algorithm, so their
distributions should be identical rather than merely close, and a practical-significance floor
only suppressed real signal at the most sensitive cases.
Two harness faults also fail the build, because both would otherwise shrink coverage in silence:
a case present in one sweep and not the other, and any mismatch between the two case lists.
Verified by mutation: a fixed offset injected into the difference fails all 23 cases and exits
non-zero; adding a case to one port only names the missing case and exits non-zero.
The report is written to
bench-report.jsonand uploaded as a build artifact, including onfailure, since when the gate trips that file identifies which cases moved. Nothing consumes it
yet — it is the baseline for comparing across commits, and that comparison is currently manual.
A legend prints above the tables. Densities are per cell so cases of different areas stay
comparable, but junction counts are integers, so a per-cell figure is always a multiple of
1/area —
0.01429at area 70 is not a small fraction but exactly one junction. Case names carrytheir area for the same reason, and each entry gives the formula behind its column rather than
the theory behind it.
Verification
make lint(0 issues),go vet,go test -race ./...,pnpm run quality:frontend(typecheck + lint + 321 tests), and
pnpm run build:frontendall pass.New tests cover the branching bias against real generation output,
fill's colour handling andthe overlay's caller-supplied colour, the visited glyph's width, the duplicate/hallucinated tool
paths, and concurrent movement against rendering under
-race.Guards are verified by mutation, not assumed. Type predicates are the recurring hazard: the
compiler takes
x is Ton trust, so a guard can promise a narrowing it does not perform andcompile silently. The exhaustive tables in
status.test.tsare driven off records keyed by theunion rather than annotated arrays — an annotated array only asks that each element belong to
the union, never that the list be complete, so it goes stale invisibly as the union grows. The
first version of the
isTooSmallStatusguard passed its own mutation and was therefore inert;that is what prompted the pattern. Widening
ViewportFitStatus,MoveActionorWallWeightnowfails compilation at the stale record, and adding the key fails the runtime guard.
TestJunctionDensityFollowsBiasMixtureholds the shape of the bias curve, so the equationabove is reproducible from the repo rather than taken on trust. Endpoints are measured at runtime
rather than hardcoded, so it states a relationship instead of memorising current numbers, and it
asserts monotonicity across all ten steps, a chord fit of R^2 >= 0.98, R^2 >= 0.995 once the
single
p(1-p)term is added, a small positive interaction coefficient, and that the mixturebeats a fitted decaying exponential by a wide margin. The rival model earns its place: a high
R^2 on any smooth monotonic series is easy, so with nothing to reject, "it fits a line" asserts
almost nothing. It runs on the same square grid and corridor cap the benchmark's
area400_20x20case uses, so the two stay directly comparable.That distinction is what mutation testing showed. Disabling the knob fails on monotonicity, as a
direction-only test would. But making the response quadratic in bias —
biasRoll < bias*bias/100— leaves it monotonic, keeps both endpoints and preserves the full 11x reduction, so every
direction-and-magnitude check still passes; only the shape assertion catches it, with the chord
fit collapsing to 0.62. That is the realistic regression: someone rescales the roll, the knob
still works in the direction everyone eyeballs, and the interpolation between difficulty levels
quietly stops being uniform. Sample count is set against its own noise floor — per-level sigma is
about 0.010, so 200 samples hold the standard error near 0.0007 against a ~0.009 step between
levels, which is what licenses a strict monotonicity check rather than a fudged tolerance.
Runtime is 0.15s.
The invariant reporting path is covered directly. No public call sequence can produce an
inconsistent state — restore matches the clock to the status it restores, and every transition
sets the clock before the status — so the tests inject the violation onto the live state and
drive a render through
cycle-walls, the one action that redraws without touching the status orthe clock and so cannot repair the violation under test. Disabling the reporting fails both
tests; removing the suppression guard fails the one asserting that a repeat is reported once
while a different violation still gets through.