Positron notebook quick-fix errors #10920
Conversation
…and update button event types. Changed fix and explain button handlers to simplify event management and removed unnecessary refs. Updated comments for clarity.
…waste tokens and create a jumbled mess in the chat.
|
E2E Tests 🚀 |
There was a problem hiding this comment.
Pull request overview
This PR adds error handling quick-fix functionality to Positron notebooks by introducing "Fix" and "Explain" buttons that appear below error outputs. These buttons integrate with Positron Assistant to help users understand and resolve errors directly from notebook cells.
- Implements split button UI with dropdown options for both "Fix" and "Explain" actions
- Primary action creates a new chat session in agent mode, dropdown option continues in existing chat
- Strips ANSI escape codes from error output before sending to assistant
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| NotebookCellQuickFix.tsx | New React component implementing split buttons with primary actions (new chat) and dropdown menus (existing chat) for fix/explain operations |
| NotebookCellQuickFix.css | Styles for split button UI matching action bar design patterns with hover states |
| CellTextOutput.tsx | Integration point that conditionally renders quick-fix buttons when output type is 'error' |
| tabIndex={0} | ||
| title={explainDropdownTooltip} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { |
There was a problem hiding this comment.
The space key handler should prevent default behavior to avoid scrolling the page. Add e.preventDefault(); inside the condition to prevent unwanted page scroll when activating the dropdown with the space key.
| * Primary click starts a fresh chat session in agent mode (with tool access to modify notebooks), | ||
| * dropdown opens in the existing chat panel to retain conversation context. |
There was a problem hiding this comment.
The comment states that the primary click "starts a fresh chat session in agent mode (with tool access to modify notebooks)", but it should also mention that the dropdown option retains conversation context in the existing chat. Consider rephrasing to: "Primary click starts a fresh chat session in agent mode (with tool access to modify notebooks). Dropdown provides option to continue in existing chat to retain conversation context."
| * Primary click starts a fresh chat session in agent mode (with tool access to modify notebooks), | |
| * dropdown opens in the existing chat panel to retain conversation context. | |
| * Primary click starts a fresh chat session in agent mode (with tool access to modify notebooks). | |
| * Dropdown provides option to continue in existing chat to retain conversation context. |
|
|
||
| const actions: IAction[] = [ | ||
| { | ||
| id: 'continue-in-existing-chat', |
There was a problem hiding this comment.
Both fix and explain dropdown menus use the same action ID 'continue-in-existing-chat'. While this works functionally, it would be better to use unique IDs like 'fix-in-existing-chat' and 'explain-in-existing-chat' for better debuggability and consistency.
| id: 'continue-in-existing-chat', | |
| id: 'fix-in-existing-chat', |
|
|
||
| const actions: IAction[] = [ | ||
| { | ||
| id: 'continue-in-existing-chat', |
There was a problem hiding this comment.
Both fix and explain dropdown menus use the same action ID 'continue-in-existing-chat'. While this works functionally, it would be better to use unique IDs like 'fix-in-existing-chat' and 'explain-in-existing-chat' for better debuggability and consistency.
| .notebook-cell-quick-fix .assistant-action-dropdown { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| padding: 0 4px; | ||
| border-left: 1px solid var(--vscode-positronActionBar-selectBorder); | ||
| cursor: pointer; | ||
| user-select: none; | ||
| align-self: stretch; | ||
| border-radius: 0 3px 3px 0; | ||
| } |
There was a problem hiding this comment.
Missing focus-visible style for keyboard navigation. Add a rule like .notebook-cell-quick-fix .assistant-action-dropdown:focus-visible { outline: 1px solid var(--vscode-focusBorder); outline-offset: -1px; } to provide visual feedback for keyboard users.
| tabIndex={0} | ||
| title={fixDropdownTooltip} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { |
There was a problem hiding this comment.
The space key handler should prevent default behavior to avoid scrolling the page. Add e.preventDefault(); inside the condition to prevent unwanted page scroll when activating the dropdown with the space key.
dhruvisompura
left a comment
dhruvisompura
left a comment
There was a problem hiding this comment.
Worked pretty well for me!
| { | ||
| type === 'error' | ||
| ? <NotebookCellQuickFix errorContent={content} /> | ||
| : null | ||
| } |
There was a problem hiding this comment.
One minor thing I did notice is that you can't use the keyboard to navigate to the fix/explain buttons in the output since the output div element isn't focusable!
Addresses #8607.
This PR adds "Fix" and "Explain" buttons to error outputs in notebooks, bringing notebook error handling to parity with the console experience. When an error occurs in a notebook cell, users now see quick action buttons directly in the output that allow them to:
The implementation integrates with Positron Assistant and strips ANSI codes from errors before sending them to avoid token waste and improve chat clarity.
Demo
quick-fix-error-demo.mov
Release Notes
New Features
Bug Fixes
QA Notes
@:positron-notebooks @:assistant