Positron nb assistant copilot fixes #10654
Conversation
|
E2E Tests 🚀 |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the GitHub Copilot chat provider wasn't receiving notebook context in its system prompt when used with Positron Assistant. The changes ensure Copilot receives complete notebook information upfront (cells, outputs, variables) rather than needing tool calls, matching the behavior of the Anthropic provider. Additionally, Copilot's native notebook tools are filtered out when Positron notebook mode is active to prevent conflicts.
Key changes:
- Added notebook context retrieval in
generateAssistantPromptto pass context to the prompt renderer - Added filtering logic to disable Copilot's native notebook tools when Positron notebook mode is active
- Imported
getAttachedNotebookContextandSerializedNotebookContextto support notebook context handling
| // Get notebook context if available, with error handling | ||
| let notebookContext: SerializedNotebookContext | undefined; | ||
| try { | ||
| notebookContext = await getAttachedNotebookContext(request); | ||
| } catch (err) { | ||
| log.error('[PositronAssistantApi] Error checking notebook context:', err); | ||
| } |
There was a problem hiding this comment.
The error handling added here for getAttachedNotebookContext is inconsistent with the Edit and Agent participant implementations in participants.ts (lines 791 and 813), which don't have try-catch blocks. Consider adding similar error handling to those implementations as well, or document why this API path needs different error handling than the participant paths.
There was a problem hiding this comment.
I just tried to test these changes locally and ran into some issues with getting a copilot chat provider to use the notebook tools 😭 . Not sure if there's a specific setting that I may be missing?
unrelated to this PR, but executing cells seems horribly borked which is making it hard to test the changes with cell outputs. Curious if you experienced this at all?
Here's what I am seeing on my end:
Screen.Recording.2025-11-20.at.3.42.49.PM.mov
seeM
left a comment
seeM
left a comment
There was a problem hiding this comment.
I'm not too in the loop with Assistant changes so will let someone from the Assistant team review this one, once Dhruvi's issue has been addressed
Addresses #10653.
This PR fixes an issue where the GitHub Copilot chat provider wasn't receiving notebook context in its system prompt when used with Positron Assistant. The Copilot provider now uses the same prompt construction path as the Anthropic provider, ensuring it receives complete notebook information (cells, outputs, variables) upfront rather than needing tool calls for basic context.
Additionally, this PR filters out Copilot's native notebook tools when Positron notebook mode is active to prevent conflicts with Positron's specialized notebook tools.
Release Notes
New Features
Bug Fixes
QA Notes
@:assistant @:notebooks
Expected:
GetNotebookCells,EditNotebookCells,RunNotebookCells) not Copilot-native ones