feat(stepper): implement PythonStepperEvaluator and add stepper dependencies by Shrey5132 · Pull Request #152 · source-academy/py-slang

Shrey5132 · 2026-06-10T06:13:30Z

This PR adds portal dependencies on @sourceacademy/common-stepper and @sourceacademy/runner-stepper from the plugins repository. It implements PythonStepperEvaluator1 and its supporting AST walking, translation, and state reduction logic to step through Python 1 code using the Conductor framework.

…dencies

gemini-code-assist

Code Review

This pull request introduces a Python substitution stepper evaluator (PythonStepperEvaluator1) and its associated runner plugin to drive the stepper in the Conductor runner. It includes AST translation to an estree-shaped node model, a substitution-model reducer, and comprehensive tests. Feedback on the implementation highlights two key issues in the reducer: first, the substitute function incorrectly substitutes the identifier of a VariableDeclarator instead of only its init expression, which can corrupt the AST; second, division/modulo by zero and raising zero to a negative power are not handled, which can propagate JS Infinity or NaN instead of throwing a Python-like ZeroDivisionError.

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.

gemini-code-assist · 2026-06-10T06:16:24Z

+export function substitute(node: StepNode, name: string, value: StepNode): StepNode {
+  switch (node.type) {
+    case 'Identifier':
+      return node.name === name ? clone(value) : node;
+    case 'ArrowFunctionExpression':
+      if (paramNames(node).includes(name)) return node;
+      return { ...node, body: substitute(node.body as StepNode, name, value) };
+    case 'FunctionDeclaration':
+      if ((node.id as StepNode).name === name || paramNames(node).includes(name)) return node;
+      return { ...node, body: substitute(node.body as StepNode, name, value) };
+    default:
+      return mapChildren(node, child => substitute(child, name, value));
+  }
+}


When substituting a variable name, the recursive substitution model falls back to the default case for VariableDeclarator nodes. This causes the id (the variable name identifier) of the declarator to be substituted with its value, resulting in a malformed AST (e.g., 5 = 6 instead of x = 6) and potential runtime crashes or unexpected behavior when evaluating subsequent re-bindings.

Adding an explicit case for VariableDeclarator to only substitute its init expression prevents this issue and ensures AST integrity.

Suggested change

export function substitute(node: StepNode, name: string, value: StepNode): StepNode {

switch (node.type) {

case 'Identifier':

return node.name === name ? clone(value) : node;

case 'ArrowFunctionExpression':

if (paramNames(node).includes(name)) return node;

return { ...node, body: substitute(node.body as StepNode, name, value) };

case 'FunctionDeclaration':

if ((node.id as StepNode).name === name || paramNames(node).includes(name)) return node;

return { ...node, body: substitute(node.body as StepNode, name, value) };

default:

return mapChildren(node, child => substitute(child, name, value));

}

}

export function substitute(node: StepNode, name: string, value: StepNode): StepNode {

switch (node.type) {

case 'Identifier':

return node.name === name ? clone(value) : node;

case 'ArrowFunctionExpression':

if (paramNames(node).includes(name)) return node;

return { ...node, body: substitute(node.body as StepNode, name, value) };

case 'FunctionDeclaration':

if ((node.id as StepNode).name === name || paramNames(node).includes(name)) return node;

return { ...node, body: substitute(node.body as StepNode, name, value) };

case 'VariableDeclarator':

return { ...node, init: substitute(node.init as StepNode, name, value) };

default:

return mapChildren(node, child => substitute(child, name, value));

}

}

gemini-code-assist · 2026-06-10T06:16:24Z

+  } else if (typeof l === 'number' && typeof r === 'number') {
+    const pyFloat = Boolean(left.pyFloat) || Boolean(right.pyFloat) || op === '/';
+    switch (op) {
+      case '+': result = numberLiteral(l + r, pyFloat); break;
+      case '-': result = numberLiteral(l - r, pyFloat); break;
+      case '*': result = numberLiteral(l * r, pyFloat); break;
+      case '/': result = numberLiteral(l / r, true); break;
+      case '//': result = numberLiteral(Math.floor(l / r), pyFloat); break;
+      case '%': result = numberLiteral(((l % r) + r) % r, pyFloat); break;
+      case '**': result = numberLiteral(l ** r, pyFloat || l ** r !== Math.floor(l ** r)); break;


The stepper does not currently handle division or modulo by zero, or raising zero to a negative power. In JavaScript, these operations result in Infinity or NaN, which can propagate through the reduction steps and cause unexpected behavior or crashes in the host renderer. Adding explicit checks to throw a ZeroDivisionError matches Python's semantics and allows the evaluator to catch and report the error gracefully.

Suggested change

} else if (typeof l === 'number' && typeof r === 'number') {

const pyFloat = Boolean(left.pyFloat) || Boolean(right.pyFloat) || op === '/';

switch (op) {

case '+': result = numberLiteral(l + r, pyFloat); break;

case '-': result = numberLiteral(l - r, pyFloat); break;

case '*': result = numberLiteral(l * r, pyFloat); break;

case '/': result = numberLiteral(l / r, true); break;

case '//': result = numberLiteral(Math.floor(l / r), pyFloat); break;

case '%': result = numberLiteral(((l % r) + r) % r, pyFloat); break;

case '**': result = numberLiteral(l ** r, pyFloat || l ** r !== Math.floor(l ** r)); break;

} else if (typeof l === 'number' && typeof r === 'number') {

if (r === 0 && (op === '/' || op === '//' || op === '%')) {

throw new Error('ZeroDivisionError: division by zero');

}

if (l === 0 && r < 0 && op === '**') {

throw new Error('ZeroDivisionError: 0.0 cannot be raised to a negative power');

}

const pyFloat = Boolean(left.pyFloat) || Boolean(right.pyFloat) || op === '/';

switch (op) {

case '+': result = numberLiteral(l + r, pyFloat); break;

case '-': result = numberLiteral(l - r, pyFloat); break;

case '*': result = numberLiteral(l * r, pyFloat); break;

case '/': result = numberLiteral(l / r, true); break;

case '//': result = numberLiteral(Math.floor(l / r), pyFloat); break;

case '%': result = numberLiteral(((l % r) + r) % r, pyFloat); break;

case '**': result = numberLiteral(l ** r, pyFloat || l ** r !== Math.floor(l ** r)); break;

Agreed, please take a look at the existing operator combinations, Python has weird properties (e.g., the pyMod function)

It doesn't work exactly the same as JS

AaravMalani

I've not checked much of the actual reduction, but there a few minor and big changes I'm requesting. Good work anyways!

AaravMalani · 2026-06-10T10:55:48Z

  "PyWasmEvaluator",
  "PySvmlEvaluator",
  "PySvmlSinterEvaluator",
+  "PythonStepperEvaluator1",


Maybe rename to PyStepperEvaluator1 for naming convention sake?

AaravMalani · 2026-06-10T10:57:30Z

    "typescript-eslint": "^8.56.1"
  },
  "dependencies": {
+    "@sourceacademy/common-stepper": "portal:../plugins/src/common/stepper",


I'm making it a draft till it gets added to the plugins repository

AaravMalani · 2026-06-10T11:00:31Z

+  return identifier(p.lexeme);
+}
+
+function translateExpr(expr: ExprNS.Expr): StepNode {


Why have a translation layer? Wouldn't it save a lot of code just by directly operating on the normal AST nodes? (Don't sweat it if it's not feasible, it's pretty minor and I'll still approve without it)

AaravMalani · 2026-06-10T11:01:14Z

+function translateExpr(expr: ExprNS.Expr): StepNode {
+  switch (expr.kind) {
+    case 'BigIntLiteral': {
+      // Python ints are arbitrary-precision; the stepper computes in JS `number`, which is exact for


Err, I don't agree with the cast here. The stepper should compute with BigInts

AaravMalani · 2026-06-10T11:04:25Z

+      };
+    }
+    case 'Pass':
+      return { type: 'ExpressionStatement', expression: identifier('pass') };


I've not fully gone through the reduction steps, but is this a valid translation?
Even if it does symbolically work, it is a misnomer. Maybe introduce a PassStatement type which does nothing?

AaravMalani · 2026-06-10T11:06:42Z

+ * renderer that dispatches on a node's `type` string and reads estree-style field names
+ * (`left`/`right`/`operator`, `test`/`consequent`/`alternate`, `callee`/`arguments`, ...). To reuse
+ * that renderer unchanged, the Python stepper does not operate on py-slang's class-based AST
+ * directly: it first {@link ../stepper/translate translates} the Python AST into these plain


(Minor) Why the ../stepper/translate instead of ./translate?

AaravMalani · 2026-06-10T11:08:11Z

+ * Each {@link reduceProgram} call performs exactly one contraction and reports the redex/result so
+ * the driver can emit before/after markers. It returns `null` once the program is in normal form.
+ *
+ * Known limitations (acceptable for a teaching stepper): integer arithmetic is computed in JS


These feel like pretty major limitations, hopefully they can be worked on

AaravMalani · 2026-06-10T11:10:42Z

+  } else if (typeof l === 'number' && typeof r === 'number') {
+    const pyFloat = Boolean(left.pyFloat) || Boolean(right.pyFloat) || op === '/';
+    switch (op) {
+      case '+': result = numberLiteral(l + r, pyFloat); break;
+      case '-': result = numberLiteral(l - r, pyFloat); break;
+      case '*': result = numberLiteral(l * r, pyFloat); break;
+      case '/': result = numberLiteral(l / r, true); break;
+      case '//': result = numberLiteral(Math.floor(l / r), pyFloat); break;
+      case '%': result = numberLiteral(((l % r) + r) % r, pyFloat); break;
+      case '**': result = numberLiteral(l ** r, pyFloat || l ** r !== Math.floor(l ** r)); break;


Agreed, please take a look at the existing operator combinations, Python has weird properties (e.g., the pyMod function)

It doesn't work exactly the same as JS

Shrey5132 · 2026-06-12T14:45:44Z

@AaravMalani Thanks for all the feedback
This is a WIP, I only opened the PR to let others see the work I've done so far.
I'm working on all the changes mentioned.

feat(stepper): implement PythonStepperEvaluator and add stepper depen…

08456f4

…dencies

gemini-code-assist Bot reviewed Jun 10, 2026

View reviewed changes

AaravMalani reviewed Jun 10, 2026

View reviewed changes

Conversation

Shrey5132 commented Jun 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaravMalani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shrey5132 commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants