Conversation
- Vite-built widget bundles ext-apps SDK + Vega-Lite + vega-interpreter into a single HTML file (no CDN deps, no eval, CSP-safe) - Widget receives tool result via MCP Apps protocol (ontoolresult) - Supports fullscreen toggle when host advertises it - create_chart returns vega_spec only (no PNG, widget renders interactively) - Register ui://sidemantic/chart resource with proper MCP Apps metadata - Remove _apps_enabled flag (MCP Apps works via protocol, not a runtime flag) - Remove vendored mcp-ui-server (widget is self-contained) - Remove --apps CLI flag dependency on mcp-ui-server import - structured_output=False on all tools (fixes Claude Desktop tool visibility) - Replace | None params with falsy defaults (removes anyOf from schemas)
- Move expand button to top-right inline with title ("Expand ↗")
- Fix fullscreen height to fill viewport minus 150px for prompt box
- Add padding in fullscreen mode (16px top, 24px sides)
- Add 5px height buffer to prevent inline scroll
- Dark mode support for expand button
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 606d450029
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const ro = new ResizeObserver(() => result.view.resize().run()); | ||
| ro.observe(container); |
There was a problem hiding this comment.
renderChart is invoked on every tool result and display-mode change, but each call creates a new ResizeObserver bound to that render's Vega view without disconnecting prior observers. After multiple chart updates in one session, resize events fan out to stale views, which causes avoidable CPU/memory growth and progressively slower interaction. Track the active observer/view and clean them up before creating a new embed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f894c5edc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .then((result) => { | ||
| activeView = result; | ||
| const ro = new ResizeObserver(() => result.view.resize().run()); | ||
| ro.observe(container); | ||
| activeObserver = ro; |
There was a problem hiding this comment.
The embed(...).then(...) callback always installs activeView/ResizeObserver even if another renderChart call has already started, so rapid updates (e.g., back-to-back tool results or display-mode changes) can let an older in-flight render reattach a stale view and extra observer after the newer render has cleared the container. Although cleanup now runs at render start, it does not protect against this out-of-order promise resolution, so stale observers and wrong chart state can still accumulate.
Useful? React with 👍 / 👎.
| container.innerHTML = '<div class="error">No chart data in tool result</div>'; | ||
| } |
There was a problem hiding this comment.
When no Vega spec is found, this branch replaces the DOM with an error message but never disconnects activeObserver or finalizes activeView. If the previous call rendered a chart and the next tool result is non-chart/error output, the old Vega view remains live in memory and can continue reacting to resize events indefinitely. Run the same cleanup used by renderChart() before setting fallback/error/loading markup.
Useful? React with 👍 / 👎.
- Generation counter ensures out-of-order embed promises discard stale results - cleanupChart() called before error/loading content to prevent leaked observers
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d4786aca1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| app.ontoolinput = () => { | ||
| cleanupChart(); | ||
| container.innerHTML = '<div class="loading">Running query...</div>'; |
There was a problem hiding this comment.
When a new tool call starts, this handler clears the DOM but does not advance renderGeneration. If a previous embed(...) promise resolves afterward, its .then(...) still passes the generation check and reattaches the stale chart/observer on top of the loading state. This race appears whenever a prior render is still in flight and ontoolinput fires before it settles, causing users to see outdated results during a new query.
Useful? React with 👍 / 👎.
| } else { | ||
| cleanupChart(); | ||
| container.innerHTML = '<div class="error">No chart data in tool result</div>'; | ||
| } |
There was a problem hiding this comment.
In the no-spec branch, the widget shows an error but leaves lastSpec unchanged. A later onhostcontextchanged (for example switching inline/fullscreen) will call renderChart(lastSpec) and resurrect the previous chart even though the latest tool result had no chart data. Resetting lastSpec here avoids replaying stale visualizations after failures/non-chart outputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d8fe76d1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| app.ontoolinput = () => { | ||
| cleanupChart(); | ||
| ++renderGeneration; | ||
| container.innerHTML = '<div class="loading">Running query...</div>'; |
There was a problem hiding this comment.
ontoolinput clears the chart and increments renderGeneration, but it leaves lastSpec populated; if a displayMode host-context update arrives before the next tool result, onhostcontextchanged will call renderChart(lastSpec) and resurrect the previous chart during the new query. This causes stale data to reappear and can overwrite the loading state, so the cached spec should be invalidated when a new run starts.
Useful? React with 👍 / 👎.
- New explore_metrics tool launches dashboard with metric series, totals, and dimension leaderboards via ui://sidemantic/explorer - App-only widget_query tool (visibility: ["app"]) handles refresh calls from the widget without LLM round-trips - WidgetModel adapter bridges anywidget's model interface to ext-apps SDK, translating set/save_changes into callServerTool requests - Data transported as base64 Arrow IPC for efficient typed transfer - Multi-entry Vite build for chart (960KB) and explorer (336KB) widgets - Add pyarrow to apps optional dependency
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e28033d1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # --- MCP Resource: Catalog Metadata --- | ||
| # --- Explorer state --- | ||
|
|
||
| _explorer_state: dict | None = None |
There was a problem hiding this comment.
The explorer backend keeps all widget context in a single module-global _explorer_state, which is overwritten on every explore_metrics call and then reused by widget_query. In HTTP/app mode, concurrent conversations or users can interleave requests, so one session can read another session’s model, filters, and date range, producing cross-session data leakage and incorrect query results. Store state by session/conversation/tool-call identifier instead of a singleton.
Useful? React with 👍 / 👎.
| safe = _escape_sql_literal(str(values[0])) | ||
| filter_exprs.append(f"{model_name}.{dim_key} = '{safe}'") |
There was a problem hiding this comment.
Dimension filter values are always coerced with str(...) and wrapped in single quotes, so booleans and numerics become string literals (for example true becomes 'True'). This breaks or misfilters on stricter dialects (e.g., boolean/text comparisons in Postgres) and is especially visible because boolean dimensions are included by default in explorer filters. Format literals by type (true/false, numeric unquoted) rather than always emitting quoted strings.
Useful? React with 👍 / 👎.
| end_str = str(date_range[1]) | ||
| start_literal = _format_date_literal(start_str) | ||
| end_literal = _format_date_literal(end_str) | ||
| filter_exprs.append(f"{model_name}.{time_dim} >= {start_literal} AND {model_name}.{time_dim} <= {end_literal}") |
There was a problem hiding this comment.
The time-range predicate uses <= end_literal even when the end input is a date-only string, which becomes CAST('YYYY-MM-DD' AS DATE). For timestamp time dimensions, that comparison includes only midnight and excludes the rest of the end date, causing undercounted metrics/totals for common date-range inputs. Use an exclusive upper bound at next day (< next_day) for date-only end values.
Useful? React with 👍 / 👎.
| // Extract data from tool result | ||
| const data = this.extractData(result); | ||
| if (data) { | ||
| this.applyData(data); |
There was a problem hiding this comment.
After awaiting callServerTool, the handler unconditionally applies returned data with no request-generation guard. If users trigger multiple refreshes quickly (e.g., changing time grain then filters), a slower earlier response can arrive last and overwrite the model with stale data that no longer matches current selections. Track a refresh token/counter and discard out-of-order responses before applyData.
Useful? React with 👍 / 👎.
1 participant
Summary
@modelcontextprotocol/ext-appsSDK