feat: issue #33 follow-ups — non_exhaustive policy and google.rs test quality#55
Merged
Conversation
…ality Item 1 — exhaustiveness policy (breaking, batched into 0.9.0): - #[non_exhaustive] on Content (enum) and Content::ToolCall (variant): new content kinds and ToolCall fields become warn-only for downstream. Add Content::tool_call()/tool_call_with_metadata() constructors. - #[non_exhaustive] on ModelConfig (0.9.0 added a field; future ones should not break downstream literals). - StopReason stays deliberately exhaustive (control-flow enum: a new variant should be a compile error downstream, not a silent wildcard). Item 2 — fix vacuous google.rs tests: - extract next_sse_data() and part_text() and drive them from the production loop; rewrite test_parse_chunk_with_crlf_sse and test_parse_chunk_with_empty_text to exercise the real helpers - drive-by fix: split at the EARLIEST separator when both \n\n and \r\n\r\n are buffered (the old inline logic preferred the LF match and could merge two events); regression test added - empty-text parts no longer skip a functionCall in the same part Item 3 — CI coverage for the PR #32 scenarios: - tests/google_stream_test.rs: wiremock-based thought-signature capture and round-trip (signature echoed, synthetic google-fc- id stripped), running on every PR instead of only under GEMINI_API_KEY Refs #33 (items 4 and 5 deferred — see issue comment) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…google.rs error surfacing Type policy (completes the non_exhaustive work coherently): - ModelConfig::custom(api, provider, base_url, id, name): the construction path for Bedrock/Vertex/Azure and future protocols — non_exhaustive had removed downstream struct literals without a replacement for those three - variant-level non_exhaustive on Message::Assistant and Content::Thinking (same PR-#32 exposure class as ToolCall); Message::assistant() + with_error_message()/with_timestamp() and Content::thinking()/ thinking_signed() constructors; external tests migrated - doc fixes: enum-level vs variant-level levers explained separately, "0.9.0" phrased as slated rather than shipped, FRU/serde(default) notes, stale enum listings in messages-events.md and CLAUDE.md updated google.rs error surfacing (pre-existing gaps in the rewired loop): - mid-stream {"error": ...} payloads now classify and return Err instead of deserializing into an empty chunk and vanishing - transport errors return Err(Network) like every other provider instead of warn + break -> fake successful turn - SAFETY/PROHIBITED_CONTENT/BLOCKLIST/SPII finish reasons map to StopReason::Refusal with an explanatory error_message Tests (closing the coverage-review gaps): - empty-text part with functionCall no longer swallowed (loop-level) - next_sse_data consumes keepalives/data-less events (Some(""), not None) - multi-event text-delta accumulation; usage mapping; in-stream error; SAFETY -> Refusal; structural thoughtSignature placement assertion; constructor field-pinning; metadata ToolCall persistence round-trip Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements items 1–3 of #33; items 4–5 are deferred with rationale (see issue comment). A 5-agent review pass ran on this branch; all findings addressed in the second commit.
Item 1 — exhaustiveness policy (breaking, must land in 0.9.0)
#[non_exhaustive]onContent(enum level: downstream matches need a wildcard) and on theToolCall,Thinking, andMessage::Assistantvariants — the field-growth exposure class that PR fix: Gemini streaming and function calling #32 hit. Constructors:Content::tool_call()/tool_call_with_metadata()/thinking()/thinking_signed(),Message::assistant()+with_error_message()/with_timestamp().#[non_exhaustive]onModelConfig, plusModelConfig::custom(api, provider, base_url, id, name)so Bedrock/Vertex/Azure users (no preset) keep a construction path.StopReason(control-flow enum — a new variant should be a compile error downstream) orAgentMessage(untagged serde + context-inclusion semantics). Policy documented on the types.Item 2 — real tests for google.rs SSE handling
next_sse_data()/part_text()extracted and driven by the production loop; the two previously-vacuous tests now exercise them. Behavior fixes that fell out: earliest-separator splitting (the old logic could merge a CRLF event into a later LF event), and an empty text part no longer skips afunctionCallin the same part.Item 3 — PR #32 scenarios now run in CI
tests/google_stream_test.rs(wiremock): thought-signature capture intoprovider_metadata, structural round-trip assertion on the outbound body (signature on the functionCall part, syntheticgoogle-fc-id stripped), empty-text+functionCall, multi-event text accumulation, and usage mapping.Bonus (from the review pass) — google.rs error surfacing
{"error": ...}payloads now classify and returnErr(previously deserialized into an empty chunk and vanished — fake successful turn, no retry).Err(Network)(retryable) instead ofwarn!+break→Ok(Stop).SAFETY/PROHIBITED_CONTENT/BLOCKLIST/SPIIfinish reasons →StopReason::Refusalwith an explanatoryerror_message.Breaking-change notes for 0.9.0
Downstream must: add wildcard arms to exhaustive
Contentmatches; constructToolCall/Thinking/Message::Assistantvia the new constructors (patterns need..); constructModelConfigvia presets orModelConfig::custom()(field mutation still works; literals/FRU do not).🤖 Generated with Claude Code