-
Notifications
You must be signed in to change notification settings - Fork 108
Improve assistant package workflow #7900
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
Improve assistant package workflow #7900
Conversation
E2E Tests 🚀 |
bd1c9df
to
1888bb6
Compare
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 couldn't get this to work for Python. In particular:
- the language model thinks all installed packages are loaded/imported
- the language model still writes
import
statements for packages it thinks are loaded
I don't think it will be possible to get this to work with the approach of invoking a Python script b/c we need to introspect the in-memory state of the specific python session the user has active at the Console.
It might be possible to co-opt the variables comm for this b/c loaded modules are currently explicitly excluded but we could include them. Alternately we could use something callMethod
on the R side to invoke sys.modules.keys()
on the Python side.
import json | ||
import importlib.metadata | ||
installed_packages = [f"{dist.metadata['Name']} ({dist.version})" for dist in importlib.metadata.distributions()] | ||
print(json.dumps(installed_packages)) |
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.
This is in the loaded/attached tool, but is querying for all installed packages. In my testing it made Assistant think that every installed package was also loaded.
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.
Thank you! I rewrote the tool to use callMethod and added a new rpc to get the loaded modules.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Melissa Barca <[email protected]>
Signed-off-by: Melissa Barca <[email protected]>
6ff40d4
to
ecf2b87
Compare
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.
Pull Request Overview
This PR improves the assistant package workflow by renaming existing R LLM tools to use language-specific names and adding corresponding Python tools for checking loaded and installed modules.
- Renames R tools in both TypeScript and package.json for clarity.
- Introduces Python tools to gather module information via a new RPC method and a helper Python script.
- Updates package assistant prompts to include a package management workflow.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
extensions/positron-r/src/llm-tools.ts | Renames R tool identifiers to include the R language prefix. |
extensions/positron-r/package.json | Updates tool names and descriptions to reflect the R-specific context. |
extensions/positron-python/src/client/llm-tools/llm-tools.ts | Implements Python tools to get loaded modules and package version. |
extensions/positron-python/src/client/extensionActivation.ts | Registers the new Python language model tools. |
extensions/positron-python/python_files/posit/positron/ui.py | Adds the new _get_loaded_modules RPC method. |
extensions/positron-python/package.json | Adds definitions for the new Python language model tools. |
extensions/positron-assistant/src/md/prompts/chat/default.md | Updates assistant prompts with package management guidelines. |
Comments suppressed due to low confidence (1)
extensions/positron-python/python_files/posit/positron/ui.py:72
- It appears that the sys module is referenced without an explicit import within this file. Confirm that sys is imported or add 'import sys' as needed to avoid runtime errors.
if not name.startswith("_") and isinstance(kernel.shell.user_ns[name], type(sys))
Co-authored-by: Copilot <[email protected]> Signed-off-by: Melissa Barca <[email protected]>
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.
33711b9
to
0dda015
Compare
return [ | ||
name | ||
for name in kernel.shell.user_ns | ||
if not name.startswith("_") and isinstance(kernel.shell.user_ns[name], type(sys)) |
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!
Addresses #6967
Make assistant more aware of the current package and module environment.
Release Notes
New Features
Bug Fixes
QA Notes