-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Allow QueryChat usage outside of Shiny app #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
and fix a few small issues along the way
pkg-py/src/querychat/_querychat.py
Outdated
| # Default to query-only for console (privacy) | ||
| if isinstance(tools, MISSING_TYPE) and (new or self._client_console is None): | ||
| tools = ("query",) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Query is the less private tool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it's not obvious to me why .console() should have different default tools from .app()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, no that comment isn't right, it's actually the opposite. I'll fix that.
.console() needs different tools from .app() because the update dashboard tool doesn't make sense in the console where there's no dashboard to update. Really, only the query tool makes sense and it's safe for that to be the default because this for interactive use, not something you'd put into production.
You could go through hoops to set up update_dashboard in a way that it works or makes sense for something you're working on, but tools = "query" is the right default choice for .console().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be a little more concrete about the hoops: .console() accepts kwargs that are passed to .client() which you could use to setup update_dashboard and reset_dashboard callbacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
BTW, this PR does start to make me think that it'd useful to have non-Shiny/reactive versions of .df(), etc., that you could read directly from the instance. And in that case, tools would also now update the values that those methods return. Seems this would be somewhat useful R and also might end up being really nice for supporting streamlit, etc on the Python side. I was sort of planning on dedicating some time to streamlit this week, so maybe I'll explore this idea once this PR is done?
It does also kinda push me a direction of thinking a .chat() and .stream() method might be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the non-reactive .df(), but as to .chat() and .stream() -- I created .client() to help us go down that path. My intuition is that in Streamlit, notebook, etc. contexts you might be able to get pretty far with superclasses that can rely on .client(), or maybe we'll need the .chat() and .stream() methods.
I don't think we should build any of these right this minute, but the goal of .client() is to make it easier to test and find out.
cpsievert
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition, thanks! Good with me once CoPilot has a final say.
Fixes #159
Fixes #117
New
client()method - QueryChat objects can now create standalone chat clients with configurable tools and callbacks, enabling use outside of Shiny applications.New
console()method - Launch interactive console-based chat sessions with your data source, with persistent conversation state across invocations (seems like a feature we might want to bring to theapp()method, but there it's a little more complicated, so that can be follow-up).Configurable tool selection - This happened to be easy to fold into this change. A new
toolsparameter allows selective inclusion of "query" and/or "update" tools, enabling privacy-focused workflows that prevent the LLM from accessing data.console()console()defaults to usingtools = "query", since theupdatetool doesn't make sense in that contextclient()defaults to the instance'stoolsvalue but can be overridden directly.client()method with the default: use the instance setting fortools