Skip to content

Add breakpointSteps to the CSE snapshot protocol#55

Merged
Akshay-2007-1 merged 3 commits into
mainfrom
feat/cse-breakpoint-steps
Jul 9, 2026
Merged

Add breakpointSteps to the CSE snapshot protocol#55
Akshay-2007-1 merged 3 commits into
mainfrom
feat/cse-breakpoint-steps

Conversation

@Akshay-2007-1

Copy link
Copy Markdown
Contributor

Summary

  • Adds breakpointSteps: number[] to CseSnapshotMessage in @sourceacademy/common-cse-machine: a run-level array of 0-based step indices where a breakpoint (e.g. Python's breakpoint(), JS's debugger;) sits on top of the control.
  • CseMachinePlugin.sendSnapshots (runner) takes it as an optional second argument (defaults to []) and forwards it in the message.
  • CseMachineHostPlugin.receiveSnapshots (web) now receives it as a second parameter, passed through from the message (?? [] if absent).
  • Adds a changeset (minor bump for all three affected packages — additive, non-breaking).

Why

Mirrors the stepper protocol's existing redexNodeType marker field, which host apps use to drive breakpoint-navigation (double-chevron) controls. The CSE machine protocol had no equivalent, so a CSE-machine-based evaluator (e.g. py-slang, for source-academy/py-slang#189) had no way to tell the host which steps to jump to. This is a companion change to a py-slang change that detects breakpoint() in the CSE machine and populates this array, and a frontend change that wires it into the existing updateBreakpointSteps Redux action.

Test plan

  • Updated/extended unit tests in all three touched packages (common.test.ts, runner.test.ts, web.test.ts) — 66 tests passing
  • tsc --noEmit clean on all three packages
  • Rebuilt each package's dist/ and confirmed the new field/param is present in the emitted bundle

Adds breakpointSteps: number[] to CseSnapshotMessage — a run-level array
of 0-based step indices where a breakpoint (e.g. Python's breakpoint())
sits on top of the control. CseMachinePlugin.sendSnapshots takes it as an
optional second argument; CseMachineHostPlugin.receiveSnapshots now
receives it as a second parameter.

Lets host apps wire up breakpoint-navigation controls for CSE-machine
evaluators, matching the stepper's existing redexNodeType contract.
Paired with a py-slang change that detects breakpoint() in the CSE
machine and populates this array (source-academy/py-slang issue #189).

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

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 55bc989

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

💥 An error occurred when fetching the changed packages and changesets in this PR
Some errors occurred when validating the changesets config:
The package or glob expression "@sourceacademy/web-stepper" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch

@Akshay-2007-1 Akshay-2007-1 self-assigned this Jul 9, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces breakpointSteps to the CSE snapshot protocol, allowing host applications to wire breakpoint-navigation controls for CSE-machine-based evaluators. The changes update CseMachinePlugin.sendSnapshots and CseMachineHostPlugin.receiveSnapshots to support and propagate this new array of step indices. The feedback suggests making breakpointSteps optional in the CseSnapshotMessage interface to ensure backward compatibility, and defensively verifying that message.breakpointSteps is an array at runtime before passing it to receiveSnapshots.

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.

Comment thread src/common/cse-machine/src/index.ts Outdated
Comment thread src/web/cse-machine/src/index.ts Outdated
Make CseSnapshotMessage.breakpointSteps optional for backward compatibility
with runners that predate this field, and defensively check
Array.isArray(message.breakpointSteps) in the web plugin before passing it
to receiveSnapshots, since messages cross a conduit channel boundary and
could come from an older or malformed runner.

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

Copy link
Copy Markdown
Contributor Author

Addressed both suggestions in 5c35f18: breakpointSteps is now optional on CseSnapshotMessage, and the web plugin defensively checks Array.isArray(message.breakpointSteps) before passing it through.

@Akshay-2007-1

Copy link
Copy Markdown
Contributor Author

@coderabbitai, any review comments?

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 55 --repo source-academy/plugins --json title,body,files,commits -q '.'
echo "---FILES---"
gh pr diff 55 --repo source-academy/plugins

Length of output: 14705


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,80p' src/web/cse-machine/src/index.ts

Length of output: 2465


The changes look solid overall — I checked the current diff (commits 668a454 and 5c35f18) and confirm both previously flagged points are addressed correctly:

  • breakpointSteps?: number[] is now optional on CseSnapshotMessage in src/common/cse-machine/src/index.ts.
  • receiveSnapshots in src/web/cse-machine/src/index.ts guards with Array.isArray(message.breakpointSteps) ? message.breakpointSteps : [] before forwarding, which correctly handles messages from older runners or malformed payloads.

A couple of minor, non-blocking observations for your consideration:

  1. No validation of breakpointSteps contents. The subscribe handler in src/web/cse-machine/src/index.ts (Line 51-61) only checks Array.isArray(message.breakpointSteps) but doesn't verify the array elements are actually numbers, or that each index falls within [0, totalSteps). Since this crosses a conduit channel boundary from a potentially malformed/older runner, a malicious or buggy payload could contain non-numeric entries or out-of-range indices, which downstream host code (e.g. the Redux updateBreakpointSteps wiring) may not expect. Given message.snapshots and message.totalSteps aren't deeply validated either, this may be an accepted tradeoff for this PR's scope — just flagging it in case you'd like tighter validation now or as a follow-up.

  2. Ordering not enforced. The doc comment states breakpointSteps should be "in ascending order," but this is documentation-only — nothing enforces it in CseMachinePlugin.sendSnapshots (src/runner/cse-machine/src/index.ts). Since the runner and the py-slang breakpoint detector are the sole producers today, this is likely fine, but consumers on the host side should probably not assume sortedness without an explicit guarantee/test.

Neither of these blocks the PR — the core protocol addition, optional-field backward compatibility, and defensive parsing are all in good shape, and the test coverage across common.test.ts, runner.test.ts, and web.test.ts looks appropriately thorough for the new field.

[resolve]

@Akshay-2007-1 Akshay-2007-1 requested a review from Shrey5132 July 9, 2026 12:47
@Akshay-2007-1 Akshay-2007-1 merged commit 6672299 into main Jul 9, 2026
1 check passed
@Akshay-2007-1 Akshay-2007-1 deleted the feat/cse-breakpoint-steps branch July 9, 2026 12:58
Akshay-2007-1 added a commit that referenced this pull request Jul 9, 2026
changesets/action execs the `version` input directly instead of through a
shell, splitting it on whitespace and running the first token with the
rest as literal argv. `yarn changeset version && yarn install` therefore
sent `&&`, `yarn`, `install` to the changeset CLI itself as extra
positional arguments, which it rejected ("Too many arguments passed to
changesets — we only accept the command name as an argument"). That
aborted the version step before it could write any changes, so the
resulting "no commits between main and changeset-release/main" made the
action fail to open a Version Packages PR (seen on the #55 merge).

Wrap the compound command in `bash -c "..."` so bash — not changesets/action
— parses the `&&`.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Akshay-2007-1 added a commit to source-academy/frontend that referenced this pull request Jul 10, 2026
…tSteps

@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>
Akshay-2007-1 added a commit to Akshay-2007-1/py-slang-local that referenced this pull request Jul 10, 2026
…tSteps

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

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
martin-henz added a commit to source-academy/frontend that referenced this pull request Jul 10, 2026
…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>
martin-henz added a commit to source-academy/py-slang that referenced this pull request Jul 10, 2026
…rker (#256)

* feat(cse-machine): support breakpoint() as a breakpoint-navigation marker

Registers breakpoint() as a no-op MISC builtin (chapters 1-4) and, in the
CSE machine's step generator, detects a zero-arg call resolving (by
identity, so aliases like `bp = breakpoint; bp()` are caught too) to that
builtin, recording the step where it sits on top of the control into
context.runtime.breakpointSteps. Threads that array out through
collectSnapshots and PyCseEvaluator's sendSnapshots call so the host's
breakpoint-navigation controls can consume it (paired with a protocol
change in source-academy/plugins to carry it across the Conductor
__cse channel). Mirrors PR #197's substitution-stepper support of the
same feature (issue #189).

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/runner-cse-machine at a local yarn-link portal
(/home/vakshay/plugins) for development. That doesn't resolve for anyone
else or CI, and runner-cse-machine can't be pinned to a plugins branch via
Yarn's git+workspace protocol either — like web-cse-machine, 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) publishing a new npm
version before it will typecheck/build against real installs.

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/runner-cse-machine ^1.0.0 -> ^3.0.0, now that
source-academy/plugins#55 has published (bumping the same two packages,
plus web-cse-machine, to carry breakpointSteps over the CSE snapshot
channel). Unblocks this PR: tsc --noEmit passes against the real published
CseMachinePlugin.sendSnapshots(snapshots, breakpointSteps?) signature.

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

* fix(cse-machine): address Gemini review on #256

Also reset context.runtime.changepointSteps and .break in evaluateChunk,
alongside breakpointSteps -- same per-run leaking-state class, since
this.context persists across evaluateChunk calls but only breakpointSteps
was being reset.

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

* chore: formattin

* Fix yarn.lock entry ordering for CI's hardened-mode install check

The root workspace's dependency block had @sourceacademy/conductor
listed after @sourceacademy/runner-cse-machine instead of before it
alphabetically -- yarn.lock wasn't re-normalized when
common-cse-machine/runner-cse-machine were bumped. This alone was
enough to fail `yarn install --frozen-lockfile` under Yarn's hardened
mode (auto-enabled for fork PRs), which treats any lockfile diff,
including pure reordering, as forbidden. Regenerated via a plain
`yarn install`; no dependency versions changed.

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

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Martin Henz <henz@nus.edu.sg>
Co-authored-by: henz <henz@comp.nus.edu.sg>
Isha-Sovasaria pushed a commit to source-academy/frontend that referenced this pull request Jul 14, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants