Skip to content

refactor response validation - #55

Merged
thorrester merged 1 commit into
mainfrom
response-matching
Mar 19, 2026
Merged

refactor response validation#55
thorrester merged 1 commit into
mainfrom
response-matching

Conversation

@thorrester

@thorrester thorrester commented Mar 19, 2026

Copy link
Copy Markdown
Member

Pull Request

Short Summary

Adds Provider::GoogleAdk and refactors ChatResponse::from_response_value to accept an optional provider hint, replacing a fragile key-only heuristic with a two-path design: direct deserialization when the provider is known, heuristic key inspection as a fallback. Fixes a bug where valid ADK responses without a "partial" key were misrouted or errored, and a second bug where Prompt::new_rs with Provider::GoogleAdk and no explicit settings immediately returned Err(InvalidModelSettings).

Context

Root problem — ADK heuristic was too narrow. The old heuristic required "partial" plus at least one other ADK field to route to AdkLlmV1. All AdkLlmResponse fields are optional, so real ADK responses without "partial" misrouted to GeminiV1 (if "candidates" was present) or fell through to Err. The fix introduces a provider-hint path that bypasses heuristics entirely when the caller knows the provider:

Before:

pub fn from_response_value(value: Value) -> Result<Self, ProviderError> {
    // key-only heuristic: "partial" required for ADK
    if obj.contains_key("choices") { ... }
    else if obj.contains_key("partial") && (obj.contains_key("model_version") || ...) { ... }
    else if obj.contains_key("candidates") { ... } // ADK responses without "partial" land here
    ...
}

After:

pub fn from_response_value(value: Value, provider: Option<&Provider>) -> Result<Self, ProviderError> {
    match provider {
        Some(p) if *p != Provider::Undefined => Self::from_response_value_with_provider(value, p),
        _ => Self::from_response_value_heuristic(value),
    }
}
// Provider::GoogleAdk → direct AdkLlmResponse deserialization, no key checks needed

Heuristic improvements. The heuristic fallback now checks for any key in ADK_SPECIFIC_KEYS (12 keys, including usage_metadata and model_version which ADK uses in snake_case vs Gemini's camelCase usageMetadata/modelVersion). This check is ordered before the Gemini "candidates" check, preventing ADK responses that include "candidates" from silently producing empty GeminiV1 results.

provider_default_settings fix. Provider::GoogleAdk previously fell through to OpenAIChatSettings::default(), but validate_provider(GoogleAdk) requires ModelSettings::GoogleChat. Any ADK prompt created without explicit settings would immediately fail. Now GoogleAdk (alongside Google, Vertex, Gemini) returns ModelSettings::GoogleChat(GeminiSettings::default()).

File Change
crates/potato_type/src/lib.rs Adds GoogleAdk variant to Provider enum; adds #[cfg(test)] with round-trip tests for all 7 variants
crates/potato_type/src/prompt/settings.rs Adds GoogleAdk arm to validate_provider; fixes provider_default_settings to return GoogleChat; adds 4 tests
crates/potato_type/src/prompt/interface.rs Adds GoogleAdk to all Google provider groups (create_message_for_provider, get_system_role, settings_from_value, is_google_provider)
crates/potato_provider/src/providers/types.rs Splits from_response_value into provider-hinted + heuristic paths; expands ADK_SPECIFIC_KEYS; updates all 15 existing test calls; adds 13 new tests
py-potato/python/potato_head/_potato_head.pyi Adds GoogleAdk: "Provider" stub entry
Cargo.toml / Cargo.lock / py-potato/pyproject.toml / py-potato/uv.lock Version bump 0.18.0 → 0.19.0

Is this a Breaking Change?

Yes — ChatResponse::from_response_value gains a required second argument (provider: Option<&Provider>). Any call site outside this crate must be updated to pass None (preserves existing heuristic behavior) or a specific Provider hint.


Open with Devin

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

Google,
Vertex,
Anthropic,
GoogleAdk,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Provider::GoogleAdk missing from MessageNum::matches_provider and convert_message_to_provider_type causes UnsupportedProviderError

Provider::GoogleAdk is added to the enum at crates/potato_type/src/lib.rs:69 but is not propagated to MessageNum::matches_provider (crates/potato_type/src/prompt/types.rs:284-293) or convert_message_to_provider_type (crates/potato_type/src/prompt/types.rs:338-349). When ChatResponse::to_message_num(&Provider::GoogleAdk) is called (crates/potato_provider/src/providers/types.rs:163-174), matches_provider returns false for a GeminiContentV1 message (which is the correct type for GoogleAdk), then convert_message_to_provider_type falls through to _ => Err(TypeError::UnsupportedProviderError). This makes it impossible to convert ADK response messages back into request messages for the GoogleAdk provider.

Prompt for agents
In crates/potato_type/src/prompt/types.rs, two functions need Provider::GoogleAdk support:

1. In matches_provider (line 284-293), add a new arm:
   | (MessageNum::GeminiContentV1(_), Provider::GoogleAdk)
   alongside the existing Gemini/Google/Vertex arms.

2. In convert_message_to_provider_type (line 338-349), add:
   Provider::GoogleAdk => self.to_google_message(),
   alongside the existing Google/Vertex/Gemini arms (before the _ wildcard).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine for now. we mainly need it for request parsing

Google,
Vertex,
Anthropic,
GoogleAdk,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Provider::GoogleAdk missing from GeminiGenerateContentRequest::match_provider causes request serialization failure

Provider::GoogleAdk is added to the enum at crates/potato_type/src/lib.rs:69 but is not propagated to GeminiGenerateContentRequest::match_provider at crates/potato_type/src/google/v1/generate/request.rs:2614-2619. When ProviderRequest::to_request(&Provider::GoogleAdk) is called (crates/potato_type/src/prompt/builder.rs:225-234), match_provider returns false and the method returns Err("ProviderRequest does not match the specified provider"). This prevents serializing any GoogleAdk prompt to its API request body.

Prompt for agents
In crates/potato_type/src/google/v1/generate/request.rs at line 2614-2619, update the match_provider function to include Provider::GoogleAdk:

fn match_provider(&self, provider: &Provider) -> bool {
    matches!(
        provider,
        Provider::Gemini | Provider::Google | Provider::Vertex | Provider::GoogleAdk
    )
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@thorrester
thorrester merged commit f1bf86f into main Mar 19, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant