-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Add AI assistance, markdown support for notebooks, and refactor… #40
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
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 pull request adds AI chat and notebook interoperability, enhanced markdown rendering with highlight.js and marked.js, refactored SQL templates into dedicated modules, and improved AI provider support with custom system prompts and updated default models.
Key changes:
- AI-generated SQL can now be sent directly to open PostgreSQL notebooks with visual feedback
- Integrated marked.js and highlight.js for improved markdown rendering and syntax highlighting
- Refactored SQL templates from
src/commands/helper.tsintosrc/commands/sql/helper.tsfor better maintainability - Enhanced AI service with custom system prompt support and updated default models (gpt-4o, gemini-1.5-flash)
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/providers/chat/webviewHtml.ts | Added notebook integration UI, marked.js/highlight.js support, CSP configuration, and new code block rendering with "Add to Notebook" button |
| src/providers/ChatViewProvider.ts | Implemented _handleOpenInNotebook method to insert SQL into active notebooks with error handling |
| src/providers/chat/AiService.ts | Added custom system prompt parameter, improved conversation history construction, updated default models, added timeout for model selection |
| src/notebookProvider.ts | Enhanced cell deserialization to support both markdown and SQL cell types |
| src/commands/sql/helper.ts | New file containing extracted SQL templates, query builders, and maintenance templates (1018 lines) |
| src/commands/helper.ts | Re-exports SQL utilities from sql/helper.ts for backward compatibility |
| src/commands/aiAssist.ts | Refactored to use AiService class, simplified task selector with reduced instruction templates |
| src/commands/sql/index.ts | Added exports for SQL_TEMPLATES, QueryBuilder, and MaintenanceTemplates |
| resources/marked.min.js | Added marked.js v15.0.12 library for markdown parsing |
| resources/highlight.css | Added highlight.js GitHub Dark theme CSS |
| docs/index.html | Updated version badge to 0.5.3 |
| README.md | Added export data feature documentation and SQL template directory structure |
| MARKETPLACE.md | Added export data feature to feature list |
| CHANGELOG.md | Added comprehensive release notes for versions 0.3.0 through 0.5.3 |
| if (!str) return ''; | ||
| return str | ||
| .replace(/&/g, '&') | ||
| .replace(/"/g, '"') |
Copilot
AI
Dec 13, 2025
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.
The escapeAttribute function is incomplete and does not handle single quotes. This could lead to attribute injection vulnerabilities when the escaped string is used in HTML attributes. The function should also escape single quotes by replacing them with ' or '.
| .replace(/"/g, '"') | |
| .replace(/"/g, '"') | |
| .replace(/'/g, ''') |
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${cspSource} 'unsafe-inline'; script-src ${cspSource} 'unsafe-inline';"> |
Copilot
AI
Dec 13, 2025
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.
The CSP (Content Security Policy) allows 'unsafe-inline' for both style-src and script-src. This weakens security protections against XSS attacks. Consider using nonces or hashes instead of 'unsafe-inline', especially for scripts. If inline scripts are necessary for the webview, at least restrict style-src to not use 'unsafe-inline'.
| <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${cspSource} 'unsafe-inline'; script-src ${cspSource} 'unsafe-inline';"> | |
| <meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src ${cspSource} 'unsafe-inline'; script-src ${cspSource};"> |
| code = codeElement.textContent || ''; | ||
| } | ||
| const rawCode = codeElement.getAttribute('data-raw'); | ||
| const code = rawCode !== null ? rawCode : (codeElement.textContent || ''); |
Copilot
AI
Dec 13, 2025
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.
The rawCode variable uses getAttribute which returns null if the attribute doesn't exist. The code correctly handles this with rawCode !== null, but then uses textContent which could also be null. This should use optional chaining or explicitly handle the null case: const code = rawCode !== null ? rawCode : (codeElement.textContent ?? '');
| const code = rawCode !== null ? rawCode : (codeElement.textContent || ''); | |
| const code = rawCode !== null ? rawCode : (codeElement.textContent ?? ''); |
| body = { | ||
| systemInstruction: systemPrompt ? { parts: [{ text: systemPrompt }] } : undefined, | ||
| contents: [ | ||
| ...conversationHistory.map(msg => ({ | ||
| role: msg.role === 'assistant' ? 'model' : 'user', | ||
| parts: [{ text: msg.content }] | ||
| })), | ||
| { role: 'user', parts: [{ text: userMessage }] } | ||
| ] | ||
| }; |
Copilot
AI
Dec 13, 2025
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.
The systemInstruction property in the Gemini API body is only added when systemPrompt is truthy, but the property itself is set to systemPrompt ? { parts: [{ text: systemPrompt }] } : undefined. This means when systemPrompt is an empty string or falsy, the property will be undefined. It would be cleaner to conditionally construct the body object to exclude the property entirely when systemPrompt is not provided.
|
|
||
|
|
||
| if (!content && provider === 'custom') { | ||
| content = JSON.stringify(response); // Fallback |
Copilot
AI
Dec 13, 2025
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.
The error handling fallback on line 305 stringifies the entire response object which could contain sensitive information (like API keys in error responses). This should log a generic error message or sanitize the response before including it in the error message.
| content = JSON.stringify(response); // Fallback | |
| content = response?.error?.message | |
| ? `API error: ${response.error.message}` | |
| : 'No content returned from API.'; |
| kind?: 'markdown' | 'sql'; | ||
| language?: 'markdown' | 'sql'; |
Copilot
AI
Dec 13, 2025
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.
The new markdown and SQL cell type support is added, but the Cell interface adds optional properties 'kind' and 'language' that are redundant. The NotebookCellKind enum already distinguishes between Code and Markup cells, and the language is specified in the NotebookCellData constructor. These optional properties in the interface may cause confusion and should be removed or documented why they're needed for backward compatibility.
| async selectTask(): Promise<string | undefined> { | ||
| const selection = await vscode.window.showQuickPick(this.tasks, { | ||
| placeHolder: 'Select an AI task for your SQL query', | ||
| ignoreFocusOut: true, | ||
| placeHolder: 'Select an AI task or enter custom instruction', | ||
| matchOnDescription: true, | ||
| matchOnDetail: true | ||
| matchOnDetail: true, | ||
| ignoreFocusOut: true | ||
| }); | ||
|
|
||
| if (!selection) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Find matching instruction key by extracting label text (remove icon) | ||
| const labelText = selection.label.replace(/^\$\([^)]+\)\s*/, ''); | ||
| const key = Object.keys(this.instructions).find(k => labelText === k); | ||
|
|
||
| if (!key) { | ||
| return undefined; | ||
| if (selection.label.includes('Custom Instruction')) { | ||
| return await vscode.window.showInputBox({ | ||
| placeHolder: 'e.g., "Rewrite this query to filter by user status"', | ||
| prompt: 'Enter your specific instruction for the AI', | ||
| ignoreFocusOut: true | ||
| }); | ||
| } | ||
|
|
||
| const instruction = this.instructions[key as keyof typeof AiTaskSelector.instructions]; | ||
|
|
||
| // If custom instruction, prompt user | ||
| if (instruction === null) { | ||
| const input = await vscode.window.showInputBox({ | ||
| placeHolder: 'Describe how you want to modify this query...', | ||
| prompt: 'Enter detailed instructions for the AI (e.g., "Add a join with the orders table and filter by date range")', | ||
| // For specific tasks that might need extra input | ||
| if (selection.label.includes('Add WHERE Clause')) { | ||
| const condition = await vscode.window.showInputBox({ | ||
| placeHolder: 'e.g., status = \'active\' AND created_at > NOW() - INTERVAL \'1 day\'', | ||
| prompt: 'Enter the filtering condition', | ||
| ignoreFocusOut: true | ||
| }); | ||
| return input; | ||
| if (!condition) return undefined; | ||
| return `Add WHERE clause: ${condition}`; | ||
| } | ||
|
|
||
| return instruction; | ||
| return selection.label.replace(/\$\([a-z-]+\)\s/, ''); // Return clean label as instruction | ||
| } |
Copilot
AI
Dec 13, 2025
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.
The simplified AI task selector removes detailed instruction templates and replaces them with basic label text. This significantly reduces the quality and specificity of prompts sent to the AI. For example, "Explain Query" previously had a comprehensive template explaining what to document, but now just sends the label text "Explain Query". This may result in less useful AI responses. Consider keeping the instruction templates or at least providing more detailed prompts.
… chart views, including `chart.js` dependency and related utilities.
…nnel to PgStudio.
This pull request delivers a new release (v0.5.3) of the PostgreSQL Explorer extension, focusing on bug fixes, improved stability, and significant enhancements to AI integration, notebook support, and code maintainability. The most notable changes include improved AI chat and notebook interoperability, expanded export capabilities, refactoring for better code organization, and updated documentation to reflect new features.
AI Integration and Notebook Interoperability
AI Provider Improvements
gpt-4o, Gemini usesgemini-1.5-flash). [1] [2] [3] [4] [5] [6] [7]Export and Output Enhancements
Codebase Refactoring and Maintainability
src/commands/sql/, improving maintainability and separation of concerns. [1] [2]UI and Resource Updates
highlight.jsand CSS) into the chat view for improved code rendering. [1] [2]These changes collectively enhance the extension's usability, developer experience, and AI-powered workflows.… SQL utilities.