Skip to content

CSE machine: support for StreamViz drawing arrows#1939

Merged
martin-henz merged 18 commits intomasterfrom
streamMapping
Apr 17, 2026
Merged

CSE machine: support for StreamViz drawing arrows#1939
martin-henz merged 18 commits intomasterfrom
streamMapping

Conversation

@of89p
Copy link
Copy Markdown
Contributor

@of89p of89p commented Apr 6, 2026

Description

The map streamLineage (Found in context object, types.ts) that maps the ID of nullary functions to the pairs it returns.

Everytime a CALL0 is called in the control, we push a tuple (nullaryFnID, controlStackHeight) to the pendingStreamFnStack[]. Within generateCSEMachineStateStream, we check the whether the height of the control stack has reached the height of i-1, where i is the controlStackHeight when we pendingStreamFnStack.peek(). If so, we pop pendingStreamFnStack.pop() and we map (using streamLineage) the nullaryFnID to the pair that is at the top of the stash.

We store breakpoints in streamPointSteps[]. This array is calculated within envChangingStreams. The criteria for a breakpoint is exactly the same as updating streamLineage.

See: source-academy/frontend#3727 (comment)
and source-academy/frontend#3747

This PR is blocking: source-academy/frontend#3747

Type of change

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

How to test

Checklist

  • I have tested this code.
    • Tests pass with yarn test
    • js-slang executable is verified to work
    • Did I have to reduce the strength of some tests?
      • If so, why...
  • I have updated the documentation

@martin-henz
Copy link
Copy Markdown
Member

/gemini review

Copy link
Copy Markdown
Contributor

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

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 stream visualization features by tracking stream lineage and execution steps. It adds streamLineage, streamsPointSteps, and pendingStreamFnStack to the execution context and updates the CSE machine interpreter to manage these properties. Review feedback highlights opportunities to improve type safety by using specific tuple types for the function stack and to enhance performance by storing control stack heights as numbers rather than strings, thereby avoiding redundant parsing. Additionally, a redundant property in the runtime initialization was identified for removal to maintain consistency with the defined interfaces.

Comment thread src/createContext.ts Outdated
Comment thread src/types.ts Outdated
Comment thread src/cse-machine/interpreter.ts Outdated
Comment thread src/cse-machine/interpreter.ts Outdated
Comment thread src/cse-machine/utils.ts Outdated
Copy link
Copy Markdown
Contributor

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

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 infrastructure for stream visualization by tracking stream lineage and execution steps within the CSE machine. Key changes include the addition of streamLineage, streamsPointSteps, and a pendingStreamFnStack to the execution context to map nullary function calls to their results. The review feedback focuses on improving type safety by storing stack heights as numbers rather than strings, removing redundant property initializations that deviate from the defined interface, and addressing minor style issues such as strict equality and trailing whitespace.

Comment thread src/createContext.ts Outdated
Comment thread src/types.ts Outdated
Comment thread src/cse-machine/interpreter.ts Outdated
Comment thread src/cse-machine/interpreter.ts Outdated
Comment thread src/cse-machine/utils.ts Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented Apr 8, 2026

Coverage Report for CI Build 24542781792

Coverage increased (+0.03%) to 78.391%

Details

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

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 8736
Covered Lines: 7019
Line Coverage: 80.35%
Relevant Branches: 4078
Covered Branches: 3026
Branch Coverage: 74.2%
Branches in Coverage %: Yes
Coverage Strength: 184022.22 hits per line

💛 - Coveralls

@martin-henz
Copy link
Copy Markdown
Member

/gemini review
Can you check that all review comments are properly handled?

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Thank you for the update. I have reviewed the changes in this pull request. The implementation correctly introduces the streamLineage map and pendingStreamFnStack to track nullary function calls and their resulting pairs, which aligns with the requirements for the stream visualizer. The logic in generateCSEMachineStateStream and the helper envChangingStreams correctly handles the state updates. All necessary context modifications and mock updates appear to be in place. The implementation looks solid and addresses the requirements described.

@martin-henz
Copy link
Copy Markdown
Member

martin-henz commented Apr 8, 2026

/gemini review
can you describe how the trace that the CSE machine of js-slang passes to the frontend represents the links between nullary functions and the pairs they return?

@martin-henz martin-henz marked this pull request as draft April 8, 2026 16:56
@martin-henz martin-henz marked this pull request as ready for review April 10, 2026 01:13
@martin-henz martin-henz changed the title StreamVis drawing arrows StreamViz drawing arrows Apr 13, 2026
@martin-henz martin-henz self-requested a review April 13, 2026 13:13
Copy link
Copy Markdown
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

I think we should add regression tests for this change because it affects the core of the CSE machine: function calls

@martin-henz martin-henz changed the title StreamViz drawing arrows CSE machine: support for StreamViz drawing arrows Apr 13, 2026
@martin-henz martin-henz dismissed their stale review April 17, 2026 01:22

tests were added

Copy link
Copy Markdown
Member

@martin-henz martin-henz left a comment

Choose a reason for hiding this comment

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

Looks good!

@martin-henz martin-henz merged commit 680bcd3 into master Apr 17, 2026
5 checks passed
@martin-henz martin-henz deleted the streamMapping branch April 17, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants