Skip to content

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

Open
AmaadMartin wants to merge 1 commit into
mainfrom
feat/pin-cfc-model-gate-eap-rejection
Open

Test: pin the CFC model gate's rejection of unversioned Gemini Early Access ids#910
AmaadMartin wants to merge 1 commit into
mainfrom
feat/pin-cfc-model-gate-eap-rejection

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 Compositional Function Calling (CFC) gate in Runner.runAsync rejects an unversioned Gemini Early Access id such as gemini-flash-early-exp, and no test pins that. A contributor could widen isGemini2OrAbove, or swap the gate for isGeminiModel, and admit an unvalidated model family without a single test turning red.

Solution: A new suite pins the six outcomes of the gate. The diff is test-only, so there is no behaviour change and no source file is touched.

Cross-language note: adk-python rejects the same two ids at its own gate (Runner._new_invocation_context), so the pinned cases agree in both runtimes. The gemini-3.x and Vertex publisher-path cases diverge between the runtimes; they are out of scope here and are tracked as a separate task.

Collision check: gh pr list --repo AmaadMartin/adk-js --state open plus searches for cfc, model gate and gemini early access. Open PR #716 adds the same test file but asserts nothing about Early Access ids. This PR supersedes #716, so #716 should be closed. I did not stack on it, because the stacked PR would have to revert part of its base: PR #716 asserts gemini-3.0-pro and the Vertex publisher path, which are the out-of-scope cases named above. PRs #372, #615 and #618 touch core/src/utils/model_name.ts; this change does not.

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.

npx vitest run --project unit:core core/test/runner/runner_cfc_test.ts   # 6 passed
npx prettier --check core/test/runner/runner_cfc_test.ts   # clean
npx eslint core/test/runner/runner_cfc_test.ts             # clean
npx tsc --noEmit   # 292 errors, identical to the untouched tree; 0 of them in the new file
npm run build -w core   # succeeds

The 292 tsc errors are a pre-existing baseline on this branch point. I measured the tree with and without the new file and the count and the file list are the same.

Note on the base. The first CI run failed on ubuntu-latest with seven tests/integration/workflows sample failures. Those are unrelated to this change: the fork's main carried the identity-preamble prompt change without the follow-up that repairs the recorded workflow samples, so every pull request on that base failed the same way. I fast-forwarded the fork's main to the upstream commit that carries the repair and rebased onto it.

Coverage: the suite executes every statement and every branch of the gate block in core/src/runner/runner.ts. The change adds no source line, so there is no new source code to cover.

Proof that each test can fail. I mutated the gate once per test, re-ran the suite, then restored the gate.

Mutation in core/src/runner/runner.ts Result
Gate on isGeminiModel instead of isGemini2OrAbove 2 failed, 4 passed — both Early Access cases
Delete the rejection throw 3 failed — both Early Access cases and the non-Gemini case
Never install a BuiltInCodeExecutor 1 failed — the accept case
Drop the isBuiltInCodeExecutor guard 1 failed — the already-installed case
Drop the runConfig.supportCfc && condition 1 failed — the supportCfc: false case

The first mutation is the one that matters, and its output is the argument for the decision:

AssertionError: expected 'Gemini code execution tool is not sup…' to be 'CFC is not supported for model: gemin…'
Expected: "CFC is not supported for model: gemini-flash-early-exp in agent: cfc_agent"
Received: "Gemini code execution tool is not supported for model gemini-flash-early-exp"

Widening the gate does not make an Early Access id work. It moves the failure later, into BuiltInCodeExecutor.processLlmRequest, which gates on isGemini2OrAbove itself, and the message gets worse.

Review note. An earlier revision also added a three-line comment above the gate, claiming the narrow rule keeps the run off the unimplemented CFC path. That claim is wrong: a versioned id reaches the same CFC is not yet supported in callLlmAsync throw, as the accept-case test asserts. I removed the comment, so the diff is now test-only.

Manual End-to-End (E2E) Tests:
No E2E test applies. A CFC run cannot complete in adk-js, because LlmAgent.callLlmAsync refuses the path unconditionally. The vitest command above is the acceptance check. To reproduce by hand:

npm install && npm run build -w core
npx vitest run --project unit:core core/test/runner/runner_cfc_test.ts

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.

@AmaadMartin
AmaadMartin force-pushed the feat/pin-cfc-model-gate-eap-rejection branch from 86ef9f6 to eeb74f6 Compare August 11, 2026 05:25
…i Early Access ids

The gate in Runner.runAsync admits only a Gemini id with a parseable major
version of 2 or above. Unversioned Early Access ids such as
gemini-flash-early-exp fall outside that rule, and no test held them there.

The new suite pins the six gate outcomes. Nothing changes at runtime.
@AmaadMartin
AmaadMartin force-pushed the feat/pin-cfc-model-gate-eap-rejection branch from eeb74f6 to 4c93f6b Compare August 11, 2026 05:46
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