Tracking non-blocking items from the review of #32 (Gemini streaming and function calling). None of these block the merge; grouping them here so they don't get lost.
1. Mark Content (or Content::ToolCall) #[non_exhaustive]
PR #32 adds a new required field provider_metadata to the public Content::ToolCall variant, which is a breaking change for every downstream crate that constructs the variant by name. We're paying the breaking-change cost once for 0.8.0 — we should bake in the escape hatch in the same release so future field additions (e.g. Anthropic cache breakpoints, OpenAI reasoning IDs) are warn-only for downstream consumers, not hard errors.
Suggested fix: add #[non_exhaustive] to the Content enum in src/types.rs, or at minimum to the ToolCall variant.
2. Fix vacuous tests in src/provider/google.rs
Two of the new unit tests don't actually exercise the production code they claim to cover:
test_parse_chunk_with_crlf_sse (src/provider/google.rs:497) re-implements the separator detection inline inside the test body and asserts on its own copy. If the real streaming loop at google.rs:88 were broken, this test would still pass.
test_parse_chunk_with_empty_text (src/provider/google.rs:489) only verifies that serde can parse {"text": ""}. It does not exercise the if text.is_empty() { continue; } guard at google.rs:118.
Fix: extract the SSE splitting + empty-text filtering into a private helper function that can be unit-tested directly, and rewrite both tests to drive that helper.
3. Make Gemini integration tests runnable in CI
tests/integration_gemini.rs adds two behavioral tests for the fix, but both are #[ignore] and require GEMINI_API_KEY. They will never run in CI → zero regression protection for the exact scenarios the PR is fixing.
Fix: port the thought-signature round-trip and multi-turn function-call scenarios to MockProvider (or a stubbed HTTP server using wiremock) so they run on every PR. The existing test_tool_call_with_provider_metadata_executes in tests/agent_loop_test.rs is a good template.
Also: tests/integration_gemini.rs currently pins gemini-3-flash-preview — when these get un-ignored, prefer a GA model name like gemini-2.5-flash for stability.
4. Replace Option<serde_json::Value> with a typed ProviderMetadata enum
provider_metadata is currently a fully untyped bag. There's no schema, no namespacing, and no compile-time link between the metadata shape and the producing provider. Two providers could silently collide on keys.
Suggested shape:
#[derive(Clone, Debug, Serialize, Deserialize)]
#[serde(tag = "provider", rename_all = "snake_case")]
pub enum ProviderMetadata {
Gemini { thought_signature: String },
// Anthropic { cache_control: ... },
// OpenAi { reasoning_id: String },
}
This gives compile-time exhaustiveness, prevents key collisions, and makes valid shapes obvious to readers. Can land in a later release (would itself be another breaking change, so ideally batch with other 0.9.0 items).
5. google.rs synthetic-ID magic-string coupling
Both content_to_google_parts and build_request_body in src/provider/google.rs detect synthetic IDs via id.starts_with("google-fc-") to strip them before sending back to Gemini. Functional but couples the formatter and serializer through a literal prefix. Cleaner alternatives: a was_synthetic: bool marker in provider_metadata, or an Option<String> wire ID distinct from the internal correlation ID.
Context: #32 merged the Gemini streaming fix. See that PR for the full discussion.
Tracking non-blocking items from the review of #32 (Gemini streaming and function calling). None of these block the merge; grouping them here so they don't get lost.
1. Mark
Content(orContent::ToolCall)#[non_exhaustive]PR #32 adds a new required field
provider_metadatato the publicContent::ToolCallvariant, which is a breaking change for every downstream crate that constructs the variant by name. We're paying the breaking-change cost once for 0.8.0 — we should bake in the escape hatch in the same release so future field additions (e.g. Anthropic cache breakpoints, OpenAI reasoning IDs) are warn-only for downstream consumers, not hard errors.Suggested fix: add
#[non_exhaustive]to theContentenum insrc/types.rs, or at minimum to theToolCallvariant.2. Fix vacuous tests in
src/provider/google.rsTwo of the new unit tests don't actually exercise the production code they claim to cover:
test_parse_chunk_with_crlf_sse(src/provider/google.rs:497) re-implements the separator detection inline inside the test body and asserts on its own copy. If the real streaming loop atgoogle.rs:88were broken, this test would still pass.test_parse_chunk_with_empty_text(src/provider/google.rs:489) only verifies that serde can parse{"text": ""}. It does not exercise theif text.is_empty() { continue; }guard atgoogle.rs:118.Fix: extract the SSE splitting + empty-text filtering into a private helper function that can be unit-tested directly, and rewrite both tests to drive that helper.
3. Make Gemini integration tests runnable in CI
tests/integration_gemini.rsadds two behavioral tests for the fix, but both are#[ignore]and requireGEMINI_API_KEY. They will never run in CI → zero regression protection for the exact scenarios the PR is fixing.Fix: port the thought-signature round-trip and multi-turn function-call scenarios to
MockProvider(or a stubbed HTTP server usingwiremock) so they run on every PR. The existingtest_tool_call_with_provider_metadata_executesintests/agent_loop_test.rsis a good template.Also:
tests/integration_gemini.rscurrently pinsgemini-3-flash-preview— when these get un-ignored, prefer a GA model name likegemini-2.5-flashfor stability.4. Replace
Option<serde_json::Value>with a typedProviderMetadataenumprovider_metadatais currently a fully untyped bag. There's no schema, no namespacing, and no compile-time link between the metadata shape and the producing provider. Two providers could silently collide on keys.Suggested shape:
This gives compile-time exhaustiveness, prevents key collisions, and makes valid shapes obvious to readers. Can land in a later release (would itself be another breaking change, so ideally batch with other 0.9.0 items).
5.
google.rssynthetic-ID magic-string couplingBoth
content_to_google_partsandbuild_request_bodyinsrc/provider/google.rsdetect synthetic IDs viaid.starts_with("google-fc-")to strip them before sending back to Gemini. Functional but couples the formatter and serializer through a literal prefix. Cleaner alternatives: awas_synthetic: boolmarker inprovider_metadata, or anOption<String>wire ID distinct from the internal correlation ID.Context: #32 merged the Gemini streaming fix. See that PR for the full discussion.