From 50f6e648864eab5940bab9701b3511444ee8148b Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 20 Aug 2026 10:13:19 -0500 Subject: [PATCH 01/19] feat(py): use native schema tool display --- pkg-py/src/querychat/tools.py | 117 ++++++++++++++++---------------- pkg-py/tests/test_tools.py | 121 ++++++++++++++++++++++++---------- 2 files changed, 146 insertions(+), 92 deletions(-) diff --git a/pkg-py/src/querychat/tools.py b/pkg-py/src/querychat/tools.py index 9ef9a81c..076c5f49 100644 --- a/pkg-py/src/querychat/tools.py +++ b/pkg-py/src/querychat/tools.py @@ -1,17 +1,14 @@ from __future__ import annotations import html -import json from collections.abc import Callable from typing import TYPE_CHECKING, Any, Protocol, TypedDict, runtime_checkable -from chatlas import ContentToolRequest, ContentToolResult, Tool -from htmltools import HTMLDependency, TagList, tags +from chatlas import ContentToolResult, Tool +from htmltools import Tag, tags from pydantic import Field -from shinychat import message_content_chunk -from shinychat.types import ChatMessage, ToolResultDisplay +from shinychat.types import ToolResultDisplay -from .__version import __version__ from ._datasource import ColumnMeta, format_schema from ._icons import bs_icon from ._utils import ( @@ -47,54 +44,6 @@ class GetSchemaResult(ContentToolResult): columns: list[ColumnMeta] = Field(default_factory=list) -def _col_to_dict(col: ColumnMeta) -> dict[str, Any]: - return { - "name": col.name, - "sql_type": col.sql_type, - "units": col.units, - "description": col.description, - "min_val": str(col.min_val) if col.min_val is not None else None, - "max_val": str(col.max_val) if col.max_val is not None else None, - "categories": col.categories, - "constraints": col.constraints, - } - - -_orig_request_handler = message_content_chunk.dispatch(ContentToolRequest) - - -@message_content_chunk.register -def _(request: ContentToolRequest) -> ChatMessage: - if request.name == "querychat_get_schema": - return ChatMessage(content="") - return _orig_request_handler(request) - - -@message_content_chunk.register -def _(message: GetSchemaResult) -> ChatMessage: - columns_json = json.dumps([_col_to_dict(c) for c in message.columns]) - content = TagList( - tags.span( - class_="qc-schema-collector", - data_table=message.table_name, - data_schema=str(message.value), - data_schema_json=columns_json, - style="display:none", - ), - _schema_dep(), - ) - return ChatMessage(content=content) - - -def _schema_dep() -> HTMLDependency: - return HTMLDependency( - "querychat-schema-display", - __version__, - source={"package": "querychat", "subdir": "static"}, - script=[{"src": "js/schema-display.js"}], - ) - - def _get_schema_impl( data_dicts: list[DataDict], executor: QueryExecutor, @@ -115,7 +64,19 @@ def get_schema(table_name: str) -> ContentToolResult: schema_text = format_schema(table_name, columns) return GetSchemaResult( - value=schema_text, table_name=table_name, columns=columns + value=schema_text, + table_name=table_name, + columns=columns, + extra={ + "display": ToolResultDisplay( + label=table_name, + value_preview=f"{len(columns)} column" + f"{'' if len(columns) == 1 else 's'}", + html=_schema_table(columns), + show_request=False, + open=False, + ) + }, ) return get_schema @@ -156,7 +117,51 @@ def tool_get_schema( return Tool.from_func( impl, name="querychat_get_schema", - annotations={"title": "Get Schema"}, + annotations={"title": "Fetch schemas"}, + ) + + +def _schema_table(columns: list[ColumnMeta]) -> Tag: + headers = ("Column", "Type", "Description", "Constraints", "Range / Values") + rows: list[Tag] = [] + + for column in columns: + type_cell = tags.span(column.sql_type) + if column.units: + type_cell.append( + " ", + tags.span(column.units, class_="text-body-secondary"), + ) + + range_values = "" + if ( + column.kind in ("numeric", "date") + and column.min_val is not None + and column.max_val is not None + ): + range_values = f"{column.min_val} to {column.max_val}" + elif column.categories: + range_values = ", ".join(f"'{value}'" for value in column.categories) + + rows.append( + tags.tr( + tags.th(column.name, scope="row"), + tags.td(type_cell), + tags.td(column.description or ""), + tags.td(", ".join(column.constraints)), + tags.td(range_values), + ) + ) + + return tags.div( + tags.table( + tags.thead( + tags.tr(*(tags.th(header, scope="col") for header in headers)) + ), + tags.tbody(*rows), + class_="table table-sm mb-0", + ), + class_="table-responsive", ) diff --git a/pkg-py/tests/test_tools.py b/pkg-py/tests/test_tools.py index e7d63147..a5c5638c 100644 --- a/pkg-py/tests/test_tools.py +++ b/pkg-py/tests/test_tools.py @@ -1,15 +1,14 @@ """Tests for tool functions and utilities.""" -import html as html_module import warnings import narwhals.stable.v1 as nw import pandas as pd import polars as pl import pytest -from htmltools import TagList +from htmltools import Tag from querychat._data_dict import ColumnRange, ColumnSpec, DataDict, TableSpec -from querychat._datasource import DataFrameSource +from querychat._datasource import ColumnMeta, DataFrameSource from querychat._query_executor import DataSourceExecutor from querychat._utils import querychat_tool_starts_open from querychat.tools import ( @@ -17,9 +16,9 @@ UpdateDashboardData, _get_schema_impl, _query_impl, + tool_get_schema, tool_reset_dashboard, ) -from shinychat import message_content_chunk @pytest.fixture @@ -191,20 +190,60 @@ def _make_executor_and_table( return executor, [table_name] -def test_get_schema_impl_with_data_dict() -> None: +def test_get_schema_result_preserves_metadata_for_model_and_display() -> None: dd = DataDict( tables={ "orders": TableSpec( - columns=[ColumnSpec(name="amount", range=ColumnRange(min=0, max=100))] + columns=[ + ColumnSpec( + name="amount", + description="Gross & tax", + units="USD", + constraints=[">= 0", "required"], + range=ColumnRange(min=0, max=100), + ), + ColumnSpec( + name="status", + values=["pending", "shipped & paid"], + ), + ] ) }, ) - df = pl.DataFrame({"amount": [10, 20]}) + df = pl.DataFrame( + { + "amount": [10, 20], + "status": ["pending", "shipped & paid"], + } + ) executor, table_names = _make_executor_and_table(df, "orders") fn = _get_schema_impl([dd], executor, table_names, categorical_threshold=10) + result = fn("orders") - assert "amount" in str(result.value) - assert "Range: 0 to 100" in str(result.value) + + assert isinstance(result, GetSchemaResult) + assert result.value == ( + "Table: orders\n" + "Columns:\n" + "- amount (INTEGER) [USD]\n" + " Description: Gross & tax\n" + " Constraints: >= 0, required\n" + " Range: 0 to 100\n" + "- status (TEXT)\n" + " Categorical values: 'pending', 'shipped & paid'" + ) + assert all(isinstance(column, ColumnMeta) for column in result.columns) + assert [column.name for column in result.columns] == ["amount", "status"] + + assert result.extra is not None + display = result.extra["display"] + assert isinstance(display.html, Tag) + rendered = display.html.render()["html"] + assert "Gross <amount> & tax" in rendered + assert 'USD' in rendered + assert ">= 0, required" in rendered + assert "0 to 100" in rendered + assert "'pending', 'shipped & paid'" in rendered def test_get_schema_impl_without_data_dict() -> None: @@ -224,31 +263,41 @@ def test_get_schema_impl_unknown_table_returns_error() -> None: assert "nonexistent" in str(result.error) -def test_get_schema_result_sentinel_has_data_attributes(): - result = GetSchemaResult( - value="Table: orders\nColumns:\n- id (INTEGER)", - table_name="orders", +def test_get_schema_tool_uses_native_display() -> None: + df = pl.DataFrame( + { + "order_id": [1, 2], + "status": ["pending", "shipped"], + } ) - msg = message_content_chunk(result) - # msg.content is a pre-rendered HTML string; wrap in TagList to render again - rendered = TagList(msg.content).render() - html = rendered["html"] - assert "qc-schema-collector" in html - assert 'data-table="orders"' in html - assert "display:none" in html - - -def test_get_schema_result_sentinel_embeds_schema(): - schema = "Table: orders\nColumns:\n- id (INTEGER)" - result = GetSchemaResult(value=schema, table_name="orders") - msg = message_content_chunk(result) - # ChatMessage pre-renders TagList to a string; unescape HTML entities to check schema - assert schema in html_module.unescape(msg.content) - - -def test_get_schema_result_includes_js_dependency(): - result = GetSchemaResult(value="Table: t\nColumns:\n- x (TEXT)", table_name="t") - msg = message_content_chunk(result) - # ChatMessage extracts HTMLDependency objects into html_deps - dep_names = [d.name for d in msg.html_deps] - assert "querychat-schema-display" in dep_names + executor, table_names = _make_executor_and_table(df, "orders") + tool = tool_get_schema([], executor, table_names, categorical_threshold=10) + + result = tool.func(table_name="orders") + display = result.extra["display"] + + assert tool.annotations is not None + assert tool.annotations["title"] == "Fetch schemas" + assert display.label == "orders" + assert display.value_preview == "2 columns" + assert display.show_request is False + assert display.open is False + assert isinstance(display.html, Tag) + + html = display.html.render()["html"] + assert '
' in html + assert '' in html + for header in ("Column", "Type", "Description", "Constraints", "Range / Values"): + assert f'' in html + assert '' in html + assert "INTEGER" in html + + +def test_get_schema_tool_uses_singular_column_preview() -> None: + df = pl.DataFrame({"order_id": [1]}) + executor, table_names = _make_executor_and_table(df, "orders") + tool = tool_get_schema([], executor, table_names, categorical_threshold=10) + + result = tool.func(table_name="orders") + + assert result.extra["display"].value_preview == "1 column" From 10d788642b291e038e712cb9ae905c4e784a66e3 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 20 Aug 2026 10:20:15 -0500 Subject: [PATCH 02/19] feat(r): use native schema tool display --- pkg-r/NAMESPACE | 1 - pkg-r/R/querychat_tools.R | 128 ++++++++++++++------ pkg-r/tests/testthat/test-querychat_tools.R | 95 ++++++++++----- 3 files changed, 150 insertions(+), 74 deletions(-) diff --git a/pkg-r/NAMESPACE b/pkg-r/NAMESPACE index 4361f351..e5f9a84c 100644 --- a/pkg-r/NAMESPACE +++ b/pkg-r/NAMESPACE @@ -20,4 +20,3 @@ import(rlang) importFrom(R6,R6Class) importFrom(bslib,sidebar) importFrom(lifecycle,deprecated) -importFrom(shinychat,contents_shinychat) diff --git a/pkg-r/R/querychat_tools.R b/pkg-r/R/querychat_tools.R index d4316c74..36e4ee2d 100644 --- a/pkg-r/R/querychat_tools.R +++ b/pkg-r/R/querychat_tools.R @@ -3,29 +3,10 @@ GetSchemaResult <- S7::new_class( "GetSchemaResult", parent = ellmer::ContentToolResult, properties = list( - table_name = S7::class_character, - columns_json = S7::new_property(S7::class_character, default = "") + table_name = S7::class_character ) ) -#' @importFrom shinychat contents_shinychat -rlang::on_load({ - S7::method(contents_shinychat, GetSchemaResult) <- get_schema_result_display - - orig_request_contents <- S7::method( - contents_shinychat, - ellmer::ContentToolRequest - ) - S7::method(contents_shinychat, ellmer::ContentToolRequest) <- function( - content - ) { - if (identical(content@name, "querychat_get_schema")) { - return(NULL) - } - orig_request_contents(content) - } -}) - tool_get_schema <- function( data_dicts, executor, @@ -52,11 +33,22 @@ tool_get_schema <- function( categorical_threshold, table_spec = table_spec ) - columns_json <- jsonlite::toJSON(schema_result$columns, auto_unbox = TRUE) + column_count <- length(schema_result$columns) GetSchemaResult( value = schema_result$text, table_name = table_name, - columns_json = as.character(columns_json) + extra = list( + display = shinychat::tool_result_display( + label = table_name, + value_preview = paste( + column_count, + if (column_count == 1) "column" else "columns" + ), + html = schema_table(schema_result$columns), + show_request = FALSE, + open = FALSE + ) + ) ) }, name = "querychat_get_schema", @@ -66,7 +58,7 @@ tool_get_schema <- function( "The name of the table to retrieve schema for." ) ), - annotations = ellmer::tool_annotations(title = "Get Schema") + annotations = ellmer::tool_annotations(title = "Fetch schemas") ) } @@ -364,25 +356,81 @@ querychat_tool_result <- function( ) } -schema_dep <- function() { - htmltools::htmlDependency( - name = "querychat-schema-display", - version = utils::packageVersion("querychat"), - package = "querychat", - src = "htmldep", - script = "schema-display.js" +schema_table <- function(columns) { + headers <- c( + "Column", + "Type", + "Description", + "Constraints", + "Range / Values" ) -} + rows <- lapply(columns, function(column) { + units <- schema_scalar_text(column$units) + type_cell <- htmltools::tagList( + htmltools::tags$span(schema_scalar_text(column$sql_type)), + if (nzchar(units)) { + htmltools::tagList( + " ", + htmltools::tags$span(units, class = "text-body-secondary") + ) + } + ) + + constraints <- vapply( + column$constraints, + schema_scalar_text, + character(1) + ) + constraints <- paste(constraints[nzchar(constraints)], collapse = ", ") -get_schema_result_display <- function(content) { - htmltools::tagList( - htmltools::tags$span( - class = "qc-schema-collector", - `data-table` = content@table_name, - `data-schema` = content@value, - `data-schema-json` = content@columns_json, - style = "display:none" + categories <- vapply( + column$categories, + schema_scalar_text, + character(1) + ) + categories <- categories[nzchar(categories)] + min_val <- schema_scalar_text(column$min_val) + max_val <- schema_scalar_text(column$max_val) + range_values <- if (nzchar(min_val) && nzchar(max_val)) { + paste(min_val, "to", max_val) + } else if (length(categories) > 0) { + paste0("'", categories, "'", collapse = ", ") + } else { + "" + } + + htmltools::tags$tr( + htmltools::tags$th( + htmltools::tags$code(schema_scalar_text(column$name)), + scope = "row" + ), + htmltools::tags$td(type_cell), + htmltools::tags$td(schema_scalar_text(column$description)), + htmltools::tags$td(constraints), + htmltools::tags$td(range_values) + ) + }) + + htmltools::tags$div( + htmltools::tags$table( + htmltools::tags$thead( + htmltools::tags$tr( + lapply(headers, function(header) { + htmltools::tags$th(header, scope = "col") + }) + ) + ), + htmltools::tags$tbody(rows), + class = "table table-sm mb-0" ), - schema_dep() + class = "table-responsive" ) } + +schema_scalar_text <- function(value) { + if (is.null(value) || length(value) == 0 || is.na(value[[1]])) { + return("") + } + + as.character(value[[1]]) +} diff --git a/pkg-r/tests/testthat/test-querychat_tools.R b/pkg-r/tests/testthat/test-querychat_tools.R index 96cb6bcc..d2e5d63f 100644 --- a/pkg-r/tests/testthat/test-querychat_tools.R +++ b/pkg-r/tests/testthat/test-querychat_tools.R @@ -514,38 +514,67 @@ describe("tool_update_dashboard_impl()", { }) }) -describe("get_schema_result_display()", { - it("returns a sentinel span with data-table attribute", { - result <- GetSchemaResult( - value = "Table: orders\nColumns:\n- id (INTEGER)", - table_name = "orders" - ) - html <- get_schema_result_display(result) - html_str <- as.character(html) - expect_true(grepl("qc-schema-collector", html_str)) - expect_true(grepl('data-table="orders"', html_str)) - expect_true( - grepl("display:none", html_str) || grepl("display: none", html_str) - ) - }) - - it("embeds schema text in data-schema attribute", { - schema <- "Table: orders\nColumns:\n- id (INTEGER)" - result <- GetSchemaResult(value = schema, table_name = "orders") - html <- get_schema_result_display(result) - html_str <- as.character(html) - expect_true(grepl("data-schema", html_str)) - expect_true(grepl("orders", html_str)) - }) - - it("includes querychat-schema-display HTML dependency", { - result <- GetSchemaResult( - value = "Table: t\nColumns:\n- x (TEXT)", - table_name = "t" - ) - html <- get_schema_result_display(result) - deps <- htmltools::findDependencies(html) - dep_names <- vapply(deps, function(d) d$name, character(1)) - expect_true("querychat-schema-display" %in% dep_names) +describe("tool_get_schema()", { + skip_if_no_dataframe_engine() + + it("returns schema text with a native rich display", { + df_source <- local_data_frame_source(new_test_df()) + executor <- local_executor(df_source) + expected <- executor$get_schema_result("test_table", 20) + tool <- tool_get_schema( + data_dicts = list(), + executor = executor, + table_names = "test_table", + categorical_threshold = 20 + ) + + result <- tool(table_name = "test_table") + display <- result@extra$display + html <- as.character(display$html) + + expect_equal(tool@annotations$title, "Fetch schemas") + expect_equal(result@value, expected$text) + expect_s3_class(display, "shinychat_tool_result_display") + expect_equal(display$label, "test_table") + expect_equal(display$value_preview, "3 columns") + expect_false(display$show_request) + expect_false(display$open) + expect_match(html, '
', fixed = TRUE) + expect_match(html, '
{header}order_id
', fixed = TRUE) + for (header in c( + "Column", + "Type", + "Description", + "Constraints", + "Range / Values" + )) { + expect_match( + html, + paste0('"), + fixed = TRUE + ) + } + expect_match( + html, + '' + ) + expect_match(html, "INTEGER", fixed = TRUE) + expect_match(html, "1 to 5", fixed = TRUE) + expect_match(html, "'A', 'B', 'C', 'D', 'E'", fixed = TRUE) + }) + + it("uses a singular preview for one column", { + df_source <- local_data_frame_source(data.frame(id = 1:3)) + executor <- local_executor(df_source) + tool <- tool_get_schema( + data_dicts = list(), + executor = executor, + table_names = "test_table", + categorical_threshold = 20 + ) + + result <- tool(table_name = "test_table") + + expect_equal(result@extra$display$value_preview, "1 column") }) }) From ba035fb4a5e4600fede5ac495e7db817f8d58a9e Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 20 Aug 2026 10:24:38 -0500 Subject: [PATCH 03/19] build: remove obsolete schema display bundle --- js/build.mjs | 8 - js/src/schema-display.js | 229 ------------------ .../src/querychat/static/js/schema-display.js | 154 ------------ pkg-r/inst/htmldep/schema-display.js | 154 ------------ 4 files changed, 545 deletions(-) delete mode 100644 js/src/schema-display.js delete mode 100644 pkg-py/src/querychat/static/js/schema-display.js delete mode 100644 pkg-r/inst/htmldep/schema-display.js diff --git a/js/build.mjs b/js/build.mjs index dda68cb8..6b95400d 100644 --- a/js/build.mjs +++ b/js/build.mjs @@ -24,14 +24,6 @@ const jsTargets = [ source: "src/viz.ts", output: "../pkg-r/inst/htmldep/viz.js", }, - { - source: "src/schema-display.js", - output: "../pkg-py/src/querychat/static/js/schema-display.js", - }, - { - source: "src/schema-display.js", - output: "../pkg-r/inst/htmldep/schema-display.js", - }, ]; const cssTargets = [ diff --git a/js/src/schema-display.js b/js/src/schema-display.js deleted file mode 100644 index 95966542..00000000 --- a/js/src/schema-display.js +++ /dev/null @@ -1,229 +0,0 @@ -let lastDisplay = null; -let lastDisplayTime = 0; -const BATCH_MS = 1000; -let activePanel = null; - -// -- Schema text parser -------------------------------------------------- - -function parseColumnsJson(json) { - return JSON.parse(json).map((col) => ({ - name: col.name, - type: col.sql_type, - units: col.units || null, - description: col.description || null, - constraints: col.constraints && col.constraints.length > 0 ? col.constraints.join(', ') : null, - range: - col.min_val != null && col.max_val != null ? `${col.min_val} to ${col.max_val}` : null, - categories: - col.categories && col.categories.length > 0 - ? col.categories.map((v) => `'${v}'`).join(', ') - : null, - })); -} - -// -- Table rendering ----------------------------------------------------- - -function esc(s) { - return String(s) - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"'); -} - -const TH = - 'padding:0.35em 0.75em;text-align:left;white-space:nowrap;font-weight:600;' + - 'border-bottom:2px solid var(--bs-border-color,#dee2e6);' + - 'background:var(--bs-tertiary-bg,#f8f9fa);' + - 'position:sticky;top:0;z-index:1;'; -const TD_MONO = - 'padding:0.3em 0.75em;white-space:nowrap;' + - 'font-family:var(--bs-font-monospace,monospace);font-size:0.875em;' + - 'border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));'; -const TD_WRAP = - 'padding:0.3em 0.75em;max-width:22em;overflow-wrap:break-word;' + - 'border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));'; -const TD_NOWRAP = - 'padding:0.3em 0.75em;white-space:nowrap;' + - 'border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));'; - -function renderTable(columns) { - const rows = columns - .map((col) => { - let typeCell = esc(col.type); - if (col.units) { - typeCell += ` [${esc(col.units)}]`; - } - const details = col.range - ? esc(col.range) - : col.categories - ? esc(col.categories) - : ''; - - return ( - `` + - `` + - `` + - `` + - `` + - `` + - `` - ); - }) - .join(''); - - return ( - `
', header, "\\s*id\\s*
${esc(col.name)}${typeCell}${col.description ? esc(col.description) : ''}${col.constraints ? esc(col.constraints) : ''}${details}
` + - `` + - `` + - `` + - `` + - `` + - `` + - `` + - `${rows}` + - `
ColumnTypeDescriptionConstraintsRange / Values
` - ); -} - -// -- Panel positioning & lifecycle --------------------------------------- - -const PANEL_STYLE = - 'position:fixed;z-index:9999;' + - 'background:var(--bs-body-bg,#fff);color:var(--bs-body-color,#212529);' + - 'border:1px solid var(--bs-border-color,#dee2e6);' + - 'border-radius:var(--bs-border-radius,0.375rem);' + - 'box-shadow:0 4px 16px rgba(0,0,0,.15);' + - 'overflow:auto;' + - 'max-height:min(420px,60vh);'; - -function positionPanel(btn, panel) { - const rect = btn.getBoundingClientRect(); - const vw = window.innerWidth; - const vh = window.innerHeight; - - const pw = Math.min(Math.max(360, vw * 0.55), vw - 16); - panel.style.width = `${pw}px`; - panel.style.left = `${Math.max(8, Math.min(rect.left, vw - pw - 8))}px`; - - // Prefer below; fall back to above if there's more room there - const spaceBelow = vh - rect.bottom - 8; - const spaceAbove = rect.top - 8; - if (spaceBelow >= 120 || spaceBelow >= spaceAbove) { - panel.style.top = `${rect.bottom + 4}px`; - } else { - const panelH = Math.min(420, spaceAbove); - panel.style.top = `${Math.max(8, rect.top - panelH - 4)}px`; - } -} - -function closePanel() { - if (activePanel) { - activePanel.panel.hidden = true; - activePanel.btn.setAttribute('aria-expanded', 'false'); - activePanel = null; - } -} - -document.addEventListener('click', closePanel); -document.addEventListener('keydown', (e) => { - if (e.key === 'Escape') closePanel(); -}); -window.addEventListener( - 'scroll', - (e) => { - if (activePanel && !activePanel.panel.contains(/** @type {Node} */ (e.target))) { - closePanel(); - } - }, - true, -); -window.addEventListener('resize', closePanel); - -// -- Button + panel construction ----------------------------------------- - -function createBtn(tableName, columnsJson) { - const columns = parseColumnsJson(columnsJson); - - const btn = document.createElement('button'); - btn.type = 'button'; - btn.style.cssText = - 'background:none;border:none;padding:0;color:inherit;' + - 'text-decoration:underline dotted;cursor:pointer;font-size:inherit;border-radius:2px;'; - btn.textContent = tableName; - btn.setAttribute('aria-label', `Show schema for ${tableName}`); - btn.setAttribute('aria-expanded', 'false'); - btn.setAttribute('aria-haspopup', 'dialog'); - - const panel = document.createElement('div'); - panel.setAttribute('role', 'dialog'); - panel.setAttribute('aria-label', `${tableName} schema`); - panel.style.cssText = PANEL_STYLE; - panel.hidden = true; - panel.innerHTML = renderTable(columns); - document.body.appendChild(panel); - - btn.addEventListener('click', (e) => { - e.stopPropagation(); - if (activePanel && activePanel.panel === panel) { - closePanel(); - return; - } - closePanel(); - positionPanel(btn, panel); - panel.hidden = false; - btn.setAttribute('aria-expanded', 'true'); - activePanel = { btn, panel }; - }); - - panel.addEventListener('click', (e) => e.stopPropagation()); - - return btn; -} - -// -- Focus ring for keyboard users (Bootstrap resets button outline) ----- - -const style = document.createElement('style'); -style.textContent = - '.qc-schema-display button:focus-visible{' + - 'outline:2px solid currentColor;outline-offset:2px;border-radius:2px}'; -document.head.appendChild(style); - -// -- MutationObserver --------------------------------------------------- - -function processCollector(sentinel) { - const now = Date.now(); - const tableName = sentinel.dataset.table; - const btn = createBtn(tableName, sentinel.dataset.schemaJson); - - if (lastDisplay && document.contains(lastDisplay) && now - lastDisplayTime < BATCH_MS) { - lastDisplay.appendChild(document.createTextNode(', ')); - lastDisplay.appendChild(btn); - sentinel.remove(); - } else { - const p = document.createElement('p'); - p.className = 'qc-schema-display'; - p.style.cssText = - 'color:var(--bs-secondary-color,#6c757d);font-size:0.875em;margin:0.1rem 0;'; - p.appendChild(document.createTextNode('🔍 Fetched schemas: ')); - p.appendChild(btn); - sentinel.replaceWith(p); - lastDisplay = p; - } - lastDisplayTime = now; -} - -new MutationObserver((mutations) => { - for (const { addedNodes } of mutations) { - for (const node of addedNodes) { - if (node.nodeType !== 1) continue; - if (/** @type {Element} */ (node).classList.contains('qc-schema-collector')) { - processCollector(/** @type {HTMLElement} */ (node)); - } else { - /** @type {Element} */ (node) - .querySelectorAll('.qc-schema-collector') - .forEach((el) => processCollector(/** @type {HTMLElement} */ (el))); - } - } - } -}).observe(document.body, { subtree: true, childList: true }); diff --git a/pkg-py/src/querychat/static/js/schema-display.js b/pkg-py/src/querychat/static/js/schema-display.js deleted file mode 100644 index b21376c6..00000000 --- a/pkg-py/src/querychat/static/js/schema-display.js +++ /dev/null @@ -1,154 +0,0 @@ -/* Generated file. Source: js/src/schema-display.js. Do not edit directly. */ - -"use strict"; -(() => { - // src/schema-display.js - var lastDisplay = null; - var lastDisplayTime = 0; - var BATCH_MS = 1e3; - var activePanel = null; - function parseColumnsJson(json) { - return JSON.parse(json).map((col) => ({ - name: col.name, - type: col.sql_type, - units: col.units || null, - description: col.description || null, - constraints: col.constraints && col.constraints.length > 0 ? col.constraints.join(", ") : null, - range: col.min_val != null && col.max_val != null ? `${col.min_val} to ${col.max_val}` : null, - categories: col.categories && col.categories.length > 0 ? col.categories.map((v) => `'${v}'`).join(", ") : null - })); - } - function esc(s) { - return String(s).replace(/&/g, "&").replace(//g, ">").replace(/"/g, """); - } - var TH = "padding:0.35em 0.75em;text-align:left;white-space:nowrap;font-weight:600;border-bottom:2px solid var(--bs-border-color,#dee2e6);background:var(--bs-tertiary-bg,#f8f9fa);position:sticky;top:0;z-index:1;"; - var TD_MONO = "padding:0.3em 0.75em;white-space:nowrap;font-family:var(--bs-font-monospace,monospace);font-size:0.875em;border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));"; - var TD_WRAP = "padding:0.3em 0.75em;max-width:22em;overflow-wrap:break-word;border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));"; - var TD_NOWRAP = "padding:0.3em 0.75em;white-space:nowrap;border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));"; - function renderTable(columns) { - const rows = columns.map((col) => { - let typeCell = esc(col.type); - if (col.units) { - typeCell += ` [${esc(col.units)}]`; - } - const details = col.range ? esc(col.range) : col.categories ? esc(col.categories) : ""; - return `${esc(col.name)}${typeCell}${col.description ? esc(col.description) : ""}${col.constraints ? esc(col.constraints) : ""}${details}`; - }).join(""); - return `${rows}
ColumnTypeDescriptionConstraintsRange / Values
`; - } - var PANEL_STYLE = "position:fixed;z-index:9999;background:var(--bs-body-bg,#fff);color:var(--bs-body-color,#212529);border:1px solid var(--bs-border-color,#dee2e6);border-radius:var(--bs-border-radius,0.375rem);box-shadow:0 4px 16px rgba(0,0,0,.15);overflow:auto;max-height:min(420px,60vh);"; - function positionPanel(btn, panel) { - const rect = btn.getBoundingClientRect(); - const vw = window.innerWidth; - const vh = window.innerHeight; - const pw = Math.min(Math.max(360, vw * 0.55), vw - 16); - panel.style.width = `${pw}px`; - panel.style.left = `${Math.max(8, Math.min(rect.left, vw - pw - 8))}px`; - const spaceBelow = vh - rect.bottom - 8; - const spaceAbove = rect.top - 8; - if (spaceBelow >= 120 || spaceBelow >= spaceAbove) { - panel.style.top = `${rect.bottom + 4}px`; - } else { - const panelH = Math.min(420, spaceAbove); - panel.style.top = `${Math.max(8, rect.top - panelH - 4)}px`; - } - } - function closePanel() { - if (activePanel) { - activePanel.panel.hidden = true; - activePanel.btn.setAttribute("aria-expanded", "false"); - activePanel = null; - } - } - document.addEventListener("click", closePanel); - document.addEventListener("keydown", (e) => { - if (e.key === "Escape") closePanel(); - }); - window.addEventListener( - "scroll", - (e) => { - if (activePanel && !activePanel.panel.contains( - /** @type {Node} */ - e.target - )) { - closePanel(); - } - }, - true - ); - window.addEventListener("resize", closePanel); - function createBtn(tableName, columnsJson) { - const columns = parseColumnsJson(columnsJson); - const btn = document.createElement("button"); - btn.type = "button"; - btn.style.cssText = "background:none;border:none;padding:0;color:inherit;text-decoration:underline dotted;cursor:pointer;font-size:inherit;border-radius:2px;"; - btn.textContent = tableName; - btn.setAttribute("aria-label", `Show schema for ${tableName}`); - btn.setAttribute("aria-expanded", "false"); - btn.setAttribute("aria-haspopup", "dialog"); - const panel = document.createElement("div"); - panel.setAttribute("role", "dialog"); - panel.setAttribute("aria-label", `${tableName} schema`); - panel.style.cssText = PANEL_STYLE; - panel.hidden = true; - panel.innerHTML = renderTable(columns); - document.body.appendChild(panel); - btn.addEventListener("click", (e) => { - e.stopPropagation(); - if (activePanel && activePanel.panel === panel) { - closePanel(); - return; - } - closePanel(); - positionPanel(btn, panel); - panel.hidden = false; - btn.setAttribute("aria-expanded", "true"); - activePanel = { btn, panel }; - }); - panel.addEventListener("click", (e) => e.stopPropagation()); - return btn; - } - var style = document.createElement("style"); - style.textContent = ".qc-schema-display button:focus-visible{outline:2px solid currentColor;outline-offset:2px;border-radius:2px}"; - document.head.appendChild(style); - function processCollector(sentinel) { - const now = Date.now(); - const tableName = sentinel.dataset.table; - const btn = createBtn(tableName, sentinel.dataset.schemaJson); - if (lastDisplay && document.contains(lastDisplay) && now - lastDisplayTime < BATCH_MS) { - lastDisplay.appendChild(document.createTextNode(", ")); - lastDisplay.appendChild(btn); - sentinel.remove(); - } else { - const p = document.createElement("p"); - p.className = "qc-schema-display"; - p.style.cssText = "color:var(--bs-secondary-color,#6c757d);font-size:0.875em;margin:0.1rem 0;"; - p.appendChild(document.createTextNode("\u{1F50D} Fetched schemas: ")); - p.appendChild(btn); - sentinel.replaceWith(p); - lastDisplay = p; - } - lastDisplayTime = now; - } - new MutationObserver((mutations) => { - for (const { addedNodes } of mutations) { - for (const node of addedNodes) { - if (node.nodeType !== 1) continue; - if ( - /** @type {Element} */ - node.classList.contains("qc-schema-collector") - ) { - processCollector( - /** @type {HTMLElement} */ - node - ); - } else { - node.querySelectorAll(".qc-schema-collector").forEach((el) => processCollector( - /** @type {HTMLElement} */ - el - )); - } - } - } - }).observe(document.body, { subtree: true, childList: true }); -})(); diff --git a/pkg-r/inst/htmldep/schema-display.js b/pkg-r/inst/htmldep/schema-display.js deleted file mode 100644 index b21376c6..00000000 --- a/pkg-r/inst/htmldep/schema-display.js +++ /dev/null @@ -1,154 +0,0 @@ -/* Generated file. Source: js/src/schema-display.js. Do not edit directly. */ - -"use strict"; -(() => { - // src/schema-display.js - var lastDisplay = null; - var lastDisplayTime = 0; - var BATCH_MS = 1e3; - var activePanel = null; - function parseColumnsJson(json) { - return JSON.parse(json).map((col) => ({ - name: col.name, - type: col.sql_type, - units: col.units || null, - description: col.description || null, - constraints: col.constraints && col.constraints.length > 0 ? col.constraints.join(", ") : null, - range: col.min_val != null && col.max_val != null ? `${col.min_val} to ${col.max_val}` : null, - categories: col.categories && col.categories.length > 0 ? col.categories.map((v) => `'${v}'`).join(", ") : null - })); - } - function esc(s) { - return String(s).replace(/&/g, "&").replace(//g, ">").replace(/"/g, """); - } - var TH = "padding:0.35em 0.75em;text-align:left;white-space:nowrap;font-weight:600;border-bottom:2px solid var(--bs-border-color,#dee2e6);background:var(--bs-tertiary-bg,#f8f9fa);position:sticky;top:0;z-index:1;"; - var TD_MONO = "padding:0.3em 0.75em;white-space:nowrap;font-family:var(--bs-font-monospace,monospace);font-size:0.875em;border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));"; - var TD_WRAP = "padding:0.3em 0.75em;max-width:22em;overflow-wrap:break-word;border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));"; - var TD_NOWRAP = "padding:0.3em 0.75em;white-space:nowrap;border-bottom:1px solid var(--bs-border-color-translucent,rgba(0,0,0,.08));"; - function renderTable(columns) { - const rows = columns.map((col) => { - let typeCell = esc(col.type); - if (col.units) { - typeCell += ` [${esc(col.units)}]`; - } - const details = col.range ? esc(col.range) : col.categories ? esc(col.categories) : ""; - return `${esc(col.name)}${typeCell}${col.description ? esc(col.description) : ""}${col.constraints ? esc(col.constraints) : ""}${details}`; - }).join(""); - return `${rows}
ColumnTypeDescriptionConstraintsRange / Values
`; - } - var PANEL_STYLE = "position:fixed;z-index:9999;background:var(--bs-body-bg,#fff);color:var(--bs-body-color,#212529);border:1px solid var(--bs-border-color,#dee2e6);border-radius:var(--bs-border-radius,0.375rem);box-shadow:0 4px 16px rgba(0,0,0,.15);overflow:auto;max-height:min(420px,60vh);"; - function positionPanel(btn, panel) { - const rect = btn.getBoundingClientRect(); - const vw = window.innerWidth; - const vh = window.innerHeight; - const pw = Math.min(Math.max(360, vw * 0.55), vw - 16); - panel.style.width = `${pw}px`; - panel.style.left = `${Math.max(8, Math.min(rect.left, vw - pw - 8))}px`; - const spaceBelow = vh - rect.bottom - 8; - const spaceAbove = rect.top - 8; - if (spaceBelow >= 120 || spaceBelow >= spaceAbove) { - panel.style.top = `${rect.bottom + 4}px`; - } else { - const panelH = Math.min(420, spaceAbove); - panel.style.top = `${Math.max(8, rect.top - panelH - 4)}px`; - } - } - function closePanel() { - if (activePanel) { - activePanel.panel.hidden = true; - activePanel.btn.setAttribute("aria-expanded", "false"); - activePanel = null; - } - } - document.addEventListener("click", closePanel); - document.addEventListener("keydown", (e) => { - if (e.key === "Escape") closePanel(); - }); - window.addEventListener( - "scroll", - (e) => { - if (activePanel && !activePanel.panel.contains( - /** @type {Node} */ - e.target - )) { - closePanel(); - } - }, - true - ); - window.addEventListener("resize", closePanel); - function createBtn(tableName, columnsJson) { - const columns = parseColumnsJson(columnsJson); - const btn = document.createElement("button"); - btn.type = "button"; - btn.style.cssText = "background:none;border:none;padding:0;color:inherit;text-decoration:underline dotted;cursor:pointer;font-size:inherit;border-radius:2px;"; - btn.textContent = tableName; - btn.setAttribute("aria-label", `Show schema for ${tableName}`); - btn.setAttribute("aria-expanded", "false"); - btn.setAttribute("aria-haspopup", "dialog"); - const panel = document.createElement("div"); - panel.setAttribute("role", "dialog"); - panel.setAttribute("aria-label", `${tableName} schema`); - panel.style.cssText = PANEL_STYLE; - panel.hidden = true; - panel.innerHTML = renderTable(columns); - document.body.appendChild(panel); - btn.addEventListener("click", (e) => { - e.stopPropagation(); - if (activePanel && activePanel.panel === panel) { - closePanel(); - return; - } - closePanel(); - positionPanel(btn, panel); - panel.hidden = false; - btn.setAttribute("aria-expanded", "true"); - activePanel = { btn, panel }; - }); - panel.addEventListener("click", (e) => e.stopPropagation()); - return btn; - } - var style = document.createElement("style"); - style.textContent = ".qc-schema-display button:focus-visible{outline:2px solid currentColor;outline-offset:2px;border-radius:2px}"; - document.head.appendChild(style); - function processCollector(sentinel) { - const now = Date.now(); - const tableName = sentinel.dataset.table; - const btn = createBtn(tableName, sentinel.dataset.schemaJson); - if (lastDisplay && document.contains(lastDisplay) && now - lastDisplayTime < BATCH_MS) { - lastDisplay.appendChild(document.createTextNode(", ")); - lastDisplay.appendChild(btn); - sentinel.remove(); - } else { - const p = document.createElement("p"); - p.className = "qc-schema-display"; - p.style.cssText = "color:var(--bs-secondary-color,#6c757d);font-size:0.875em;margin:0.1rem 0;"; - p.appendChild(document.createTextNode("\u{1F50D} Fetched schemas: ")); - p.appendChild(btn); - sentinel.replaceWith(p); - lastDisplay = p; - } - lastDisplayTime = now; - } - new MutationObserver((mutations) => { - for (const { addedNodes } of mutations) { - for (const node of addedNodes) { - if (node.nodeType !== 1) continue; - if ( - /** @type {Element} */ - node.classList.contains("qc-schema-collector") - ) { - processCollector( - /** @type {HTMLElement} */ - node - ); - } else { - node.querySelectorAll(".qc-schema-collector").forEach((el) => processCollector( - /** @type {HTMLElement} */ - el - )); - } - } - } - }).observe(document.body, { subtree: true, childList: true }); -})(); From 028b1c4004716495586be9ecf4377244d9c5cee1 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 20 Aug 2026 10:49:14 -0500 Subject: [PATCH 04/19] style: polish schema tool tables --- pkg-py/src/querychat/tools.py | 7 ++++--- pkg-py/tests/test_tools.py | 9 +++++++-- pkg-r/R/querychat_tools.R | 5 +++-- pkg-r/tests/testthat/test-querychat_tools.R | 7 ++++++- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/pkg-py/src/querychat/tools.py b/pkg-py/src/querychat/tools.py index 076c5f49..1a6170ee 100644 --- a/pkg-py/src/querychat/tools.py +++ b/pkg-py/src/querychat/tools.py @@ -145,7 +145,7 @@ def _schema_table(columns: list[ColumnMeta]) -> Tag: rows.append( tags.tr( - tags.th(column.name, scope="row"), + tags.th(tags.code(column.name), scope="row"), tags.td(type_cell), tags.td(column.description or ""), tags.td(", ".join(column.constraints)), @@ -156,10 +156,11 @@ def _schema_table(columns: list[ColumnMeta]) -> Tag: return tags.div( tags.table( tags.thead( - tags.tr(*(tags.th(header, scope="col") for header in headers)) + tags.tr(*(tags.th(header, scope="col") for header in headers)), + class_="table-light", ), tags.tbody(*rows), - class_="table table-sm mb-0", + class_="table table-sm table-hover align-middle mb-0", ), class_="table-responsive", ) diff --git a/pkg-py/tests/test_tools.py b/pkg-py/tests/test_tools.py index a5c5638c..914887db 100644 --- a/pkg-py/tests/test_tools.py +++ b/pkg-py/tests/test_tools.py @@ -1,5 +1,6 @@ """Tests for tool functions and utilities.""" +import re import warnings import narwhals.stable.v1 as nw @@ -286,10 +287,14 @@ def test_get_schema_tool_uses_native_display() -> None: html = display.html.render()["html"] assert '
' in html - assert '' in html + assert '
' in html + assert '' in html for header in ("Column", "Type", "Description", "Constraints", "Range / Values"): assert f'' in html - assert '' in html + assert re.search( + r'', + html, + ) assert "INTEGER" in html diff --git a/pkg-r/R/querychat_tools.R b/pkg-r/R/querychat_tools.R index 36e4ee2d..12600655 100644 --- a/pkg-r/R/querychat_tools.R +++ b/pkg-r/R/querychat_tools.R @@ -418,10 +418,11 @@ schema_table <- function(columns) { lapply(headers, function(header) { htmltools::tags$th(header, scope = "col") }) - ) + ), + class = "table-light" ), htmltools::tags$tbody(rows), - class = "table table-sm mb-0" + class = "table table-sm table-hover align-middle mb-0" ), class = "table-responsive" ) diff --git a/pkg-r/tests/testthat/test-querychat_tools.R b/pkg-r/tests/testthat/test-querychat_tools.R index d2e5d63f..3ac8bd49 100644 --- a/pkg-r/tests/testthat/test-querychat_tools.R +++ b/pkg-r/tests/testthat/test-querychat_tools.R @@ -540,7 +540,12 @@ describe("tool_get_schema()", { expect_false(display$show_request) expect_false(display$open) expect_match(html, '
', fixed = TRUE) - expect_match(html, '
{header}order_id\s*order_id\s*
', fixed = TRUE) + expect_match( + html, + '
', + fixed = TRUE + ) + expect_match(html, '', fixed = TRUE) for (header in c( "Column", "Type", From 1aec4a6dcd7e21ea68483f824b624c837bda8c21 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 20 Aug 2026 11:02:42 -0500 Subject: [PATCH 05/19] style: reorder schema table columns --- pkg-py/src/querychat/tools.py | 4 +- pkg-py/tests/test_tools.py | 27 ++++++++++- pkg-r/R/querychat_tools.R | 8 +-- pkg-r/tests/testthat/test-querychat_tools.R | 54 ++++++++++++++++----- 4 files changed, 73 insertions(+), 20 deletions(-) diff --git a/pkg-py/src/querychat/tools.py b/pkg-py/src/querychat/tools.py index 1a6170ee..2abb1cd7 100644 --- a/pkg-py/src/querychat/tools.py +++ b/pkg-py/src/querychat/tools.py @@ -122,7 +122,7 @@ def tool_get_schema( def _schema_table(columns: list[ColumnMeta]) -> Tag: - headers = ("Column", "Type", "Description", "Constraints", "Range / Values") + headers = ("Column", "Type", "Range / Values", "Description", "Constraints") rows: list[Tag] = [] for column in columns: @@ -147,9 +147,9 @@ def _schema_table(columns: list[ColumnMeta]) -> Tag: tags.tr( tags.th(tags.code(column.name), scope="row"), tags.td(type_cell), + tags.td(range_values), tags.td(column.description or ""), tags.td(", ".join(column.constraints)), - tags.td(range_values), ) ) diff --git a/pkg-py/tests/test_tools.py b/pkg-py/tests/test_tools.py index 914887db..0b0c64a4 100644 --- a/pkg-py/tests/test_tools.py +++ b/pkg-py/tests/test_tools.py @@ -191,7 +191,7 @@ def _make_executor_and_table( return executor, [table_name] -def test_get_schema_result_preserves_metadata_for_model_and_display() -> None: +def test_get_schema_tool_preserves_metadata_for_model_and_display() -> None: dd = DataDict( tables={ "orders": TableSpec( @@ -240,6 +240,29 @@ def test_get_schema_result_preserves_metadata_for_model_and_display() -> None: display = result.extra["display"] assert isinstance(display.html, Tag) rendered = display.html.render()["html"] + expected_headers = ( + "Column", + "Type", + "Range / Values", + "Description", + "Constraints", + ) + header_positions = [ + rendered.index(f'') + for header in expected_headers + ] + amount_cell_positions = [ + rendered.index(content) + for content in ( + "amount", + "INTEGER ", + "", + "", + "", + ) + ] + assert header_positions == sorted(header_positions) + assert amount_cell_positions == sorted(amount_cell_positions) assert "Gross <amount> & tax" in rendered assert 'USD' in rendered assert ">= 0, required" in rendered @@ -289,7 +312,7 @@ def test_get_schema_tool_uses_native_display() -> None: assert '
' in html assert '
{header}0 to 100Gross <amount> & tax>= 0, required
' in html assert '' in html - for header in ("Column", "Type", "Description", "Constraints", "Range / Values"): + for header in ("Column", "Type", "Range / Values", "Description", "Constraints"): assert f'' in html assert re.search( r'', diff --git a/pkg-r/R/querychat_tools.R b/pkg-r/R/querychat_tools.R index 12600655..ed59cd53 100644 --- a/pkg-r/R/querychat_tools.R +++ b/pkg-r/R/querychat_tools.R @@ -360,9 +360,9 @@ schema_table <- function(columns) { headers <- c( "Column", "Type", + "Range / Values", "Description", - "Constraints", - "Range / Values" + "Constraints" ) rows <- lapply(columns, function(column) { units <- schema_scalar_text(column$units) @@ -405,9 +405,9 @@ schema_table <- function(columns) { scope = "row" ), htmltools::tags$td(type_cell), + htmltools::tags$td(range_values), htmltools::tags$td(schema_scalar_text(column$description)), - htmltools::tags$td(constraints), - htmltools::tags$td(range_values) + htmltools::tags$td(constraints) ) }) diff --git a/pkg-r/tests/testthat/test-querychat_tools.R b/pkg-r/tests/testthat/test-querychat_tools.R index 3ac8bd49..46fbbc0f 100644 --- a/pkg-r/tests/testthat/test-querychat_tools.R +++ b/pkg-r/tests/testthat/test-querychat_tools.R @@ -520,9 +520,25 @@ describe("tool_get_schema()", { it("returns schema text with a native rich display", { df_source <- local_data_frame_source(new_test_df()) executor <- local_executor(df_source) - expected <- executor$get_schema_result("test_table", 20) + table_spec <- list( + columns = list( + list( + name = "id", + description = "Primary key", + units = "rows", + constraints = c("positive", "required"), + range = list(min = 1, max = 5) + ) + ) + ) + data_dicts <- list(list(tables = list(test_table = table_spec))) + expected <- executor$get_schema_result( + "test_table", + 20, + table_spec = table_spec + ) tool <- tool_get_schema( - data_dicts = list(), + data_dicts = data_dicts, executor = executor, table_names = "test_table", categorical_threshold = 20 @@ -546,19 +562,33 @@ describe("tool_get_schema()", { fixed = TRUE ) expect_match(html, '', fixed = TRUE) - for (header in c( + expected_headers <- c( "Column", "Type", + "Range / Values", "Description", - "Constraints", - "Range / Values" - )) { - expect_match( - html, - paste0('"), - fixed = TRUE - ) - } + "Constraints" + ) + header_positions <- vapply( + paste0('"), + function(header) regexpr(header, html, fixed = TRUE)[[1]], + integer(1) + ) + id_cell_positions <- vapply( + c( + "id", + "INTEGER", + "", + "", + "" + ), + function(content) regexpr(content, html, fixed = TRUE)[[1]], + integer(1) + ) + expect_true(all(header_positions > 0)) + expect_true(all(id_cell_positions > 0)) + expect_true(all(diff(header_positions) > 0)) + expect_true(all(diff(id_cell_positions) > 0)) expect_match( html, '' From c8970c0c3f92c8ef7c4602e5c655c6397dba4351 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 20 Aug 2026 19:26:04 -0500 Subject: [PATCH 06/19] feat: use framed Shiny Chat visualization results --- js/src/viz.css | 42 ++++++++--- pkg-py/examples/10-viz-app.py | 2 + pkg-py/src/querychat/_viz_tools.py | 3 +- pkg-py/src/querychat/static/css/viz.css | 43 ++++++++---- pkg-py/tests/playwright/test_11_viz_footer.py | 56 ++++++++++++++- pkg-py/tests/test_viz_footer.py | 69 +++++++++++++++++++ pkg-r/DESCRIPTION | 4 +- pkg-r/R/querychat_viz.R | 5 +- pkg-r/inst/htmldep/viz.css | 43 ++++++++---- pkg-r/tests/testthat/test-viz-tool.R | 25 +++++++ pyproject.toml | 4 +- 11 files changed, 253 insertions(+), 43 deletions(-) diff --git a/js/src/viz.css b/js/src/viz.css index 6c41e224..04e46886 100644 --- a/js/src/viz.css +++ b/js/src/viz.css @@ -35,8 +35,8 @@ display: inline-flex; align-items: center; gap: 4px; - padding: 2px 8px; - height: 28px; + padding: 1px 0.25rem; + height: 24px; border: none; border-radius: var(--bs-border-radius, 4px); background: transparent; @@ -53,24 +53,36 @@ } .querychat-query-chevron { - font-size: 0.625rem; - transition: transform 150ms; - display: inline-block; + display: grid; + place-items: center; + width: 0.9em; + height: 0.9em; + flex: none; + opacity: var(--_activity-opacity-muted, 0.65); + transform: rotate(-90deg); + transition: transform 300ms ease-in-out; +} + +.querychat-query-chevron svg { + display: block; + width: 100%; + height: 100%; } .querychat-query-chevron--expanded { - transform: rotate(90deg); + transform: rotate(0); } .querychat-icon { - width: 14px; - height: 14px; + width: 1em; + height: 1em; } .querychat-dropdown-chevron { - width: 12px; - height: 12px; + width: 0.9em; + height: 0.9em; margin-left: 2px; + opacity: var(--_activity-opacity-muted, 0.65); } .querychat-save-dropdown { @@ -116,7 +128,8 @@ display: none; position: relative; border-top: 1px solid var(--bs-border-color, #dee2e6); - margin: 8px -16px -8px; + margin: 0.25rem calc(-1 * var(--_querychat-footer-pad-x)) + calc(-1 * var(--_querychat-footer-pad-y)); } .querychat-query-section--visible { @@ -128,10 +141,17 @@ .shiny-tool-card:has(.querychat-viz-container) { max-height: 700px; overflow: hidden; + opacity: 1; } .shiny-tool-card:has(.querychat-viz-container) > .card-footer { + --_querychat-footer-pad-y: 0.125rem; + --_querychat-footer-pad-x: calc(var(--_row-pad, 0.4rem) - 0.25rem); + flex: 0 0 auto; + padding: var(--_querychat-footer-pad-y) var(--_querychat-footer-pad-x); + border-top: var(--bs-border-width, 1px) solid + var(--bs-border-color, #dee2e6); } .shiny-tool-card[fullscreen]:has(.querychat-viz-container) { diff --git a/pkg-py/examples/10-viz-app.py b/pkg-py/examples/10-viz-app.py index 857e8421..250c121b 100644 --- a/pkg-py/examples/10-viz-app.py +++ b/pkg-py/examples/10-viz-app.py @@ -17,3 +17,5 @@ qc.ui() ui.page_opts(fillable=True, title="QueryChat Visualization Demo") + +ui.input_dark_mode() \ No newline at end of file diff --git a/pkg-py/src/querychat/_viz_tools.py b/pkg-py/src/querychat/_viz_tools.py index 531d9087..468c17a1 100644 --- a/pkg-py/src/querychat/_viz_tools.py +++ b/pkg-py/src/querychat/_viz_tools.py @@ -141,6 +141,7 @@ def __init__( full_screen=True, icon=bs_icon("graph-up"), footer=footer, + presentation="framed", ), } @@ -319,7 +320,7 @@ def build_viz_footer( "data-querychat-action": "show-query", "data-target": query_section_id, }, - tags.span({"class": "querychat-query-chevron"}, "\u25b6"), + bs_icon("chevron-down", cls="querychat-query-chevron"), tags.span({"class": "querychat-query-label"}, "Show Query"), ), ), diff --git a/pkg-py/src/querychat/static/css/viz.css b/pkg-py/src/querychat/static/css/viz.css index bbf54e6e..04e46886 100644 --- a/pkg-py/src/querychat/static/css/viz.css +++ b/pkg-py/src/querychat/static/css/viz.css @@ -1,4 +1,3 @@ -/* Generated file. Source: js/src/viz.css. Do not edit directly. */ /* Hide Vega's built-in action dropdown (we have our own save button) */ .querychat-viz-container details:has(> .vega-actions) { display: none !important; @@ -36,8 +35,8 @@ display: inline-flex; align-items: center; gap: 4px; - padding: 2px 8px; - height: 28px; + padding: 1px 0.25rem; + height: 24px; border: none; border-radius: var(--bs-border-radius, 4px); background: transparent; @@ -54,24 +53,36 @@ } .querychat-query-chevron { - font-size: 0.625rem; - transition: transform 150ms; - display: inline-block; + display: grid; + place-items: center; + width: 0.9em; + height: 0.9em; + flex: none; + opacity: var(--_activity-opacity-muted, 0.65); + transform: rotate(-90deg); + transition: transform 300ms ease-in-out; +} + +.querychat-query-chevron svg { + display: block; + width: 100%; + height: 100%; } .querychat-query-chevron--expanded { - transform: rotate(90deg); + transform: rotate(0); } .querychat-icon { - width: 14px; - height: 14px; + width: 1em; + height: 1em; } .querychat-dropdown-chevron { - width: 12px; - height: 12px; + width: 0.9em; + height: 0.9em; margin-left: 2px; + opacity: var(--_activity-opacity-muted, 0.65); } .querychat-save-dropdown { @@ -117,7 +128,8 @@ display: none; position: relative; border-top: 1px solid var(--bs-border-color, #dee2e6); - margin: 8px -16px -8px; + margin: 0.25rem calc(-1 * var(--_querychat-footer-pad-x)) + calc(-1 * var(--_querychat-footer-pad-y)); } .querychat-query-section--visible { @@ -129,10 +141,17 @@ .shiny-tool-card:has(.querychat-viz-container) { max-height: 700px; overflow: hidden; + opacity: 1; } .shiny-tool-card:has(.querychat-viz-container) > .card-footer { + --_querychat-footer-pad-y: 0.125rem; + --_querychat-footer-pad-x: calc(var(--_row-pad, 0.4rem) - 0.25rem); + flex: 0 0 auto; + padding: var(--_querychat-footer-pad-y) var(--_querychat-footer-pad-x); + border-top: var(--bs-border-width, 1px) solid + var(--bs-border-color, #dee2e6); } .shiny-tool-card[fullscreen]:has(.querychat-viz-container) { diff --git a/pkg-py/tests/playwright/test_11_viz_footer.py b/pkg-py/tests/playwright/test_11_viz_footer.py index 408f7f8b..e8b29e71 100644 --- a/pkg-py/tests/playwright/test_11_viz_footer.py +++ b/pkg-py/tests/playwright/test_11_viz_footer.py @@ -9,6 +9,7 @@ from __future__ import annotations +import re import time from pathlib import Path from typing import TYPE_CHECKING @@ -118,6 +119,59 @@ def _send_viz_prompt( _wait_for_stable_position(page.locator(".querychat-show-query-btn")) +class TestVizFrame: + def test_expanded_visualization_has_complete_frame(self, page: Page) -> None: + group = page.locator(".shiny-chat-tool-group--single").filter( + has_text="Passengers by Class" + ) + card = page.locator(".shiny-tool-card:has(.querychat-viz-container)") + summary = group.locator(":scope > .shiny-chat-tool-group__row") + footer = card.locator(":scope > .card-footer") + + expect(card).to_have_css("opacity", "1") + expect(summary).to_have_css("opacity", "1") + for side in ("top", "right", "bottom", "left"): + expect(group).to_have_css(f"border-{side}-width", "1px") + expect(card).to_have_css(f"border-{side}-width", "0px") + expect(summary).to_have_css("border-bottom-width", "1px") + expect(summary).to_have_css("border-radius", "0px") + expect(footer).to_have_css("border-top-width", "1px") + expect(footer).to_have_css("padding-top", "2px") + expect(footer).to_have_css("padding-bottom", "2px") + + header_box = summary.bounding_box() + footer_box = footer.bounding_box() + header_glyph_box = summary.locator( + ".shiny-chat-tool-group__glyph" + ).bounding_box() + header_chevron_box = summary.locator( + ".shiny-chat-tool-group__chevron" + ).bounding_box() + show_chevron_box = footer.locator( + ".querychat-query-chevron" + ).bounding_box() + save_chevron_box = footer.locator( + ".querychat-dropdown-chevron" + ).bounding_box() + + assert header_box is not None + assert footer_box is not None + assert header_glyph_box is not None + assert header_chevron_box is not None + assert show_chevron_box is not None + assert save_chevron_box is not None + assert abs(header_box["height"] - footer_box["height"]) <= 1 + assert abs(header_glyph_box["x"] - show_chevron_box["x"]) <= 0.5 + assert abs( + (header_chevron_box["x"] + header_chevron_box["width"]) + - (save_chevron_box["x"] + save_chevron_box["width"]) + ) <= 0.5 + + summary.click() + for side in ("top", "right", "bottom", "left"): + expect(group).to_have_css(f"border-{side}-width", "0px") + + class TestShowQueryToggle: """Tests for the Show Query / Hide Query toggle button.""" @@ -152,7 +206,7 @@ def test_chevron_rotates_on_expand(self, page: Page) -> None: expect(chevron).not_to_have_class("querychat-query-chevron--expanded") btn.click() expect(chevron).to_have_class( - "querychat-query-chevron querychat-query-chevron--expanded" + re.compile(r"\bquerychat-query-chevron--expanded\b") ) def test_toggle_hides_section_again(self, page: Page) -> None: diff --git a/pkg-py/tests/test_viz_footer.py b/pkg-py/tests/test_viz_footer.py index 1943fcdb..bee4512d 100644 --- a/pkg-py/tests/test_viz_footer.py +++ b/pkg-py/tests/test_viz_footer.py @@ -6,6 +6,8 @@ shinychat renders this in the card footer area. """ +import re +from pathlib import Path from unittest.mock import MagicMock import narwhals.stable.v1 as nw @@ -14,6 +16,8 @@ from htmltools import TagList, tags from querychat._datasource import DataFrameSource +VIZ_CSS_PATH = Path(__file__).parents[2] / "js" / "src" / "viz.css" + @pytest.fixture def sample_df(): @@ -86,6 +90,21 @@ def test_cls_parameter_injects_class(self): html = str(bs_icon("download", cls="querychat-icon")) assert "querychat-icon" in html + def test_show_query_uses_shinychat_disclosure_chevron(self): + from querychat._viz_tools import build_viz_footer + + rendered = TagList( + build_viz_footer( + "SELECT * FROM test_data VISUALISE x, y DRAW point", + "Chart", + dom_widget_id="querychat_viz_raw", + ) + ).render()["html"] + + assert "querychat-query-chevron" in rendered + assert "bi-chevron-down" in rendered + assert "\u25b6" not in rendered + class TestVizPreloadMarkup: def test_preload_markup_has_no_inline_script(self): @@ -119,3 +138,53 @@ def test_build_viz_footer_uses_resolved_dom_widget_id(self): assert 'data-widget-id="module-querychat_viz_raw"' in rendered assert 'data-widget-id="querychat_viz_raw"' not in rendered + + +class TestVizFrameStyles: + def test_visualization_card_is_opaque_and_footer_is_compact(self): + css = VIZ_CSS_PATH.read_text(encoding="utf-8") + + card = re.search( + r"\.shiny-tool-card:has\(\.querychat-viz-container\)\s*" + r"\{(?P[^}]*)\}", + css, + ) + assert card is not None + assert "opacity: 1;" in card["declarations"] + + footer = re.search( + r"\.shiny-tool-card:has\(\.querychat-viz-container\)" + r"\s*> \.card-footer\s*\{(?P[^}]*)\}", + css, + ) + assert footer is not None + assert "--_querychat-footer-pad-y: 0.125rem;" in footer["declarations"] + assert ( + "--_querychat-footer-pad-x: " + "calc(var(--_row-pad, 0.4rem) - 0.25rem);" + in footer["declarations"] + ) + assert ( + "padding: var(--_querychat-footer-pad-y) " + "var(--_querychat-footer-pad-x);" + in footer["declarations"] + ) + + buttons = re.search( + r"\.querychat-show-query-btn,\s*" + r"\.querychat-save-btn\s*\{(?P[^}]*)\}", + css, + ) + assert buttons is not None + assert "height: 24px;" in buttons["declarations"] + assert "padding: 1px 0.25rem;" in buttons["declarations"] + + def test_visualization_requests_shinychat_framed_presentation(self): + source = ( + Path(__file__).parents[1] + / "src" + / "querychat" + / "_viz_tools.py" + ).read_text(encoding="utf-8") + + assert 'presentation="framed"' in source diff --git a/pkg-r/DESCRIPTION b/pkg-r/DESCRIPTION index ba35b545..6a0ab31d 100644 --- a/pkg-r/DESCRIPTION +++ b/pkg-r/DESCRIPTION @@ -59,8 +59,8 @@ Suggests: VignetteBuilder: knitr Remotes: - posit-dev/shinychat/pkg-r -Config/roxygen2/version: 8.1.0 + posit-dev/shinychat/pkg-r@feat/framed-tool-result-presentation +Config/roxygen2/version: 8.0.0 Config/testthat/edition: 3 Config/testthat/parallel: true Encoding: UTF-8 diff --git a/pkg-r/R/querychat_viz.R b/pkg-r/R/querychat_viz.R index c84d15bd..69f57c37 100644 --- a/pkg-r/R/querychat_viz.R +++ b/pkg-r/R/querychat_viz.R @@ -153,7 +153,8 @@ visualize_result <- function( open = querychat_tool_starts_open("visualize"), full_screen = TRUE, icon = viz_icon(), - footer = freeze_tags(footer) + footer = freeze_tags(footer), + presentation = "framed" ) ) @@ -242,7 +243,7 @@ build_viz_footer <- function( class = "querychat-show-query-btn", `data-querychat-action` = "show-query", `data-target` = query_section_id, - shiny::tags$span(class = "querychat-query-chevron", "\u25b6"), + bsicons::bs_icon("chevron-down", class = "querychat-query-chevron"), shiny::tags$span(class = "querychat-query-label", "Show Query") ) ), diff --git a/pkg-r/inst/htmldep/viz.css b/pkg-r/inst/htmldep/viz.css index bbf54e6e..04e46886 100644 --- a/pkg-r/inst/htmldep/viz.css +++ b/pkg-r/inst/htmldep/viz.css @@ -1,4 +1,3 @@ -/* Generated file. Source: js/src/viz.css. Do not edit directly. */ /* Hide Vega's built-in action dropdown (we have our own save button) */ .querychat-viz-container details:has(> .vega-actions) { display: none !important; @@ -36,8 +35,8 @@ display: inline-flex; align-items: center; gap: 4px; - padding: 2px 8px; - height: 28px; + padding: 1px 0.25rem; + height: 24px; border: none; border-radius: var(--bs-border-radius, 4px); background: transparent; @@ -54,24 +53,36 @@ } .querychat-query-chevron { - font-size: 0.625rem; - transition: transform 150ms; - display: inline-block; + display: grid; + place-items: center; + width: 0.9em; + height: 0.9em; + flex: none; + opacity: var(--_activity-opacity-muted, 0.65); + transform: rotate(-90deg); + transition: transform 300ms ease-in-out; +} + +.querychat-query-chevron svg { + display: block; + width: 100%; + height: 100%; } .querychat-query-chevron--expanded { - transform: rotate(90deg); + transform: rotate(0); } .querychat-icon { - width: 14px; - height: 14px; + width: 1em; + height: 1em; } .querychat-dropdown-chevron { - width: 12px; - height: 12px; + width: 0.9em; + height: 0.9em; margin-left: 2px; + opacity: var(--_activity-opacity-muted, 0.65); } .querychat-save-dropdown { @@ -117,7 +128,8 @@ display: none; position: relative; border-top: 1px solid var(--bs-border-color, #dee2e6); - margin: 8px -16px -8px; + margin: 0.25rem calc(-1 * var(--_querychat-footer-pad-x)) + calc(-1 * var(--_querychat-footer-pad-y)); } .querychat-query-section--visible { @@ -129,10 +141,17 @@ .shiny-tool-card:has(.querychat-viz-container) { max-height: 700px; overflow: hidden; + opacity: 1; } .shiny-tool-card:has(.querychat-viz-container) > .card-footer { + --_querychat-footer-pad-y: 0.125rem; + --_querychat-footer-pad-x: calc(var(--_row-pad, 0.4rem) - 0.25rem); + flex: 0 0 auto; + padding: var(--_querychat-footer-pad-y) var(--_querychat-footer-pad-x); + border-top: var(--bs-border-width, 1px) solid + var(--bs-border-color, #dee2e6); } .shiny-tool-card[fullscreen]:has(.querychat-viz-container) { diff --git a/pkg-r/tests/testthat/test-viz-tool.R b/pkg-r/tests/testthat/test-viz-tool.R index 89964ff0..ef6a5256 100644 --- a/pkg-r/tests/testthat/test-viz-tool.R +++ b/pkg-r/tests/testthat/test-viz-tool.R @@ -445,3 +445,28 @@ describe("collapse_validation_errors()", { ) }) }) + +describe("build_viz_footer()", { + it("uses the Shiny Chat disclosure chevron for Show Query", { + footer <- build_viz_footer( + "SELECT * FROM test_table VISUALISE value AS x DRAW histogram", + "Chart", + "querychat_viz_raw", + "querychat_viz_raw" + ) + html <- as.character(footer) + + expect_match(html, "querychat-query-chevron", fixed = TRUE) + expect_match(html, "bi-chevron-down", fixed = TRUE) + expect_no_match(html, "\u25b6", fixed = TRUE) + }) +}) + +describe("visualization tool display", { + it("requests Shiny Chat's framed presentation", { + source <- readLines(test_path("..", "..", "R", "querychat_viz.R")) + source <- paste(source, collapse = "\n") + + expect_match(source, 'presentation = "framed"', fixed = TRUE) + }) +}) diff --git a/pyproject.toml b/pyproject.toml index 56c51065..d574c7d3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,9 +22,9 @@ maintainers = [ dependencies = [ "duckdb", "shiny>=1.6.2", - "shinychat @ git+https://github.com/posit-dev/shinychat.git@main", + "shinychat @ git+https://github.com/posit-dev/shinychat.git@refs/pull/331/head", "htmltools", - "chatlas>=0.18.0", + "chatlas @ git+https://github.com/posit-dev/chatlas.git@refs/pull/396/head", "narwhals>=2.2.0", "chevron", "sqlalchemy>=2.0.0", # Using 2.0+ for improved type hints and API From 815fc1549d7a8dc7c7a5cec6280f5c352ae081c0 Mon Sep 17 00:00:00 2001 From: cpsievert Date: Fri, 21 Aug 2026 00:29:06 +0000 Subject: [PATCH 07/19] `devtools::document()` (GitHub Actions) --- pkg-r/DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg-r/DESCRIPTION b/pkg-r/DESCRIPTION index 6a0ab31d..8afbf2c0 100644 --- a/pkg-r/DESCRIPTION +++ b/pkg-r/DESCRIPTION @@ -60,7 +60,7 @@ VignetteBuilder: knitr Remotes: posit-dev/shinychat/pkg-r@feat/framed-tool-result-presentation -Config/roxygen2/version: 8.0.0 +Config/roxygen2/version: 8.1.0 Config/testthat/edition: 3 Config/testthat/parallel: true Encoding: UTF-8 From 7a5ee718c1acdd0f1030239458a80e025a4831ae Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 21 Aug 2026 18:25:13 -0500 Subject: [PATCH 08/19] Update pyproject.toml --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d574c7d3..f0cbc7ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,7 @@ dependencies = [ "shiny>=1.6.2", "shinychat @ git+https://github.com/posit-dev/shinychat.git@refs/pull/331/head", "htmltools", - "chatlas @ git+https://github.com/posit-dev/chatlas.git@refs/pull/396/head", + "chatlas>=0.21.2", "narwhals>=2.2.0", "chevron", "sqlalchemy>=2.0.0", # Using 2.0+ for improved type hints and API From 62b0492c2388a6ab87da08a69d297acf8ccb1132 Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 21 Aug 2026 18:47:42 -0500 Subject: [PATCH 09/19] test: check framed presentation behaviorally, not via source file 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. --- pkg-r/tests/testthat/test-viz-tool.R | 44 ++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/pkg-r/tests/testthat/test-viz-tool.R b/pkg-r/tests/testthat/test-viz-tool.R index ef6a5256..2f57c02e 100644 --- a/pkg-r/tests/testthat/test-viz-tool.R +++ b/pkg-r/tests/testthat/test-viz-tool.R @@ -463,10 +463,48 @@ describe("build_viz_footer()", { }) describe("visualization tool display", { + skip_if_no_dataframe_engine() + skip_if_not_installed("ggsql") + it("requests Shiny Chat's framed presentation", { - source <- readLines(test_path("..", "..", "R", "querychat_viz.R")) - source <- paste(source, collapse = "\n") + ds <- local_data_frame_source(new_test_df()) + session <- structure( + list( + output = list(), + ns = identity + ), + class = "MockShinySession" + ) + + local_mocked_bindings( + execute_ggsql = function(...) structure(list(), class = "ggsql_spec"), + build_viz_footer = function(...) htmltools::tagList(), + .package = "querychat" + ) + local_mocked_bindings( + ggsql_validate = function(...) { + structure(list(valid = TRUE), class = "ggsql_validated") + }, + ggsql_has_visual = function(...) TRUE, + renderGgsql = function(...) shiny::renderText("ok"), + ggsqlOutput = function(id) htmltools::div(id = id), + ggsql_save = function(...) rlang::abort("V8 is not installed"), + .package = "ggsql" + ) + + tool <- tool_visualize_dashboard( + ds, + session = session, + update_fn = function(data) {} + ) + + suppressWarnings( + result <- tool( + ggsql = "SELECT * FROM test_table VISUALISE value AS x DRAW histogram", + title = "Test" + ) + ) - expect_match(source, 'presentation = "framed"', fixed = TRUE) + expect_identical(result@extra$display$presentation, "framed") }) }) From e18e98ce6e8f7fb34872bbd72846f47804e5c44d Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 21 Aug 2026 18:47:42 -0500 Subject: [PATCH 10/19] chore: sync generated viz.css web assets --- pkg-py/src/querychat/static/css/viz.css | 1 + pkg-r/inst/htmldep/viz.css | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg-py/src/querychat/static/css/viz.css b/pkg-py/src/querychat/static/css/viz.css index 04e46886..743d3718 100644 --- a/pkg-py/src/querychat/static/css/viz.css +++ b/pkg-py/src/querychat/static/css/viz.css @@ -1,3 +1,4 @@ +/* Generated file. Source: js/src/viz.css. Do not edit directly. */ /* Hide Vega's built-in action dropdown (we have our own save button) */ .querychat-viz-container details:has(> .vega-actions) { display: none !important; diff --git a/pkg-r/inst/htmldep/viz.css b/pkg-r/inst/htmldep/viz.css index 04e46886..743d3718 100644 --- a/pkg-r/inst/htmldep/viz.css +++ b/pkg-r/inst/htmldep/viz.css @@ -1,3 +1,4 @@ +/* Generated file. Source: js/src/viz.css. Do not edit directly. */ /* Hide Vega's built-in action dropdown (we have our own save button) */ .querychat-viz-container details:has(> .vega-actions) { display: none !important; From 22602d9ad622217644da656fa6473ccae43782ae Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 21 Aug 2026 19:18:09 -0500 Subject: [PATCH 11/19] refactor: adopt shinychat's open_style display field shinychat renamed the tool-result display field presentation -> open_style (with values minimal/framed); request the framed open style for visualization results. --- pkg-py/src/querychat/_viz_tools.py | 2 +- pkg-py/tests/test_viz_footer.py | 4 ++-- pkg-r/R/querychat_viz.R | 2 +- pkg-r/tests/testthat/test-viz-tool.R | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg-py/src/querychat/_viz_tools.py b/pkg-py/src/querychat/_viz_tools.py index 468c17a1..cebe8164 100644 --- a/pkg-py/src/querychat/_viz_tools.py +++ b/pkg-py/src/querychat/_viz_tools.py @@ -141,7 +141,7 @@ def __init__( full_screen=True, icon=bs_icon("graph-up"), footer=footer, - presentation="framed", + open_style="framed", ), } diff --git a/pkg-py/tests/test_viz_footer.py b/pkg-py/tests/test_viz_footer.py index bee4512d..df93c81b 100644 --- a/pkg-py/tests/test_viz_footer.py +++ b/pkg-py/tests/test_viz_footer.py @@ -179,7 +179,7 @@ def test_visualization_card_is_opaque_and_footer_is_compact(self): assert "height: 24px;" in buttons["declarations"] assert "padding: 1px 0.25rem;" in buttons["declarations"] - def test_visualization_requests_shinychat_framed_presentation(self): + def test_visualization_requests_shinychat_framed_open_style(self): source = ( Path(__file__).parents[1] / "src" @@ -187,4 +187,4 @@ def test_visualization_requests_shinychat_framed_presentation(self): / "_viz_tools.py" ).read_text(encoding="utf-8") - assert 'presentation="framed"' in source + assert 'open_style="framed"' in source diff --git a/pkg-r/R/querychat_viz.R b/pkg-r/R/querychat_viz.R index 69f57c37..27bbb10d 100644 --- a/pkg-r/R/querychat_viz.R +++ b/pkg-r/R/querychat_viz.R @@ -154,7 +154,7 @@ visualize_result <- function( full_screen = TRUE, icon = viz_icon(), footer = freeze_tags(footer), - presentation = "framed" + open_style = "framed" ) ) diff --git a/pkg-r/tests/testthat/test-viz-tool.R b/pkg-r/tests/testthat/test-viz-tool.R index 2f57c02e..d20ec28a 100644 --- a/pkg-r/tests/testthat/test-viz-tool.R +++ b/pkg-r/tests/testthat/test-viz-tool.R @@ -466,7 +466,7 @@ describe("visualization tool display", { skip_if_no_dataframe_engine() skip_if_not_installed("ggsql") - it("requests Shiny Chat's framed presentation", { + it("requests Shiny Chat's framed open style", { ds <- local_data_frame_source(new_test_df()) session <- structure( list( @@ -505,6 +505,6 @@ describe("visualization tool display", { ) ) - expect_identical(result@extra$display$presentation, "framed") + expect_identical(result@extra$display$open_style, "framed") }) }) From 791695986d1be56ed4f3de67e54496f4f147d31a Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 21 Aug 2026 20:11:46 -0500 Subject: [PATCH 12/19] Fix install warning --- pkg-r/R/querychat-package.R | 4 - .../_problems/test-querychat_tools-254.R | 162 ++++++++++++++++++ .../_problems/test-querychat_tools-27.R | 11 ++ .../_problems/test-querychat_tools-36.R | 19 ++ .../_problems/test-querychat_tools-44.R | 26 +++ 5 files changed, 218 insertions(+), 4 deletions(-) create mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-254.R create mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-27.R create mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-36.R create mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-44.R diff --git a/pkg-r/R/querychat-package.R b/pkg-r/R/querychat-package.R index e5a51df4..cc965634 100644 --- a/pkg-r/R/querychat-package.R +++ b/pkg-r/R/querychat-package.R @@ -76,10 +76,6 @@ NULL #' @rawNamespace if (getRversion() < "4.3.0") importFrom("S7", "@") NULL -.onLoad <- function(libname, pkgname) { - rlang::run_on_load() -} - release_bullets <- function() { c( "Run `staticimports::import()` to update static imports", diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R new file mode 100644 index 00000000..4f630b92 --- /dev/null +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R @@ -0,0 +1,162 @@ +# Extracted from test-querychat_tools.R:254 + +# test ------------------------------------------------------------------------- +skip_if_no_dataframe_engine() +it("returns successful result for valid query action", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table WHERE id = 1", + action = "query" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_null(result@error) + expect_s3_class(result@value, "data.frame") + expect_equal(nrow(result@value), 1) + }) +it("returns successful result for valid update action", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table WHERE value > 20", + title = "High values", + action = "update" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_null(result@error) + expect_equal( + result@value, + "Dashboard updated. Use `querychat_query` tool to review results, if needed." + ) + }) +it("returns successful result for reset action", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = NULL, + action = "reset" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_null(result@error) + expect_equal(result@value, "The dashboard has been reset to show all data.") + }) +it("handles query errors appropriately", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM nonexistent_table", + action = "query" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_s3_class(result@error, "error") + expect_null(result@value) + }) +it("handles update errors appropriately", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "INVALID SQL", + action = "update" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_s3_class(result@error, "error") + expect_null(result@value) + }) +it("formats query results with details block", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table LIMIT 1", + action = "query" + ) + + markdown <- result@extra$display$markdown + expect_match(markdown, "```sql") + expect_match(markdown, "SELECT \\* FROM test_table LIMIT 1") + expect_match(markdown, "") + }) +it("formats update results with button HTML", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + title = "Test Filter", + action = "update" + ) + + markdown <- result@extra$display$markdown + expect_match(markdown, "```sql") + expect_match(markdown, "SELECT \\* FROM test_table") + expect_match(markdown, "button") + expect_match(markdown, "Apply Filter") + expect_match(markdown, "data-query") + expect_match(markdown, "data-title") + }) +it("formats reset results with button HTML", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = NULL, + action = "reset" + ) + + markdown <- result@extra$display$markdown + expect_match(markdown, "button") + expect_match(markdown, "Reset Filter") + }) +it("includes title in extra display metadata for update action", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + title = "Custom Title", + action = "update" + ) + + expect_equal(result@extra$display$title, "Custom Title") + }) +it("does not include title for query action", { + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + title = "Should be ignored", + action = "query" + ) + + expect_null(result@extra$display$title) + }) +it("sets open state based on action and tool details option", { + df_source <- local_data_frame_source(new_test_df()) + withr::local_options(querychat.tool_details = NULL) + + query_result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + action = "query" + ) + expect_true(query_result@extra$display$open) + + reset_result <- querychat_tool_result( + df_source, + query = NULL, + action = "reset" + ) + expect_false(reset_result@extra$display$open) + }) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R new file mode 100644 index 00000000..a3072533 --- /dev/null +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R @@ -0,0 +1,11 @@ +# Extracted from test-querychat_tools.R:27 + +# test ------------------------------------------------------------------------- +it("uses the tool default when options are unset", { + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) + + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) + }) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R new file mode 100644 index 00000000..56d86366 --- /dev/null +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R @@ -0,0 +1,19 @@ +# Extracted from test-querychat_tools.R:36 + +# test ------------------------------------------------------------------------- +it("uses the tool default when options are unset", { + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) + + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) + }) +it("uses the tool default when envvar is 'default'", { + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") + + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) + }) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R new file mode 100644 index 00000000..e29baaf5 --- /dev/null +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R @@ -0,0 +1,26 @@ +# Extracted from test-querychat_tools.R:44 + +# test ------------------------------------------------------------------------- +it("uses the tool default when options are unset", { + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) + + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) + }) +it("uses the tool default when envvar is 'default'", { + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") + + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) + }) +it("uses the tool default when option is 'default'", { + withr::local_options(querychat.tool_details = "default") + + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) + }) From 79e210dec7af6199b6b3eae0411d4ac803d889fb Mon Sep 17 00:00:00 2001 From: cpsievert Date: Sat, 22 Aug 2026 01:14:25 +0000 Subject: [PATCH 13/19] `air format` (GitHub Actions) --- .../_problems/test-querychat_tools-254.R | 286 +++++++++--------- .../_problems/test-querychat_tools-27.R | 12 +- .../_problems/test-querychat_tools-36.R | 24 +- .../_problems/test-querychat_tools-44.R | 34 +-- 4 files changed, 178 insertions(+), 178 deletions(-) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R index 4f630b92..084e7b9a 100644 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R @@ -3,160 +3,160 @@ # test ------------------------------------------------------------------------- skip_if_no_dataframe_engine() it("returns successful result for valid query action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table WHERE id = 1", - action = "query" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_null(result@error) - expect_s3_class(result@value, "data.frame") - expect_equal(nrow(result@value), 1) - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table WHERE id = 1", + action = "query" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_null(result@error) + expect_s3_class(result@value, "data.frame") + expect_equal(nrow(result@value), 1) +}) it("returns successful result for valid update action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table WHERE value > 20", - title = "High values", - action = "update" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_null(result@error) - expect_equal( - result@value, - "Dashboard updated. Use `querychat_query` tool to review results, if needed." - ) - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table WHERE value > 20", + title = "High values", + action = "update" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_null(result@error) + expect_equal( + result@value, + "Dashboard updated. Use `querychat_query` tool to review results, if needed." + ) +}) it("returns successful result for reset action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = NULL, - action = "reset" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_null(result@error) - expect_equal(result@value, "The dashboard has been reset to show all data.") - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = NULL, + action = "reset" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_null(result@error) + expect_equal(result@value, "The dashboard has been reset to show all data.") +}) it("handles query errors appropriately", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM nonexistent_table", - action = "query" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_s3_class(result@error, "error") - expect_null(result@value) - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM nonexistent_table", + action = "query" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_s3_class(result@error, "error") + expect_null(result@value) +}) it("handles update errors appropriately", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "INVALID SQL", - action = "update" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_s3_class(result@error, "error") - expect_null(result@value) - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "INVALID SQL", + action = "update" + ) + + expect_s7_class(result, ellmer::ContentToolResult) + expect_s3_class(result@error, "error") + expect_null(result@value) +}) it("formats query results with details block", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table LIMIT 1", - action = "query" - ) - - markdown <- result@extra$display$markdown - expect_match(markdown, "```sql") - expect_match(markdown, "SELECT \\* FROM test_table LIMIT 1") - expect_match(markdown, "") - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table LIMIT 1", + action = "query" + ) + + markdown <- result@extra$display$markdown + expect_match(markdown, "```sql") + expect_match(markdown, "SELECT \\* FROM test_table LIMIT 1") + expect_match(markdown, "") +}) it("formats update results with button HTML", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - title = "Test Filter", - action = "update" - ) - - markdown <- result@extra$display$markdown - expect_match(markdown, "```sql") - expect_match(markdown, "SELECT \\* FROM test_table") - expect_match(markdown, "button") - expect_match(markdown, "Apply Filter") - expect_match(markdown, "data-query") - expect_match(markdown, "data-title") - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + title = "Test Filter", + action = "update" + ) + + markdown <- result@extra$display$markdown + expect_match(markdown, "```sql") + expect_match(markdown, "SELECT \\* FROM test_table") + expect_match(markdown, "button") + expect_match(markdown, "Apply Filter") + expect_match(markdown, "data-query") + expect_match(markdown, "data-title") +}) it("formats reset results with button HTML", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = NULL, - action = "reset" - ) - - markdown <- result@extra$display$markdown - expect_match(markdown, "button") - expect_match(markdown, "Reset Filter") - }) + df_source <- local_data_frame_source(new_test_df()) + + result <- querychat_tool_result( + df_source, + query = NULL, + action = "reset" + ) + + markdown <- result@extra$display$markdown + expect_match(markdown, "button") + expect_match(markdown, "Reset Filter") +}) it("includes title in extra display metadata for update action", { - df_source <- local_data_frame_source(new_test_df()) + df_source <- local_data_frame_source(new_test_df()) - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - title = "Custom Title", - action = "update" - ) + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + title = "Custom Title", + action = "update" + ) - expect_equal(result@extra$display$title, "Custom Title") - }) + expect_equal(result@extra$display$title, "Custom Title") +}) it("does not include title for query action", { - df_source <- local_data_frame_source(new_test_df()) + df_source <- local_data_frame_source(new_test_df()) - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - title = "Should be ignored", - action = "query" - ) + result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + title = "Should be ignored", + action = "query" + ) - expect_null(result@extra$display$title) - }) + expect_null(result@extra$display$title) +}) it("sets open state based on action and tool details option", { - df_source <- local_data_frame_source(new_test_df()) - withr::local_options(querychat.tool_details = NULL) - - query_result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - action = "query" - ) - expect_true(query_result@extra$display$open) - - reset_result <- querychat_tool_result( - df_source, - query = NULL, - action = "reset" - ) - expect_false(reset_result@extra$display$open) - }) + df_source <- local_data_frame_source(new_test_df()) + withr::local_options(querychat.tool_details = NULL) + + query_result <- querychat_tool_result( + df_source, + query = "SELECT * FROM test_table", + action = "query" + ) + expect_true(query_result@extra$display$open) + + reset_result <- querychat_tool_result( + df_source, + query = NULL, + action = "reset" + ) + expect_false(reset_result@extra$display$open) +}) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R index a3072533..6333dcd9 100644 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R @@ -2,10 +2,10 @@ # test ------------------------------------------------------------------------- it("uses the tool default when options are unset", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) - }) + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) +}) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R index 56d86366..2adca07b 100644 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R @@ -2,18 +2,18 @@ # test ------------------------------------------------------------------------- it("uses the tool default when options are unset", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) - }) + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) +}) it("uses the tool default when envvar is 'default'", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) - }) + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) +}) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R index e29baaf5..ddd2d9f1 100644 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R +++ b/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R @@ -2,25 +2,25 @@ # test ------------------------------------------------------------------------- it("uses the tool default when options are unset", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) - }) + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) +}) it("uses the tool default when envvar is 'default'", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") + withr::local_options(querychat.tool_details = NULL) + withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) - }) + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) +}) it("uses the tool default when option is 'default'", { - withr::local_options(querychat.tool_details = "default") + withr::local_options(querychat.tool_details = "default") - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) - }) + expect_true(querychat_tool_starts_open("query")) + expect_true(querychat_tool_starts_open("update")) + expect_false(querychat_tool_starts_open("reset")) +}) From 81678f3788cc17c4bf4984d5b3ec84aa1ea45228 Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 25 Aug 2026 16:06:55 -0500 Subject: [PATCH 14/19] build: track shinychat main now that framed presentation PR is merged --- pkg-r/DESCRIPTION | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg-r/DESCRIPTION b/pkg-r/DESCRIPTION index 8afbf2c0..ba35b545 100644 --- a/pkg-r/DESCRIPTION +++ b/pkg-r/DESCRIPTION @@ -59,7 +59,7 @@ Suggests: VignetteBuilder: knitr Remotes: - posit-dev/shinychat/pkg-r@feat/framed-tool-result-presentation + posit-dev/shinychat/pkg-r Config/roxygen2/version: 8.1.0 Config/testthat/edition: 3 Config/testthat/parallel: true diff --git a/pyproject.toml b/pyproject.toml index f0cbc7ba..cf228908 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,7 +22,7 @@ maintainers = [ dependencies = [ "duckdb", "shiny>=1.6.2", - "shinychat @ git+https://github.com/posit-dev/shinychat.git@refs/pull/331/head", + "shinychat @ git+https://github.com/posit-dev/shinychat.git@main", "htmltools", "chatlas>=0.21.2", "narwhals>=2.2.0", From a4a408c1bfbe81b180a6bf320ed2a2be79d72254 Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 25 Aug 2026 16:50:18 -0500 Subject: [PATCH 15/19] test(py): fix viz Playwright tests for shinychat framed presentation - 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 --- js/src/viz.css | 15 +++++++++- pkg-py/src/querychat/static/css/viz.css | 15 +++++++++- pkg-py/tests/playwright/test_11_viz_footer.py | 30 +++++++++++-------- pkg-r/inst/htmldep/viz.css | 15 +++++++++- 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/js/src/viz.css b/js/src/viz.css index 04e46886..a6af0000 100644 --- a/js/src/viz.css +++ b/js/src/viz.css @@ -144,7 +144,20 @@ opacity: 1; } -.shiny-tool-card:has(.querychat-viz-container) > .card-footer { +/* shinychat dims tool activity (opacity: .7); the framed viz group header + should stay fully opaque like the card body it introduces */ +.shiny-chat-tool-group--framed:has(.querychat-viz-container) + > .shiny-chat-tool-group__row { + opacity: 1; +} + +.shiny-tool-card:has(.querychat-viz-container) > .card-footer, +/* Match (and beat, via load order) the specificity of shinychat's framed + group footer rule, which would otherwise force card-cap padding */ +.shiny-chat-tool-group--framed + > .shiny-chat-tool-call-row__detail + > .shiny-tool-card:has(.querychat-viz-container) + > .card-footer { --_querychat-footer-pad-y: 0.125rem; --_querychat-footer-pad-x: calc(var(--_row-pad, 0.4rem) - 0.25rem); diff --git a/pkg-py/src/querychat/static/css/viz.css b/pkg-py/src/querychat/static/css/viz.css index 743d3718..0376c644 100644 --- a/pkg-py/src/querychat/static/css/viz.css +++ b/pkg-py/src/querychat/static/css/viz.css @@ -145,7 +145,20 @@ opacity: 1; } -.shiny-tool-card:has(.querychat-viz-container) > .card-footer { +/* shinychat dims tool activity (opacity: .7); the framed viz group header + should stay fully opaque like the card body it introduces */ +.shiny-chat-tool-group--framed:has(.querychat-viz-container) + > .shiny-chat-tool-group__row { + opacity: 1; +} + +.shiny-tool-card:has(.querychat-viz-container) > .card-footer, +/* Match (and beat, via load order) the specificity of shinychat's framed + group footer rule, which would otherwise force card-cap padding */ +.shiny-chat-tool-group--framed + > .shiny-chat-tool-call-row__detail + > .shiny-tool-card:has(.querychat-viz-container) + > .card-footer { --_querychat-footer-pad-y: 0.125rem; --_querychat-footer-pad-x: calc(var(--_row-pad, 0.4rem) - 0.25rem); diff --git a/pkg-py/tests/playwright/test_11_viz_footer.py b/pkg-py/tests/playwright/test_11_viz_footer.py index e8b29e71..59cb663c 100644 --- a/pkg-py/tests/playwright/test_11_viz_footer.py +++ b/pkg-py/tests/playwright/test_11_viz_footer.py @@ -121,8 +121,9 @@ def _send_viz_prompt( class TestVizFrame: def test_expanded_visualization_has_complete_frame(self, page: Page) -> None: - group = page.locator(".shiny-chat-tool-group--single").filter( - has_text="Passengers by Class" + # Filter structurally, not by the LLM-chosen chart title + group = page.locator( + ".shiny-chat-tool-group--single:has(.querychat-viz-container)" ) card = page.locator(".shiny-tool-card:has(.querychat-viz-container)") summary = group.locator(":scope > .shiny-chat-tool-group__row") @@ -147,12 +148,8 @@ def test_expanded_visualization_has_complete_frame(self, page: Page) -> None: header_chevron_box = summary.locator( ".shiny-chat-tool-group__chevron" ).bounding_box() - show_chevron_box = footer.locator( - ".querychat-query-chevron" - ).bounding_box() - save_chevron_box = footer.locator( - ".querychat-dropdown-chevron" - ).bounding_box() + show_chevron_box = footer.locator(".querychat-query-chevron").bounding_box() + save_chevron_box = footer.locator(".querychat-dropdown-chevron").bounding_box() assert header_box is not None assert footer_box is not None @@ -162,14 +159,21 @@ def test_expanded_visualization_has_complete_frame(self, page: Page) -> None: assert save_chevron_box is not None assert abs(header_box["height"] - footer_box["height"]) <= 1 assert abs(header_glyph_box["x"] - show_chevron_box["x"]) <= 0.5 - assert abs( - (header_chevron_box["x"] + header_chevron_box["width"]) - - (save_chevron_box["x"] + save_chevron_box["width"]) - ) <= 0.5 + assert ( + abs( + (header_chevron_box["x"] + header_chevron_box["width"]) + - (save_chevron_box["x"] + save_chevron_box["width"]) + ) + <= 0.5 + ) summary.click() + # Collapsing unmounts the card (and the viz container with it), so + # the :has() locator above no longer matches. The viz group is the + # last tool group in the chat. + collapsed_group = page.locator(".shiny-chat-tool-group--single").last for side in ("top", "right", "bottom", "left"): - expect(group).to_have_css(f"border-{side}-width", "0px") + expect(collapsed_group).to_have_css(f"border-{side}-width", "0px") class TestShowQueryToggle: diff --git a/pkg-r/inst/htmldep/viz.css b/pkg-r/inst/htmldep/viz.css index 743d3718..0376c644 100644 --- a/pkg-r/inst/htmldep/viz.css +++ b/pkg-r/inst/htmldep/viz.css @@ -145,7 +145,20 @@ opacity: 1; } -.shiny-tool-card:has(.querychat-viz-container) > .card-footer { +/* shinychat dims tool activity (opacity: .7); the framed viz group header + should stay fully opaque like the card body it introduces */ +.shiny-chat-tool-group--framed:has(.querychat-viz-container) + > .shiny-chat-tool-group__row { + opacity: 1; +} + +.shiny-tool-card:has(.querychat-viz-container) > .card-footer, +/* Match (and beat, via load order) the specificity of shinychat's framed + group footer rule, which would otherwise force card-cap padding */ +.shiny-chat-tool-group--framed + > .shiny-chat-tool-call-row__detail + > .shiny-tool-card:has(.querychat-viz-container) + > .card-footer { --_querychat-footer-pad-y: 0.125rem; --_querychat-footer-pad-x: calc(var(--_row-pad, 0.4rem) - 0.25rem); From ae5dd639e87d63bb33eb0c18ad6131c95bcef3a3 Mon Sep 17 00:00:00 2001 From: Carson Date: Tue, 25 Aug 2026 17:34:15 -0500 Subject: [PATCH 16/19] fix(py): fall back to result value when display markdown is None 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. --- pkg-py/src/querychat/_querychat_core.py | 6 ++- pkg-py/tests/test_querychat_core.py | 51 +++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 pkg-py/tests/test_querychat_core.py diff --git a/pkg-py/src/querychat/_querychat_core.py b/pkg-py/src/querychat/_querychat_core.py index 59cae534..0c33df3c 100644 --- a/pkg-py/src/querychat/_querychat_core.py +++ b/pkg-py/src/querychat/_querychat_core.py @@ -101,8 +101,10 @@ def format_chunk(chunk: Union[str, Content]) -> str: def format_tool_result(result: ContentToolResult) -> str: """Extract displayable text from a tool result.""" display_info = result.extra.get("display") if result.extra else None - if display_info and hasattr(display_info, "markdown"): - return display_info.markdown + if display_info: + markdown = getattr(display_info, "markdown", None) + if markdown is not None: + return markdown if result.value is not None: return str(result.value) return "" diff --git a/pkg-py/tests/test_querychat_core.py b/pkg-py/tests/test_querychat_core.py new file mode 100644 index 00000000..707618d3 --- /dev/null +++ b/pkg-py/tests/test_querychat_core.py @@ -0,0 +1,51 @@ +"""Tests for querychat._querychat_core formatting helpers.""" + +import pytest +from chatlas import ContentToolResult +from shinychat.types import ToolResultDisplay + +from querychat._querychat_core import format_chunk, format_tool_result + + +def test_format_tool_result_without_display_returns_value(): + result = ContentToolResult(value="schema text") + assert format_tool_result(result) == "schema text" + + +def test_format_tool_result_with_markdown_display_returns_markdown(): + result = ContentToolResult( + value="schema text", + extra={"display": ToolResultDisplay(markdown="**schema**")}, + ) + assert format_tool_result(result) == "**schema**" + + +def test_format_tool_result_with_none_markdown_falls_back_to_value(): + # ToolResultDisplay.markdown defaults to None; the display's mere + # existence must not shadow the result value. + result = ContentToolResult( + value="schema text", + extra={"display": ToolResultDisplay(label="titanic")}, + ) + assert format_tool_result(result) == "schema text" + + +def test_format_tool_result_without_value_returns_empty_string(): + result = ContentToolResult( + value=None, + extra={"display": ToolResultDisplay(label="titanic")}, + ) + assert format_tool_result(result) == "" + + +def test_format_chunk_wraps_tool_result_without_crashing(): + result = ContentToolResult( + value="schema text", + extra={"display": ToolResultDisplay(label="titanic")}, + ) + assert format_chunk(result) == "\n\nschema text\n\n" + + +def test_format_chunk_rejects_unknown_type(): + with pytest.raises(ValueError, match="Unknown chunk type"): + format_chunk(42) # type: ignore[arg-type] From 5cfe70e122ebec54f91ac4cb48b2d9f1747e3a94 Mon Sep 17 00:00:00 2001 From: Carson Date: Wed, 26 Aug 2026 09:20:02 -0500 Subject: [PATCH 17/19] style: fix import sorting in test_querychat_core.py --- pkg-py/tests/test_querychat_core.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg-py/tests/test_querychat_core.py b/pkg-py/tests/test_querychat_core.py index 707618d3..2d6ca63d 100644 --- a/pkg-py/tests/test_querychat_core.py +++ b/pkg-py/tests/test_querychat_core.py @@ -2,9 +2,8 @@ import pytest from chatlas import ContentToolResult -from shinychat.types import ToolResultDisplay - from querychat._querychat_core import format_chunk, format_tool_result +from shinychat.types import ToolResultDisplay def test_format_tool_result_without_display_returns_value(): From c1ae6a3783f6ffdc3b9429d45afeef466b41e3aa Mon Sep 17 00:00:00 2001 From: Carson Date: Wed, 26 Aug 2026 18:27:35 -0500 Subject: [PATCH 18/19] test(r): remove accidentally committed testthat _problems files These were extracted failing-test snippets from a local run, committed by mistake in 79169598. --- .../_problems/test-querychat_tools-254.R | 162 ------------------ .../_problems/test-querychat_tools-27.R | 11 -- .../_problems/test-querychat_tools-36.R | 19 -- .../_problems/test-querychat_tools-44.R | 26 --- 4 files changed, 218 deletions(-) delete mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-254.R delete mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-27.R delete mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-36.R delete mode 100644 pkg-r/tests/testthat/_problems/test-querychat_tools-44.R diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R deleted file mode 100644 index 084e7b9a..00000000 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-254.R +++ /dev/null @@ -1,162 +0,0 @@ -# Extracted from test-querychat_tools.R:254 - -# test ------------------------------------------------------------------------- -skip_if_no_dataframe_engine() -it("returns successful result for valid query action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table WHERE id = 1", - action = "query" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_null(result@error) - expect_s3_class(result@value, "data.frame") - expect_equal(nrow(result@value), 1) -}) -it("returns successful result for valid update action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table WHERE value > 20", - title = "High values", - action = "update" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_null(result@error) - expect_equal( - result@value, - "Dashboard updated. Use `querychat_query` tool to review results, if needed." - ) -}) -it("returns successful result for reset action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = NULL, - action = "reset" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_null(result@error) - expect_equal(result@value, "The dashboard has been reset to show all data.") -}) -it("handles query errors appropriately", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM nonexistent_table", - action = "query" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_s3_class(result@error, "error") - expect_null(result@value) -}) -it("handles update errors appropriately", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "INVALID SQL", - action = "update" - ) - - expect_s7_class(result, ellmer::ContentToolResult) - expect_s3_class(result@error, "error") - expect_null(result@value) -}) -it("formats query results with details block", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table LIMIT 1", - action = "query" - ) - - markdown <- result@extra$display$markdown - expect_match(markdown, "```sql") - expect_match(markdown, "SELECT \\* FROM test_table LIMIT 1") - expect_match(markdown, "") -}) -it("formats update results with button HTML", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - title = "Test Filter", - action = "update" - ) - - markdown <- result@extra$display$markdown - expect_match(markdown, "```sql") - expect_match(markdown, "SELECT \\* FROM test_table") - expect_match(markdown, "button") - expect_match(markdown, "Apply Filter") - expect_match(markdown, "data-query") - expect_match(markdown, "data-title") -}) -it("formats reset results with button HTML", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = NULL, - action = "reset" - ) - - markdown <- result@extra$display$markdown - expect_match(markdown, "button") - expect_match(markdown, "Reset Filter") -}) -it("includes title in extra display metadata for update action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - title = "Custom Title", - action = "update" - ) - - expect_equal(result@extra$display$title, "Custom Title") -}) -it("does not include title for query action", { - df_source <- local_data_frame_source(new_test_df()) - - result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - title = "Should be ignored", - action = "query" - ) - - expect_null(result@extra$display$title) -}) -it("sets open state based on action and tool details option", { - df_source <- local_data_frame_source(new_test_df()) - withr::local_options(querychat.tool_details = NULL) - - query_result <- querychat_tool_result( - df_source, - query = "SELECT * FROM test_table", - action = "query" - ) - expect_true(query_result@extra$display$open) - - reset_result <- querychat_tool_result( - df_source, - query = NULL, - action = "reset" - ) - expect_false(reset_result@extra$display$open) -}) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R deleted file mode 100644 index 6333dcd9..00000000 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-27.R +++ /dev/null @@ -1,11 +0,0 @@ -# Extracted from test-querychat_tools.R:27 - -# test ------------------------------------------------------------------------- -it("uses the tool default when options are unset", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) - - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) -}) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R deleted file mode 100644 index 2adca07b..00000000 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-36.R +++ /dev/null @@ -1,19 +0,0 @@ -# Extracted from test-querychat_tools.R:36 - -# test ------------------------------------------------------------------------- -it("uses the tool default when options are unset", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) - - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) -}) -it("uses the tool default when envvar is 'default'", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") - - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) -}) diff --git a/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R b/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R deleted file mode 100644 index ddd2d9f1..00000000 --- a/pkg-r/tests/testthat/_problems/test-querychat_tools-44.R +++ /dev/null @@ -1,26 +0,0 @@ -# Extracted from test-querychat_tools.R:44 - -# test ------------------------------------------------------------------------- -it("uses the tool default when options are unset", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = NA) - - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) -}) -it("uses the tool default when envvar is 'default'", { - withr::local_options(querychat.tool_details = NULL) - withr::local_envvar(QUERYCHAT_TOOL_DETAILS = "default") - - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) -}) -it("uses the tool default when option is 'default'", { - withr::local_options(querychat.tool_details = "default") - - expect_true(querychat_tool_starts_open("query")) - expect_true(querychat_tool_starts_open("update")) - expect_false(querychat_tool_starts_open("reset")) -}) From a030d6935409f6a2099033ff9e18f77c318505d5 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Wed, 26 Aug 2026 18:37:05 -0500 Subject: [PATCH 19/19] Apply suggestion from @cpsievert --- pkg-py/examples/10-viz-app.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg-py/examples/10-viz-app.py b/pkg-py/examples/10-viz-app.py index 250c121b..ba3ac306 100644 --- a/pkg-py/examples/10-viz-app.py +++ b/pkg-py/examples/10-viz-app.py @@ -16,6 +16,4 @@ qc.ui() -ui.page_opts(fillable=True, title="QueryChat Visualization Demo") - -ui.input_dark_mode() \ No newline at end of file +ui.page_opts(fillable=True, title="QueryChat Visualization Demo") \ No newline at end of file
{header}\s*order_id\s*
', header, "', expected_headers, "1 to 5Primary keypositive, required\\s*id\\s*