Skip to content

Fix crash when stored playground language state is invalid - #3646

Merged
martin-henz merged 9 commits into
masterfrom
invalidLang
Jul 5, 2026
Merged

martin-henz merged 9 commits into
masterfrom
invalidLang

Conversation

@martin-henz

@martin-henz martin-henz commented Mar 10, 2026 •

Copy link
Copy Markdown
Member

AI-generated PR, fixing #3645

Generated with Claude Code, Sonnet 4.6.

Description

Validate the stored chapter+variant combination against ALL_LANGUAGES when loading from localStorage. Falls back to the default language config if the combination is invalid (e.g. chapter 1 + explicit-control).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

Start frontend using localhost, switch to conductor in feature flags, choose Source 1. Then reload in the browser. This would crash the frontend when the feature flag is reset to false, because the "Source 1" language does not have a CSE machine implementation.

Checklist

  • I have tested this code
  • I have updated the documentation

Validate the stored chapter+variant combination against ALL_LANGUAGES
when loading from localStorage. Falls back to the default language config
if the combination is invalid (e.g. chapter 1 + explicit-control).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@RichDom2185 RichDom2185 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works but behaves differently than what the issue suggests.

@coveralls

coveralls commented Mar 10, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 28746807720

Coverage increased (+0.02%) to 41.4%

Details

  • Coverage increased (+0.02%) from the base build.
  • Patch coverage: 5 of 5 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 14127
Covered Lines: 6293
Line Coverage: 44.55%
Relevant Branches: 7506
Covered Branches: 2663
Branch Coverage: 35.48%
Branches in Coverage %: Yes
Coverage Strength: 27.08 hits per line

💛 - Coveralls

@martin-henz

Copy link
Copy Markdown
Member Author

This works but behaves differently than what the issue suggests.

In what way is this different from what the issue suggests?

@RichDom2185

Copy link
Copy Markdown
Member

This works but behaves differently than what the issue suggests.

In what way is this different from what the issue suggests?

The issue suggests:

invalid combination of lang + variant → fallback to default lang + variant combination

But the logic implemented is:

invalid lang → fallback to default lang && invalid variant → fallback to default variant

@martin-henz

Copy link
Copy Markdown
Member Author

This works but behaves differently than what the issue suggests.

In what way is this different from what the issue suggests?

The issue suggests:

invalid combination of lang + variant → fallback to default lang + variant combination

But the logic implemented is:

invalid lang → fallback to default lang && invalid variant → fallback to default variant

Hmm, the implementation checks:

          const validCombination = ALL_LANGUAGES.some(
            lang => lang.chapter === chapter && lang.variant === variant
          );

so it looks for a valid combination. It falls back on the default lang/variant only if the combination is not valid.

martin-henz and others added 2 commits July 6, 2026 00:02
# Conflicts:
#	src/pages/createStore.ts
When the stored chapter+variant combination is not in ALL_LANGUAGES,
reset to `defaultLanguageConfig` (a known-valid pair) rather than to the
individual default context fields, so the fallback is explicitly "the
default language config" as the issue describes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 5, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e126f818-e6c6-4391-8568-82092e75bd1e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

The change updates the loadStore function in createStore.ts to validate persisted chapter/variant combinations against ALL_LANGUAGES before rehydrating playground.context, falling back to defaultLanguageConfig when the stored combination is invalid.

Changes

Language config validation

Layer / File(s) Summary
Validate persisted chapter/variant on load
src/pages/createStore.ts
Import is expanded to include ALL_LANGUAGES and defaultLanguageConfig; loadStore now computes and validates the stored chapter/variant pair against ALL_LANGUAGES, falling back to defaultLanguageConfig when the combination is invalid.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

flowchart TD
loadStore --> ReadPersistedContext
ReadPersistedContext --> CheckAllLanguages
CheckAllLanguages -->|valid combination| UseStoredChapterVariant
CheckAllLanguages -->|invalid combination| UseDefaultLanguageConfig
UseStoredChapterVariant --> SetPlaygroundContext
UseDefaultLanguageConfig --> SetPlaygroundContext
Loading

Related issues: None specified.

Related PRs: None specified.

Suggested labels: None specified.

Suggested reviewers: None specified.

🐰 A chapter stored, a variant too,
Checked against languages tried and true,
If the pairing's amiss, no need to fret,
Defaults step in, no crash just yet!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main bug fix in the PR.
Description check ✅ Passed The description includes the required summary, type of change, testing steps, and checklist sections.
Linked Issues check ✅ Passed The change validates stored chapter+variant state against ALL_LANGUAGES and falls back to the default config as required by #3645.
Out of Scope Changes check ✅ Passed The PR stays focused on the localStorage validation fix and does not introduce unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@martin-henz

Copy link
Copy Markdown
Member Author

Revived this: merged master in (branch was ~227 commits behind) and resolved the conflict in createStore.ts.

@RichDom2185 — to address your note that it behaved differently from what the issue suggests: the invalid-combination fallback now resets to defaultLanguageConfig (a known-valid chapter+variant pair from ALL_LANGUAGES) rather than the individual default context fields, so it explicitly falls back to the default language config as the issue describes. When the combo is invalid we reset the whole language wholesale rather than trying to preserve a valid chapter and only fix the variant — since this is a crash-fix, dropping to the known-good default seemed the safest, most predictable behavior. Let me know if you had the chapter-preserving variant in mind instead.

Prettier/eslint/tsc clean; CI re-running.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/pages/createStore.ts (1)

87-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fallback logic correctly validates the combined pair.

This correctly matches the PR intent: it checks the stored chapter+variant as a single combination against ALL_LANGUAGES and falls back to defaultLanguageConfig for both fields together (rather than resolving chapter and variant independently), avoiding the "Language config not found" crash described in the linked issue.

One nit: this validity check duplicates the same ALL_LANGUAGES.find logic already encapsulated in getLanguageConfig (src/commons/application/ApplicationTypes.ts). Consider reusing that helper (wrapped in try/catch) instead of re-implementing the lookup here, to keep a single source of truth for "what counts as a valid chapter/variant pair."

♻️ Optional refactor using existing helper
         context: (() => {
           const chapter = loadedStore.playgroundSourceChapter
             ? loadedStore.playgroundSourceChapter
             : defaultState.workspaces.playground.context.chapter;
           const variant = loadedStore.playgroundSourceVariant
             ? loadedStore.playgroundSourceVariant
             : defaultState.workspaces.playground.context.variant;
-          const validCombination = ALL_LANGUAGES.some(
-            lang => lang.chapter === chapter && lang.variant === variant,
-          );
+          let validCombination = true;
+          try {
+            getLanguageConfig(chapter, variant);
+          } catch {
+            validCombination = false;
+          }
           // Invalid stored combinations (e.g. chapter 1 + explicit-control, which can be
           // persisted when toggling feature flags) would crash on load, so fall back to
           // the default language config as a coherent, always-valid pair.
           return {
             ...defaultState.workspaces.playground.context,
             chapter: validCombination ? chapter : defaultLanguageConfig.chapter,
             variant: validCombination ? variant : defaultLanguageConfig.variant,
           };
         })(),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/createStore.ts` around lines 87 - 105, The fallback in
createStore() is validating chapter and variant correctly, but it reimplements
the language-pair lookup logic already centralized in getLanguageConfig. Update
the playground context initialization to reuse getLanguageConfig from
ApplicationTypes instead of duplicating ALL_LANGUAGES.some, and wrap that helper
call in try/catch so invalid stored combinations still fall back to
defaultLanguageConfig as a pair.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/pages/createStore.ts`:
- Around line 87-105: The fallback in createStore() is validating chapter and
variant correctly, but it reimplements the language-pair lookup logic already
centralized in getLanguageConfig. Update the playground context initialization
to reuse getLanguageConfig from ApplicationTypes instead of duplicating
ALL_LANGUAGES.some, and wrap that helper call in try/catch so invalid stored
combinations still fall back to defaultLanguageConfig as a pair.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 50216437-776a-4e6f-9d01-7d8a5f6a9aec

📥 Commits

Reviewing files that changed from the base of the PR and between 339cb22 and 08645da.

📒 Files selected for processing (1)
  • src/pages/createStore.ts

@martin-henz
martin-henz merged commit 5b9b157 into master Jul 5, 2026
10 checks passed
@martin-henz
martin-henz deleted the invalidLang branch July 5, 2026 16:11
Isha-Sovasaria pushed a commit that referenced this pull request Jul 14, 2026
* Fix crash when stored playground language state is invalid

Validate the stored chapter+variant combination against ALL_LANGUAGES
when loading from localStorage. Falls back to the default language config
if the combination is invalid (e.g. chapter 1 + explicit-control).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fall back to defaultLanguageConfig for invalid stored language state

When the stored chapter+variant combination is not in ALL_LANGUAGES,
reset to `defaultLanguageConfig` (a known-valid pair) rather than to the
individual default context fields, so the fallback is explicitly "the
default language config" as the issue describes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
RichDom2185 added a commit that referenced this pull request Jul 19, 2026
* Fix crash when stored playground language state is invalid

Validate the stored chapter+variant combination against ALL_LANGUAGES
when loading from localStorage. Falls back to the default language config
if the combination is invalid (e.g. chapter 1 + explicit-control).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fall back to defaultLanguageConfig for invalid stored language state

When the stored chapter+variant combination is not in ALL_LANGUAGES,
reset to `defaultLanguageConfig` (a known-valid pair) rather than to the
individual default context fields, so the fallback is explicitly "the
default language config" as the issue describes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
martin-henz added a commit that referenced this pull request Jul 29, 2026
* Add Louis bot rendering for sicpy

* Pass language to backend

* Update @sourceacademy/autocomplete digest to d476416 (#4049)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency react-simple-keyboard to v3.8.231 (#4064)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @sentry/react to v10.63.0 (#4067)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @rsbuild/core to v2.1.3 (#4059)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Fix crash when stored playground language state is invalid (#3646)

* Fix crash when stored playground language state is invalid

Validate the stored chapter+variant combination against ALL_LANGUAGES
when loading from localStorage. Falls back to the default language config
if the combination is invalid (e.g. chapter 1 + explicit-control).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fall back to defaultLanguageConfig for invalid stored language state

When the stored chapter+variant combination is not in ALL_LANGUAGES,
reset to `defaultLanguageConfig` (a known-valid pair) rather than to the
individual default context fields, so the fallback is explicitly "the
default language config" as the issue describes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>

* docs: add local backend setup and py-slang testing instructions (#4070)

* docs: add local backend setup steps to README

Fills the gap between "point REACT_APP_BACKEND_URL at localhost:4000"
and an actually-running backend, which otherwise just shows the
"under maintenance" page. Also corrects the postgres docker command
from the backend repo's own README, which has a stray -e flag before
-p 5432:5432 that prevents the port from actually being published.

* docs: add "Running your own py-slang" section

Mirrors the existing "Running your own js-slang" section, linking
out to py-slang's own README (added in a companion PR there) rather
than duplicating the instructions here.

* Update CONTRIBUTING.md

* Update CONTRIBUTING.md

* Update README.md

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Shrey Jain <101659971+Shrey5132@users.noreply.github.com>
Co-authored-by: Martin Henz <henz@comp.nus.edu.sg>

* Update README.md

* feat(cseMachine): rename global/program frames and annotate globals (#4068)

* feat(cseMachine): rename global/program frames and annotate globals

- Rename the top two CSE Machine frame labels to match Python's LEGB
  terminology: "Global" -> "Built-in functions", "Program" -> "Globals"
  (#4042).
- Thread the new optional CseSerializedEnvFrame.globalNames field
  (companion source-academy/plugins + source-academy/py-slang PRs)
  through CseSnapshotAdapter onto the fake Environment, and render it
  as a "globals: x, y" annotation at the top of a call frame the
  moment it's created, so it's clear those names resolve straight to
  the global frame instead of the usual enclosing-scope chain (#4041).
- Add @sourceacademy/common-cse-machine as a direct dependency: it's a
  peerDependency of @sourceacademy/web-cse-machine whose types
  frontend re-exports and uses directly, but frontend never installed
  it itself, so those types were silently resolving to `any` under
  skipLibCheck.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(cseMachine): stack globals annotation below frame name, fix overflow, and occlude arrows behind header text

- Move the globals annotation to its own row stacked below the frame
  name (rather than beside it or inside the box as a binding), since
  frame names are arbitrary function names of unpredictable length
  and can't safely share a row with anything else.
- Widen the frame box to fit the annotation label the same way binding
  text already widens it (capped at the same FrameDefaultWidth), so it
  no longer gets ellipsis-truncated or rendered left of the frame's
  own border when the label is longer than the box's other content.
- Add an opaque background rect behind the frame name and the globals
  annotation so the parent/tail arrow passing through that header gap
  is fully hidden behind the text, instead of showing through the
  empty space between glyphs (arrows are already layered behind
  content, but plain Konva text has no opaque fill of its own).

Closes #4069.

* fix(cseMachine): fix assignment animation crash for statement-only-assignment languages

InstrType.ASSIGNMENT constructed AssignmentAnimation with the
*current* (post-assignment) top-of-stash item, which is only valid
because a JS assignment *expression* re-pushes its value. A Python
assignment *statement* pushes nothing back, so the stash is genuinely
empty afterwards, the non-null assertion lied, and the constructor
threw reading `stashItem.index` on undefined — silently, since the
try/catch around CseAnimation.updateAnimation() had no logging.

Use the *previous* stash's top item instead, matching how
InstrType.POP (also a pure consumer) already does this correctly.
Also add the missing console.error so future animation regressions
in this path aren't invisible again.

Closes #4072.

* fix(cseMachine): stop the arrow-occlusion patch from being wider than the text it backs

Frame.tsx's header-text backing rects used Text.width(), which floors
at Config.TextMinWidth (30px) — a minimum meant for binding key/value
alignment, unrelated to how wide the text visually is. For a short
name like a one-letter function ("f", 9px of actual ink), that floor
is wider than Config.FramePaddingX (20px, the arrow's x-offset), so
the padded-out rect covered the arrow well past where the glyph
itself ends — an oversized, unnecessary gap in the arrow line even
when the text was nowhere near wide enough to actually cross it.

Use getTextWidth(text) directly instead, sized to what's actually
rendered.

* chore: bump common-cse-machine/web-cse-machine, drop published-type workaround casts

Now that @sourceacademy/common-cse-machine@0.2.0 (declaring
globalNames) and @sourceacademy/web-cse-machine@2.0.0 are actually
published, bump both dependencies and simplify the read sites that
were casting around the not-yet-published type.

Frame.tsx still needs one cast: `Env` (js-slang's real Environment
type) has no globalNames field regardless of any package version,
since it's an ad-hoc property CseSnapshotAdapter.ts stashes onto the
fake Environment object — updated that comment to say so accurately.

Also adds @sourceacademy/common-cse-machine to .yarnrc.yml's
npmPreapprovedPackages, matching the existing entries for
web-cse-machine/conductor/js-slang: yarn's npmMinimalAgeGate (3-day
quarantine on freshly published versions) was blocking install of the
version just published in the companion plugins release.

* fix(cseMachine): scope the Python LEGB frame rename to the Conductor snapshot path only

The #4042 rename ("Global" -> "Built-in functions", "Program" ->
"Globals") was motivated by Python's LEGB terminology, but Frame.tsx's
frameNames lookup applied unconditionally, so the legacy non-conductor
path (conductor.enable off, real js-slang interpreter) picked it up
too — a real regression against js-slang's own established naming,
confirmed live and flagged by Gemini's review on this PR.

Gate the rename on Layout.snapshotMode, which is true only while
rendering a Conductor snapshot (currently Python-only) and false for
the entire legacy path — evalCodeSaga never calls
CseMachine.renderSnapshot() when conductor.enable is off, so this is
a reliable, already-existing signal rather than a new one.

Other changes in this PR are left as-is: the globalNames annotation
is already inert for legacy js-slang Environments (they never carry
that ad-hoc field), the arrow-occlusion background-rect fix is a
general visual correctness fix unrelated to any language, and the
assignment-animation and frame-transition-timing fixes don't touch
frame naming at all.

Adds regression coverage on both sides of the gate: a real js-slang
EnvTree rendered outside snapshot mode keeps "Global"/"Program", and
a Python snapshot still gets "Built-in functions"/"Globals".

* fix(cseMachine): apply center-alignment offset to the globals annotation too

reassignCoordinatesX applied the center-alignment textOffset to the
frame name but not to _globalNamesText, leaving the annotation
flush-left under a centered name whenever center-alignment is on.
Flagged independently by CodeRabbit and Shrey's review on #4068.

Also extracts the headerRowHeight formula (duplicated between the
constructor and reassignCoordinatesY) into a single module-level
constant, per CodeRabbit's nitpick.

Added a regression test; confirmed it fails without the fix (30 vs
80) and passes with it.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Shrey Jain <101659971+Shrey5132@users.noreply.github.com>

* Show a popup for input() requests instead of hanging (py-slang#190) (#4084)

* Show a popup for input() requests instead of hanging (py-slang#190)

Wires up Conductor's new receiveInputRequest hook: a handleInputRequest
saga listens for input requests from the runner, shows the existing
PromptDialog popup, and answers via hostPlugin.sendInput. The home tab
and the viz CSE Machine tab share the same evalCodeConductorSaga run,
so this single fix covers both surfaces from the issue.

Also adds isWaitingForInput workspace state, used to suspend the
execTime watchdog while a popup is open so a user typing an answer
doesn't get their run force-stopped mid-prompt.

Points @sourceacademy/conductor at the local checkout (portal:) for
dev while the protocol change there is unreleased; repin to a real
published version once that lands upstream.

* chore: deps update

* Fix uncaught rejection when the input popup is dismissed via onClose

showSimplePromptDialog's underlying promise rejects if the dialog is
dismissed via its onClose path (e.g. clicking outside) rather than a
button response. The inner try/finally in handleInputRequest reset
isWaitingForInput on that path but didn't catch the rejection, so it
escaped the saga's while(true) loop and killed the task permanently -
leaving the runner blocked forever with no way to answer this or any
future input() call. Catch it and send an empty string, same as an
explicit Cancel.

Caught by gemini-code-assist review on PR #4084.

* Repin @sourceacademy/conductor to the published npm 0.6.0

The npm publish for conductor 0.6.0 landed, so the git-tag stopgap
pin isn't needed anymore - back to the normal semver-via-npm-registry
pin this repo used before. Verified: tsc --noEmit clean against the
real published package.

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel (#4083)

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel

CseMachineHostPlugin.receiveSnapshots now takes breakpointSteps as a second
argument (paired with the @sourceacademy/web-cse-machine protocol change),
and handleCseSnapshots dispatches WorkspaceActions.updateBreakpointSteps
with it alongside the existing snapshot/stepsTotal updates. The double-
chevron breakpoint-navigation buttons in SideContentCseMachine already
read props.breakpointSteps and are rendered unconditionally for the
conductor/Python flow — they just needed real data instead of an empty
array. No new UI. Depends on a py-slang change that detects breakpoint()
in the CSE machine (source-academy/py-slang issue #189).

package.json/yarn.lock currently point @sourceacademy/common-cse-machine
and @sourceacademy/web-cse-machine at a local yarn-link portal to
/home/vakshay/plugins for development; swap for a real published semver
range once that plugins PR merges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: revert local yarn-link dev scaffolding for cse-machine packages

package.json/yarn.lock were pointing @sourceacademy/common-cse-machine and
@sourceacademy/web-cse-machine at a local yarn-link portal
(/home/vakshay/plugins) for development. That doesn't resolve for anyone
else or CI, and web-cse-machine can't be pinned to the plugins branch via
Yarn's git+workspace protocol either (it depends on the sibling
common-cse-machine workspace via workspace:*, and Yarn's single-workspace
git fetch doesn't build sibling workspaces first, so the prepack/rollup
step can't find its dist).

Reverting to the original published semver ranges so this PR's dependency
diff is clean. This PR depends on source-academy/plugins#55 (adds
breakpointSteps to the CSE snapshot protocol) merging and publishing to
npm before it will typecheck/build — opening as a draft until then.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: bump cse-machine deps to the published versions with breakpointSteps

@sourceacademy/common-cse-machine ^0.2.0 -> ^0.3.0 and
@sourceacademy/web-cse-machine ^2.0.0 -> ^3.0.0, now that
source-academy/plugins#55 has published (bumping the same two packages,
plus runner-cse-machine, to carry breakpointSteps over the CSE snapshot
channel). Unblocks this PR: tsc --noEmit passes against the real published
CseMachineHostPlugin.receiveSnapshots(snapshots, breakpointSteps) signature.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: formattin

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: henz <henz@comp.nus.edu.sg>

* Updated the format in which language is sent to backend

* Remove chatbot from JS layout; stop sending language in chat APIs

* Update README.md

adding conductor info

* Remove unused scrollRefIntoView; inline scroll, clean unused Sicp layout symbols; add host plugin callbacks; relax CseMachineHostPlugin signature

* Fix language semantics

* Fix conflict

* Update README.md

* Fix conflict

* Fix conflict

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel (#4083)

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel

CseMachineHostPlugin.receiveSnapshots now takes breakpointSteps as a second
argument (paired with the @sourceacademy/web-cse-machine protocol change),
and handleCseSnapshots dispatches WorkspaceActions.updateBreakpointSteps
with it alongside the existing snapshot/stepsTotal updates. The double-
chevron breakpoint-navigation buttons in SideContentCseMachine already
read props.breakpointSteps and are rendered unconditionally for the
conductor/Python flow — they just needed real data instead of an empty
array. No new UI. Depends on a py-slang change that detects breakpoint()
in the CSE machine (source-academy/py-slang issue #189).

package.json/yarn.lock currently point @sourceacademy/common-cse-machine
and @sourceacademy/web-cse-machine at a local yarn-link portal to
/home/vakshay/plugins for development; swap for a real published semver
range once that plugins PR merges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: revert local yarn-link dev scaffolding for cse-machine packages

package.json/yarn.lock were pointing @sourceacademy/common-cse-machine and
@sourceacademy/web-cse-machine at a local yarn-link portal
(/home/vakshay/plugins) for development. That doesn't resolve for anyone
else or CI, and web-cse-machine can't be pinned to the plugins branch via
Yarn's git+workspace protocol either (it depends on the sibling
common-cse-machine workspace via workspace:*, and Yarn's single-workspace
git fetch doesn't build sibling workspaces first, so the prepack/rollup
step can't find its dist).

Reverting to the original published semver ranges so this PR's dependency
diff is clean. This PR depends on source-academy/plugins#55 (adds
breakpointSteps to the CSE snapshot protocol) merging and publishing to
npm before it will typecheck/build — opening as a draft until then.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: bump cse-machine deps to the published versions with breakpointSteps

@sourceacademy/common-cse-machine ^0.2.0 -> ^0.3.0 and
@sourceacademy/web-cse-machine ^2.0.0 -> ^3.0.0, now that
source-academy/plugins#55 has published (bumping the same two packages,
plus runner-cse-machine, to carry breakpointSteps over the CSE snapshot
channel). Unblocks this PR: tsc --noEmit passes against the real published
CseMachineHostPlugin.receiveSnapshots(snapshots, breakpointSteps) signature.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: formattin

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: henz <henz@comp.nus.edu.sg>

* Updated the format in which language is sent to backend

* Remove chatbot from JS layout; stop sending language in chat APIs

* Update blueprintjs to v6.17.2 (#4060)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @rsbuild/core to v2.1.5 (#4077)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency react-simple-keyboard to v3.8.233 (#4081)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @rsbuild/plugin-sass to v2.0.1 (#4075)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency @rsbuild/plugin-svgr to v2.0.5 (#4076)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update vitest monorepo to v4.1.10 (#4082)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency prettier to v3.9.5 (#4087)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Follow Redux Toolkit conventions (#4089)

* Rename useTypedSelector to useAppSelector

* Create useAppDispatch hook

* Update ESLint import rule

* Migrate some components to use useAppDispatch

* Migrate more components to use useAppDispatch

* Migrate even more files to use useAppDispatch

* Fix imports

* Fix errors post-rebase

* Clean up tests

* Remove duplicated code

* Fix remaining errors

* Migrate conflicting files

* Undo incorrect change post-rebase

* Fix remaining error

* Fix ESLint errors

* Set up Knip for codebase maintenance (#4052)

* Install knip

* Deduplicate dependencies

* Add knip config

* Delete some unused files

* Delete FaceAPI side content

* Delete editable test case side content

* Remove unused export

* Clean up config warnings

* Address phantom dependencies

* Add knip docs in README

* Fix lockfile post-rebase

* Reformat README

* Resolve README discrepancies

* Fix README confusion

* Migrate from Prettier to oxfmt (#4090)

* Update .gitattributes for LF normalization

* Install oxfmt

* Migrate Prettier config to oxfmt

* Add ignore patterns

* Update VSCode configuration

* Reformat files

* Update package scripts and hooks

* Reformat Renovate config

* Group rsbuild ecosystem updates together

* Refactor SICP/SICPy layout hooks and add caching (#4092)

* Move useLocalStorageState to separate file

* Move local storage helpers to hook file

* Fix util duplication and match for parity

* Fix typo

* Standardize file layout

* Improve hook safety

* Use hook instead of low level effects

* Create SICP query keys

* Create SICP query hooks

* Use new hooks to reduce duplication

* Add special handling for index page

* Rename hook to be textbook agnostic

* Use global object to avoid stale references

* Fix loading edge case

* Prefer supplier function for performance

* feat: move Chatbot from JS layout to Python layout

* refactor(sicp): move Chatbot from JS layout to Python layout

* style(sicp): format Python layout after move

* Fix link issues

* Fix react-router useParams typing in sicpjs layout

* Fix react-router useParams typing and import formatting in sicpy layout

* Format sicpy layout with oxfmt

* Fix crash when stored playground language state is invalid (#3646)

* Fix crash when stored playground language state is invalid

Validate the stored chapter+variant combination against ALL_LANGUAGES
when loading from localStorage. Falls back to the default language config
if the combination is invalid (e.g. chapter 1 + explicit-control).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fall back to defaultLanguageConfig for invalid stored language state

When the stored chapter+variant combination is not in ALL_LANGUAGES,
reset to `defaultLanguageConfig` (a known-valid pair) rather than to the
individual default context fields, so the fallback is explicitly "the
default language config" as the issue describes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>

* docs: add local backend setup and py-slang testing instructions (#4070)

* docs: add local backend setup steps to README

Fills the gap between "point REACT_APP_BACKEND_URL at localhost:4000"
and an actually-running backend, which otherwise just shows the
"under maintenance" page. Also corrects the postgres docker command
from the backend repo's own README, which has a stray -e flag before
-p 5432:5432 that prevents the port from actually being published.

* docs: add "Running your own py-slang" section

Mirrors the existing "Running your own js-slang" section, linking
out to py-slang's own README (added in a companion PR there) rather
than duplicating the instructions here.

* Update CONTRIBUTING.md

* Update CONTRIBUTING.md

* Update README.md

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Shrey Jain <101659971+Shrey5132@users.noreply.github.com>
Co-authored-by: Martin Henz <henz@comp.nus.edu.sg>

* Update README.md

* feat(cseMachine): rename global/program frames and annotate globals (#4068)

* feat(cseMachine): rename global/program frames and annotate globals

- Rename the top two CSE Machine frame labels to match Python's LEGB
  terminology: "Global" -> "Built-in functions", "Program" -> "Globals"
  (#4042).
- Thread the new optional CseSerializedEnvFrame.globalNames field
  (companion source-academy/plugins + source-academy/py-slang PRs)
  through CseSnapshotAdapter onto the fake Environment, and render it
  as a "globals: x, y" annotation at the top of a call frame the
  moment it's created, so it's clear those names resolve straight to
  the global frame instead of the usual enclosing-scope chain (#4041).
- Add @sourceacademy/common-cse-machine as a direct dependency: it's a
  peerDependency of @sourceacademy/web-cse-machine whose types
  frontend re-exports and uses directly, but frontend never installed
  it itself, so those types were silently resolving to `any` under
  skipLibCheck.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(cseMachine): stack globals annotation below frame name, fix overflow, and occlude arrows behind header text

- Move the globals annotation to its own row stacked below the frame
  name (rather than beside it or inside the box as a binding), since
  frame names are arbitrary function names of unpredictable length
  and can't safely share a row with anything else.
- Widen the frame box to fit the annotation label the same way binding
  text already widens it (capped at the same FrameDefaultWidth), so it
  no longer gets ellipsis-truncated or rendered left of the frame's
  own border when the label is longer than the box's other content.
- Add an opaque background rect behind the frame name and the globals
  annotation so the parent/tail arrow passing through that header gap
  is fully hidden behind the text, instead of showing through the
  empty space between glyphs (arrows are already layered behind
  content, but plain Konva text has no opaque fill of its own).

Closes #4069.

* fix(cseMachine): fix assignment animation crash for statement-only-assignment languages

InstrType.ASSIGNMENT constructed AssignmentAnimation with the
*current* (post-assignment) top-of-stash item, which is only valid
because a JS assignment *expression* re-pushes its value. A Python
assignment *statement* pushes nothing back, so the stash is genuinely
empty afterwards, the non-null assertion lied, and the constructor
threw reading `stashItem.index` on undefined — silently, since the
try/catch around CseAnimation.updateAnimation() had no logging.

Use the *previous* stash's top item instead, matching how
InstrType.POP (also a pure consumer) already does this correctly.
Also add the missing console.error so future animation regressions
in this path aren't invisible again.

Closes #4072.

* fix(cseMachine): stop the arrow-occlusion patch from being wider than the text it backs

Frame.tsx's header-text backing rects used Text.width(), which floors
at Config.TextMinWidth (30px) — a minimum meant for binding key/value
alignment, unrelated to how wide the text visually is. For a short
name like a one-letter function ("f", 9px of actual ink), that floor
is wider than Config.FramePaddingX (20px, the arrow's x-offset), so
the padded-out rect covered the arrow well past where the glyph
itself ends — an oversized, unnecessary gap in the arrow line even
when the text was nowhere near wide enough to actually cross it.

Use getTextWidth(text) directly instead, sized to what's actually
rendered.

* chore: bump common-cse-machine/web-cse-machine, drop published-type workaround casts

Now that @sourceacademy/common-cse-machine@0.2.0 (declaring
globalNames) and @sourceacademy/web-cse-machine@2.0.0 are actually
published, bump both dependencies and simplify the read sites that
were casting around the not-yet-published type.

Frame.tsx still needs one cast: `Env` (js-slang's real Environment
type) has no globalNames field regardless of any package version,
since it's an ad-hoc property CseSnapshotAdapter.ts stashes onto the
fake Environment object — updated that comment to say so accurately.

Also adds @sourceacademy/common-cse-machine to .yarnrc.yml's
npmPreapprovedPackages, matching the existing entries for
web-cse-machine/conductor/js-slang: yarn's npmMinimalAgeGate (3-day
quarantine on freshly published versions) was blocking install of the
version just published in the companion plugins release.

* fix(cseMachine): scope the Python LEGB frame rename to the Conductor snapshot path only

The #4042 rename ("Global" -> "Built-in functions", "Program" ->
"Globals") was motivated by Python's LEGB terminology, but Frame.tsx's
frameNames lookup applied unconditionally, so the legacy non-conductor
path (conductor.enable off, real js-slang interpreter) picked it up
too — a real regression against js-slang's own established naming,
confirmed live and flagged by Gemini's review on this PR.

Gate the rename on Layout.snapshotMode, which is true only while
rendering a Conductor snapshot (currently Python-only) and false for
the entire legacy path — evalCodeSaga never calls
CseMachine.renderSnapshot() when conductor.enable is off, so this is
a reliable, already-existing signal rather than a new one.

Other changes in this PR are left as-is: the globalNames annotation
is already inert for legacy js-slang Environments (they never carry
that ad-hoc field), the arrow-occlusion background-rect fix is a
general visual correctness fix unrelated to any language, and the
assignment-animation and frame-transition-timing fixes don't touch
frame naming at all.

Adds regression coverage on both sides of the gate: a real js-slang
EnvTree rendered outside snapshot mode keeps "Global"/"Program", and
a Python snapshot still gets "Built-in functions"/"Globals".

* fix(cseMachine): apply center-alignment offset to the globals annotation too

reassignCoordinatesX applied the center-alignment textOffset to the
frame name but not to _globalNamesText, leaving the annotation
flush-left under a centered name whenever center-alignment is on.
Flagged independently by CodeRabbit and Shrey's review on #4068.

Also extracts the headerRowHeight formula (duplicated between the
constructor and reassignCoordinatesY) into a single module-level
constant, per CodeRabbit's nitpick.

Added a regression test; confirmed it fails without the fix (30 vs
80) and passes with it.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Shrey Jain <101659971+Shrey5132@users.noreply.github.com>

* Show a popup for input() requests instead of hanging (py-slang#190) (#4084)

* Show a popup for input() requests instead of hanging (py-slang#190)

Wires up Conductor's new receiveInputRequest hook: a handleInputRequest
saga listens for input requests from the runner, shows the existing
PromptDialog popup, and answers via hostPlugin.sendInput. The home tab
and the viz CSE Machine tab share the same evalCodeConductorSaga run,
so this single fix covers both surfaces from the issue.

Also adds isWaitingForInput workspace state, used to suspend the
execTime watchdog while a popup is open so a user typing an answer
doesn't get their run force-stopped mid-prompt.

Points @sourceacademy/conductor at the local checkout (portal:) for
dev while the protocol change there is unreleased; repin to a real
published version once that lands upstream.

* chore: deps update

* Fix uncaught rejection when the input popup is dismissed via onClose

showSimplePromptDialog's underlying promise rejects if the dialog is
dismissed via its onClose path (e.g. clicking outside) rather than a
button response. The inner try/finally in handleInputRequest reset
isWaitingForInput on that path but didn't catch the rejection, so it
escaped the saga's while(true) loop and killed the task permanently -
leaving the runner blocked forever with no way to answer this or any
future input() call. Catch it and send an empty string, same as an
explicit Cancel.

Caught by gemini-code-assist review on PR #4084.

* Repin @sourceacademy/conductor to the published npm 0.6.0

The npm publish for conductor 0.6.0 landed, so the git-tag stopgap
pin isn't needed anymore - back to the normal semver-via-npm-registry
pin this repo used before. Verified: tsc --noEmit clean against the
real published package.

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel (#4083)

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel

CseMachineHostPlugin.receiveSnapshots now takes breakpointSteps as a second
argument (paired with the @sourceacademy/web-cse-machine protocol change),
and handleCseSnapshots dispatches WorkspaceActions.updateBreakpointSteps
with it alongside the existing snapshot/stepsTotal updates. The double-
chevron breakpoint-navigation buttons in SideContentCseMachine already
read props.breakpointSteps and are rendered unconditionally for the
conductor/Python flow — they just needed real data instead of an empty
array. No new UI. Depends on a py-slang change that detects breakpoint()
in the CSE machine (source-academy/py-slang issue #189).

package.json/yarn.lock currently point @sourceacademy/common-cse-machine
and @sourceacademy/web-cse-machine at a local yarn-link portal to
/home/vakshay/plugins for development; swap for a real published semver
range once that plugins PR merges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: revert local yarn-link dev scaffolding for cse-machine packages

package.json/yarn.lock were pointing @sourceacademy/common-cse-machine and
@sourceacademy/web-cse-machine at a local yarn-link portal
(/home/vakshay/plugins) for development. That doesn't resolve for anyone
else or CI, and web-cse-machine can't be pinned to the plugins branch via
Yarn's git+workspace protocol either (it depends on the sibling
common-cse-machine workspace via workspace:*, and Yarn's single-workspace
git fetch doesn't build sibling workspaces first, so the prepack/rollup
step can't find its dist).

Reverting to the original published semver ranges so this PR's dependency
diff is clean. This PR depends on source-academy/plugins#55 (adds
breakpointSteps to the CSE snapshot protocol) merging and publishing to
npm before it will typecheck/build — opening as a draft until then.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: bump cse-machine deps to the published versions with breakpointSteps

@sourceacademy/common-cse-machine ^0.2.0 -> ^0.3.0 and
@sourceacademy/web-cse-machine ^2.0.0 -> ^3.0.0, now that
source-academy/plugins#55 has published (bumping the same two packages,
plus runner-cse-machine, to carry breakpointSteps over the CSE snapshot
channel). Unblocks this PR: tsc --noEmit passes against the real published
CseMachineHostPlugin.receiveSnapshots(snapshots, breakpointSteps) signature.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: formattin

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: henz <henz@comp.nus.edu.sg>

* Updated the format in which language is sent to backend

* Remove chatbot from JS layout; stop sending language in chat APIs

* Remove unused scrollRefIntoView; inline scroll, clean unused Sicp layout symbols; add host plugin callbacks; relax CseMachineHostPlugin signature

* Fix language semantics

* Fix conflict

* Update README.md

* Fix conflict

* Fix conflict

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel (#4083)

* feat(cseMachine): wire breakpoint step indices from the CSE snapshot channel

CseMachineHostPlugin.receiveSnapshots now takes breakpointSteps as a second
argument (paired with the @sourceacademy/web-cse-machine protocol change),
and handleCseSnapshots dispatches WorkspaceActions.updateBreakpointSteps
with it alongside the existing snapshot/stepsTotal updates. The double-
chevron breakpoint-navigation buttons in SideContentCseMachine already
read props.breakpointSteps and are rendered unconditionally for the
conductor/Python flow — they just needed real data instead of an empty
array. No new UI. Depends on a py-slang change that detects breakpoint()
in the CSE machine (source-academy/py-slang issue #189).

package.json/yarn.lock currently point @sourceacademy/common-cse-machine
and @sourceacademy/web-cse-machine at a local yarn-link portal to
/home/vakshay/plugins for development; swap for a real published semver
range once that plugins PR merges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: revert local yarn-link dev scaffolding for cse-machine packages

package.json/yarn.lock were pointing @sourceacademy/common-cse-machine and
@sourceacademy/web-cse-machine at a local yarn-link portal
(/home/vakshay/plugins) for development. That doesn't resolve for anyone
else or CI, and web-cse-machine can't be pinned to the plugins branch via
Yarn's git+workspace protocol either (it depends on the sibling
common-cse-machine workspace via workspace:*, and Yarn's single-workspace
git fetch doesn't build sibling workspaces first, so the prepack/rollup
step can't find its dist).

Reverting to the original published semver ranges so this PR's dependency
diff is clean. This PR depends on source-academy/plugins#55 (adds
breakpointSteps to the CSE snapshot protocol) merging and publishing to
npm before it will typecheck/build — opening as a draft until then.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: bump cse-machine deps to the published versions with breakpointSteps

@sourceacademy/common-cse-machine ^0.2.0 -> ^0.3.0 and
@sourceacademy/web-cse-machine ^2.0.0 -> ^3.0.0, now that
source-academy/plugins#55 has published (bumping the same two packages,
plus runner-cse-machine, to carry breakpointSteps over the CSE snapshot
channel). Unblocks this PR: tsc --noEmit passes against the real published
CseMachineHostPlugin.receiveSnapshots(snapshots, breakpointSteps) signature.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* chore: formattin

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: henz <henz@comp.nus.edu.sg>

* Updated the format in which language is sent to backend

* Remove chatbot from JS layout; stop sending language in chat APIs

* feat: move Chatbot from JS layout to Python layout

# Conflicts:
#	src/new_routes/sicpjs/_layout.tsx
#	src/new_routes/sicpy/_layout.tsx

* refactor(sicp): move Chatbot from JS layout to Python layout

* style(sicp): format Python layout after move

* Fix link issues

* Fix react-router useParams typing in sicpjs layout

* Fix react-router useParams typing and import formatting in sicpy layout

* Format sicpy layout with oxfmt

* Changes as requested

* Configure Louis chatbot per course

* Format SICP JavaScript layout

* Remove unrelated PR artifacts

* fixed code review comment

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Martin Henz <henz@comp.nus.edu.sg>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Co-authored-by: Akshay <131676168+Akshay-2007-1@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Shrey Jain <101659971+Shrey5132@users.noreply.github.com>
Co-authored-by: Zhang Yilin <yilinzh54@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants