fix(py): let server(data_source=) survive a second session - #302
Merged
cpsievert merged 1 commit intoSep 12, 2026
Merged
Conversation
add_table()'s "no changes after server initialization" guard exists to protect prompt-cache coherence once a conversation is underway, but it also blocks server(data_source=...) on every session after the first -- since every new session re-runs the server function, this made per-session registration usable for exactly one session. Extract add_table()'s guard-free core into _add_or_replace_table() and call it directly from .server(), bypassing the guard for this path only; the public add_table()/add_tables() guard is unchanged.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Session-owned resources and per-session greeting snapshots must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes repeated Shiny server(data_source=...) sessions while preserving the public table-registration guard.
Changes:
- Extracts guard-free table registration logic.
- Adds regression tests for repeated sessions and guard preservation.
- Documents the behavior in the changelog.
File summaries
| File | Summary |
|---|---|
pkg-py/tests/test_server_data_source.py |
Adds repeated-session and guard-preservation tests. |
pkg-py/src/querychat/_shiny.py |
Uses deferred registration; critical (2 votes) session resource ownership and moderate (3 votes) greeting snapshot issues remain. |
pkg-py/src/querychat/_querychat_base.py |
Adds shared registration logic; critical (1 vote) resource cleanup issue and nit (1 vote) naming issue remain. |
pkg-py/CHANGELOG.md |
Documents the session behavior. |
Review details
Suppressed comments (1)
pkg-py/src/querychat/_querychat_base.py:501
- This new method has a leading underscore even though
_querychat_base.pyis already a private module. The repository guideline for private modules requires regular names without_prefixes; rename it toadd_or_replace_tableand update the caller so this cross-module helper is not presented as a hidden API.
def _add_or_replace_table(
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| include_in_greeting=include_in_greeting, | ||
| ) | ||
|
|
||
| def _add_or_replace_table( |
|
|
||
| qc.server(data_source=other_users_df) # must not raise | ||
|
|
||
| assert list(captured_mod_server[1]["data_sources"].keys()) == ["users"] |
This was referenced Sep 12, 2026
cpsievert
added a commit
that referenced
this pull request
Sep 12, 2026
$add_table() (which $server(data_source=) calls internally) refuses to run once .server_initialized is TRUE -- a guard meant to stop reconfiguring tables after a conversation is underway, but which also blocked a second Shiny session's own $server(data_source=) call. Even bypassing that, replacing a table cleaned up the resource (and cached query executor) an earlier, still-running session depended on, and the greeting_arg closure read live private$.data_sources/greeter$tables at async-invocation time, so a later session's registration could leak into an earlier session's greeting. Extracts $add_table()'s guard-free core into a private add_or_replace_table(), used directly by $server()'s data_source= path with cleanup_replaced = FALSE so it no longer tears down an earlier session's resource. $server() now also snapshots greeter$tables and passes it (plus the already-per-session data_sources) through mod_server() to greeter$build_client(), so the lazily-invoked greeting reads a point-in-time snapshot instead of live, mutable state. Mirrors the equivalent Python fixes (#302, #303,
cpsievert
added a commit
that referenced
this pull request
Sep 12, 2026
$add_table() (which $server(data_source=) calls internally) refuses to run once .server_initialized is TRUE -- a guard meant to stop reconfiguring tables after a conversation is underway, but which also blocked a second Shiny session's own $server(data_source=) call. Even bypassing that, replacing a table cleaned up the resource (and cached query executor) an earlier, still-running session depended on, and the greeting_arg closure read live private$.data_sources/greeter$tables at async-invocation time, so a later session's registration could leak into an earlier session's greeting. Extracts $add_table()'s guard-free core into a private add_or_replace_table(), used directly by $server()'s data_source= path with cleanup_replaced = FALSE so it no longer tears down an earlier session's resource. $server() now also snapshots greeter$tables and passes it (plus the already-per-session data_sources) through mod_server() to greeter$build_client(), so the lazily-invoked greeting reads a point-in-time snapshot instead of live, mutable state. Mirrors the equivalent Python fixes (#302, #303,
cpsievert
added a commit
that referenced
this pull request
Sep 12, 2026
$add_table() (which $server(data_source=) calls internally) refuses to run once .server_initialized is TRUE -- a guard meant to stop reconfiguring tables after a conversation is underway, but which also blocked a second Shiny session's own $server(data_source=) call. Even bypassing that, replacing a table cleaned up the resource (and cached query executor) an earlier, still-running session depended on, and the greeting_arg closure read live private$.data_sources/greeter$tables at async-invocation time, so a later session's registration could leak into an earlier session's greeting. Extracts $add_table()'s guard-free core into a private add_or_replace_table(), used directly by $server()'s data_source= path with cleanup_replaced = FALSE so it no longer tears down an earlier session's resource. $server() now also snapshots greeter$tables and passes it (plus the already-per-session data_sources) through mod_server() to greeter$build_client(), so the lazily-invoked greeting reads a point-in-time snapshot instead of live, mutable state. Mirrors the equivalent Python fixes (#302, #303,
cpsievert
added a commit
that referenced
this pull request
Sep 12, 2026
$add_table() (which $server(data_source=) calls internally) refuses to run once .server_initialized is TRUE -- a guard meant to stop reconfiguring tables after a conversation is underway, but which also blocked a second Shiny session's own $server(data_source=) call. Even bypassing that, replacing a table cleaned up the resource (and cached query executor) an earlier, still-running session depended on, and the greeting_arg closure read live private$.data_sources/greeter$tables at async-invocation time, so a later session's registration could leak into an earlier session's greeting. Extracts $add_table()'s guard-free core into a private add_or_replace_table(), used directly by $server()'s data_source= path with cleanup_replaced = FALSE so it no longer tears down an earlier session's resource. $server() now also snapshots greeter$tables and passes it (plus the already-per-session data_sources) through mod_server() to greeter$build_client(), so the lazily-invoked greeting reads a point-in-time snapshot instead of live, mutable state. Mirrors the equivalent Python fixes (#302, #303,
cpsievert
added this pull request to stack #307
September 12, 2026 16:09
cpsievert
added a commit
that referenced
this pull request
Sep 12, 2026
$add_table() (which $server(data_source=) calls internally) refuses to run once .server_initialized is TRUE -- a guard meant to stop reconfiguring tables after a conversation is underway, but which also blocked a second Shiny session's own $server(data_source=) call. Even bypassing that, replacing a table cleaned up the resource (and cached query executor) an earlier, still-running session depended on, and the greeting_arg closure read live private$.data_sources/greeter$tables at async-invocation time, so a later session's registration could leak into an earlier session's greeting. Extracts $add_table()'s guard-free core into a private add_or_replace_table(), used directly by $server()'s data_source= path with cleanup_replaced = FALSE so it no longer tears down an earlier session's resource. $server() now also snapshots greeter$tables and passes it (plus the already-per-session data_sources) through mod_server() to greeter$build_client(), so the lazily-invoked greeting reads a point-in-time snapshot instead of live, mutable state. Mirrors the equivalent Python fixes (#302, #303,
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #301
Depends on #301 (restore
server(data_source=)) — review that first. This PR's diff is just the guard bypass on top of it.The problem
add_table()refuses to run once_server_initializedisTrue— a guard meant to stop you from reconfiguring tables after a conversation is already underway. Butserver(data_source=)(added in #301) reuses that same publicadd_table(), and_server_initializedgets set toTruethe very first time.server()runs.Since every new Shiny session re-runs the server function, this means
server(data_source=)only ever works for the first session. A second browser tab, a page refresh, or simply a second concurrent user hits:(This is the same bug R's existing
$server(data_source = )has today — it isn't Python-specific.)The fix
add_table()'s body (name validation, staging the table, rebuilding the system prompt/executor cache, greeting inclusion) is extracted into a guard-free_add_or_replace_table().add_table()keeps its guard and calls into that core;.server(data_source=...)calls the core directly, bypassing the guard for this path only.The public
add_table()/add_tables()guard is unchanged — you still can't call those after.server()has run.Test plan
.server(data_source=...)calls (simulating two sessions) both succeed.add_table()still raises after.server()has run, confirming the public guard is untouched.test_multi_table.pystill passes.