Repository navigation
Show a popup for input() requests instead of hanging (py-slang#190) - #4084
Conversation
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.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesInteractive conductor input
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces support for handling standard-input requests (such as Python's input()) in the runner. It adds state and actions to track when the workspace is waiting for input, implements a saga handleInputRequest to display a prompt dialog and send the user's input back to the runner, and updates the execution watchdog to suspend its timeout while waiting for input. A critical issue was identified where a rejection of the prompt dialog (e.g., if dismissed) would crash the saga and cause the runner to hang indefinitely; wrapping the dialog call in a try-catch block is recommended to handle this gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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.
Coverage Report for CI Build 29081135790Coverage decreased (-0.09%) to 42.267%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
50-50: 📐 Maintainability & Code Quality | 🔵 TrivialKeep this Git ref temporary until
@sourceacademy/conductor@0.6.0is published. Switch back to the npm release once it exists so installs regain registry integrity and reproducibility.🤖 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 `@package.json` at line 50, Update the `@sourceacademy/conductor` dependency in package.json from the temporary Git URL reference back to the published npm version `@sourceacademy/conductor`@0.6.0 once that release is available.
🤖 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.
Inline comments:
In `@src/commons/sagas/WorkspaceSaga/helpers/evalCode.ts`:
- Around line 739-762: Update the timeout branch inside the timeout saga to
resolve a truthy value, such as true, or replace it with a delay effect, so the
existing if (timedOut) condition reliably returns. Preserve the input-wait
handling and fresh timeout behavior in the surrounding race and follow-up take.
---
Nitpick comments:
In `@package.json`:
- Line 50: Update the `@sourceacademy/conductor` dependency in package.json from
the temporary Git URL reference back to the published npm version
`@sourceacademy/conductor`@0.6.0 once that release is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 016eefc3-ad84-4fbe-b85f-f3542c892076
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
package.jsonsrc/commons/application/ApplicationTypes.tssrc/commons/application/actions/InterpreterActions.tssrc/commons/sagas/WorkspaceSaga/helpers/evalCode.tssrc/commons/workspace/WorkspaceReducer.tssrc/commons/workspace/WorkspaceTypes.ts
| timeout: call(function* (): SagaIterator { | ||
| while (true) { | ||
| const { timedOut } = yield race({ | ||
| timedOut: call(() => new Promise(resolve => setTimeout(resolve, execTime + 10000))), | ||
| waiting: take( | ||
| (a: any) => | ||
| a.type === actions.setIsWaitingForInput.type && | ||
| a.payload.workspaceLocation === workspaceLocation && | ||
| a.payload.isWaitingForInput === true, | ||
| ), | ||
| }); | ||
| if (timedOut) { | ||
| return; | ||
| } | ||
| // Waiting on the input() popup: suspend the watchdog entirely until it's answered, | ||
| // then restart a fresh execTime window. | ||
| yield take( | ||
| (a: any) => | ||
| a.type === actions.setIsWaitingForInput.type && | ||
| a.payload.workspaceLocation === workspaceLocation && | ||
| a.payload.isWaitingForInput === false, | ||
| ); | ||
| } | ||
| }), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
redux-saga race result undefined when winning effect resolves undefined
💡 Result:
In redux-saga, when you use the race effect, it returns an object where keys correspond to the keys you provided in the input object [1]. If the winning effect resolves to undefined, the resulting object will contain that key with a value of undefined [1][2]. Because the result of the race is an object where all losing effects are set to undefined and only the winning effect holds its resolved value, a common pitfall occurs when checking for the result [1][2]. If you use a truthy check (e.g., if (result.key)) to determine if an effect won, that check will fail if the winning effect resolved to undefined, even though it was indeed the winner [3][2]. To correctly handle this, you should check for the presence of the key in the result object rather than the truthiness of its value. You can use the in operator or Object.prototype.hasOwnProperty.call to confirm which effect won [3][2]. Example: const result = yield race({ task: call(myFunction), cancel: take('CANCEL') }); // Avoid: if (result.task) {... } // If myFunction resolves to undefined, this will be false even if it won. // Use: if ('task' in result) { // Handle successful resolution of task, even if the value is undefined } if ('cancel' in result) { // Handle cancellation }
Citations:
- 1: https://github.com/redux-saga/redux-saga/blob/24e9a68d621fd2a57a29b3b80e4b54ecb22fa593/packages/core/types/effects.d.ts
- 2: Typing race TS 3.6 redux-saga/redux-saga#1900
- 3: https://stackoverflow.com/questions/51388284/redux-sagas-race-wont-trigger
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file structure first
ast-grep outline src/commons/sagas/WorkspaceSaga/helpers/evalCode.ts --view expanded
# Read the watchdog block and nearby control flow
sed -n '700,810p' src/commons/sagas/WorkspaceSaga/helpers/evalCode.ts
# Find any uses of redux-saga delay / race in this file for comparison
rg -n "race\\(|delay\\(|setTimeout\\(|isWaitingForInput" src/commons/sagas/WorkspaceSaga/helpers/evalCode.tsRepository: source-academy/frontend
Length of output: 5736
Make the watchdog timeout branch truthy
setTimeout(resolve, ...) resolves timedOut to undefined, so if (timedOut) never fires and the saga can hang in the follow-up take(...) instead of stopping a runaway run. Use a truthy resolution here (or delay).
Proposed fix
- timedOut: call(() => new Promise(resolve => setTimeout(resolve, execTime + 10000))),
+ timedOut: call(() => new Promise(resolve => setTimeout(() => resolve(true), execTime + 10000))),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| timeout: call(function* (): SagaIterator { | |
| while (true) { | |
| const { timedOut } = yield race({ | |
| timedOut: call(() => new Promise(resolve => setTimeout(resolve, execTime + 10000))), | |
| waiting: take( | |
| (a: any) => | |
| a.type === actions.setIsWaitingForInput.type && | |
| a.payload.workspaceLocation === workspaceLocation && | |
| a.payload.isWaitingForInput === true, | |
| ), | |
| }); | |
| if (timedOut) { | |
| return; | |
| } | |
| // Waiting on the input() popup: suspend the watchdog entirely until it's answered, | |
| // then restart a fresh execTime window. | |
| yield take( | |
| (a: any) => | |
| a.type === actions.setIsWaitingForInput.type && | |
| a.payload.workspaceLocation === workspaceLocation && | |
| a.payload.isWaitingForInput === false, | |
| ); | |
| } | |
| }), | |
| timeout: call(function* (): SagaIterator { | |
| while (true) { | |
| const { timedOut } = yield race({ | |
| timedOut: call(() => new Promise(resolve => setTimeout(() => resolve(true), execTime + 10000))), | |
| waiting: take( | |
| (a: any) => | |
| a.type === actions.setIsWaitingForInput.type && | |
| a.payload.workspaceLocation === workspaceLocation && | |
| a.payload.isWaitingForInput === true, | |
| ), | |
| }); | |
| if (timedOut) { | |
| return; | |
| } | |
| // Waiting on the input() popup: suspend the watchdog entirely until it's answered, | |
| // then restart a fresh execTime window. | |
| yield take( | |
| (a: any) => | |
| a.type === actions.setIsWaitingForInput.type && | |
| a.payload.workspaceLocation === workspaceLocation && | |
| a.payload.isWaitingForInput === false, | |
| ); | |
| } | |
| }), |
🤖 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/commons/sagas/WorkspaceSaga/helpers/evalCode.ts` around lines 739 - 762,
Update the timeout branch inside the timeout saga to resolve a truthy value,
such as true, or replace it with a delay effect, so the existing if (timedOut)
condition reliably returns. Preserve the input-wait handling and fresh timeout
behavior in the surrounding race and follow-up take.
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.
martin-henz
left a comment
There was a problem hiding this comment.
Nice example of conductor in action!
…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.
…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.
* 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>
Summary
Wires up Conductor's new
receiveInputRequesthook (source-academy/conductor#40) soinput()in Python programs shows a popup instead of hanging silently, on both the home tab and the viz CSE Machine tab.handleInputRequestsaga (evalCode.ts) listens for input requests from the runner, shows the existingPromptDialogpopup (showSimplePromptDialog, already used elsewhere e.g. Google Drive save), and answers viahostPlugin.sendInput.evalCodeConductorSagarun, so this single fix covers both surfaces from the issue — no separate viz-tab handling needed.isWaitingForInputworkspace state, used to suspend theexecTime-based execution watchdog while a popup is open (previously that raced against a small fixed window and would have killed a run mid-prompt for anyone who took a few seconds to type an answer).@sourceacademy/conductorat the local checkout via aportal:resolution for now, since the protocol change it depends on isn't released yet.Depends on
Opening as draft: CI will fail on
yarn installuntil conductor#40 is released and this branch'sportal:pin is swapped for the real version — same situation as py-slang#254.Test plan
tsc --noEmitpasses cleanportal:-only dependency in CI