playground: ?example= overrides autosave + JIT radio auto-reverts#2810
Merged
Conversation
Three deep-link bugs surfaced by the daslang.io § 01 "try <test> on the playground →" jumps: 1. `?example=<slug>` was silently overridden by localStorage autosave — visitors who'd used the playground before landed on their stale buffer instead of the workload they clicked through for. The playground-tabs.js tryInit autosave-restore now treats `?example=` the same way it already treats `#code=` / `#z=` hashes: a URL-borne intent signal that skips autosave so main.js's pageInit can apply the URL param. 2. As a consequence of #1, the JIT radio also stayed disabled — when the deep-link silently fell through to Hello world (no .wasm) instead of selecting SHA-256 (has .wasm), updateEngineAvailability HEAD-probed the wrong artifact. Fixed by #1. 3. When JIT was opted into for a sample with .wasm and the user switched to a sample without one, `updateEngineAvailability` greyed out the JIT radio but left it `checked = true`. `selectedEngine()` kept returning 'jit' and runCode() 404'd on the missing artifact. Now disabling JIT also unchecks it and re-checks interpreter. Coverage: - persistence.spec.js gains an `autosave does not override ?example=` test, mirroring the existing `#code=` test. - New engine-toggle.spec.js covers both "JIT enables when .wasm exists" and the auto-revert. HEAD probe is stubbed via page.route so the test doesn't depend on `cmake --build … all_wasm` running locally. Verified end-to-end against a local serve with my own Chrome driver: `?example=sha256` after planting stale autosave lands on the SHA-256 source; opt into JIT on SHA-256 → switch to Macros → JIT radio is now disabled+unchecked, interpreter checked. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes deep-link behavior and engine-selection correctness in the daScript playground, specifically addressing cases where URL-driven intent (?example=) and JIT availability were being overridden or left in an invalid UI state.
Changes:
- Treat
?example=as a deep-link intent signal so autosave restore doesn’t override the requested example load. - Ensure JIT radio is unchecked and interpreter re-selected when JIT becomes unavailable for the newly selected sample.
- Add Playwright coverage for
?example=vs autosave and for JIT enable/auto-revert behavior (with HEAD probing stubbed).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| web/examples/ui/src/main.js | Makes JIT disabling also revert selection to interpreter to prevent invalid 'jit' selection. |
| site/playground/playground-tabs.js | Skips autosave restore when ?example= is present so URL deep-links can take precedence. |
| site/tests/playground/persistence.spec.js | Adds e2e test ensuring autosave does not override ?example= deep-links. |
| site/tests/playground/engine-toggle.spec.js | Adds e2e coverage for JIT enablement and auto-revert (with request stubbing). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three out of four threads engaged; the helper-extraction nit is left
in place (out of scope for this PR — when a third spec needs the same
waitDropdownsPopulated body, hoist it then).
1. updateEngineAvailability: gate the HEAD response on `name ===
currentJitName` so a late-arriving response for a stale sample
doesn't clobber the radio. Repro under HTTP/2 (or any async
ordering): pick sha256 (HEAD slow), pick tree (HEAD slow); without
the gate, whichever .then resolves last wins.
2. playground-tabs.js: replace `URLSearchParams.has('example')` with a
truthy check on `.get('example')`. The pre-fix `has` form was true
for `?example=` (empty) and `?example` (no value) — autosave would
be skipped, then main.js's `params.get` would null-check, fall
through to the default sample, and the user's autosave was clobbered
for nothing. Now we only skip autosave when there's a real slug.
3. engine-toggle.spec.js: narrow `page.route` to fulfill 200 for
sha256.wasm only, 404 for every other `**/samples/examples/*.wasm`.
The previous blanket-200 stub also satisfied the startup probe for
the default Hello world sample, so the JIT-enables test passed even
when the sample-switch path was broken. Test body now explicitly
selects Functions first (asserts disabled) → SHA-256 (asserts
enabled), so the transition is what's verified, not a coincidentally-
correct end state.
Verified under both directions of the race (sha256→tree and
tree→sha256, each with an 800ms HEAD-delay monkey-patch). 32/32
no-wasm playwright tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`autosave does not override ?example=` test could race: waitTabsReady returns as soon as pgState exists (initial empty doc), but main.js's `?example=` branch still has to async-fetch data.json + sha256.das before pgLoadFiles swaps the buffer content. The bare evaluate on window.pgState.files['main.das'].getValue() could catch the pre-load empty state and fail the regex assertion. Localhost was always fast enough to race in our favor; CI under load is the realistic flake risk. Swap the bare evaluate for an expect.poll that retries until the sha256 marker appears (5s timeout). The "not stale autosave" assertion runs after the poll resolves on the final content, so it's unaffected. Verified with --repeat-each 5: my test passed 5/5 (and the 5/5 was already true pre-fix on localhost; the change is for CI hardening). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
waitForTimeout(400) over the 250ms autosave debounce leaves only a 150ms margin. Localhost always made it; CI under load (worker concurrency + system stress) consistently lost the race — saw 2/5 fail with --repeat-each 5, intermittent fail in the full-suite runs. Swap for an expect.poll on the localStorage payload, polling until both the file-set AND the active-tab pointer have been persisted by the autosave writer. Deterministic — we proceed exactly when the state has actually landed, not on a fixed wall-clock fudge. Verified with --repeat-each 10: 30/30 pass. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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
Three deep-link bugs in the playground, all surfaced by the daslang.io § 01 "try
<test>on the playground →" jumps:?example=<slug>was silently overridden by localStorage autosave. Visitors who'd used the playground before landed on their stale buffer instead of the workload they clicked through for. The fix insite/playground/playground-tabs.jsextends the existing "URL signals deep-link intent" rule (currently#code=/#z=hashes) to include?example=— autosave-restore is skipped somain.js'spageInitcan apply the URL param.JIT radio stayed disabled on the deep-linked sample. A consequence of move profile tests to other directory and allocate more memory #1: when the deep-link silently fell through to Hello world (no
.wasm) instead of selecting SHA-256 (has.wasm),updateEngineAvailabilityHEAD-probed the wrong artifact. Fixed by move profile tests to other directory and allocate more memory #1.JIT radio stayed
checkedafter becomingdisabled. When JIT was opted in for a sample with a.wasmand the user switched to a sample without one,updateEngineAvailabilitygreyed out the JIT radio but left itchecked = true.selectedEngine()kept returning'jit'andrunCode()404'd on the missing artifact. Now disabling JIT also unchecks it and re-checks interpreter (web/examples/ui/src/main.js).Coverage
site/tests/playground/persistence.spec.jsgains anautosave does not override ?example=test, mirroring the existing#code=test.site/tests/playground/engine-toggle.spec.jscovers both "JIT enables when.wasmexists" and the auto-revert. HEAD probe is stubbed viapage.routeso the test doesn't depend oncmake --build … all_wasmrunning in the test env.Test plan
playwright test --grep-invert @wasm— 32/32 pass locally?example=sha256after planting stale autosave lands on the SHA-256 source; opt into JIT on SHA-256 → switch to Macros → JIT radio is now disabled+unchecked, interpreter checked🤖 Generated with Claude Code