Skip to content

refactor(llm): type provider tool choices - #2843

Merged
nicoloboschi merged 7 commits into
vectorize-io:mainfrom
Jordan-Jarvis:upstream/refactor-typed-tool-choice-20260716
Jul 21, 2026
Merged

refactor(llm): type provider tool choices#2843
nicoloboschi merged 7 commits into
vectorize-io:mainfrom
Jordan-Jarvis:upstream/refactor-typed-tool-choice-20260716

Conversation

@Jordan-Jarvis

Copy link
Copy Markdown
Contributor

Summary

  • replace the internal str | dict tool-choice union with a canonical typed value
  • serialize provider-specific tool-choice wire shapes only at provider boundaries
  • make named-tool validation explicit and remove runtime payload-shape guessing
  • preserve provider retry configuration and response-usage tracing from current upstream main

Dependency

This is intentionally stacked on #2734 because it types the named-tool behavior introduced there. Review the final refactor commit 7a71d8c11; after #2734 lands, the base will contain the prerequisite behavior and this PR will show only the typed cutover.

Validation

  • Ruff check passed on all 17 changed files
  • Ruff format check passed on all 17 changed files
  • 41 focused provider/Reflect tests passed sequentially
  • engine compileall passed

A broader local file-level run reached 82 passing tests; four unrelated integration/setup cases failed because the local environment had no .env API key and two parallel workers timed out waiting on the shared embeddings fixture lock. The focused deterministic suite above is the authoritative change validation.


Replacement for #2737. The original PR became unreopenable after its submitting fork was deleted. This PR restores the exact reviewed head commit and preserves the original implementation without modification.

…red-tool-choice-20260716' into upstream/refactor-typed-tool-choice-20260716
…yped-tool-choice-20260716

# Conflicts:
#	hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py
#	hindsight-api-slim/hindsight_api/engine/reflect/agent.py
#	hindsight-api-slim/tests/test_tool_choice_required_downgrade.py
@Jordan-Jarvis

Copy link
Copy Markdown
Contributor Author

Ready for maintainer merge. This is the exact restored replacement for unreopenable #2737. Head 54f14f3 is already merged with current upstream main e07ca1b; the merge tree was independently reconstructed and verified byte-for-byte. All upstream checks are green. Please merge this one first because #2844 builds on the typed tool-choice contract.

@Jordan-Jarvis

Copy link
Copy Markdown
Contributor Author

cc @nicoloboschi — all checks are green; this is first in the merge order.

@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.

Approve. Behavior-preserving typed refactor: verified provider serialization maps 1:1 to the old string/dict logic across all providers, ty check passes, and the four tool-choice unit suites pass (27 tests). Non-blocking follow-ups: Anthropic provider still ignores tool_choice (pre-existing); a small direct unit test for LLMToolChoice invariants would be nice.

@nicoloboschi
nicoloboschi merged commit 1865071 into vectorize-io:main Jul 21, 2026
2 checks passed
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.

2 participants