Integrate Data Representation Tools into Multimodal tabs component#227
Integrate Data Representation Tools into Multimodal tabs component#227
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request integrates JetBrains Data Representation Tools (DRT) into the multimodal tabs component, replacing custom chart/table rendering with standardized DRT components for both the Jupyter widget and browser HTML viewer.
Changes:
- Replaced HTML-based DataFrame rendering with CSV-based data transfer to support DRT components
- Integrated
@jetbrains/drtlibrary (v0.0.4) for visualization and table rendering usingVisualizationToolandTableTool - Refactored component architecture to use data services and status management patterns compatible with DRT
Reviewed changes
Copilot reviewed 22 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| databao/multimodal/utils.py | Replaced dataframe_to_html with dataframe_to_csv for CSV-based data transfer |
| databao/multimodal/jupyter_widget.py | Updated widget to use CSV data format and removed spec_add_data dependency |
| databao/multimodal/html_viewer.py | Modified HTML viewer to send CSV data alongside spec configuration |
| client/pnpm-workspace.yaml | Added @jetbrains/drt v0.0.4 to catalog |
| client/pnpm-lock.yaml | Updated lock file with DRT and removed Vega dependencies |
| client/packages/multimodal-tabs/src/types.ts | Added Status type definition |
| client/packages/multimodal-tabs/src/hooks/useDataService.ts | Created hook for initializing DRT data services from CSV |
| client/packages/multimodal-tabs/src/components/VegaChart/VegaChart.tsx | Refactored to use DRT VisualizationTool component |
| client/packages/multimodal-tabs/src/components/DataframeTable/DataframeTable.tsx | Refactored to use DRT TableTool component |
| client/packages/multimodal-tabs/src/components/StatusRenderer/StatusRenderer.tsx | Updated to use getStatus callback pattern |
| client/packages/multimodal-tabs/src/components/Tabs/Tabs.tsx | Added forceMount to prevent tab content from unmounting |
| client/packages/multimodal-tabs/package.json | Updated dependencies to include DRT and moved React to devDependencies |
| client/apps/multimodal-jupyter/src/App.tsx | Updated to use new component APIs and added Sprite for DRT icons |
| client/apps/multimodal-html/src/App.tsx | Updated to use new component APIs and DRT integration |
Files not reviewed (1)
- client/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
client/packages/multimodal-tabs/src/components/StatusRenderer/StatusRenderer.tsx:38
- The
getStatusfunction is called multiple times (lines 25, 34, 38) within the same render. Since status calculation might involve some computation, calling it multiple times is inefficient. Consider callinggetStatus()once at the beginning and storing the result in a variable to avoid redundant computation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
943234c to
dab3b99
Compare
| @@ -49,12 +48,13 @@ class MultimodalWidget(anywidget.AnyWidget): | |||
|
|
|||
| spec = traitlets.Dict(default_value=None, allow_none=True).tag(sync=True) | |||
| spec_status = traitlets.Enum(values=LOADING_STATUS_VALUES, default_value="initial").tag(sync=True) | |||
| spec_csv_data = traitlets.Unicode("").tag(sync=True) | |||
|
|
|||
| text = traitlets.Unicode("").tag(sync=True) | |||
| text_status = traitlets.Enum(values=LOADING_STATUS_VALUES, default_value="initial").tag(sync=True) | |||
|
|
|||
There was a problem hiding this comment.
If I understood right, dataframe_csv_content and spec_csv_data are sent to frontend on every change.
So I would make sure that too large dataframes doesn't get synced, before the widget sees real data, just to avoid potential freezes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 28 changed files in this pull request and generated 9 comments.
Files not reviewed (1)
- client/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
databao/multimodal/html_viewer.py:221
data_jsonis interpolated directly into a<script>tag viatemplate.replace(...)without escaping</script>or other HTML-breaking sequences. Ifthread.text()or any dataframe cell contains</script>, it can terminate the script tag and enable script injection (XSS) in the HTML viewer. Serialize safely for embedding in HTML (e.g., replace<with\u003cor</with<\/, or embed JSON in atype="application/json"script tag and parse it).
data_object = {"text": thread.text(), "dataframeCsvContent": df_csv}
data_json = json.dumps(data_object)
template = TEMPLATE_PATH.read_text(encoding="utf-8")
html = template.replace(DATA_PLACEHOLDER, f"window.__DATA__ = {data_json};")
html_bytes = html.encode("utf-8")
client/packages/multimodal-tabs/src/components/StatusRenderer/StatusRenderer.tsx:39
getStatus()is invoked multiple times during a single render, which is unnecessary and could lead to inconsistent UI if a caller ever passes a non-pure function. Compute it once at the top of the component (e.g.,const status = getStatus()) and branch on that value.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
max_rows and max_columns
c15e90b to
231175c
Compare
and jupyter_widget to standardize CSV serialization. Defer generating the widget's dataframe CSV until the DATAFRAME modality is selected and adjust tests to reflect the lazy loading behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 28 changed files in this pull request and generated 7 comments.
Files not reviewed (1)
- client/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
client/packages/multimodal-tabs/src/components/StatusRenderer/StatusRenderer.tsx:39
getStatus()is invoked multiple times during render; if it ever becomes non-trivial or depends on changing state, this can lead to inconsistent branching within a single render. Compute it once at the top of the component (e.g.,const status = getStatus()) and use that value for the subsequent checks.
client/apps/multimodal-jupyter/src/App.tsx:20Statusis imported as a value from./types, but it’s a type-only export there. With Vite/esbuild this can leave a runtime import for a non-existent export and fail at runtime. Useimport type { Status, MultimodalTabType } ...(and keep value imports for the runtime constants/functions).
import {
isMultimodalTabType,
MULTIMODAL_TABS,
MultimodalTabType,
Status,
} from "./types";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Integrates JetBrains Data Representation Tools (DRT) components into the multimodal tabs UI for both the Jupyter widget and the browser HTML viewer, replacing custom chart/table rendering with
VisualizationToolandTableToolfrom@jetbrains/drt.Screen.Recording.2026-02-10.at.15.27.16.mov