Repository navigation
Set up Knip for codebase maintenance - #4052
Merged
Merged
Conversation
1 of 7 tasks
Contributor
|
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:
WalkthroughThis PR adds Knip configuration, scripts, and documentation for dead-code detection, then removes unused exports, components, constants, and stylesheet rules identified by that tooling. ChangesKnip Setup and Documentation
Dead Code Removal
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request integrates Knip into the codebase to detect and clean up dead code, introducing a knip.json configuration, new yarn scripts, and comprehensive documentation in the README.md. It also removes several unused components, files, and styles. Feedback on the changes highlights a discrepancy where directories and test files mentioned in the README's ignore section are missing from the actual knip.json configuration, as well as a conceptual mix-up in the README instructions regarding how to ignore flagged symbols versus dependencies.
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.
Coverage Report for CI Build 28653855154Coverage remained the same at 40.649%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
RichDom2185
marked this pull request as draft
July 3, 2026 05:13
RichDom2185
marked this pull request as ready for review
July 3, 2026 10:09
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
RichDom2185
enabled auto-merge (squash)
July 3, 2026 10:09
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
111-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor wording nits from static analysis.
"afterwards" → "afterward" is the preferred US English form (line 132); "short comment" could be tightened to "brief comment" (line 144). Both cosmetic.
🤖 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 `@README.md` around lines 111 - 155, Update the README wording in the Knip usage/triage section to match the review nits: change “afterwards” to the preferred US English “afterward” in the `yarn knip:fix` description, and tighten “short comment” to “brief comment” in the `ignoreDependencies` guidance. Keep the rest of the `README.md` maintenance guidance unchanged.Source: Linters/SAST tools
🤖 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 `@README.md`:
- Around line 111-155: Update the README wording in the Knip usage/triage
section to match the review nits: change “afterwards” to the preferred US
English “afterward” in the `yarn knip:fix` description, and tighten “short
comment” to “brief comment” in the `ignoreDependencies` guidance. Keep the rest
of the `README.md` maintenance guidance unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77eaed7b-9d6d-416c-a908-d8d186ee63ad
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
README.mdknip.jsonpackage.jsonsrc/commons/navigationBar/subcomponents/AcademyNavigationBar.tsxsrc/commons/sideContent/content/SideContentCanvasOutput.tsxsrc/commons/sideContent/content/SideContentFaceapiDisplay.tsxsrc/commons/sideContent/content/githubAssessments/SideContentEditableTestcaseCard.tsxsrc/commons/sideContent/content/githubAssessments/SideContentMarkdownEditor.tsxsrc/commons/sideContent/content/githubAssessments/SideContentTaskEditor.tsxsrc/features/cseMachine/components/arrows/Arrow.tsxsrc/features/game/state/GameStateConstants.tssrc/styles/_workspace.scss
💤 Files with no reviewable changes (8)
- src/commons/sideContent/content/githubAssessments/SideContentTaskEditor.tsx
- src/commons/sideContent/content/githubAssessments/SideContentEditableTestcaseCard.tsx
- src/features/cseMachine/components/arrows/Arrow.tsx
- src/features/game/state/GameStateConstants.ts
- src/commons/sideContent/content/SideContentFaceapiDisplay.tsx
- src/commons/sideContent/content/SideContentCanvasOutput.tsx
- src/styles/_workspace.scss
- src/commons/sideContent/content/githubAssessments/SideContentMarkdownEditor.tsx
RichDom2185
marked this pull request as draft
July 3, 2026 11:35
auto-merge was automatically disabled
July 3, 2026 11:35
Pull request was converted to draft
RichDom2185
marked this pull request as ready for review
July 13, 2026 13:39
Isha-Sovasaria
pushed a commit
that referenced
this pull request
Jul 19, 2026
* 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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please merge #4051 first.
Also resolves phantom dependency issues and deletes some unused files. Remaining files and issues flagged out by knip are to be triaged manually (as it may be e.g. intentionally unused at the moment to facilitate upcoming feature work).
As such, also does not run knip in CI verification.
Type of change
How to test
Everything should still work
Checklist
This is part 2 of 2 in a stack made with GitButler: