WIP: Make Conductor-based assessment workspaces (Run + testcase grading) work for non-js-slang languages#4057
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for running code and testcases under Conductor, which includes skipping separate prepend evaluation and debugger statement insertion, concatenating code blocks for single-run testcase execution, and normalizing undefined results. It also refactors CSE machine arrow routing to better target closure and continuation visual geometries. The review feedback highlights two critical issues: first, testcase grading in runTestCase.ts may fail if the last output is an interpreter result rather than a console log, requiring a backward search for logs; second, arrow routing in ArrowFromText.tsx defaults leftFaceX and rightFaceX to zero when the target is not a function, which breaks routing for array targets and requires falling back to the target's actual coordinates.
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.
|
Important Review skippedDraft detected. 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:
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 |
Akshay-2007-1
force-pushed
the
fix/conductor-prepend-concat
branch
from
ffab40e to
b4ba27e
Compare
…ting it separately Conductor runs each evalCodeSaga call in a fresh, isolated worker that is terminated after use, so bindings from a silently-evaluated prepend never carry over into the entrypoint's own worker. This made quest/mission/path prepend definitions (e.g. pair, pizza) appear unrecognised when running under Conductor. Concatenate prepend + entrypoint into a single file for a single Conductor run instead, mirroring the existing Variant.TYPED handling.
insertDebuggerStatements() used context.chapter to decide whether to run js-slang's parser as a pre-check. Assessment questions carry a legacy chapter field (backend only accepts Source chapters 1-4, with no non-Source option), so a Conductor/Python question's chapter still reads as a Source chapter and js-slang's parser was being run against non-JS source before Conductor ever got to evaluate it, producing spurious syntax errors like "Missing semicolon at the end of statement". debugger; is JS/Source-specific anyway, so skip this entirely whenever Conductor is enabled.
…d+testcase as one file evalTestCode.ts still hardcodes js-slang's runInContext directly, and the legacy prepend/postpend/testcase flow relies on a single mutable js-slang Context shared across four separate evalCodeSaga calls - neither works under Conductor, since each call gets a fresh, isolated worker that's torn down afterwards. When Conductor is enabled, skip the elevated-context/blockExtraMethods privilege dance (a js-slang-specific concept with no Conductor equivalent) and instead concatenate prepend, the student's code, postpend, and the testcase expression into a single file for one Conductor run. The result is read back from the workspace's last output entry and re-dispatched as evalTestcaseSuccess/Failure so the existing testcase UI keeps working unchanged.
Python statements aren't value-producing at the top level the way Source/js-slang's REPL-style last-expression semantics are - a bare expression as the last line of a real Python script produces nothing observable, confirmed against an actual Python interpreter. Compare the last printed line (captured via the same consoleLogs the stdout channel already populates) against the testcase's answer instead of trying to capture a "result value" that mostly won't exist. The captured line is wrapped in a toReplString() so the existing stringify(testcase.result) === testcase.answer comparison in SideContentTestcaseCard renders it verbatim rather than adding JSON-style quotes around it.
Covers the three new behaviors added for Conductor support, none of which had any tests before: - insertDebuggerStatements skips js-slang parsing entirely when Conductor is enabled (and still works as before when it isn't) - evalEditorSaga concatenates prepend into the entrypoint for a single Conductor call, and leaves the entrypoint untouched when there's no prepend - runTestCaseConductor runs prepend+code+postpend+testcase as one file, grades by the last printed line rather than a returned value, and reports failure without crashing when the run errors Exports runTestCaseConductor so it can be exercised directly instead of only through the full runTestCase/workspaceSaga dispatch chain.
…channel
Same class of bug as the result channel: BasicHostPlugin's error channel
subscription calls receiveError with errorMessage.error directly, with no
guarantee it's defined, and redux-saga's eventChannel rejects undefined
outright ("Saga or channel was provided with an undefined action"). This
is unpatched in the conductor package's current upstream source too, not
just an older version. Normalise to null at the point it enters the
channel, same fix as already applied to the result channel.
Akshay-2007-1
force-pushed
the
fix/conductor-prepend-concat
branch
from
2fc7c34 to
b8e0e22
Compare
…st output entry Fixes a real issue flagged by review: consoleLogs isn't guaranteed to be on the absolute last entry of state.workspaces[loc].output. stdout and result messages travel on separate Conductor channels with no guaranteed relative ordering, and output is deliberately never cleared between testcase runs, so a trailing 'result' entry can legitimately have no consoleLogs of its own even though the testcase did print something. Search backward for the last entry that actually carries console logs instead of assuming it's the last entry outright.
1 participant
Summary
Conductor-enabled languages (e.g. Python via py-slang) work in Playground today, but the same code paths break in assessment workspaces (missions/quests/paths), because several parts of the assessment eval/grading pipeline assume js-slang's execution model: a single, persistent, mutable
Contextshared across separate eval calls. Conductor instead runs every call in a fresh, isolated, one-shot worker with no shared state between calls. This PR reconciles that mismatch for the "Run" and testcase-grading paths.evalEditor.ts/insertDebuggerStatements.ts: prepend is concatenated into the entrypoint for a single Conductor run instead of evaluated as a separate silent call (which never shared state with the student's own run); a leftover js-slang debugger-statement pre-parse (keyed off a legacychapterfield assessment questions are forced to carry regardless of actual language) is skipped when Conductor is active.runTestCase.ts: testcase grading concatenates prepend + student code + postpend + the testcase expression into a single Conductor run, and grades by comparing printed output rather than a returned value — Python doesn't have Source/JS's REPL-style "value of the last expression" semantics at the script level.BrowserHostPlugin.ts: fixes a crash where aNone/undefined-shaped Conductor result broke a redux-saga channel that explicitly disallowsundefinedas an action payload.Status
This is a work in progress, opened early for visibility/CI feedback. Known gaps not addressed here:
WorkspaceSaga.test.tssuite passes unmodified, confirming no regression to the non-Conductor path).Test plan
WorkspaceSaga.test.tsand fullcommons/sagas/suite pass unmodifiedtsc --noEmit) and lint clean on all touched files