Skip to content

feat: frame visualization tool results with Shiny Chat - #274

Merged
cpsievert merged 19 commits into
fix/e2e-shiny-app-subprocessfrom
fix/visualize-tool-borders
Aug 27, 2026
Merged

feat: frame visualization tool results with Shiny Chat#274
cpsievert merged 19 commits into
fix/e2e-shiny-app-subprocessfrom
fix/visualize-tool-borders

Conversation

@cpsievert

@cpsievert cpsievert commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Why this matters

Visualization tool results now use Shiny Chat's native framed presentation, so QueryChat no longer reaches into its condensed tool-display structure to create the surrounding frame.

Builds on posit-dev/shinychat#331 (merged; Python and R dependencies now track shinychat main).

What changes

  • Request presentation="framed" for visualization results in Python and R.
  • Remove QueryChat's Shiny Chat frame-selector overrides while keeping visualization-specific sizing and footer styling.
  • Refresh served CSS copies and update focused tests.
  • Update viz Playwright suites for the framed DOM (.shiny-tool-card inside .shiny-chat-tool-group, not .shiny-tool-result), and keep the framed viz header/footer undimmed and compact against shinychat main's framed-group CSS.

Verification

  • uv run pytest pkg-py/tests/test_viz_footer.py -q
  • uv run pytest pkg-py/tests/playwright/test_10_viz_inline.py pkg-py/tests/playwright/test_11_viz_footer.py pkg-py/tests/playwright/test_12_viz_bookmark.py — 22 passed
  • Rscript -e ...test-viz-tool.R

Stacked on #279 (which now carries the viz Playwright DOM-selector updates for shinychat main; this PR keeps only the framed-presentation feature and its tests).

Comment thread pyproject.toml Outdated
@cpsievert

This comment was marked as resolved.

@cpsievert
cpsievert marked this pull request as ready for review August 25, 2026 21:50
cpsievert added a commit that referenced this pull request Aug 26, 2026
shinychat main (#283) renders routed tool results as .shiny-tool-card
inside .shiny-chat-tool-group; the .shiny-tool-result wrapper is now
fallback-only. Update the viz Playwright suites to match.

Moved from #274, where these changes were mixed with the framed
presentation feature.
@cpsievert
cpsievert force-pushed the fix/visualize-tool-borders branch from 2fe9c58 to c0acec9 Compare August 26, 2026 20:57
@cpsievert
cpsievert changed the base branch from main to fix/e2e-shiny-app-subprocess August 26, 2026 20:57
cpsievert and others added 17 commits August 26, 2026 15:59
The previous test read R/querychat_viz.R from the package source tree,
which does not exist when tests run under R CMD check. Exercise the
visualize tool with mocked ggsql/session and assert on
result@extra instead.
shinychat renamed the tool-result display field presentation -> open_style
(with values minimal/framed); request the framed open style for
visualization results.
- Wait for .shiny-tool-card:has(.querychat-viz-container); framed results
  no longer render a .shiny-tool-result element
- Select the viz tool group structurally, not by LLM-chosen chart title
- Re-resolve the group after collapse (shinychat unmounts the card)
- Undim the framed viz group header and restore compact footer padding
  against shinychat main's framed-group CSS
ToolResultDisplay on shinychat main gained a markdown field defaulting
to None, so the hasattr() check in format_tool_result() started
returning None for every tool result, crashing format_chunk() with a
TypeError on every chat turn in Streamlit/Gradio/Dash.
@cpsievert
cpsievert force-pushed the fix/visualize-tool-borders branch from c0acec9 to 5cfe70e Compare August 26, 2026 20:59
These were extracted failing-test snippets from a local run, committed
by mistake in 7916959.
Comment thread pkg-py/examples/10-viz-app.py Outdated
@cpsievert
cpsievert merged commit a74c959 into main Aug 27, 2026
18 checks passed
@cpsievert
cpsievert deleted the fix/visualize-tool-borders branch August 27, 2026 00:06
cpsievert added a commit that referenced this pull request Aug 27, 2026
…ssue) (#279)

* build: install shinychat from git main

Demonstrates that shinychat main alone breaks the Shiny sidebar E2E
tests (test_03_sidebar_apps.py title tests) due to a timing regression
introduced in posit-dev/shinychat#280 interacting with the in-process
app servers used by the Playwright fixtures.

* test(e2e): run Shiny apps as subprocesses via shiny.pytest.create_app_fixture

The threaded in-process uvicorn fixtures shared Shiny's process-global,
loop-bound reactive lock across apps. When two apps on different event
loops contended for the lock, the session's message handler crashed
with 'bound to a different event loop' and the chat message was dropped,
hanging the sidebar title tests. shinychat main (posit-dev/shinychat#280)
widened that race window enough to fail CI reliably.

create_app_fixture runs each app in its own subprocess (as Shiny's own
test suite does), giving each app its own loop and lock. This matches
how the Streamlit fixtures already work.

* test(e2e): update cancel button selectors for shinychat #350

shinychat main (176a8fc) replaced the conditional send/cancel/spinner
button classes with a single button using data-state. The
.shiny-chat-btn-cancel class no longer exists.

* test(e2e): update viz test selectors for shinychat main's tool DOM

shinychat main (#283) renders routed tool results as .shiny-tool-card
inside .shiny-chat-tool-group; the .shiny-tool-result wrapper is now
fallback-only. Update the viz Playwright suites to match.

Moved from #274, where these changes were mixed with the framed
presentation feature.

* test(e2e): fix test_13 to use ShinyAppProc.url

app_01_hello is now a create_app_fixture (ShinyAppProc), so page.goto
needs .url. Missed in 89caac3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant