Skip to content

refactor(models): Refine MessageAgentThought SQLAlchemy typing - #11

Open
tomerqodo wants to merge 2 commits into
codex_full_base_refactormodels_refine_messageagentthought_sqlalchemy_typing_pr11from
codex_full_head_refactormodels_refine_messageagentthought_sqlalchemy_typing_pr11
Open

refactor(models): Refine MessageAgentThought SQLAlchemy typing#11
tomerqodo wants to merge 2 commits into
codex_full_base_refactormodels_refine_messageagentthought_sqlalchemy_typing_pr11from
codex_full_head_refactormodels_refine_messageagentthought_sqlalchemy_typing_pr11

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5209691271

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 500 to 503
tool_call_response.append(
ToolPromptMessage(
content=tool_responses.get(tool, agent_thought.observation),
content=str(tool_inputs.get(tool, agent_thought.observation)),
name=tool,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use tool responses when building tool messages

The tool response message is currently populated from tool_inputs rather than the parsed observations, so when agent_thought.observation contains the actual tool output (JSON or plain text), the reconstructed tool message will echo the input instead of the response. This changes the conversation history fed back to the model and can lead to incorrect reasoning or summaries because the LLM never sees the tool output. The earlier logic used tool_responses.get(...); restoring that (or otherwise using tool_responses) will keep tool-call results consistent with stored observations.

Useful? React with 👍 / 👎.

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.

1 participant