Skip to content

Fix Codex named tool_choice serialization in reflect#734

Merged
nicoloboschi merged 1 commit into
vectorize-io:mainfrom
Sapientropic:SDY/openai-codex-reflect-pr-slim-20260328
Mar 30, 2026
Merged

Fix Codex named tool_choice serialization in reflect#734
nicoloboschi merged 1 commit into
vectorize-io:mainfrom
Sapientropic:SDY/openai-codex-reflect-pr-slim-20260328

Conversation

@Sapientropic

Copy link
Copy Markdown
Contributor

Summary

This fixes a Codex provider compatibility issue in reflect/tool-calling paths.

Reflect can force a specific tool on early iterations using the legacy named tool choice shape:

{"type":"function","function":{"name":"recall"}}

The current Codex Responses API expects the function name at the top level instead:

{"type":"function","name":"recall"}

Without that normalization, Codex rejects the request and reflect can end up with empty based_on.memories even when recallable evidence exists.

What changed

  • normalize legacy named tool_choice dicts in CodexLLM.call_with_tools()
  • add a regression test that verifies the serialized payload no longer contains tool_choice.function
  • add a regression test that verifies forced tool choice still yields tool calls

Why this belongs in the provider

This follows the same design direction as the existing provider-specific tool-choice compatibility handling for LM Studio / OpenAI-compatible providers:

  • the reflect agent keeps its provider-agnostic intent
  • each provider adapts the request shape it needs

Validation

Targeted regression:

python -m pytest -o addopts="" --noconftest hindsight-api-slim/tests/test_codex_tool_choice.py -q

Result:

2 passed

I also verified the original failure mode before the patch via a live Codex-backed daemon, where reflect requests were rejected with:

400 Unknown parameter: 'tool_choice.function'

After this patch, the same integration recovered real reflect evidence instead of failing in the forced-tool iterations.

Non-goals

This PR does not claim to fix every possible reflect empty-evidence case.
It specifically fixes the Codex provider request-shape incompatibility for forced named tool choice.

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thansk, can you use pytest for the test? Just follow the codebase standard

@chriscoey

chriscoey commented Mar 28, 2026

Copy link
Copy Markdown

Can confirm this bug. I'm using openai-codex provider with gpt-5.4 and reflect was returning "I don't have information" despite recall finding 12 results for the same query. The Responses API expects {"type": "function", "name": "recall"} but the agent sends {"type": "function", "function": {"name": "recall"}}. Applied the same fix locally and reflect works correctly now.

@nicoloboschi nicoloboschi merged commit 3573e53 into vectorize-io:main Mar 30, 2026
nicoloboschi added a commit that referenced this pull request Mar 30, 2026
Follow-up to #734: replace unittest.TestCase + manual sys.path
manipulation with idiomatic pytest + @pytest.mark.asyncio,
matching the rest of the test suite.
nicoloboschi added a commit that referenced this pull request Mar 30, 2026
Follow-up to #734: replace unittest.TestCase + manual sys.path
manipulation with idiomatic pytest + @pytest.mark.asyncio,
matching the rest of the test suite.
nicoloboschi added a commit that referenced this pull request Mar 30, 2026
* Convert codex tool_choice test to pytest style

Follow-up to #734: replace unittest.TestCase + manual sys.path
manipulation with idiomatic pytest + @pytest.mark.asyncio,
matching the rest of the test suite.

* Fix test_hierarchical_fields_categorization for new configurable fields

Update expected count from 20 to 21 and add assertions for fields
added by recent PRs: retain_default_strategy, retain_strategies,
max_observations_per_scope, reflect_source_facts_max_tokens,
llm_gemini_safety_settings, mcp_enabled_tools.

* Add LlamaIndex doc to v0.4 versioned docs and sidebars

The LlamaIndex integration doc was added to docs/ (next version) in
#672 but not to versioned_docs/version-0.4/, causing a broken link
on the /integrations page which resolves to the latest version.

* Regenerate docs skill references

Run generate-docs-skill.sh to pick up new integration pages
(codex, llamaindex) and updated configuration docs.

* Add Codex integration doc to v0.4 versioned docs and sidebar

Same issue as LlamaIndex: doc was added to docs/ (next) but not
versioned_docs/version-0.4/, causing broken link on /integrations.
@Sapientropic Sapientropic deleted the SDY/openai-codex-reflect-pr-slim-20260328 branch March 30, 2026 11:25
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.

4 participants