From 13c9fb738fec81aa98c61a8aaba429c955eba00a Mon Sep 17 00:00:00 2001 From: Carson Date: Wed, 26 Nov 2025 10:37:55 -0600 Subject: [PATCH 1/2] fix(pkg-py): .server() now returns reactives, making them truly session-specific --- .../{datasource.py => _datasource.py} | 0 pkg-py/src/querychat/_deprecated.py | 2 +- pkg-py/src/querychat/_querychat.py | 313 ++++++++---------- pkg-py/src/querychat/_querychat_module.py | 35 +- pkg-py/src/querychat/tools.py | 2 +- pkg-py/src/querychat/types/__init__.py | 9 + pkg-py/tests/test_datasource.py | 2 +- pkg-py/tests/test_df_to_html.py | 2 +- pkg-py/tests/test_querychat.py | 18 - 9 files changed, 192 insertions(+), 191 deletions(-) rename pkg-py/src/querychat/{datasource.py => _datasource.py} (100%) create mode 100644 pkg-py/src/querychat/types/__init__.py diff --git a/pkg-py/src/querychat/datasource.py b/pkg-py/src/querychat/_datasource.py similarity index 100% rename from pkg-py/src/querychat/datasource.py rename to pkg-py/src/querychat/_datasource.py diff --git a/pkg-py/src/querychat/_deprecated.py b/pkg-py/src/querychat/_deprecated.py index 2e2ea19d..9ec355e5 100644 --- a/pkg-py/src/querychat/_deprecated.py +++ b/pkg-py/src/querychat/_deprecated.py @@ -11,7 +11,7 @@ import sqlalchemy from narwhals.stable.v1.typing import IntoFrame - from .datasource import DataSource + from ._datasource import DataSource def init( diff --git a/pkg-py/src/querychat/_querychat.py b/pkg-py/src/querychat/_querychat.py index 815e69f6..d0af227a 100644 --- a/pkg-py/src/querychat/_querychat.py +++ b/pkg-py/src/querychat/_querychat.py @@ -15,9 +15,9 @@ from shiny.session import get_current_session from shinychat import output_markdown_stream +from ._datasource import DataFrameSource, DataSource, SQLAlchemySource from ._icons import bs_icon -from ._querychat_module import ModServerResult, mod_server, mod_ui -from .datasource import DataFrameSource, DataSource, SQLAlchemySource +from ._querychat_module import ServerValues, mod_server, mod_ui if TYPE_CHECKING: import pandas as pd @@ -72,9 +72,6 @@ def __init__( self._client.set_turns([]) self._client.system_prompt = prompt - # Populated when ._server() gets called (in an active session) - self._server_values: ModServerResult | None = None - def app( self, *, bookmark_store: Literal["url", "server", "disable"] = "url" ) -> App: @@ -130,15 +127,21 @@ def app_ui(request): ) def app_server(input: Inputs, output: Outputs, session: Session): - self._server(enable_bookmarking=enable_bookmarking) + vals = mod_server( + self.id, + data_source=self._data_source, + greeting=self.greeting, + client=self._client, + enable_bookmarking=enable_bookmarking, + ) @render.text def query_title(): - return self.title() or "SQL Query" + return vals.title() or "SQL Query" @render.ui def ui_reset(): - req(self.sql()) + req(vals.sql()) return ui.input_action_button( "reset_query", "Reset Query", @@ -148,17 +151,17 @@ def ui_reset(): @reactive.effect @reactive.event(input.reset_query) def _(): - self.sql("") - self.title(None) + vals.sql.set("") + vals.title.set(None) @render.data_frame def dt(): - return self.df() + return vals.df() @render.ui def sql_output(): - sql = self.sql() or f"SELECT * FROM {table_name}" - sql_code = f"```sql\n{sql}\n```" + sql_value = vals.sql() or f"SELECT * FROM {table_name}" + sql_code = f"```sql\n{sql_value}\n```" return output_markdown_stream( "sql_code", content=sql_code, @@ -218,142 +221,6 @@ def ui(self, **kwargs): """ return mod_ui(self.id, **kwargs) - def _server(self, *, enable_bookmarking: bool = False) -> None: - """ - Initialize the server module. - - Note: - ---- - This is a private method since it is called automatically in Express mode. - - """ - # Must be called within an active Shiny session - session = get_current_session() - if session is None: - raise RuntimeError( - "A Shiny session must be active in order to initialize QueryChat's server logic. " - "If you're using Shiny Core, make sure to call .server() within your server function." - ) - - # No-op for Express' stub session (i.e., it's 1st run) - if session.is_stub_session(): - return - - # Call the server module - self._server_values = mod_server( - self.id, - data_source=self._data_source, - greeting=self.greeting, - client=self._client, - enable_bookmarking=enable_bookmarking, - ) - - return - - def df(self) -> pd.DataFrame: - """ - Reactively read the current filtered data frame that is in effect. - - Returns - ------- - : - The current filtered data frame as a pandas DataFrame. If no query - has been set, this will return the unfiltered data frame from the - data source. - - Raises - ------ - RuntimeError - If `.server()` has not been called yet. - - """ - vals = self._server_values - if vals is None: - raise RuntimeError("Must call .server() before accessing .df()") - - return vals.df() - - @overload - def sql(self, query: None = None) -> str: ... - - @overload - def sql(self, query: str) -> bool: ... - - def sql(self, query: Optional[str] = None) -> str | bool: - """ - Reactively read (or set) the current SQL query that is in effect. - - Parameters - ---------- - query - If provided, sets the current SQL query to this value. - - Returns - ------- - : - If no `query` is provided, returns the current SQL query as a string - (possibly `""` if no query has been set). If a `query` is provided, - returns `True` if the query was changed to a new value, or `False` - if it was the same as the current value. - - Raises - ------ - RuntimeError - If `.server()` has not been called yet. - - """ - vals = self._server_values - if vals is None: - raise RuntimeError("Must call .server() before accessing .sql()") - - if query is None: - return vals.sql() - else: - return vals.sql.set(query) - - @overload - def title(self, value: None = None) -> str | None: ... - - @overload - def title(self, value: str) -> bool: ... - - def title(self, value: Optional[str] = None) -> str | None | bool: - """ - Reactively read (or set) the current title that is in effect. - - The title is a short description of the current query that the LLM - provides to us whenever it generates a new SQL query. It can be used as - a status string for the data dashboard. - - Parameters - ---------- - value - If provided, sets the current title to this value. - - Returns - ------- - : - If no `value` is provided, returns the current title as a string, or - `None` if no title has been set due to no SQL query being set. If a - `value` is provided, sets the current title to this value and - returns `True` if the title was changed to a new value, or `False` - if it was the same as the current value. - - Raises - ------ - RuntimeError - If `.server()` has not been called yet. - - """ - vals = self._server_values - if vals is None: - raise RuntimeError("Must call .server() before accessing .title()") - - if value is None: - return vals.title() - else: - return vals.title.set(value) - def generate_greeting(self, *, echo: Literal["none", "output"] = "none"): """ Generate a welcome greeting for the chat. @@ -381,20 +248,17 @@ def generate_greeting(self, *, echo: Literal["none", "output"] = "none"): return str(client.chat(prompt, echo=echo)) @property - def client(self): + def system_prompt(self) -> str: """ - Get the (session-specific) chat client. + Get the system prompt. Returns ------- : - The current chat client. + The system prompt string. """ - vals = self._server_values - if vals is None: - raise RuntimeError("Must call .server() before accessing .client") - return vals.client + return self._client.system_prompt or "" @property def data_source(self): @@ -476,7 +340,7 @@ class QueryChat(QueryChatBase): """ - def server(self, *, enable_bookmarking: bool = False) -> None: + def server(self, *, enable_bookmarking: bool = False) -> ServerValues: """ Initialize Shiny server logic. @@ -515,15 +379,15 @@ def app_ui(request): def server(input, output, session): - qc.server(enable_bookmarking=True) + qc_vals = qc.server(enable_bookmarking=True) @render.data_frame def data_table(): - return qc.df() + return qc_vals.df() @render.text - def title(): - return qc.title() or "My Data" + def title_text(): + return qc_vals.title() or "My Data" app = App(app_ui, server, bookmark_store="url") @@ -532,10 +396,24 @@ def title(): Returns ------- : - None + A ServerValues dataclass containing session-specific reactive values + and the chat client. See ServerValues documentation for details on + the available attributes. """ - return self._server(enable_bookmarking=enable_bookmarking) + session = get_current_session() + if session is None: + raise RuntimeError( + ".server() must be called within an active Shiny session (i.e., within the server function). " + ) + + return mod_server( + self.id, + data_source=self._data_source, + greeting=self.greeting, + client=self._client, + enable_bookmarking=enable_bookmarking, + ) class QueryChatExpress(QueryChatBase): @@ -645,6 +523,14 @@ def __init__( prompt_template: Optional[str | Path] = None, enable_bookmarking: Literal["auto", True, False] = "auto", ): + # Sanity check: Express should always have a (stub/real) session + session = get_current_session() + if session is None: + raise RuntimeError( + "Unexpected error: No active Shiny session found." + "Is express.QueryChat() being called outside of a Shiny Express app?", + ) + super().__init__( data_source, table_name, @@ -661,8 +547,7 @@ def __init__( # querychat's bookmarking enable: bool if enable_bookmarking == "auto": - session = get_current_session() - if session and isinstance(session, ExpressStubSession): + if isinstance(session, ExpressStubSession): store = session.app_opts.get("bookmark_store", "disable") enable = store != "disable" else: @@ -670,7 +555,103 @@ def __init__( else: enable = enable_bookmarking - self._server(enable_bookmarking=enable) + self._vals = mod_server( + self.id, + data_source=self._data_source, + greeting=self.greeting, + client=self._client, + enable_bookmarking=enable, + ) + + def df(self) -> pd.DataFrame: + """ + Reactively read the current filtered data frame that is in effect. + + Returns + ------- + : + The current filtered data frame as a pandas DataFrame. If no query + has been set, this will return the unfiltered data frame from the + data source. + + """ + return self._vals.df() + + @overload + def sql(self, query: None = None) -> str: ... + + @overload + def sql(self, query: str) -> bool: ... + + def sql(self, query: Optional[str] = None) -> str | bool: + """ + Reactively read (or set) the current SQL query that is in effect. + + Parameters + ---------- + query + If provided, sets the current SQL query to this value. + + Returns + ------- + : + If no `query` is provided, returns the current SQL query as a string + (possibly `""` if no query has been set). If a `query` is provided, + returns `True` if the query was changed to a new value, or `False` + if it was the same as the current value. + + """ + if query is None: + return self._vals.sql() + else: + return self._vals.sql.set(query) + + @overload + def title(self, value: None = None) -> str | None: ... + + @overload + def title(self, value: str) -> bool: ... + + def title(self, value: Optional[str] = None) -> str | None | bool: + """ + Reactively read (or set) the current title that is in effect. + + The title is a short description of the current query that the LLM + provides to us whenever it generates a new SQL query. It can be used as + a status string for the data dashboard. + + Parameters + ---------- + value + If provided, sets the current title to this value. + + Returns + ------- + : + If no `value` is provided, returns the current title as a string, or + `None` if no title has been set due to no SQL query being set. If a + `value` is provided, sets the current title to this value and + returns `True` if the title was changed to a new value, or `False` + if it was the same as the current value. + + """ + if value is None: + return self._vals.title() + else: + return self._vals.title.set(value) + + @property + def client(self): + """ + Get the (session-specific) chat client. + + Returns + ------- + : + The current chat client. + + """ + return self._vals.client def normalize_data_source( diff --git a/pkg-py/src/querychat/_querychat_module.py b/pkg-py/src/querychat/_querychat_module.py index 4fd83570..7e8beb68 100644 --- a/pkg-py/src/querychat/_querychat_module.py +++ b/pkg-py/src/querychat/_querychat_module.py @@ -16,7 +16,7 @@ from shiny import Inputs, Outputs, Session from shiny.bookmark import BookmarkState, RestoreState - from .datasource import DataSource + from ._datasource import DataSource ReactiveString = reactive.Value[str] """A reactive string value.""" @@ -44,7 +44,36 @@ def mod_ui(**kwargs): @dataclass -class ModServerResult: +class ServerValues: + """ + Session-specific reactive values and client returned by QueryChat.server(). + + This dataclass contains all the session-specific reactive state for a QueryChat + instance. Each session gets its own ServerValues to ensure proper isolation + between concurrent sessions. + + Attributes + ---------- + df + A reactive Calc that returns the current filtered data frame. If no SQL + query has been set, this returns the unfiltered data from the data source. + Call it like `.df()` to reactively read the current data frame. + sql + A reactive Value containing the current SQL query string. Access the value + by calling `.sql()`, or set it with `.sql.set("SELECT ...")`. + An empty string `""` indicates no query has been set. + title + A reactive Value containing the current title for the query. The LLM + provides this title when generating a new SQL query. Access it with + `.title()`, or set it with `.title.set("...")`. Returns + `None` if no title has been set. + client + The session-specific chat client instance. This is a deep copy of the + base client configured for this specific session, containing the chat + history and tool registrations for this session only. + + """ + df: Callable[[], pd.DataFrame] sql: ReactiveString title: ReactiveStringOrNone @@ -150,4 +179,4 @@ def _on_restore(x: RestoreState) -> None: if "querychat_has_greeted" in vals: has_greeted.set(vals["querychat_has_greeted"]) - return ModServerResult(df=filtered_df, sql=sql, title=title, client=chat) + return ServerValues(df=filtered_df, sql=sql, title=title, client=chat) diff --git a/pkg-py/src/querychat/tools.py b/pkg-py/src/querychat/tools.py index d908364a..06970150 100644 --- a/pkg-py/src/querychat/tools.py +++ b/pkg-py/src/querychat/tools.py @@ -11,8 +11,8 @@ from ._utils import df_to_html if TYPE_CHECKING: + from ._datasource import DataSource from ._querychat_module import ReactiveString, ReactiveStringOrNone - from .datasource import DataSource def _read_prompt_template(filename: str, **kwargs) -> str: diff --git a/pkg-py/src/querychat/types/__init__.py b/pkg-py/src/querychat/types/__init__.py new file mode 100644 index 00000000..339579a2 --- /dev/null +++ b/pkg-py/src/querychat/types/__init__.py @@ -0,0 +1,9 @@ +from .._datasource import DataFrameSource, DataSource, SQLAlchemySource # noqa: A005 +from .._querychat_module import ServerValues + +__all__ = ( + "DataFrameSource", + "DataSource", + "SQLAlchemySource", + "ServerValues", +) diff --git a/pkg-py/tests/test_datasource.py b/pkg-py/tests/test_datasource.py index 4453207e..99f51495 100644 --- a/pkg-py/tests/test_datasource.py +++ b/pkg-py/tests/test_datasource.py @@ -3,7 +3,7 @@ from pathlib import Path import pytest -from querychat.datasource import SQLAlchemySource +from querychat._datasource import SQLAlchemySource from sqlalchemy import create_engine, text diff --git a/pkg-py/tests/test_df_to_html.py b/pkg-py/tests/test_df_to_html.py index 7a09a327..a2cf58e6 100644 --- a/pkg-py/tests/test_df_to_html.py +++ b/pkg-py/tests/test_df_to_html.py @@ -4,8 +4,8 @@ import pandas as pd import pytest +from querychat._datasource import DataFrameSource, SQLAlchemySource from querychat._utils import df_to_html -from querychat.datasource import DataFrameSource, SQLAlchemySource from sqlalchemy import create_engine diff --git a/pkg-py/tests/test_querychat.py b/pkg-py/tests/test_querychat.py index 5ffce90b..893ff824 100644 --- a/pkg-py/tests/test_querychat.py +++ b/pkg-py/tests/test_querychat.py @@ -61,21 +61,3 @@ def test_querychat_custom_id(sample_df): assert qc.id == "custom_id" - -def test_querychat_core_reactive_access_before_server_raises(sample_df): - """Test that accessing reactive properties before .server() raises error.""" - qc = QueryChat( - data_source=sample_df, - table_name="test_table", - greeting="Hello!", - ) - - # Accessing reactive properties before .server() should raise - with pytest.raises(RuntimeError, match="Must call \\.server\\(\\)"): - qc.title() - - with pytest.raises(RuntimeError, match="Must call \\.server\\(\\)"): - qc.sql() - - with pytest.raises(RuntimeError, match="Must call \\.server\\(\\)"): - qc.df() From 50deaf29534896d5bf5f150ce34ca274b9192150 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Wed, 26 Nov 2025 11:02:12 -0600 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg-py/src/querychat/_querychat.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg-py/src/querychat/_querychat.py b/pkg-py/src/querychat/_querychat.py index d0af227a..c0a11b35 100644 --- a/pkg-py/src/querychat/_querychat.py +++ b/pkg-py/src/querychat/_querychat.py @@ -386,7 +386,7 @@ def data_table(): return qc_vals.df() @render.text - def title_text(): + def title(): return qc_vals.title() or "My Data" @@ -527,7 +527,7 @@ def __init__( session = get_current_session() if session is None: raise RuntimeError( - "Unexpected error: No active Shiny session found." + "Unexpected error: No active Shiny session found. " "Is express.QueryChat() being called outside of a Shiny Express app?", )