Skip to content

Conversation

@diego-coder
Copy link

Fixes #308

This PR creates end-to-end tool-calling support for ChatMLX, allowing it to correctly use tools with compatible models running locally on Apple Silicon.

Context and History

This is a new, comprehensive approach to resolving #308 (assigned by @mdrxy). This work supersedes the previous attempt in PR #133, which was confirmed to be non-functional and is now stale with unresolved conflicts. This new implementation is fully functional, follows LangChain Core conventions, and is covered by extensive testing.

Technical Implementation

The solution is broken down into three main parts:

  1. Prompt Formatting: The _to_chat_prompt method has been updated to accept a tools argument, which is then passed to the tokenizer's apply_chat_template. This ensures the model receives tool definitions in the correct format, enabling it to generate tool-call responses.

  2. Response Parsing: The _to_chat_result method has been completely overhauled with a dual-strategy parsing approach:

  • It first checks for structured tool call data in generation_info, leveraging a model's native tool-calling capabilities when available.

  • If no native tool calls are found, it falls back to a ReAct-style text parser (Action: / Action Input:) to extract tool calls from the raw text, ensuring compatibility with a wide range of models.

  • It correctly uses langchain_core helpers to create ToolCall and InvalidToolCall objects.

  1. Comprehensive Testing: The PR adds new tests to validate the full functionality:
  • Unit tests to verify that tool definitions are correctly passed to the tokenizer.
  • An end-to-end integration test using an upgraded model mlx-community/phi-3-mini-128k-instruct to confirm the entire tool-calling lifecycle (prompting, generation, and parsing) works as expected.

@diego-coder
Copy link
Author

Sup @mdrxy! Checked for conflicts, updated, pull request is now ready for CI checks. This fixes #308.

@mdrxy
Copy link
Contributor

mdrxy commented Nov 5, 2025

@diego-coder thank you and apologies for the delay. working thru the queue

@diego-coder
Copy link
Author

Honestly @mdrxy you work too hard, I see you! I see what you do around here. I will update my others as well since I am on. I just updated #209 which you just checked on with a merge, so conflicts there should now be resolved. I will send like, one more message with the complete package so I'm not repeatedly pinging you, I'm guessing your inbox is a four alarm 🔥. Get to them whenever you can

@diego-coder diego-coder force-pushed the codex/update-_to_chat_prompt-to-accept-tools-parameter branch from df2c96a to b06ac9d Compare November 5, 2025 02:54
@diego-coder diego-coder force-pushed the codex/update-_to_chat_prompt-to-accept-tools-parameter branch from df9948b to 3d45f27 Compare November 5, 2025 04:43
@diego-coder
Copy link
Author

Hi @mdrxy,

As promised, here is the final summary of my open PRs. I've just finished running the full rebase and lint-fix workflow on all of them.

They are all up-to-date with main and all CI errors (including inherited lint/test failures) should now be resolved. They are just awaiting CI approval to run.


Ready for Review (All CI should pass)

Special case here

  • #32255 (Pydantic Schema): This is the one outlier. I successfully rebased and fixed all the complex merge conflicts. I also ran the full lint-fix process, which auto-fixed 56 errors.
  • However, the CI is still failing with 38 remaining linting errors (mostly UP038 and S603). These errors are all in main branch files that my PR doesn't otherwise touch.
  • I'm hesitant to manually patch 38 unrelated files across the core library. So I've pushed the branch "as-is" with all the manual conflict-fixes and the auto-fixes applied.
  • Question: What's the best path forward here? I'm happy to do all the manual fixes if you'd like (would want to make sure they will get reviewed at some point), or we can wait for a project-wide lint.

Thanks again for all your help and for working through this backlog with me. No rush

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants