Thanks for the PR — fixes real Gemini compatibility issues and the test coverage is solid. Two design concerns worth discussing before merging:

1. __thought_signature leaks into tool execution

The signature stashing in arguments round-trips correctly back to Gemini, but between parse and re-serialization the tool call flows through the agent loop like any other, so tools actually receive __thought_signature in their arguments when execute() is called. Most built-in tools ignore unknown keys, but OpenAPI-adapted tools (see src/openapi/) and MCP tools that do strict JSON schema validation may reject it.

A cleaner fix would be to add an optional field to Content::ToolCall:

ToolCall {
    id: String,
    name: String,
    arguments: serde_json::Value,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    provider_metadata: Option<serde_json::Value>,
},

This follows the precedent of Content::Thinking { signature } and keeps provider-specific data out of the tool-facing payload. Happy to do this as a follow-up if you'd prefer to land this PR as-is.

2. "google-fc-" prefix as a sentinel

The id filter uses !tool_call_id.starts_with("google-fc-") to distinguish synthetic from real IDs. If Google ever returns an ID that happens to start with "google-fc-", the ID would be silently dropped and multi-turn function calling would break — the exact thing this PR is fixing. An Option<String> on Content::ToolCall (or storing synthetic-ness out-of-band) would be more robust, but this is a minor nit given the low likelihood.

Neither is a blocker. Nice work on the unit + integration test coverage.

One small thing for the integration tests: gemini-3-flash-preview isn't a real model name — likely gemini-2.5-flash or similar. Since they're #[ignore]d it won't break CI, but worth updating so someone can actually run them.

🤖 Generated with Claude Code

@cablehead thanks for trusting yoagent!

Thanks for the thoughtful review -- and for building yoagent, it's a great foundation to work with.

Pushed updates addressing both points:

1. provider_metadata instead of __thought_signature in arguments

Added an optional provider_metadata: Option<serde_json::Value> field to Content::ToolCall. The Gemini provider stores {"thought_signature": "..."} there, and reads it back when building the next request. Tools never see it.

2. Dropped the "google-fc-" prefix

Gemini returns real function call IDs, so the synthetic fallback was unnecessary. Now it's just fc.id.clone().unwrap_or_default() and the check is !id.is_empty(). No sentinel strings.

Re: gemini-3-flash-preview -- it does resolve and work (tested against the live API), but happy to change it to gemini-2.5-flash if you'd prefer a GA model name in the tests.

Thanks for the iteration — provider_metadata and dropping the google-fc- sentinel are exactly the right shape. But the refactor (looks like cargo fix autoexpansion) introduced a load-bearing pattern bug that reintroduces the very behavior this PR is fixing.

Blocking: literal provider_metadata: None used as a match guard

The new field is destructured with a literal None in five places. That makes it a refinement filter, not a default — any ToolCall carrying Some(metadata) falls through and is silently dropped.

src/agent_loop.rs:382 — the headline regression:

.filter_map(|c| match c {
    Content::ToolCall {
        provider_metadata: None,   // ← excludes Some(...)
        id, name, arguments,
    } => Some(...)

Gemini sets Some({"thought_signature": ...}) on every function call when thinking is on. The agent loop extracts zero tool calls, dispatches nothing, and the loop terminates. End-to-end, the PR's fix doesn't run tools on the exact path it's targeting.

Same pattern in four more sites — latent today (Azure/Responses never set metadata, Vertex sets None), but each one is a landmine that silently corrupts the next provider that populates the field:

• src/provider/azure_openai.rs:164 — StopReason::ToolUse detection
• src/provider/openai_responses.rs:184 — same
• src/provider/azure_openai.rs:248, openai_responses.rs:271, openai_compat.rs:275, google_vertex.rs:302 — request-body builders silently strip ToolCalls with metadata from outgoing requests

Fix: replace provider_metadata: None, with .. in all non-construction sites. Construction sites (where you build a fresh Content::ToolCall) are correct as written.

Important

Empty function call IDs collide — src/provider/google.rs:139 uses fc.id.clone().unwrap_or_default(). Gemini commonly omits id outside parallel function calling, so every call gets id = "". The agent loop correlates tool results to calls by id; two empty IDs in the same turn collide silently. Suggest preserving the synthetic fallback for the None branch:

let id = fc.id.clone().unwrap_or_else(|| format!("google-fc-{}", content.len()));

Breaking API change — adding provider_metadata to Content::ToolCall (a public enum variant) breaks any downstream crate that constructs the variant by name. Marking Content (or just the variant) #[non_exhaustive] would prevent this from biting again.

SSE parse errors swallowed at debug! — src/provider/google.rs:107. A schema change from Google would silently truncate turns with no signal in default logging. Should be warn! at minimum.

Test coverage gaps

• No agent-loop test for ToolCall with Some(provider_metadata) — a MockProvider-based test in tests/agent_loop_test.rs would have caught the blocking bug above. MockProvider (src/provider/mock.rs:99) hardcodes provider_metadata: None and needs an injection point.
• test_parse_chunk_with_crlf_sse is vacuous — it re-implements the separator detection inside the test body and asserts on its own copy. Breaking the real loop wouldn't break the test.
• test_parse_chunk_with_empty_text only verifies that serde parses {"text": ""}, not the continue guard at google.rs:118.
• Integration tests in tests/integration_gemini.rs are #[ignore] + require GEMINI_API_KEY → won't run in CI → zero regression protection. The thought-signature round-trip can be expressed against MockProvider.

Minor

• tests/integration_gemini.rs still uses gemini-3-flash-preview — confirmed working live, but a GA name would be safer.
• Option<serde_json::Value> for provider_metadata is fully untyped; future-proofing this as enum ProviderMetadata { Gemini { thought_signature: String }, ... } would give compile-time exhaustiveness and prevent key collisions across providers. Not blocking — just worth considering.

Suggested action

A single sweep replacing literal provider_metadata: None, with .. in the five non-construction sites unblocks the merge. The other items can land in follow-ups.

Good catch on the provider_metadata: None pattern bug -- that was a mechanical sed that went wrong. Fixed in all 7 non-construction sites (replaced with ..).

Also addressed:

• Restored synthetic google-fc-{n} fallback for models that omit id
• Upgraded SSE parse error logging from debug! to warn!

The test coverage gaps are fair points. Happy to add a MockProvider-based test for ToolCall with Some(provider_metadata) if you'd like that in this PR, or as a follow-up.

Follow-up commit adds the MockProvider test: test_tool_call_with_provider_metadata_executes verifies the agent loop executes tools when provider_metadata is Some(...). Added provider_metadata field to MockToolCall for injection.

Re-reviewed after 368c0ee. All blocking issues are resolved:

• agent_loop.rs:382 — literal provider_metadata: None replaced with ..; Gemini tool calls with thought signatures now execute end-to-end
• Same anti-pattern cleaned up in azure_openai.rs, openai_responses.rs, openai_compat.rs stop-reason checks and request builders
• Empty function call ID collision fixed — google.rs:139 restores the synthetic google-fc-{n} fallback
• SSE parse errors bumped from debug! to warn!
• test_tool_call_with_provider_metadata_executes is a proper regression guard — would have caught the original bug, and passes now
• MockToolCall now exposes provider_metadata so future tests can inject metadata cleanly

Verified locally: cargo check clean, 89 unit tests + the new agent-loop test all green on the branch.

LGTM — approving. Thanks for the careful fixup.

A few non-blocking follow-ups I'll track in a separate issue (breaking-change #[non_exhaustive], vacuous CRLF/empty-text tests, MockProvider-based coverage for the Gemini integration tests so they run in CI, typed ProviderMetadata enum). None of these block merge.

Note for the release: adding a required field to the public Content::ToolCall variant is a breaking change, so the next release should be 0.8.0, not 0.7.6. Confirmed this breaks at least one downstream construction site in our own yoyo-evolve — we'll fix that when we bump.