Skip to content

Conversation

@cpsievert
Copy link
Contributor

@cpsievert cpsievert commented Nov 24, 2025

Follow up to #101.

In that PR, I added the following methods to set initial values after initialization:

  1. set_system_prompt
  2. set_data_source
  3. set_client

Problem is, calling these methods after .server() is too late since they're effectively setting input values for the Shiny server module. This makes them effectively useless with Express.

Since these methods weren't all that useful to begin with, this PR removes them. It will, however, keep the ability to mutate the session-specific .client (where you can also change the .system_prompt). Also, by setting up a fork of the client earlier, we can drop the separate .system_prompt state, and make the per-session forks more efficient.

@cpsievert cpsievert changed the title fix(pkg-py): fix/disallow setting of initialization values post-server initializtion fix(pkg-py): fix/disallow setting of initialization values post-initialization Nov 24, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the ability to set initialization values (system_prompt, data_source, client) after initialization, as these methods were ineffective when called after .server() in Express mode. The changes optimize per-session chat client initialization by creating a forked, configured client during object construction rather than per-session.

Key changes:

  • Removed set_system_prompt(), set_data_source(), and set_client() methods
  • Converted data_source and client to private attributes with property accessors
  • Moved client forking and system prompt configuration to initialization time

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pkg-py/src/querychat/_querychat.py Removed setter methods, converted attributes to private with property accessors, moved client initialization to __init__, added categorical_threshold parameter
pkg-py/src/querychat/_querychat_module.py Removed system_prompt parameter and related setup since client now comes pre-configured

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cpsievert cpsievert marked this pull request as ready for review November 24, 2025 18:22
@cpsievert cpsievert merged commit ce0d0d3 into main Nov 24, 2025
11 checks passed
@cpsievert cpsievert deleted the fix/py-set-methods branch November 24, 2025 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants