Conversation
- Enhanced the executeQuery method to better handle conversation history and user messages, ensuring compliance with SDK requirements. - Introduced a default tools array for allowedTools, simplifying the options setup. - Updated the getAvailableModels method to use type assertions for model tiers and ensured proper return type with TypeScript's satisfies operator. - Added error handling during query execution to log and propagate errors effectively.
… handling - Added a postinstall script in package.json to set permissions for spawn-helper on macOS. - Refactored the terminal WebSocket connection handling in index.ts for improved readability and consistency. - Enhanced error logging and connection management in the terminal service. - Cleaned up formatting and indentation across multiple files for better code clarity.
- Refactored init.sh to introduce a reusable function for killing processes on specified ports, improving code clarity and maintainability. - Added a cleanup function to ensure proper resource management on exit. - Updated server startup logic in index.ts to handle port conflicts gracefully, providing clear error messages and suggestions for resolution. - Improved logging for server status and health checks during initialization.
Summary of ChangesHello @webdevcody, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the stability and user experience of the server and development environment. Key changes include more robust server startup procedures with better port conflict resolution, refined handling of conversation history within the Claude AI provider for improved multi-turn interactions, and a crucial fix for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a number of improvements, primarily focused on code formatting, developer experience, and robustness. Key changes include:
- Improved startup error handling in
apps/server/src/index.ts, especially for port conflicts (EADDRINUSE), which is a great addition for developers. - Refactoring of the
init.shscript to be more robust, with better process cleanup and conditional server startup. - Several fixes and modernizations in
apps/server/src/providers/claude-provider.ts, such as addingtry-catchblocks, fixing a bug with multipart prompts, and using modern TypeScript features likesatisfies.
However, I've identified a critical issue in claude-provider.ts where changes to handle conversation history inadvertently drop assistant messages, leading to a loss of conversation context for the model. My review includes a suggestion to fix this while also improving the code's structure.
| // Convert history to SDK message format | ||
| // Note: When using async generator, SDK only accepts SDKUserMessage (type: 'user') | ||
| // So we filter to only include user messages to avoid SDK errors | ||
| const historyMessages = convertHistoryToMessages(conversationHistory); | ||
| const hasAssistantMessages = historyMessages.some( | ||
| (msg) => msg.type === "assistant" | ||
| ); | ||
|
|
||
| if (hasAssistantMessages) { | ||
| // If we have assistant messages, use async generator but filter to only user messages | ||
| // This maintains conversation flow while respecting SDK type constraints | ||
| promptPayload = (async function* () { | ||
| // Filter to only user messages - SDK async generator only accepts SDKUserMessage | ||
| const userHistoryMessages = historyMessages.filter( | ||
| (msg) => msg.type === "user" | ||
| ); | ||
| for (const msg of userHistoryMessages) { | ||
| yield msg; | ||
| } | ||
|
|
||
| // Yield current prompt | ||
| const normalizedPrompt = normalizeContentBlocks(prompt); | ||
| const currentPrompt = { | ||
| type: "user" as const, | ||
| session_id: "", | ||
| message: { | ||
| role: "user" as const, | ||
| content: normalizedPrompt, | ||
| }, | ||
| parent_tool_use_id: null, | ||
| }; | ||
| yield currentPrompt; | ||
| })(); | ||
| } else { | ||
| // Only user messages in history - can use async generator normally | ||
| promptPayload = (async function* () { | ||
| for (const msg of historyMessages) { | ||
| yield msg; | ||
| } | ||
|
|
||
| // Yield current prompt | ||
| const normalizedPrompt = normalizeContentBlocks(prompt); | ||
| const currentPrompt = { | ||
| type: "user" as const, | ||
| session_id: "", | ||
| message: { | ||
| role: "user" as const, | ||
| content: normalizedPrompt, | ||
| }, | ||
| parent_tool_use_id: null, | ||
| }; | ||
| yield currentPrompt; | ||
| })(); | ||
| } |
There was a problem hiding this comment.
This change introduces a critical issue. By filtering out assistant messages from the conversation history (lines 88-90), you are sending an incomplete history to the model. This will lead to incorrect responses as the model will lack the full context of the conversation. The comment on line 85, // This maintains conversation flow..., is incorrect as this change breaks the conversation flow.
While the SDK's async generator for the prompt may only accept SDKUserMessage (with type: 'user'), a better approach is to switch to a different payload format when assistant messages are present. Given that you've updated promptPayload's type to include Array<any>, you can pass the entire history as an array. This avoids using the async generator in that case and preserves the full conversation context.
The suggested change below fixes this critical bug and also reduces code duplication by defining currentPrompt once.
// Convert history to SDK message format
const historyMessages = convertHistoryToMessages(conversationHistory);
const hasAssistantMessages = historyMessages.some(
(msg) => msg.type === "assistant"
);
const normalizedPrompt = normalizeContentBlocks(prompt);
const currentPrompt = {
type: "user" as const,
session_id: "",
message: {
role: "user" as const,
content: normalizedPrompt,
},
parent_tool_use_id: null,
};
if (hasAssistantMessages) {
// Note: When using an async generator, the SDK may only accept user messages.
// To pass a full conversation with assistant replies, we must pass the history as an array.
// This preserves the full conversation context.
promptPayload = [...historyMessages, currentPrompt];
} else {
// Only user messages in history, so we can use the async generator.
promptPayload = (async function* () {
for (const msg of historyMessages) {
yield msg;
}
yield currentPrompt;
})();
}- Added a hasInstallScript property to package-lock.json. - Refactored the app-store.ts file for improved readability by formatting function parameters and object properties. - Updated the default terminal shortcut from "Cmd+`" to "T" and implemented migration logic for state persistence. - Incremented version number in the terminal state management to reflect breaking changes.
… service - Added a default categories.json file to the project initialization structure. - Improved code formatting and readability in the auto-mode-service.ts file by restructuring console log statements and method calls. - Updated feature status checks to include "backlog" in addition to "pending" and "ready".
- Updated the stopAutoLoop method to emit the "auto_mode_stopped" event immediately when the loop is explicitly stopped, enhancing event handling. - Improved code readability by restructuring feature retrieval calls in integration tests for better clarity.
1 participant
No description provided.