Positron Notebooks: Add ghost cell suggestions for AI-powered next step hints#11622
Positron Notebooks: Add ghost cell suggestions for AI-powered next step hints#11622
Conversation
|
E2E Tests 🚀 |
nstrayer
force-pushed
the
positron-nb-ghost-suggestions
branch
from
7d29cdf to
7acc211
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds AI-powered ghost cell suggestions to Positron Notebooks that appear after successful cell execution with suggested next code steps. The feature integrates with existing multi-provider AI infrastructure and includes streaming support, per-notebook settings, and a refined UX with split buttons for accept/dismiss actions.
Changes:
- Added ghost cell suggestion feature that appears after successful cell execution with AI-generated next step recommendations
- Created reusable SplitButton component and refactored NotebookCellQuickFix to use it
- Added per-notebook and global configuration settings for ghost cell suggestions with debounce delay control
- Updated notebook metadata handling to correctly persist settings in ipynb format via metadata.metadata path
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/positronNotebook/common/positronNotebookConfig.ts | Added configuration keys for ghost cell suggestions and delay settings |
| src/vs/workbench/contrib/positronNotebook/common/notebookAssistantMetadata.ts | Updated metadata handling to use metadata.metadata path for ipynb persistence and added ghost cell settings support |
| src/vs/workbench/contrib/positronNotebook/browser/utilityComponents/SplitButton.tsx | New reusable split button component with main action and dropdown menu |
| src/vs/workbench/contrib/positronNotebook/browser/utilityComponents/SplitButton.css | Styling for split button component with hover and focus states |
| src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellQuickFix.tsx | Refactored to use new SplitButton component, simplified code |
| src/vs/workbench/contrib/positronNotebook/browser/notebookCells/NotebookCellQuickFix.css | Updated CSS to work with SplitButton component |
| src/vs/workbench/contrib/positronNotebook/browser/notebookCells/GhostCellInfoModalDialog.tsx | Info modal explaining ghost cell feature with link to settings |
| src/vs/workbench/contrib/positronNotebook/browser/notebookCells/GhostCellInfoModalDialog.css | Styling for info modal |
| src/vs/workbench/contrib/positronNotebook/browser/notebookCells/GhostCell.tsx | Main ghost cell component with loading, streaming, ready, and error states |
| src/vs/workbench/contrib/positronNotebook/browser/notebookCells/GhostCell.css | Comprehensive styling for ghost cell states and animations with accessibility support |
| src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts | Core logic for ghost cell state management, debouncing, and command integration |
| src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookComponent.tsx | Added GhostCell component to notebook rendering |
| src/vs/workbench/contrib/positronNotebook/browser/IPositronNotebookInstance.ts | Added GhostCellState type and interface methods for ghost cell operations |
| src/vs/workbench/contrib/positronNotebook/browser/AssistantPanel/AssistantPanel.tsx | Added ghost cell suggestions toggle to assistant panel settings |
| extensions/positron-assistant/src/notebookAssistantMetadata.ts | Added ghost cell suggestions metadata handling in extension |
| extensions/positron-assistant/src/md/prompts/notebook/ghost-cell.md | AI prompt template for generating ghost cell suggestions |
| extensions/positron-assistant/src/ghostCellSuggestions.ts | Extension logic for generating ghost cell suggestions with LLM |
| extensions/positron-assistant/src/extension.ts | Command registration for ghost cell suggestion generation |
| .ghost-cell-info-button:hover { | ||
| color: var(--vscode-foreground); | ||
| } | ||
|
|
There was a problem hiding this comment.
The info button (ghost-cell-info-button) has hover styles but is missing focus-visible styles for keyboard accessibility. Users navigating with keyboard should see a clear focus indicator when the button receives focus. Consider adding a :focus-visible rule similar to other focusable elements in the component (e.g., like the regenerate button at lines 144-150).
| .ghost-cell-info-button:focus-visible { | |
| color: var(--vscode-foreground); | |
| outline: 1px solid var(--vscode-focusBorder); | |
| outline-offset: 2px; | |
| } |
| dismissGhostCell(disableForNotebook?: boolean): void { | ||
| // Cancel any pending request | ||
| if (this._ghostCellDebounceTimer) { | ||
| clearTimeout(this._ghostCellDebounceTimer); | ||
| this._ghostCellDebounceTimer = undefined; | ||
| } | ||
|
|
||
| if (this._ghostCellCancellationToken) { | ||
| this._ghostCellCancellationToken.cancel(); | ||
| this._ghostCellCancellationToken.dispose(); | ||
| this._ghostCellCancellationToken = undefined; | ||
| } | ||
|
|
||
| // Hide ghost cell | ||
| this._ghostCellState.set({ status: 'hidden' }, undefined); | ||
|
|
||
| // Optionally disable for this notebook | ||
| if (disableForNotebook) { | ||
| const textModel = this._textModel.get(); | ||
| if (textModel) { | ||
| const newMetadata = setAssistantSettings({ ...textModel.metadata }, { ghostCellSuggestions: 'disabled' }); | ||
| textModel.applyEdits([{ | ||
| editType: CellEditType.DocumentMetadata, | ||
| metadata: newMetadata | ||
| }], true, undefined, () => undefined, undefined, true); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The PR description states "Creating a new cell should dismiss the ghost cell," but this behavior is not implemented. When a user manually adds a new cell (not via accepting the ghost cell suggestion), the ghost cell should be dismissed to avoid confusion. Consider adding logic to dismiss the ghost cell in response to cell additions, either by listening to cell change events or by calling dismissGhostCell from relevant methods.
| const dismissActions = React.useMemo((): IAction[] => [ | ||
| { | ||
| id: 'ghost-cell-dismiss', | ||
| label: dismissLabel, | ||
| tooltip: dismissLabel, | ||
| class: undefined, | ||
| enabled: true, | ||
| run: handleDismiss | ||
| }, | ||
| { | ||
| id: 'ghost-cell-disable-for-notebook', | ||
| label: dontSuggestInNotebookLabel, | ||
| tooltip: dontSuggestInNotebookLabel, | ||
| class: undefined, | ||
| enabled: true, | ||
| run: handleDisableForNotebook | ||
| }, | ||
| { | ||
| id: 'ghost-cell-dont-suggest-again', | ||
| label: dontSuggestAgainLabel, | ||
| tooltip: dontSuggestAgainLabel, | ||
| class: undefined, | ||
| enabled: true, | ||
| run: handleDisableGlobally | ||
| } | ||
| ], [handleDismiss, handleDisableForNotebook, handleDisableGlobally]); |
There was a problem hiding this comment.
The dismiss dropdown includes "Dismiss" as the first option, which duplicates the main button's action. This is redundant and may confuse users. Consider removing the "Dismiss" option from the dropdown and only including the "Don't suggest for this notebook" and "Don't suggest again" options. The main button already provides the basic dismiss functionality.
src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts
Outdated
Show resolved
Hide resolved
| - After an error: The most likely fix | ||
| - After calculations: One way to inspect or visualize the result | ||
|
|
||
| ## Output Format | ||
|
|
||
| You MUST return only valid XML in the output, and nothing else. Use the following structure: | ||
|
|
||
| ```xml | ||
| <suggestion> | ||
| <explanation>Brief description of what this code does and why it's a logical next step (1-2 sentences)</explanation> | ||
| <code> | ||
| # Comment explaining the suggestion | ||
| your_code_here() | ||
| </code> | ||
| </suggestion> | ||
| ``` | ||
|
|
||
| ## Examples | ||
|
|
||
| ### Example 1: After loading a DataFrame | ||
|
|
||
| Context: User just ran `df = pd.read_csv('data.csv')` and got successful output showing the DataFrame loaded | ||
|
|
||
| ```xml | ||
| <suggestion> | ||
| <explanation>Preview the first few rows to see what the data looks like.</explanation> | ||
| <code> | ||
| df.head() | ||
| </code> | ||
| </suggestion> | ||
| ``` | ||
|
|
||
| ### Example 2: After creating a visualization | ||
|
|
||
| Context: User just created a scatter plot with `plt.scatter(x, y)` | ||
|
|
||
| ```xml | ||
| <suggestion> | ||
| <explanation>Add axis labels to make the plot easier to interpret.</explanation> | ||
| <code> | ||
| plt.xlabel('X Variable') | ||
| plt.ylabel('Y Variable') | ||
| plt.title('Scatter Plot') | ||
| plt.show() | ||
| </code> | ||
| </suggestion> | ||
| ``` | ||
|
|
||
| ### Example 3: After an error | ||
|
|
||
| Context: User got a KeyError when trying to access a column | ||
|
|
||
| ```xml | ||
| <suggestion> | ||
| <explanation>Check the available columns to find the correct column name.</explanation> | ||
| <code> | ||
| df.columns.tolist() | ||
| </code> | ||
| </suggestion> | ||
| ``` |
There was a problem hiding this comment.
The prompt template mentions "After an error: The most likely fix" (line 30) and includes Example 3 for error scenarios (lines 78-89), but the PositronNotebookInstance.ts implementation only triggers ghost cell suggestions after successful cell execution (when lastRunSuccess === true). This discrepancy means the LLM is being instructed to handle error cases that will never occur in practice. Consider either removing the error-related guidance from the prompt or updating the implementation to also suggest cells after error cases (if that's the desired behavior).
| } else { | ||
| // No suggestion generated, hide ghost cell | ||
| this._ghostCellState.set({ status: 'hidden' }, undefined); | ||
| } |
There was a problem hiding this comment.
The callback command disposable is not disposed when the suggestion result is null (line 2317). While the disposable will eventually be garbage collected, it should be explicitly disposed to follow consistent cleanup patterns and avoid potential memory leaks. Consider wrapping the promise chain in a finally block or ensuring disposal happens in all paths.
| // Auto-dismiss error after 5 seconds | ||
| setTimeout(() => { | ||
| const currentState = this._ghostCellState.get(); | ||
| if (currentState.status === 'error') { | ||
| this._ghostCellState.set({ status: 'hidden' }, undefined); | ||
| } | ||
| }, 5000); |
There was a problem hiding this comment.
The auto-dismiss error timeout (line 2334) is not cleaned up when the instance is disposed. If the instance is disposed while the 5-second timeout is pending, the callback will still execute and try to access the disposed instance's state. This should be stored as a class field and cleared in the dispose() method, similar to how _ghostCellDebounceTimer is handled.
| // Auto-dismiss error after 5 seconds | |
| setTimeout(() => { | |
| const currentState = this._ghostCellState.get(); | |
| if (currentState.status === 'error') { | |
| this._ghostCellState.set({ status: 'hidden' }, undefined); | |
| } | |
| }, 5000); | |
| // Auto-dismiss error after 5 seconds. Tie the timeout to the current cancellation token | |
| const errorAutoDismissTimeout = setTimeout(() => { | |
| if (token.isCancellationRequested) { | |
| return; | |
| } | |
| const currentState = this._ghostCellState.get(); | |
| if (currentState.status === 'error') { | |
| this._ghostCellState.set({ status: 'hidden' }, undefined); | |
| } | |
| }, 5000); | |
| token.onCancellationRequested(() => { | |
| clearTimeout(errorAutoDismissTimeout); | |
| }); |
Adds AI-powered ghost cell suggestions that appear after code execution, suggesting potential next steps based on notebook context. The feature includes: - GhostCell component with loading, streaming, and error states - LLM integration via positron-assistant extension - Configuration setting to enable/disable the feature - Keyboard shortcuts (Tab to accept, Escape to dismiss) - Assistant panel controls for toggling suggestions
Insert accepted ghost cell at end of notebook (where ghost is displayed) rather than after the executed cell. Also documents known rough edges including streaming, error checking, and missing tests.
- Add "Single Responsibility" guideline - each suggestion should do ONE thing - Add "Role Distinction" section explaining ghost cells vs chat pane - Simplify examples to show focused, bite-sized suggestions - Remove rigid "under 20 lines" constraint in favor of natural sizing
- Only trigger ghost cell suggestions after successful cell execution (skip failed cells to avoid suggesting fixes for errors) - Register a callback command to receive streaming partial content during suggestion generation for responsive UI updates
Introduces a reusable SplitButton component that combines a primary action button with a dropdown menu for additional options. This pattern is used in ghost cells and quick fix buttons to provide secondary actions without cluttering the UI.
- Change ghost cell primary action from "Accept" to "Accept and Run" - Add split button dropdowns to ghost cell actions with additional options - Add "Don't suggest again" option to dismiss dropdown that disables ghost cell suggestions globally - Refactor NotebookCellQuickFix to use the new SplitButton component, reducing code duplication
Wrap buildFixQuery and buildExplainQuery in useCallback to properly track dependencies in the useMemo hooks for dropdown actions.
Adds an info button (i icon) next to the sparkle icon in the ghost cell header. Clicking it opens a modal dialog explaining what ghost cell suggestions are, how they work, and how to disable them. The dialog includes a link to open the assistant panel settings.
- Fix metadata access path to use metadata.metadata.positron (matching ipynb serializer behavior) instead of metadata.positron - Refactor setAssistantSettings to iterate over updates dynamically instead of individual if-statements per setting - Extract updateNotebookSettings helper in AssistantPanel to reduce code duplication across setting change handlers - Remove debug console.log statements
Adds a new setting `positron.assistant.notebook.ghostCellSuggestions.delay` to control how long to wait after cell execution before triggering ghost cell suggestions. Default is 2000ms (range: 500-10000ms). This replaces the previously hardcoded 3 second delay.
Change margin-left from 19px to 12px so the ghost cell's dashed border aligns with regular notebook cell borders.
Add _enabledThisSession flag to bypass config read delays when user clicks Enable. Change enableGhostCellSuggestions() to sync with fire-and-forget config updates, matching disableGhostCellSuggestions pattern.
Rename ghostCellSuggestions to ghostCellSuggestions.enabled for consistency with ghostCellSuggestions.delay. Update info modal to show a clickable link that opens the setting directly instead of an icon button that opened the assistant panel.
Add a "pull mode" option that shows a placeholder after cell execution instead of automatically requesting suggestions. Users can trigger suggestions via button or Cmd+Shift+G keyboard shortcut. Includes a toggle in the ghost cell UI to switch between automatic and on-demand modes, and supports per-notebook override via metadata.
Change the info modal to use "settings" as link text instead of the raw setting name, matching the common pattern used elsewhere in the codebase. Also update the query to show all ghost cell settings, not just enabled.
Add thoughts/ to .gitignore to prevent accidental commits.
Register positron.assistant.notebook.ghostCellSuggestions.model setting to configure which model generates ghost cell suggestions. The setting follows the same pattern as other ghost cell settings. Also display the selected model name in the ghost cell info modal when a suggestion is ready, so users can see which model generated it.
seeM
left a comment
seeM
left a comment
There was a problem hiding this comment.
Really epic PR @nstrayer! 🚀 This worked really smoothly for me in my testing. I have lots of thoughts for improvement further below.
My only blocking issue is that "Don't suggest in this notebook again" worked, but I couldn't figure out how to re-enable it again. I wonder if a setting might work well there so it can be easily edited?
Detailed feedback
I ran a cell that loaded a dataframe, and ~2 seconds later saw an inline prompt to enable suggestions ✅ . BTW I really like the feeling of this inline prompt UI, wonder where else it'd be useful!
I like the difference between normal vs hover styling of the prompt. Currently, the hover transitions over a short delay which differs from the other notebook transitions that are instant. Thoughts on making this consistent - either putting the transition everywhere or removing it from here?
I clicked enable, saw it say "Generating suggestions..." for a split second and then disappear ❌ bug?
I ran the same cell again, and it said "Generating suggestions..." followed by the suggestion cell ✅:
The description text is truncated and I can view the full text by hovering and seeing the tooltip, but a tooltip doesn't feel like a comfortable way to read a medium-sized description. In all of my examples, the description was much longer than the space allowed and truncated. Maybe a collapse/expand approach?
I can toggle the Automatic/On-demand setting.
Double clicking Automatic/On-demand probably shouldn't select the text though.
This might be me being a bit silly, but after toggling Automatic/On-demand a few times I lost track of which visual treatment meant which one was actually selected 😅
Most things in Positron have subtle hover style changes - could we add something subtle for hovering on the Automatic/On-demand toggle?
It also feels a little odd that the Automatic/On-demand button uses blue but the Accept and Run button uses gray and Dismiss white.
Thoughts on moving the About button to the right hand side? The left feels like really valuable real-estate and the About button feels like it'll be infrequently used.
I also wonder if the sparkles icon is necessary, given the dashed border and otherwise pretty distinguished look of the suggestion cell?
The vertical black line feels out of place here:
Switching to another editor and back keeps the suggestion cell ✅ but there is a small animation as it appears.
Thoughts on making the cursor a pointer when over the about button instead of a question mark? Not sure what is preferred but I am generally surprised when the cursor becomes a question mark, maybe because that's not often used.
The blue borders around the Dismiss and Regenerate buttons feel inconsistent with buttons elsewhere in the notebook and in Positron.
Clicking Accept & Run added a cell with the contents and ran it ✅ and followed up with another cell suggestion - that immediate followup was super satisfying BTW 👌
Idea: I felt like I wanted to use the keyboard to navigate to the suggestion as if it were just another cell type, and then use Shift+Enter to accept and run it. Felt natural and surprised that I couldn't. But we might also not want to go this route.
I didn't change the settings but was surprised to see these in my settings file Can we avoid adding settings manually and instead changing our defaults?
"positron.assistant.notebook.ghostCellSuggestions.hasOptedIn": true,
"positron.assistant.notebook.ghostCellSuggestions.mode": "push",
"positron.assistant.notebook.ghostCellSuggestions.enabled": true,
Dismiss button worked ✅
Can we streamline the UI for on-demand mode? I feel we could do without "AI suggestion available on request", and perhaps we could move the buttons next to the Automatic/on-demand toggle so it can be a slim single line.
Future: Might be nice to get syntax highlighting in the suggestions source since it makes it easier to review the suggestion.
| 1. **Single Responsibility**: Each suggestion should do ONE thing. If you're tempted to chain multiple operations, pick the most valuable one. | ||
| 2. **Be Actionable**: The suggested code should run immediately without modification | ||
| 3. **Be Obvious**: Suggest the natural, low-friction next step - not a multi-step analysis pipeline | ||
| 4. **Be Contextual**: Base your suggestion on what the user just executed and its results |
There was a problem hiding this comment.
These seemed to be followed well in my testing
src/vs/workbench/contrib/positronNotebook/browser/IPositronNotebookInstance.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/positronNotebook/browser/ContextKeysManager.ts
Outdated
Show resolved
Hide resolved
Adds a quick pick UI for selecting which model to use for ghost cell suggestions. Users can access the picker via: - Command palette: "Select Ghost Cell Suggestion Model" - Settings: Click "Select a model" link in the setting description - Ghost cell footer: Click the model name after a suggestion is shown Also adds fallback warning indicator when configured model is unavailable.
nstrayer
force-pushed
the
positron-nb-ghost-suggestions
branch
from
0eabbd7 to
da22b87
Compare
When users click "Don't suggest in this notebook again", they previously had no discoverable way to re-enable suggestions. This adds: - New command "Enable Ghost Cell Suggestions for This Notebook" in the command palette (searchable via F1) - Info notification with "Re-enable" button shown when disabling - enableGhostCellSuggestionsForNotebook() method that clears the per-notebook disabled metadata and triggers a suggestion
When clicking Enable, the workbench fires async config updates then immediately triggers a suggestion. The extension reads the stale config and returns null, causing the ghost cell to disappear. Added skipConfigCheck parameter to bypass the extension-side config check when the workbench has already verified the suggestion should proceed (e.g., user just clicked Enable).
- Add user-select: none to prevent text selection on double-click - Add hover effect for non-selected toggle buttons - Update sizing to match ActionBarToggle (22px container, 20px buttons) - Update border-radius to match ActionBarToggle (6px)
Refactor suggestion mode from string enum ('push'/'pull') to boolean
(automatic). Remove separate hasOptedIn setting - now detect user choice
by inspecting whether enabled setting has been explicitly set.
UI improvements:
- Add expand/collapse for truncated explanations
- Simplify awaiting-request layout to single row
- Move info button to footer alongside model indicator
…us styles Remove fade-in animation that replayed when switching editors, change transitions from 200ms to 0.1s to match notebook patterns, use :focus-visible for button outlines, and change info button cursor to pointer. Update PR feedback tracking with completed items.
These internal development documents were accidentally committed and should not be part of the feature PR.
Create a standalone SegmentedToggle component in positronComponents that encapsulates the two-option segmented toggle pattern previously duplicated across AssistantPanel and GhostCell. Refactor both consumers to use it. Also extract SettingToggleRow in AssistantPanel to reduce repetition across the three settings rows, and consolidate renderGhostCellState parameters into a context object for readability.
Move all ghost cell state, config, actions, and logic out of PositronNotebookInstance into a self-contained GhostCellController contribution, following the same pattern as contrib/find. This keeps the notebook instance focused on core notebook concerns and makes the ghost cell feature independently maintainable.
Remove noise from the branch diff: trailing blank line in IPositronNotebookInstance, empty ghost-cell region comment block, unused INotificationService injection, and extra blank line between _register() calls.
| const positron = metadata?.positron as Record<string, unknown> | undefined; | ||
| // Access inner metadata (this is what gets serialized to ipynb file) | ||
| const innerMetadata = metadata?.metadata as Record<string, unknown> | undefined; | ||
| const positron = innerMetadata?.positron as Record<string, unknown> | undefined; |
There was a problem hiding this comment.
The same breaking change exists in the extension version of this file. Existing notebooks with assistant settings stored at metadata.positron will have those settings ignored after this change. Add migration logic to check the old path (metadata.positron) as a fallback when the new path (metadata.metadata.positron) is not found.
| const positron = innerMetadata?.positron as Record<string, unknown> | undefined; | |
| const positronFromInner = innerMetadata?.positron as Record<string, unknown> | undefined; | |
| // Migration fallback: older notebooks stored settings at metadata.positron.assistant | |
| const positronFromRoot = metadata?.positron as Record<string, unknown> | undefined; | |
| const positron = positronFromInner ?? positronFromRoot; |
|
|
||
| // State for ghost cell suggestions setting | ||
| const [ghostCellSuggestionsOverride, setGhostCellSuggestionsOverride] = useState<GhostCellSuggestionsOverride>(undefined); | ||
| const globalGhostCellSuggestions = configurationService.getValue<boolean>(POSITRON_NOTEBOOK_GHOST_CELL_SUGGESTIONS_KEY) ?? true; |
There was a problem hiding this comment.
The default value for globalGhostCellSuggestions is set to true, but according to the config file (config.ts line 38), the actual default is false. This mismatch means the UI will show an incorrect state when the user hasn't explicitly set a value. Change the default to false to match the configuration default.
| const globalGhostCellSuggestions = configurationService.getValue<boolean>(POSITRON_NOTEBOOK_GHOST_CELL_SUGGESTIONS_KEY) ?? true; | |
| const globalGhostCellSuggestions = configurationService.getValue<boolean>(POSITRON_NOTEBOOK_GHOST_CELL_SUGGESTIONS_KEY) ?? false; |
| // Setting enabled to false marks the user's explicit choice | ||
| // Use undefined to remove when setting matches default (false) |
There was a problem hiding this comment.
The comment says "Setting enabled to false marks the user's explicit choice" but the code actually sets the value to undefined (line 381). The comment is misleading. Update the comment to accurately reflect that setting to undefined removes the user override and falls back to the default (false).
| // Setting enabled to false marks the user's explicit choice | |
| // Use undefined to remove when setting matches default (false) | |
| // Remove user override so the setting falls back to its default (false/disabled) |
| const resizeObserver = new ResizeObserver(checkTruncation); | ||
| if (spanRef.current) { | ||
| resizeObserver.observe(spanRef.current); | ||
| } | ||
|
|
||
| return () => resizeObserver.disconnect(); | ||
| }, [text, isExpanded]); | ||
|
|
There was a problem hiding this comment.
The ResizeObserver created on line 94 is not properly cleaned up when the element changes. If spanRef.current changes between renders, the observer will continue observing the old element. Save the observed element in the effect and only disconnect/observe when the element reference actually changes. Also, the effect dependencies should include spanRef.current to handle ref changes.
| const resizeObserver = new ResizeObserver(checkTruncation); | |
| if (spanRef.current) { | |
| resizeObserver.observe(spanRef.current); | |
| } | |
| return () => resizeObserver.disconnect(); | |
| }, [text, isExpanded]); | |
| const currentSpan = spanRef.current; | |
| if (!currentSpan) { | |
| return; | |
| } | |
| const resizeObserver = new ResizeObserver(checkTruncation); | |
| resizeObserver.observe(currentSpan); | |
| return () => { | |
| resizeObserver.unobserve(currentSpan); | |
| resizeObserver.disconnect(); | |
| }; | |
| }, [text, isExpanded, spanRef.current]); |
| tabIndex={disabled ? -1 : 0} | ||
| title={dropdownTooltip} | ||
| onKeyDown={(e) => { | ||
| if ((e.key === 'Enter' || e.key === ' ') && !disabled) { |
There was a problem hiding this comment.
The dropdown button handles keyboard events for Enter and Space keys but doesn't prevent the default browser behavior. For Space key specifically, this could cause the page to scroll when activating the dropdown. Add e.preventDefault() inside the keyboard handler to prevent unwanted default behaviors.
| if ((e.key === 'Enter' || e.key === ' ') && !disabled) { | |
| if ((e.key === 'Enter' || e.key === ' ') && !disabled) { | |
| e.preventDefault(); |
| const assistant = positron?.assistant as Record<string, unknown> | undefined; | ||
|
|
There was a problem hiding this comment.
The metadata access path has changed from metadata?.positron to metadata?.metadata?.positron. This is a breaking change that affects how metadata is read from notebooks. The comment explains this is to match ipynb serialization, but existing notebooks with settings stored at the old path (metadata.positron) will lose their assistant settings after this change. Consider adding migration logic to read from the old path as a fallback to preserve existing user settings.
| const assistant = positron?.assistant as Record<string, unknown> | undefined; | |
| let assistant = positron?.assistant as Record<string, unknown> | undefined; | |
| // Fallback for legacy metadata location: metadata.positron.assistant | |
| if (!assistant && metadata) { | |
| const legacyRoot = metadata as unknown as Record<string, unknown>; | |
| const legacyPositron = legacyRoot.positron as Record<string, unknown> | undefined; | |
| assistant = legacyPositron?.assistant as Record<string, unknown> | undefined; | |
| } |
| metadata: newMetadata | ||
| }], true, undefined, () => undefined, undefined, true); | ||
|
|
||
| commandService.executeCommand('workbench.action.files.save'); |
There was a problem hiding this comment.
The updateNotebookSettings helper automatically saves the file after updating metadata (line 520), but this happens synchronously without waiting for the save to complete or handling potential save failures. The fire-and-forget executeCommand call could fail silently. Consider awaiting the save command and handling errors, or at least logging if the save fails to give users feedback when their settings changes don't persist.
| // Type narrowing for cell execution events | ||
| const cellEvent = event; | ||
| if (cellEvent.type !== NotebookExecutionType.cell) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The type narrowing check on line 104 is redundant since line 98 already checks that event.type !== NotebookExecutionType.cell and returns early. The additional check on line 104 can never be true due to the earlier guard. Remove this redundant type check.
| // Type narrowing for cell execution events | |
| const cellEvent = event; | |
| if (cellEvent.type !== NotebookExecutionType.cell) { | |
| return; | |
| } | |
| // Type narrowing for cell execution events (already ensured by the guard above) | |
| const cellEvent = event; |
| item.mime === 'application/vnd.code.notebook.error' || | ||
| item.mime === 'application/vnd.code.notebook.stderr' |
There was a problem hiding this comment.
The error detection logic treats stderr output as an error (line 125), but stderr is commonly used for warnings, progress bars, or informational messages in data science workflows, not just errors. This could lead to inappropriate ghost cell suggestions after non-error stderr output. Consider checking for actual error mime types only (application/vnd.code.notebook.error) or examining the execution result status more carefully.
| item.mime === 'application/vnd.code.notebook.error' || | |
| item.mime === 'application/vnd.code.notebook.stderr' | |
| item.mime === 'application/vnd.code.notebook.error' |
| const timeoutPromise = new Promise<never>((_, reject) => { | ||
| setTimeout(() => reject(new Error('Ghost cell suggestion timed out')), timeoutMs); | ||
| }); | ||
|
|
||
| const responsePromise = model.sendRequest([systemMessage, userMessage], {}, token); | ||
| const response = await Promise.race([responsePromise, timeoutPromise]); |
There was a problem hiding this comment.
The timeout mechanism creates a Promise that rejects after 30 seconds but doesn't clear the setTimeout timer if the race completes early. This means the timer continues running unnecessarily and the error handler closure stays in memory. Store the timeout ID and clear it in a finally block or when the race completes.
| const timeoutPromise = new Promise<never>((_, reject) => { | |
| setTimeout(() => reject(new Error('Ghost cell suggestion timed out')), timeoutMs); | |
| }); | |
| const responsePromise = model.sendRequest([systemMessage, userMessage], {}, token); | |
| const response = await Promise.race([responsePromise, timeoutPromise]); | |
| let timeoutId: ReturnType<typeof setTimeout>; | |
| const timeoutPromise = new Promise<never>((_, reject) => { | |
| timeoutId = setTimeout(() => reject(new Error('Ghost cell suggestion timed out')), timeoutMs); | |
| }); | |
| const responsePromise = model.sendRequest([systemMessage, userMessage], {}, token); | |
| const response = await (async () => { | |
| try { | |
| return await Promise.race([responsePromise, timeoutPromise]); | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } | |
| })(); |
seeM
left a comment
seeM
left a comment
There was a problem hiding this comment.
Changes are looking good! This seems good to merge but still have a few suggestions from the previous batch and some new:
- Top padding of description and buttons shouldn't shift down when expanding description
- Not sure how to re-enable for notebook after "Don't suggest in this notebook again"
- Thoughts on removing the hover animation - make instant?
- Add hover state to the selected (white) toggle button
- Remove Dismiss border on hover
- Lighter color for Accept button vertical separator
- 12px font for model indicator
Future:
- Add a way to accept & run via keyboard
- Can it make suggestions at any point in the notebook - not only the end?
- Could we update the notebooks rule with a short instruction about notebook editor extensions/contributions?
| * Hook to access the GhostCellController contribution from the current notebook instance. | ||
| * Must be used within a NotebookInstanceProvider. | ||
| */ | ||
| export function useGhostCellController(): GhostCellController { |
| @@ -0,0 +1,677 @@ | |||
| /*--------------------------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
Thoughts on moving this to contrib/ghostCell as well?
| @@ -30,8 +30,10 @@ import { IConfigurationService } from '../../../../../platform/configuration/com | |||
| import { IChatEditingService, IModifiedFileEntry, ModifiedFileEntryState } from '../../../chat/common/chatEditingService.js'; | |||
There was a problem hiding this comment.
Could also move to assistant notebook contribution?
2 participants
Addresses #11413.
Summary
Adds ghost cell suggestions to Positron Notebooks -- an AI-powered feature that suggests a logical next code cell after successful cell execution. The ghost cell appears at the bottom of the notebook with suggested code that users can accept, dismiss, or regenerate.
Key implementation details:
haiku/miniNotebook: Enable Ghost Cell Suggestions for This Notebook) available in the command palette after per-notebook disableScreenshots/Demo
next-suggestions.mov
New setting for notebook/global
Info Modal
Dismiss Options
Release Notes
New Features
Bug Fixes
QA Notes
@:positron-notebooks @:assistant
Prerequisites:
Test scenarios:
Opt-in prompt (first run):
Basic flow (automatic/push mode - default when enabled):
import pandas as pd; df = pd.read_csv('data.csv'))On-demand/pull mode:
positron.assistant.notebook.ghostCellSuggestions.automatictofalseMode toggle:
Model picker:
Accept actions:
Dismiss actions:
Re-enable for notebook:
Notebook: Enable Ghost Cell Suggestions for This Notebookfrom the command paletteRegenerate:
Info modal:
AssistantPanel settings:
Settings:
positron.assistant.notebook.ghostCellSuggestions.enabledpositron.assistant.notebook.ghostCellSuggestions.delay(500-10000ms)positron.assistant.notebook.ghostCellSuggestions.automatic(true/false)positron.assistant.notebook.ghostCellSuggestions.model(array of patterns)Edge cases:
Show Your Work Extension Link: vscode://nstrayer.show-your-work/open?gist=f863c156bf49e6d314070b51faa86a2a