Skip to content

Fix: Drop the Gemini version floor at the CFC gate in Runner - #219

Open
AmaadMartin wants to merge 3 commits into
mainfrom
fix/cfc-gate-drop-gemini-version-floor
Open

Fix: Drop the Gemini version floor at the CFC gate in Runner#219
AmaadMartin wants to merge 3 commits into
mainfrom
fix/cfc-gate-drop-gemini-version-floor

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A

  2. Or, if no issue exists, describe the change:

Problem: The CFC gate in Runner._new_invocation_context tested model_name.startswith('gemini-2'). Compositional Function Calling routes through the Gemini Live API, so the gate raised ValueError on ADK's own LlmAgent.DEFAULT_LIVE_MODEL (gemini-live-2.5-flash-native-audio). It also rejected every Gemini 3 id and every path- or provider-prefixed id. A ValueError has no workaround short of renaming the model.

Solution: The gate now asks "is this a Gemini model", and honors ADK_DISABLE_GEMINI_MODEL_ID_CHECK. This is the predicate the BuiltInCodeExecutor two lines below already uses, so one constraint is checked once. Commit 745de0ac moved every production caller onto is_gemini_model and deprecated both version helpers; runners.py was the last version-bucketing site. Dropping the floor also admits Early Access ids such as gemini-flash-early-exp, which was the intended answer and not an accident.

Non-Gemini models still fail fast, with the same ValueError type and the same message text.

Why not a normalising major >= 2 parse: it does not fix the reported defect. gemini-live-2.5-flash-native-audio yields the version token live, which no version parser accepts, so a major >= 2 gate rejects the Live default model too.

Collision check: I ran gh pr list --repo AmaadMartin/adk-python --state open --limit 100 and read the diff of every adjacent PR. Two PRs touch the same line and reach the opposite decision:

Neither lands this change, so this is not a duplicate. I branched from main rather than stacking, because stacking would add a helper and then delete its only caller in the same review. The conflict with each is the single predicate line plus the TestRunnerCfcModelGate class name. A maintainer should merge one of the three.

Cross-runtime note: adk-js carries the same defect in core/src/runner/runner.ts. It is tracked as a separate change, because its BuiltInCodeExecutor still uses isGemini2OrAbove and must be migrated in the same commit. No file under adk-js is touched here.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

TestRunnerCfcModelGate in tests/unittests/test_runners.py adds 16 tests. No test asserted this gate before, so this is net-new coverage.

uv run pytest tests/unittests/test_runners.py -q            # 94 passed
uv run pytest tests/unittests/runners \
  tests/unittests/code_executors/test_built_in_code_executor.py \
  tests/unittests/utils/test_model_name_utils.py -q         # 118 passed, 1 skipped, 3 xfailed
uv run pyink --check src/google/adk/runners.py tests/unittests/test_runners.py   # clean
uv run isort --check src/google/adk/runners.py tests/unittests/test_runners.py   # clean
uv run ruff check src/google/adk/runners.py tests/unittests/test_runners.py      # clean

Coverage of the changed block is 100% line and 100% branch, measured with coverage run --branch: no missing lines and no partial branches in runners.py:2211-2219.

Mutation results. I ran the new tests against seven mutations of the gate. Each test dies to at least one.

Mutation Tests that fail
Restore startswith('gemini-2') 9, e.g. test_cfc_gate_accepts_any_gemini_model[gemini-live-2.5-flash-native-audio]: AssertionError: assert isinstance(None, BuiltInCodeExecutor)
Predicate always true 4, e.g. test_cfc_gate_rejects_a_non_gemini_model[claude-3-5-sonnet]: DID NOT RAISE <class 'ValueError'>
Predicate always false 10
Drop or is_gemini_model_id_check_disabled() test_cfc_gate_accepts_any_model_when_the_id_check_is_disabled
Drop the support_cfc condition test_cfc_gate_is_inert_when_support_cfc_is_false
Drop the hasattr(canonical_model) condition test_cfc_gate_is_inert_for_an_agent_without_a_canonical_model
Always install a fresh executor test_cfc_gate_keeps_an_already_installed_code_executor

One deviation from the plan. models/gemini-2.5-pro cannot reach the gate as a plain string: LLMRegistry does not match that form, so canonical_model raises first. That is an LLMRegistry limit, not a CFC one, and is out of scope. test_cfc_gate_accepts_a_models_prefixed_gemini_id supplies the id on a real Gemini instance instead, which is how a user reaches the gate with that form.

Manual End-to-End (E2E) Tests:
test_cfc_run_async_completes_on_the_default_live_model drives the whole public path: runner.run_async(run_config=RunConfig(support_cfc=True)) on DEFAULT_LIVE_MODEL, through the real gate and the real run_live flow, and asserts the model's reply arrives. The only fake is testing_utils.MockModel at the connection boundary. The live loop does not end on its own, so the test stops at the first agent reply, which is the pattern testing_utils already uses.

To reproduce by hand:

python -c "
import asyncio
from google.adk.agents.llm_agent import LlmAgent
from google.adk.agents.run_config import RunConfig
from google.adk.runners import InMemoryRunner

async def main():
  for m in ('gemini-live-2.5-flash-native-audio', 'gemini-3-pro-preview',
            'gemini-flash-early-exp', 'gemini-2.5-flash', 'claude-3-5-sonnet'):
    a = LlmAgent(name='a', model=m)
    r = InMemoryRunner(agent=a)
    s = await r.session_service.create_session(app_name='a', user_id='u')
    try:
      r._new_invocation_context(s, run_config=RunConfig(support_cfc=True))
      print(f'{m}: accepted, code_executor={type(a.code_executor).__name__}')
    except ValueError as e:
      print(f'{m}: rejected -> {e}')
asyncio.run(main())
"

The first four are accepted with code_executor=BuiltInCodeExecutor, and claude-3-5-sonnet is rejected with the unchanged message. Before this change all four are rejected.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

CI

Unit Tests pass on Python 3.10 to 3.14. Mypy Check passes on 3.10 to 3.13. A2A v0.3 Tests pass on 3.10 to 3.14.

Pre-commit Linter fails, and the cause is not in this diff. The only failing hook is update-constraints, which regenerates constraints-3.10.txt through constraints-3.14.txt because upstream package versions moved past the recorded snapshot date. This PR touches neither those files nor pyproject.toml. The same hook fails on the sibling PRs #216 and #217, and PR #214 tracks the fix.

Amaad Martin added 3 commits August 10, 2026 22:02
The gate tested `model_name.startswith('gemini-2')`, so it raised
ValueError on ADK's own LlmAgent.DEFAULT_LIVE_MODEL
('gemini-live-2.5-flash-native-audio'), on every Gemini 3 id, and on
path- or provider-prefixed ids. Compositional Function Calling routes
through the Gemini Live API, so the gate rejected the model family the
feature exists for.

Commit 745de0a moved every production caller onto is_gemini_model and
deprecated the version helpers. This was the last version-bucketing
site. The gate now asks the same question as the BuiltInCodeExecutor it
installs two lines later: is this a Gemini model, or is
ADK_DISABLE_GEMINI_MODEL_ID_CHECK set. Non-Gemini models still fail
fast with the unchanged message.
CI compares mypy output against main and fails on new errors. Every test
method now declares its return type, and the live-turn test narrows
`Content.parts` before it indexes.
…test

`_new_invocation_context` is annotated `-> InvocationContext`, so the
isinstance check on its return value cannot fail. The code_executor
assertion on the next line already proves the call did not raise, which
is what the accept case pins. The sibling cases assert only that.
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