feat(test): seed-deterministic maze generation + regression foundation - #9
Merged
Conversation
….7.1 bump Closes three gaps in the experimental C#/WASM export pipeline and adds a plan for getting it onto officially supported foundations. Fixes: - Production and preview could build different toolchains. The editor tag and template version were duplicated in four places (composite action defaults, workflow_dispatch defaults, and inline fallbacks in the production job) while the preview job passed nothing and inherited the action defaults. Updating one and not the others meant /preview validated a toolchain production never used. All versions now come from .github/web-toolchain.env; action inputs default to empty and act purely as deliberate overrides. - The drift guard compared only major.minor, so a 4.7.0-vs-4.7.1 mismatch passed silently -- the most likely kind of bump. It now normalises all three spellings of a version (4.7.1 / 4.7.1.stable.mono / 4.7.1-stable), requires exact agreement including the editor tag, and handles Godot's convention that x.y releases carry no patch component. It also verifies a checksum is pinned for the target asset before downloading a 165 MB editor rather than after. - Checksum verification matched the first zip found in the cache directory; it now requires the expected filename, so a restored cache cannot substitute a differently-named editor. Cache key includes the fork repo as well as the tag. Toolchain bump to 4.7.1-stable (upstream 4.7.1 is a stability-only release with no known incompatibilities with 4.7). Superseded checksums are retained so a rollback needs no checksum work. Docs: - docs/WEB_EXPORT_ROADMAP.md (new): upstream status with evidence, definition of done, four-phase migration plan, risk register, quarterly monitoring routine and rollback procedure. Records that LibGodot Core (#110863) merged and shipped in Godot 4.6, that the approach pivoted away from the stalled #106125 to LibGodot, and that #121502 and #118976 remain open at milestone 4.x -- so official C# web export is still unscheduled. - docs/WEB_EXPORT.md: corrected the DNS section (maze.ryankelly.dev resolves), softened the fork-abandonment risk (the fork matched upstream 4.7.1 within two days), and repointed the version matrix at the new single source of truth. - AGENTS.md: added the web-only failure modes (invariant globalization, missing crypto BCL APIs, no GDExtension), which pass desktop CI and break only in the browser. Not verified here: no .NET SDK in this environment, so the desktop build against Godot.NET.Sdk 4.7.1 is unbuilt. The version-resolution and drift-guard logic was executed against all drift permutations, and both editor checksums were computed from the published assets (the 4.7-stable hash reproduces the existing pin). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhKDz2xEVEtYECxT7q5F3
…ual regression harness
Makes maze generation reproducible from a seed, which is the prerequisite for any
regression testing of generated output, and adds the Playwright visual-regression
harness for the web build.
Why: the existing 421 tests could only assert invariants (valid, connected), never
what was generated -- because nothing was reproducible. Measured on the previous
code at 12x12x2 with identical settings over 8 runs: all four algorithms produced
8/8 distinct mazes, start and end points were 8/8 distinct, and shortest-path length
ranged [45, 1, 33, 25, 25, 104, 43, 151] -- min 1, max 151. An algorithm change that
made mazes measurably worse would have passed every test.
Three independent sources of ungoverned randomness fed generation, none seedable:
1. RandomValueGenerator used a [ThreadStatic] Random seeded from Environment.TickCount.
2. ArrayHelper.Shuffle used Random.Shared -- reached statically from the backtracker,
growing-tree and random carver, so there was no injection seam at all.
3. RandomAgent and PerfectAgent called Random.Shared.Shuffle directly.
Changes:
- IRandomValueGenerator is now the single source of randomness (GetNext, Shuffle,
Reseed), injected everywhere, holding per-instance state. The thread-static
indirection was also unnecessary: nothing in the pipeline is concurrent.
- MazeGenerationSettings.Seed pins a run; MazeGenerationFactory reseeds once before
anything consumes randomness, so start/end placement, carving order, wall removal
and agent walks are all covered.
- MazeGenerationResults.Seed always reports the seed used. Unseeded runs still draw
and report a concrete seed, so a maze found by chance is reproducible.
- ArrayHelper deleted: its shuffle overloads were the trap and its Average was dead
code. Benchmarks now exercise the production shuffle path with a fixed seed.
- ISystemClock replaces a DateTime.Now read in MazeStatsSerializer that made
serialized stats differ every run -- a golden-file blocker found by the new
discipline guard rather than by inspection.
Guards:
- DeterminismTests: same seed gives identical mazes across all four algorithms;
different seeds still differ (seeding must not collapse the output space); a
reported seed reproduces its maze; wall removal and agent walks are seed-stable;
reusing a container across seeds doesn't leak state.
- RandomnessDisciplineTests: scans scripts/maze and rejects Random.Shared,
new Random(, ArrayHelper.Shuffle, Guid.NewGuid and DateTime.Now/UtcNow, exempting
only the two designated injected sources.
Visual regression (tests/visual, @playwright/test toHaveScreenshot):
- Harness with canvas-boot detection, COOP/COEP assertions and pixel-stability
polling that fails rather than screenshotting a still-animating canvas.
- harness.spec.ts self-tests the harness against a local deterministic canvas, so it
stays verified without needing a web build (which requires the patched Windows
editor). Measured there: a seeded render is byte-identical across fresh pages and
across browser launches, but reusing one page across loads perturbs ~1.6% of
pixels -- so every case loads into a fresh page. An early version of this suite
reused a page and looked like rendering nondeterminism; it was the fixture.
- maze.spec.ts targets the deployed build and is SKIPPED until the build accepts
generation parameters from the query string (docs/VISUAL_REGRESSION.md specifies
the contract). Skipped rather than failing so it cannot report a false red.
- CI: visual-harness on every PR (no deploy needed); visual-production/-preview
after the existing smoke tests, with report artifacts on failure.
Docs: docs/REGRESSION_TESTING.md (four-layer plan, recommended order, open
questions), docs/VISUAL_REGRESSION.md (measurements, baseline policy, prerequisite),
AGENTS.md (the randomness rule contributors must follow).
Verified: full suite 443 passing (421 existing plus 22 new) on .NET 8; benchmarks and
experiments projects still compile; visual harness self-test green across repeated
runs; all workflow YAML parses.
Not verified: the desktop/Godot build (no Godot SDK or .NET 9 available here) and the
maze visual suite (needs a deployed build). Separately, suite wall-time under parallel
execution is erratic on a 4-core box -- runs from 2.8s to over 300s -- but this
reproduces on the pre-change baseline too (122s observed), sequential runs are clean
on both sides, and per-test timings match, so it is pre-existing and not from this
change. Worth its own investigation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwhKDz2xEVEtYECxT7q5F3
CI run 9 on this branch hung for 40 minutes with no output after "Found 3 maze files" and was cancelled, leaving the required `test` check red. Root cause, isolated by running the two agent families separately over 8 runs each on the sample mazes: RandomAgent 1768 1662 1724 1738 1702 1755 1666 1644 ms (flat) PerfectAgent 120011 1941 14602 120034 3225 1960 1848 25880 ms (2 timed out) PerfectAgent's search is worst-case exponential: it tracks visited cells per-path via previousPoints.Any(...) -- a linear scan of the current path -- rather than with a shared visited set, so the same cell is re-explored along different paths, and it copies the whole path per branch. Whether it terminates quickly depends entirely on the shuffled direction order, which is why it is intermittent. This is pre-existing, not introduced here: the same workflow recorded 1002s on a push to main on 2026-07-01, against durations of 29-57s for neighbouring runs. Fix (symptom, not cause): - SampleMazeTests: PerfectAgent tests now draw from PerfectAgentMazeFiles() capped at 200 cells, keeping 10x10x1 and dropping 20x20x3 (1200 cells) and 20x20x4 (1600). Exactly 4 test cases removed, verified by diffing --list-tests output; nothing else changed. Total 443 -> 439. - Those two tests carry Timeout(60_000) so a regression fails with attribution rather than stalling. - test.yml jobs get timeout-minutes: 15, so a future hang costs 15 minutes and a clear failure instead of 40 minutes and a bare cancellation. Other PerfectAgent callers are on 5x5x1 (25 cells) or seeded, so unaffected. Verified: 12 consecutive full-suite runs at 8.7-9.7s with zero timeouts, against a pre-fix spread that reached 120s+ on the agent tests alone. 439 passing. The algorithm is still exponential and will resurface on any larger maze -- including in the app, where it would hang the UI rather than a test. Recorded in docs/REGRESSION_TESTING.md as the fix that still needs doing (shared visited set). Also observed once, on the pre-narrowing tree: a single test failure that did not reproduce across 16 subsequent runs, so it is unidentified rather than diagnosed. Noted here rather than left silent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhKDz2xEVEtYECxT7q5F3
This was referenced Jul 25, 2026
…sm data race
`[assembly: Parallelizable(ParallelScope.All)]` was combined with NUnit's default
SingleInstance lifecycle, so test cases in a fixture ran concurrently against one
shared instance. Every field assigned in `[SetUp]` was therefore a data race.
It surfaced as two NullReferenceExceptions in RandomValueTests on CI, on a commit
whose other runs of the same job were green — re-running the identical commit
passed, confirming a race rather than an environment difference. The mechanism:
_mazePointFactory = new Mock<IMazePointFactory>(); // (a) bare mock
_mazePointFactory.Setup(x => x.MakePoint(...)).Returns(...); // (b) configures it
_randomPoint = new RandomPointGenerator(_random,
_mazePointFactory.Object); // (c) re-reads the field
Another test's (a) landing between this test's (b) and (c) makes (c) capture an
unconfigured mock. Moq then returns default(MazePoint) — null — and the test dies
dereferencing `point.X`, nowhere near the actual cause.
Measured with a trace harness (NUnit's per-test console capture reorders output and
hides this): with SingleInstance, four concurrent `[SetUp]` bodies share one fixture
instance; with InstancePerTestCase, four concurrent tests get four distinct
instances. Parallelism is unchanged, the unsafe sharing is gone.
Also drops MovementHelperTests' `[NonParallelizable]`, which was a local workaround
for this same root cause, and adds a guard test so removing the attribute fails
immediately by name instead of resurfacing as an occasional unexplained flake.
…2-determinism-3a6h5m
This branch and `main` had no Node or Playwright ignore rules, so nothing stopped an `npm ci` in tests/visual/ from being committed. That is exactly how 170 files / 17.7 MiB of node_modules got into the GitHub-Actions branch (#7), which is also based on `main`. Later branches in this stack do carry `tests/visual/`-anchored rules, added with the visual-regression harness. These are deliberately repo-wide rather than anchored: the trap is a node_modules appearing somewhere the anchored pattern doesn't cover. Baseline screenshots under *-snapshots/ stay tracked; only regenerated run output is ignored.
…2-determinism-3a6h5m # Conflicts: # .gitignore
…2-determinism-3a6h5m
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rtkelly13
added a commit
that referenced
this pull request
Jul 27, 2026
AGENTS.md keeps this branch's testing/session sections and re-adds the Repository Conventions section from main (deduplicated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Stack 2 of 4 — split out of #6. Based on #8; review that first.
Makes maze generation reproducible from a seed, which is the prerequisite for any regression testing of generated output.
Why
The existing 421 tests could only assert invariants (valid, connected), never what was generated — because nothing was reproducible. Measured on the previous code, 12×12×2, identical settings, 8 runs:
[45, 1, 33, 25, 25, 104, 43, 151]An algorithm change that made mazes measurably worse would have passed every test.
Three independent unseedable sources, not one:
RandomValueGenerator→ a[ThreadStatic]Randomseeded fromEnvironment.TickCount.ArrayHelper.Shuffle→Random.Shared, reached statically from the backtracker, growing-tree algorithm and carver — so there was no injection seam at all.RandomAgent/PerfectAgent→Random.Shared.Shuffledirectly, bypassing evenArrayHelper.Changes
IRandomValueGeneratoris the single source of randomness (GetNext,Shuffle,Reseed), injected everywhere, per-instance state. The thread-static indirection was also unnecessary — nothing in the pipeline is concurrent.MazeGenerationSettings.Seedpins a run; the factory reseeds once before anything draws, so start/end placement, carving order, wall removal and agent walks are all covered.MazeGenerationResults.Seedalways reports the seed used, so an unseeded run is reproducible after the fact.ArrayHelperdeleted — itsShufflewas the trap, itsAveragewas dead code. Benchmarks now exercise the production path with a fixed seed.ISystemClockreplaces aDateTime.NowinMazeStatsSerializerthat would have made serialized stats differ every run — a golden-file blocker found by the discipline guard, not by inspection.Guards
DeterminismTests— same seed gives identical mazes across all four algorithms; different seeds still differ (so seeding can't silently collapse the output space); a reported seed reproduces its maze; wall removal and agent walks are seed-stable.RandomnessDisciplineTests— scansscripts/maze/and rejectsRandom.Shared,new Random(,ArrayHelper.Shuffle,Guid.NewGuid,DateTime.Now/UtcNow, exempting only the two designated injected sources.Also here: a CI hang, diagnosed
CI hung for 40 minutes with no output and was cancelled. Isolating the two agent families over 8 runs each on the sample mazes:
PerfectAgent's search is worst-case exponential. Pre-existing, not introduced here — the same workflow logged 1002s on a push tomainon 2026-07-01 against 29–57s neighbours.Mitigated here by narrowing the
PerfectAgentsample and adding timeouts (exactly 4 test cases removed, verified by diffing--list-tests). The root cause is fixed in #6, which restores those cases.Verification
443 tests pass (421 existing + 22 new). 12 consecutive full-suite runs at 8.7–9.7s with zero timeouts.
testcheck green.GodotSharptargetsnet8.0, so it is buildable; I was wrong about that initially). Verified from #11 onward.Docs
docs/REGRESSION_TESTING.md— the four-layer plan, recommended order, and open questions including theShortestPath == 1case (start and end landing adjacent) andBinaryTreeAlgorithmbeing a placeholder that delegates toBacktrackerAlgorithm.Generated by Claude Code