Add Data Visualizer plugin for Conductor - #71
Merged
martin-henz merged 12 commits intoJul 28, 2026
Conversation
Shared, language-agnostic protocol package for the data visualizer plugin pair (Conductor migration of draw_data), mirroring common-stepper's structure. Classification/cycle-detection logic is intentionally kept out of this package and out of per-language runners — it lives once in the host plugin (web-data-visualizer, added in a later PR), since pair/list/tree shapes are shared across languages unlike ASTs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JmkCBZfioraGKf1tR6K3n
BaseDataVisualizerRunnerPlugin<TValue>: the language-agnostic runner half of the data visualizer. Owns channel wiring, row accumulation across a run, and request/replay — mirrors BaseStepperRunnerPlugin's structure, but sendDrawing() fires once per draw_data(...) call (not once per run) since that's a builtin invoked from arbitrary points in student code, not something the evaluator drives top-down like stepping. The single abstract method (toNode) is a purely mechanical value-to-node conversion; RefIdAllocator gives it row-scoped identity tracking (WeakMap-by-reference, same technique as py-slang's PyCseMachinePlugin) without requiring any cycle-detection or classification logic in the adapter itself — that stays host-side, once, shared across languages. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JmkCBZfioraGKf1tR6K3n
classify.ts: cycle-detection and pair/list/tree classification for one SerializedDataVisualizerNode, ported from the old pre-Conductor frontend's DataVisualizer (hasCycle/isBinaryTree/isGeneralTree/initializeTreeMetaData), generalized from a fixed 2-element-pair representation to the N-ary "array" node (needed for native Python lists, which aren't fixed at length 2 the way a Source/SICP pair-chain is). Two deliberate behavior changes from the old version: a DFS-path-scoped cycle check (the old WeakSet-based one flagged any shared-but-acyclic DAG as cyclic) that also now reports isSharedStructure separately, and per-node classification instead of the old global-static first-argument-only behavior. isGeneralTreeNode is a clean structural re-derivation rather than a line-for-line port — the original overloaded one function for two different roles and leaned on several JS loose-equality quirks with no clean equivalent on a typed node representation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JmkCBZfioraGKf1tR6K3n
Ports the presentational layer from frontend/src/features/dataVisualizer/{tree,drawable}/ into this package: TreeNode variants, the three drawers (Original/BinaryTree/GeneralTree), and the Konva-based drawables, refactored off DataVisualizer.* global static state into explicit parameters (ClassificationResult/TreeLayout from classify.ts, threaded through Tree/drawer constructors instead of read from a singleton).
DataVisualizerHostPlugin.tsx is new — mirrors StepperHostPlugin.tsx: subscribes to the channel, holds rows as an external store, registers a tab. DataVisualizerView.tsx is new UI (the old rendering was spread across SideContentDataVisualizer.tsx too, not ported 1:1) with a view-mode toggle mirroring the old three-button UI as local component state.
Two gaps found while porting, fixed in classify.ts: a function value anywhere in the structure now forces isBinaryTree/isGeneralTree false (matching Tree.fromSourceStructure's old constructFunction side effect, previously missed), and TreeLayout now exposes nodeCountByDepth (needed by GeneralTreeDrawer, previously only had positions/colors). DataTreeNode no longer needs the old displaySpecialContent fallback-counter mechanism — every leaf/function node already carries a ready-to-show displayValue from the runner side, so there's no case left where the renderer has to invent a placeholder label.
Verified with a throwaway Playwright-driven harness (not part of this commit) rendering hand-built fixtures directly through Tree.fromSerializedNode(...).draw(viewMode) — simple pairs, a SICP list, a proper binary tree in both original and binary-tree view, a non-binary-tree correctly showing the warning, a general tree, a cyclic list (backward arrow), shared-but-acyclic structure, a native N-ary array, a function value, and string truncation all render correctly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011JmkCBZfioraGKf1tR6K3n
…sets RefId doc comment: this was left uncommitted since right after PR1 (the common-data-visualizer package commit) — the actual precision fix (refId is unique per-row, not per-message, since a fresh RefIdAllocator is created per sendDrawing() call) had already been applied and used correctly by PR2 onward, just never staged into a commit. .changeset/config.json: ignore @sourceacademy/web-data-visualizer for independent changeset versioning, matching web-stepper's existing treatment (both are bundled host plugins, not semver-published libraries). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011JmkCBZfioraGKf1tR6K3n
Bundling react-konva/konva pulled in a CJS dependency that branches on process.env.NODE_ENV at module-init time (ReferenceError: process is not defined - there's no process global in the browser), and separately gave react-konva its own private react-reconciler wired to a duplicate React instance, breaking JSX's internal owner dispatcher at runtime (TypeError: dispatcher.getOwner is not a function). Stop bundling konva/react-konva - the frontend's requireProvider already exposes both for its own dynamic modules, so mark them external and peer-provided, matching how react/@blueprintjs/core are already handled.
sendDrawing() shared one RefIdAllocator across every argument of a single draw_data(...) call. Since the host renders each argument as its own independent Konva Stage (a separate <canvas>), a value shared *across* arguments got serialized as a "ref" node in the second argument pointing at a tree that only exists in the first argument's separately-built tree - unresolvable, so the host silently fell back to rendering it as empty. Give each argument its own fresh allocator instead, matching the old pre-Conductor frontend's per-argument `visitedStructures` scoping exactly: a value shared across arguments is now fully re-serialized in each one (matching legacy), while sharing *within* one argument's own structure still collapses to a resolvable ref.
- Call-by-call pagination: show one draw_data(...) call at a time with Previous/Next + "Call X/Y", instead of stacking every call in one scrolling view. Matches the old SideContentDataVisualizer.tsx layout, including per-call "Structure i" cards for multi-argument calls and ArrowLeft/ArrowRight keyboard navigation. - Fix draw() being called with an arbitrary (10, 10) offset instead of Config.StrokeWidth / 2 (matching the old code's leftMargin/topMargin exactly) - the Stage's width/height was sized assuming content starts at x=0, so the wrong offset silently clipped ~10px off the right/bottom edge of every box. - Drop the Binary Tree / General Tree view modes - classify.ts, BinaryTreeDrawer.tsx, and GeneralTreeDrawer.tsx are now fully unused and deleted, along with the nodeColor/nodePos fields they were the only consumers of. Always draws the original box-and-pointer layout.
showTab() both reveals a tab AND focuses/selects it - correct for a plugin that decides mid-session it wants attention (e.g. sound the moment it starts playing), wrong for a plugin's constructor, which runs as soon as the language loads and would otherwise hijack whichever tab the student already had open (typically the welcome tab) on every single language switch. Add revealTab(id): visibility only, no selection change. Implemented here as the ITabService contract; frontend's TabService and DeferredConductorTabService need matching implementations (frontend PR incoming). DataVisualizerHostPlugin now reveals its tab instead of showing it, and fixes its tab icon to match the old tool's (eye-open, was diagram-tree).
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
added 3 commits
July 28, 2026 13:02
CI's --immutable install was rejecting the lockfile: package.json gained konva/react-konva peerDependencies when data-visualizer externalized them, but the lockfile was never regenerated to match.
These files predate the repo's prettier config being applied to them in CI (format:ci checks but doesn't autofix), so format:ci was flagging quote-style/wrapping diffs on every file I'd touched.
Neither method awaits anything (channel.send is fire-and-forget, same as the stepper runner plugin's send calls) so lint's require-await correctly flagged them. Callers already `await` both call sites, which is a harmless no-op on a non-Promise return.
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
2 participants
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.
Summary
Adds a Data Visualizer plugin for the Conductor framework, ported from the pre-Conductor
frontend'sdraw_datavisualizer, following the same host/runner plugin split already established by the Stepper migration. Language-agnostic: the cycle-detection, pair/list/tree classification, and box-and-pointer rendering live once, host-side; each language's runner adapter is purely mechanical (dispatch on the language's own value representation, emit a generic tagged node).@sourceacademy/common-data-visualizer: wire-format types shared between runner and host.@sourceacademy/runner-data-visualizer:BaseDataVisualizerRunnerPlugin, the per-language base class (see the py-slang PR opened alongside this one for the Python subclass).@sourceacademy/web-data-visualizer: the host plugin — tab registration, cycle/shared-structure handling, box-and-pointer drawing (ported fromfrontend/src/features/dataVisualizer/).@sourceacademy/common-tabs: addsITabService.revealTab()(see below).Bugs found and fixed during implementation
All found via live testing against a real evaluator (Python §4, through
./dev.sh --py), not just unit tests:ReferenceError: process is not defined, thenTypeError: dispatcher.getOwner is not a function) — bundlingreact-konva/konvapulled in a CJS dep branching onprocess.env.NODE_ENV, and gavereact-konvaits own privatereact-reconcilerwired to a duplicate React instance. Fixed by markingkonva/react-konvaexternal and peer-provided (the frontend'srequireProvideralready exposes both for its own dynamic modules), matching howreact/@blueprintjs/coreare already handled.sendDrawing()shared oneRefIdAllocatoracross every argument of adraw_data(...)call. Since the host renders each argument as an independent KonvaStage(a separate<canvas>— no way to draw an arrow across two of them), a value shared across arguments got serialized as an unresolvable "ref" node, silently rendering as empty. Fixed by giving each argument its own fresh allocator, matching the old pre-Conductor frontend's per-argument identity-map scoping exactly.draw()was called with an arbitrary(10, 10)offset instead ofConfig.StrokeWidth / 2(matching the old code'sleftMargin/topMarginexactly), desyncing the canvas's width/height sizing (assumes content starts atx=0) from where content actually started, clipping ~10px off the right/bottom edge of every box.showTab()both reveals a tab and focuses/selects it; calling it from the plugin's constructor (which runs as soon as the language loads) hijacked whichever tab the student had open, every time. No existing API could reveal a tab without also selecting it, so addedITabService.revealTab()(visibility only). Filed plugins#70 — Stepper has the identical bug and needs the same one-line fix.frontendPR for the tab-order fix, and this PR's "Rework UI to match legacy" commit for the rest.Related
@sourceacademy/conductorpackage (confirmed present through 0.8.2) that brokehostLoadPlugin()during this work.py-slang(Python adapter +draw_databuiltin),plugin-directory(plugin registration),frontend(tab-gating fix,revealTabimplementation, CSE-Machine-after-dynamic-tabs ordering).Test plan
yarn vitest run src/runner/data-visualizer src/common/data-visualizer src/common/tabs— 11/11 passing, including a new regression test for the cross-argument sharing fix.