Repository navigation
fix(build): pin let-go to v1.13.0, and fix the runner it silently zeroes - #195
Conversation
A bundle built with the pinned let-go does not start. It dies at namespace
load:
error loading xsofy.input: ExecutionError: calling dev?
caused by bit-shift-left expected Int
`dev?` runs when `xsofy.input` loads, so this is not a dev-mode-only path.
The pin was the bare commit cf195a67, which is nooga/let-go#378 itself, taken
on 2026-07-16 because no release carried the `#app` host mount yet. v1.12.2
shipped #378 four days later, so the SHA was already unnecessary; what this
fixes is that it had also gone stale against the rest of the tree.
Nothing shipped broken. `deploy-pages.yml` fires only on `v*` tags and the last
one, v0.0.2, predates the SHA pin — it deployed with v1.11.1, which is why the
live site serves a pre-#378 bundle mounting `#terminal`. The next tag would
have been the first to carry this.
The floor lane did not catch it because it compiles the test namespaces, runs
the suite, and runs the Layer-1 smoke, but never builds or boots the WASM
bundle — the one thing `deploy-pages.yml` does. All of those pass on the old
pin, which is why it looked healthy. Adding a boot check there is worth doing
separately.
Verified on v1.12.2: 328 tests / 2857 assertions, the Layer-1 smoke, a full
`make wasm`, and the resulting bundle loading in a browser under COI headers —
`#app` present, no console errors, title screen and a playable map. The same
browser check on the old pin fails 3 out of 3.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
v1.13.0 ports clojure.test for real (nooga/let-go#863), which gives run-tests Clojure's actual arities: no arguments now means the CURRENT namespace. xsofy/test/run.lg calls a bare (run-tests) from xsofy.test.run, a namespace holding no deftests, so on v1.13.0 the aggregate suite ran zero tests and still exited 0 — CI would have gone green having executed nothing. Measured, same tree, same command: v1.12.2 Tests: 328 Pass: 2857 Fail: 0 v1.13.0 Ran 0 tests containing 0 assertions (exit 0) v1.13.0 + run-all-tests Ran 328 tests containing 2857 assertions, 0 failures Selecting the namespaces by regex keeps the :require list as the source of truth, since all-ns only sees what has been loaded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mparrett
changed the title
v1.13.0 warns when a namespace replaces a core referral, and xsofy.check trips it on every load: WARNING: lazy-map already refers to: #'clojure.core/lazy-map in namespace: xsofy.check, being replaced by: #'xsofy.check/lazy-map The collision predates this pin bump — core has carried `(def lazy-map map)` since before the SHA this PR replaces — but the warning is new, so the bump is what surfaces it. The two are not the same function. Core's is an alias for eager `map`; xsofy.check's is a cons-based lazy version, one of the pair documented at check.lg:66 as existing because let-go's `map` and `mapcat` are eager and rose trees need laziness. Excluding the core referral would assert a replacement this isn't, so rename instead: the helper keeps its `lazy-concat` sibling's naming with a `*` marking it as ours. Private and file-local, so the rename is confined to check.lg's seven call sites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The second core-referral warning the pin bump surfaces: WARNING: peek already refers to: #'clojure.core/peek in namespace: xsofy.dag, being replaced by: #'xsofy.dag/peek Opposite fix to the one in check.lg, for the opposite reason. That `lazy-map` was private, file-local, and a different function from core's eager alias, so it was renamed. `xsofy.dag/peek` is public, called from ten sites in dag_test.lg, and is the domain term: reading the world at a commit-id in a namespace modelling a git-like DAG. Renaming it would churn a public API to quiet a warning. Excluding the referral says what is true here — in this namespace `peek` means the DAG read — and leaves every call site alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nooga
left a comment
nooga
left a comment
There was a problem hiding this comment.
Reviewed via automated multi-angle analysis, including building lg from both the old pin (cf195a67) and v1.13.0 and running the actual WASM build pipeline against both.
Verdict: approve, with one open question for the author and a pre-existing gap worth a follow-up.
The re-pin itself is sound. v1.13.0 is a real, published tag (go list -m -versions confirms it against the module proxy) and is cf195a67's descendant 290 commits later, including the #859/#862/#906 PRs cited. Verified those two are genuinely no-ops for this bundle: stock GOOS=js GOARCH=wasm already has 64-bit int (PtrSize: 8 in Go's own arch.go), so they don't explain any crash even if the crash is real. grep -rn "cf195a6" across the repo hits only .let-go-version — nothing else needed updating.
The "silently zeroes" half is confirmed real and correctly fixed, independent of the crash claim. Not a .let-go-version-parsing issue (both CI extraction sites are format-agnostic between a SHA and a tag). The actual bug: at cf195a67, run-tests with zero args ran every already-loaded namespace with tests ((doseq [[s bs] *registered-tests*] ...)), which is what gave "328/2857." At v1.13.0, post the clojure.test port, zero-arg run-tests means only the calling namespace — xsofy.test.run defines no tests itself, so the old bare (run-tests) now silently reports 0 tests and exits 0. run-all-tests #"xsofy\.test\..*" (new in v1.13.0) is the correct semantic fix, not a workaround — restores coverage by construction rather than an explicit namespace list that could drift.
One thing I couldn't verify: I built the real WASM bundle at cf195a67 via the actual make wasm/build-wasm action pipeline and booted main.wasm under Node with wasm_exec.js, with and without ?mode=dev — no crash, no "error loading xsofy.input" in either case. Doesn't mean the claim is wrong (a JS-host or Worker-thread-specific path my single-threaded harness doesn't exercise could still trigger it), but it's currently unverified on my end. Would ask for the actual repro (browser + flags) so the root cause is nailed down, though it doesn't change whether re-pinning to a real tag is the right move regardless.
Pre-existing gap, not introduced by this PR: no automatic, non-label-gated, non-tag-gated CI lane actually boots the WASM bundle in a browser — test.yml's WASM step only builds it. The only lanes that boot it (tests/browser/smoke.mjs) are gated behind a v* tag push or an opt-in PR label. So the exact failure mode this PR's headline bug describes (builds clean, crashes at runtime) would go unnoticed by default PR CI again in the future. Worth a follow-up issue, not a blocker here. Also: nothing asserts a minimum test/assertion count post-fix, and the runner doesn't fail the CI step on individual is failures — same class of silent-pass risk in a different shape.
Minor: the new .let-go-version comment says the floor lane "never builds or boots" the bundle — it does build it, just doesn't boot it. Small wording nit, not consequential.
Diff mechanics check out: lazy-map → lazy-map* rename in xsofy/check.lg is complete (def + all 6 call sites, private, no external callers possible), and dag.lg's new (:refer-clojure :exclude [peek]) is safe (no internal use of core peek that the exclusion could break). CI green on both required lanes.
🤖 Generated with Claude Code
nooga's review of #195 caught the claim. test.yml's floor lane has run the build-wasm action since #69, so it does build what the deploy builds; what it never does is load the result. That distinction is the whole point — this failure is at namespace load, and a build-only lane cannot see it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reproChromium via Playwright, COI headers, no flags — plain The boot probe times out after 60s waiting for the title screen; Two details that make this easy to miss, and may be why your harness came back clean. It arrives as
You were right about the floor laneIt does build the bundle. The correction sharpens the point rather than blunting it: the gap is not a missing build, it is a missing boot, and a build-only lane cannot catch a namespace-load failure by construction. #196 carries the same wrong claim in its title and body and needs the same fix; it already tracks the CI gap you flagged, including the assertion-count floor. |
|
The v1 salt format writes a term's byte length ahead of its bytes, which reads as though the length has to be known first. It does not. The encoder was running utf8-len over the string to get that byte, then conj-utf8 over the same string to emit the bytes — decoding every codepoint twice, and range-checking it twice along the way. Writing the length as a placeholder and patching it once the real length is the distance the accumulator moved removes the measuring pass outright. The checked appender stays on the live path, so there is no second, unchecked encoder to keep in step with the first, and codepoint->utf8-len, add-utf8-len and utf8-len go with the pass they existed to serve. Interleaved A/B, 20000 iterations of [:combat :hit "attacker-17" "target-93"], all three arms alternating per round, five rounds, min to min: base 614 ms two-pass 529 ms -13.8% one-pass 451 ms -26.5% (-14.7% against two-pass) Output is byte-identical to base: encode-salt and combine agree over 25 cases spanning the empty string, 1-4 byte codepoints, keywords, int64 extremes and nested vectors; the 253/254/255/256/257-byte boundaries agree in both ASCII and two-byte forms; and the over-255 string, keyword and vector cases throw the same assertion text, as do the unsupported-type cases. Suite is 330 tests and 2864 assertions, 0 failures — a real count, since this is rebased past #195 and the runner no longer reports zero. Follow-up to the non-blocking note on #194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The v1 salt format writes a term's byte length ahead of its bytes, which reads as though the length has to be known first. It does not. The encoder was running utf8-len over the string to get that byte, then conj-utf8 over the same string to emit the bytes — decoding every codepoint twice, and range-checking it twice along the way. Writing the length as a placeholder and patching it once the real length is the distance the accumulator moved removes the measuring pass outright. The checked appender stays on the live path, so there is no second, unchecked encoder to keep in step with the first, and codepoint->utf8-len, add-utf8-len and utf8-len go with the pass they existed to serve. Taking the length from the accumulator also closes a seam. It used to come from codepoint->utf8-len's cond ladder while the bytes came from conj-codepoint->utf8's, so two ladders had to agree about the 0x80/0x800/0x10000 thresholds indefinitely, with nothing checking that they did. One of them is now gone, and the other cannot disagree with itself. Interleaved A/B, 20000 iterations of [:combat :hit "attacker-17" "target-93"], all three arms alternating per round, five rounds, min to min: base 614 ms two-pass 529 ms -13.8% one-pass 451 ms -26.5% (-14.7% against two-pass) Output is byte-identical to base: encode-salt and combine agree over 25 cases spanning the empty string, 1-4 byte codepoints, keywords, int64 extremes and nested vectors; the 253/254/255/256/257-byte boundaries agree in both ASCII and two-byte forms; and the over-255 string, keyword and vector cases throw the same assertion text, as do the unsupported-type cases. Suite is 330 tests and 2864 assertions, 0 failures — a real count, since this is rebased past #195 and the runner no longer reports zero. Changing the patch index from (inc start) to start fails 15 of them. Follow-up to the non-blocking note on #194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A bundle built with the pinned let-go does not start. It dies at namespace load:
dev?runs whenxsofy.inputloads, so this is not a dev-mode-only path: the bundle never reaches the title screen.Why the pin was a SHA, and why it no longer needs to be
.let-go-versionheld the bare commit cf195a67, which is nooga/let-go#378 itself, taken on 2026-07-16 because no release carried the#apphost mount yet. The file said so, and said what to do about it:v1.12.2 shipped #378 on 2026-07-20, so the SHA stopped being necessary four days after it was added. What this changes is not only that bookkeeping: the pin had also gone stale against the rest of the tree, and the failure above is the result.
This targets v1.13.0, released 2026-09-20, rather than v1.12.2. It carries #378 like v1.12.2 does, and also #859 and #862, which make
vm.Int64 bits on every host and lower a let-go:inttoint64. Both are no-ops for this bundle — they widen 32-bit-int hosts (linux/386, TinyGo wasm), and stock Go wasm was already 64-bit — so the worlds existing seeds name are unchanged. What those seeds play is not — see “Worlds survive the bump; play does not” below.The runner fix is not optional
v1.13.0 ports
clojure.testfor real (nooga/let-go#863), which givesrun-testsClojure's actual arities: no arguments means the current namespace.xsofy/test/run.lgcalls a bare(run-tests)fromxsofy.test.run, a namespace holding no deftests. On v1.13.0 that runs zero tests and exits 0, so this lane would have gone green having executed nothing.Same tree, same command, three runs:
Selecting the namespaces by regex keeps the
:requirelist as the source of truth, sinceall-nsonly sees what has been loaded.The release notes call out the opcode set and bundle format as breaking, but not this. Anything downstream using a bare
(run-tests)as an aggregate runner has the same silent hole.Two warnings the bump surfaces
v1.13.0 warns when a namespace replaces a core referral, and the tree trips it twice:
Both collisions predate this bump — core has carried
(def lazy-map map)andpeeksince before the SHA being replaced — so what is new is the warning, not the shadowing.They are fixed in opposite directions, because they are different situations.
xsofy.check/lazy-mapis renamed tolazy-map*. It is private and file-local, and it is a different function from core's: core's is an alias for eagermap, while check.lg's is a cons-based lazy version, one of the pair documented at check.lg:66 as existing because let-go'smapandmapcatare eager and rose trees need laziness. Excluding the referral would assert a replacement this is not. Seven call sites, one file.xsofy.dag/peekkeeps its name, under(:refer-clojure :exclude [peek]). It is public, called from ten sites in dag_test.lg, and it is the domain term: reading the world at a commit-id in a namespace modelling a git-like DAG. Renaming it would churn a public API to quiet a warning, where excluding says what is true — in this namespace,peekmeans the DAG read.The rule, for anyone hitting this on another bump: rename when the local is a different function that happens to share a name, exclude when the local is the intended meaning of that name. The assertion counts below are unchanged by either.
Worlds survive the bump; play does not
The worlds a seed names are unchanged. What a seed plays is not.
(world/make-world 40 25 12345)on the current pincf195a67and on v1.13.0369e2a69returns the same:seed, the same 25 entities with byte-identical generated ids, and the same player position[25 20]. Worldgen is untouched.What changes is the order
(keys (:entities w))comes back in:process-creaturestakes creature acting order straight from(vals (:entities world))— #187 — so the same seed plays out differently either side of this bump. Measured on the balance harness over 40 runs with the tree held fixed and the lg version the only variable, mean survival moves from 132.2 turns to 163.4.The assertion counts below do not speak to this. The replay and determinism tests replay within one lg version, comparing a run against itself, so they pass either way.
Nothing here blocks the bump: the bundle is broken on the current pin and the fix is to move off it. But a replay code recorded before this lands is not guaranteed to reproduce after it, and #187 is what would make that guarantee hold. The evidence is on #187.
Nothing shipped broken
deploy-pages.ymlfires only onv*tags, and the last one, v0.0.2, predates the SHA pin. That release deployed with v1.11.1, which is why xsofy.quest currently serves a pre-#378 bundle mounting#terminalrather than#app. The live site is healthy.The next tag would have been the first to carry the SHA pin, so this is a latent release blocker rather than an outage.
Why the floor lane is green
The
pinned (deploy floor)lane compiles each test namespace, runs the suite, runs the Layer-1 runtime smoke, and — since #69 — builds the WASM bundle through the samebuild-wasmaction the deploy uses. What it never does is boot the bundle it just built. This failure is at namespace load, so every one of those steps passes on the old pin and the lane has no way to see it.An earlier revision of this body, and of the
.let-go-versioncomment, said the lane never builds the bundle. That was wrong, and the correction sharpens the point: the gap is not a missing build, it is a missing load. Issue #196 carries the same error and needs the same correction.Verification
On v1.13.0: 328 tests and 2857 assertions with the fixed runner, a full production-faithful bundle build, and the resulting bundle loaded headless under COI headers —
crossOriginIsolatedtrue,#apppresent, title screen, and the map screen after a keypress. The only console message is a 404 forbuild-info.json, which the local build path does not generate.The same browser check against the old SHA pin fails, with the error above. CI installs the pin with
go install …@<version>, which takes a tag and a SHA alike, so the resolution path is unchanged.