Skip to content

Comments

fix(orm): fixed history context in chat mode#30

Merged
OriNachum merged 1 commit intomainfrom
bug/chat-conversation-context
May 19, 2025
Merged

fix(orm): fixed history context in chat mode#30
OriNachum merged 1 commit intomainfrom
bug/chat-conversation-context

Conversation

@OriNachum
Copy link
Collaborator

No description provided.

@OriNachum OriNachum requested a review from Copilot May 19, 2025 22:54
Copy link
Contributor

Copilot AI left a 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 PR fixes the history context in chat mode by enhancing conversation history management and updating chat completions processing. The key changes include:

  • Adding Node version control commands (nvm install/use) to test tools scripts.
  • Introducing a global conversation history dictionary with trimming logic in the server.
  • Updating functions in the chat completions flow to store and merge conversation history.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
test_tools/call_codex_mcp.sh Inserts nvm install/use commands to ensure the correct Node version.
test_tools/call_codex.sh Inserts nvm install/use commands similar to the MCP variant.
src/open_responses_server/server.py Adds conversation history logic and updates chat completions processing.
docs/prompts/add-chat-completions-history.md Documents new conversation history behavior.

messages.append(assistant_message)

# Add the tool response for immediate tools
if is_mcp:
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that 'is_mcp' is defined in this scope before its usage. For example, assign is_mcp = is_mcp_tool(tool_call['function']['name']) prior to the if-check to avoid undefined variable errors.

Copilot uses AI. Check for mistakes.
logger.info(f"Saved conversation history for response_id {response_id} with {len(messages)} messages")

# Trim conversation history if it grows too large
if len(conversation_history) > MAX_CONVERSATION_HISTORY:
Copy link

Copilot AI May 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider refactoring the conversation history trimming logic into a helper function to avoid code duplication and improve maintainability.

Copilot uses AI. Check for mistakes.
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@OriNachum OriNachum merged commit 8a06308 into main May 19, 2025
8 of 11 checks passed
@OriNachum OriNachum deleted the bug/chat-conversation-context branch May 19, 2025 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex chat mode: Conversation not kept within the request or between messages

1 participant