Skip to content

Test: pin the CFC model gate's rejection of unversioned Gemini Early Access ids - #189

Open
AmaadMartin wants to merge 1 commit into
mainfrom
fix/cfc-gate-early-access-decision
Open

Test: pin the CFC model gate's rejection of unversioned Gemini Early Access ids#189
AmaadMartin wants to merge 1 commit into
mainfrom
fix/cfc-gate-early-access-decision

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 8, 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 model gate in Runner._new_invocation_context has no test. Deleting its raise, or widening its predicate to admit unversioned Gemini Early Access ids, breaks nothing in CI. Nothing at the call site says why the gate is narrower than BuiltInCodeExecutor's "is this a Gemini id" check, so readers keep re-deriving the reasoning and reading the difference as an accidental inconsistency.

Solution: Unversioned Gemini Early Access ids stay rejected at the CFC gate, in both adk-python and adk-js. The accept path does not just install BuiltInCodeExecutor: it reroutes the invocation to the Live API (flows/llm_flows/base_llm_flow.py), so the gate asks whether the model can serve CFC there. In adk-js the same accept path throws CFC is not yet supported in callLlmAsync, so no reading of "admit Early Access" produces working behaviour there. This change adds TestRunnerCfcModelGate and a four-line comment; the gate's logic is byte-identical, and the matching adk-js test is queued as a separate task.

Collision check: I listed all 187 open pull requests on the fork and matched their changed files. No open pull request touches src/google/adk/runners.py or tests/unittests/test_runners.py.

Deviation from the plan: the new async test carries an explicit @pytest.mark.asyncio. asyncio_mode is auto today, so the marker is not required, but every other async test in this file declares it and separate open work switches pytest-asyncio to strict mode.

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 adds 8 test cases in tests/unittests/test_runners.py. Cases 1-6 drive _new_invocation_context directly, matching the existing TestRunnerCacheConfig precedent in the same file. Case 7 drives runner.run_async, which proves the rejection reaches a caller of the public entry point. The accept path reroutes to the Live API, so a unit test does not drive it end to end.

The tests use real objects: a real LlmAgent, a real Runner, a real InMemorySessionService, and one BaseLlm fake (testing_utils.MockModel) for the non-Gemini case, which a plain string cannot reach because LLMRegistry does not resolve it.

$ .venv/bin/python -m pytest tests/unittests/test_runners.py -q
86 passed, 17 warnings in 1.69s

Coverage of the gate block, measured with coverage run --branch: every line and every branch is executed, with no missing branch pairs.

Mutation evidence. I ran the new tests against seven mutations of the gate. Every mutation is caught, and every one of the 8 cases is killed by at least one mutation.

Mutation Failing tests
Delete the raise ValueError(...) 4 failed: both Early Access ids, the non-Gemini case, run_async
Widen the predicate to admit early-exp ids 3 failed: both Early Access ids, run_async
Install the executor before the model check 4 failed, on agent.code_executor is None
Always reinstall the executor 1 failed: keeps_an_already_installed_code_executor
Drop the run_config.support_cfc condition 1 failed: is_inert_when_support_cfc_is_false
Drop the hasattr(self.agent, 'canonical_model') guard 1 failed: is_inert_for_an_agent_without_a_canonical_model
Never install the executor 1 failed: accepts_gemini_2_and_installs_the_code_executor

Sample failure message from the first mutation:

E     Failed: DID NOT RAISE ValueError
FAILED tests/unittests/test_runners.py::TestRunnerCfcModelGate::test_cfc_gate_rejects_unversioned_early_access_model_ids[gemini-flash-early-exp]

The tests assert nothing about gemini-3.0-pro, gemini-1.5-pro, or Vertex publisher paths. The gate's handling of those ids is a separate, real divergence from adk-js and is tracked as its own task; pinning today's answers would cement it.

CI: Unit Tests (3.10-3.14), Mypy Check (3.10-3.13) and A2A v0.3 Tests (3.10-3.14) all pass. The Pre-commit Linter fails for a reason unrelated to this change: its update-constraints hook regenerates constraints-*.txt with today's snapshot date, so the hook fails on every open pull request in this fork. Open pull request #163 covers that hook.

Manual End-to-End (E2E) Tests:
No runtime behaviour changes, so there is nothing a user can observe differently. Reviewer-facing verification is the comment at the gate plus the mutation table above.

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.

…ni Early Access ids

The gate in Runner._new_invocation_context had no test, so deleting its
raise or widening its predicate broke nothing. Add TestRunnerCfcModelGate,
which covers all four decision points both ways, and a comment recording
why the gate is narrower than BuiltInCodeExecutor's Gemini-id check: the
accept path reroutes the invocation to the Live API.

The gate's logic is unchanged.
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