ai: discover tools in deterministic order - #4902
Conversation
Tools.Discover inherited the registry iteration order, and the memory registry (and mDNS in practice) iterates a map — so the tool list was shuffled on every discovery. That silently defeats provider prompt caching: Anthropic cache_control and Gemini implicit caching both key on a byte-identical request prefix, and the tool catalogue is the bulk of that prefix. A shuffled catalogue is a cache write on every turn instead of a hit. Sort discovered tools by name. Custom tools added via AgentTool keep their insertion order after the discovered set. A test registers five services and asserts the order is sorted and survives repeated discovery across map-iteration shuffles.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4398f9f401
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This pull request makes tool discovery deterministic by sorting the discovered tool list by name, preventing registry map-iteration randomness from shuffling tools between calls. This is important for provider-side prompt caching (e.g., Anthropic prefix caching and Gemini implicit caching), which depends on a byte-identical request prefix and is heavily influenced by the serialized tools catalogue.
Changes:
- Sort
Tools.Discover()results by tool name to ensure a stable ordering across runs. - Add a unit test that registers multiple services and asserts the discovered tool list is sorted and remains identical across repeated discovery calls.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
ai/tools.go |
Sorts discovered tools by name to ensure deterministic tool ordering. |
ai/tools_test.go |
Adds a regression test asserting tool discovery order is sorted and stable across repeated calls. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review feedback: ListServices returns one entry per registered version of a service, so Discover resolved each name once per version and duplicated its tools — and GetService returns versions in map order, so full[0] chose a different version (and therefore different endpoint schemas and descriptions) on different discoveries, churning the serialized catalogue even with the names sorted. Dedupe service names, sort the resolved versions and take the highest so the same version wins every time, and make the final sort stable with OriginalName as tiebreak. The determinism test now registers two versions of one service with different endpoints and asserts exactly one tool set, from the highest version, on every round.
…y fixes (#4903) * docs(changelog): record the provider tool-loop, caching, and discovery fixes Add [Unreleased] entries for the three merged provider fixes: the cross-provider tool-execution loop (#4900), Anthropic prompt caching with the WithoutCache opt-out (#4901), and deterministic tool discovery (#4902) — so the next release rolls them into its section. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Why
Follow-up to #4901 (Anthropic prompt caching), and the answer to "what's the Gemini equivalent."
Investigating Gemini caching turned up a bug that undermines both:
Tools.Discoverinherits the registry's iteration order, and the memory registry ranges over a map — so the tool list is shuffled on every discovery. Provider prompt caching (Anthropiccache_control, Gemini implicit caching) keys on a byte-identical request prefix, and the tool catalogue is the bulk of that prefix. A shuffled catalogue means a cache write on every turn instead of a hit — paying the write premium repeatedly while never caching.Change
Sort discovered tools by name in
Tools.Discover(the freeDiscoverToolsdelegates to it). Custom tools added viaAgentToolkeep their insertion order after the discovered set. Go'sencoding/jsonalready sorts map keys, so with the slice order fixed the serialized prefix is fully deterministic.New test registers five services and asserts the order is sorted and survives repeated discovery across map-iteration shuffles.
On Gemini specifically
No provider code needed: implicit caching is on by default for Gemini 2.5+ models (go-micro's default is
gemini-2.5-flash) with ~90% off cached prefix tokens — it just requires the stable prefix this PR provides. ExplicitcachedContentscaching is a stateful resource with TTL + storage billing; enabling it silently could raise costs for idle agents, so it's deliberately not added. If wanted later, it should be an explicit opt-in.Verification
go test ./ai/(new determinism test),./agent/,./cmd/micro/, mock conformance 6/6 — green; gofmt/vet clean.🤖 Generated with Claude Code
Generated by Claude Code