Skip to content

Fix: tighten the Gemini-1 boundary and split the EAP gate out of isGemini2OrAbove (adk-python parity) - #615

Open
AmaadMartin wants to merge 4 commits into
fix/extract-model-name-parityfrom
fix/model-name-eap-and-path-parity
Open

Fix: tighten the Gemini-1 boundary and split the EAP gate out of isGemini2OrAbove (adk-python parity)#615
AmaadMartin wants to merge 4 commits into
fix/extract-model-name-parityfrom
fix/model-name-eap-and-path-parity

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 4, 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: core/src/utils/model_name.ts is the single place adk-js classifies model
ids, and it diverges from adk-python's src/google/adk/utils/model_name_utils.py in three
independent ways. Every built-in tool and code executor that gates on those predicates
inherits the divergence.

Collision check (required, and it found real overlap).
gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 surfaced two live sibling
PRs touching this exact module:

GAPs 1 and 2 are therefore already landed and are not reimplemented here. This PR
stacks on fix/extract-model-name-parity (#471)
and contributes the remaining delta:
GAP 3, which neither sibling touches, plus the predicate split that #372's approach
requires in order not to regress the CFC gate (see the design decision below).
#470 (ADK_DISABLE_GEMINI_MODEL_ID_CHECK bypass in the same two call sites) is adjacent
but based on main and does not overlap in behaviour; only its || modelCheckDisabled
term will need to survive the eventual merge.

GAP 3 — the Gemini-1 boundary was a prefix test, not a version test (fixed here; not
covered by either sibling PR)
. adk-python uses re.match(r'^gemini-1\.\d+', name)
(model_name_utils.py:120); adk-js used modelName.startsWith('gemini-1'). Four built-in
tools (GoogleSearchTool, GoogleMapsGroundingTool, VertexAiSearchTool,
EnterpriseWebSearchTool) branch on this predicate, so a future double-digit Gemini major
would be routed down the legacy Gemini-1 path.

// before
isGemini1Model('gemini-10.0-pro'); // true   (adk-python: false)
isGemini1Model('gemini-1-pro'); // true   (adk-python: false)
isGemini1Model('gemini-1'); // true   (adk-python: false)
// after
isGemini1Model('gemini-10.0-pro'); // false
isGemini1Model('gemini-1.5-flash'); // true  (unchanged)

GAP 1 — EAP model ids were rejected (already fixed by #372, in the stack base).
gemini-flash-early-exp carries no numeric version, so isGemini2OrAbove returned false
and UrlContextTool / BuiltInCodeExecutor refused it. What this PR changes is where
that acceptance lives — see the design decision.

GAP 2 — extractModelName only understood the Vertex path form (already fixed by
#471, in the stack base)
. apigee/gemini-2.5-flash, models/gemini-2.5-pro and
gemini/gemini-2.5-flash now reduce to their bare model name.

Solution:

Chosen: introduce isGeminiEapOr2OrAbove and migrate the two parity call
sites; leave isGemini2OrAbove semantically unchanged.

Rationale — isGemini2OrAbove has three callers in core/src
(code_executors/built_in_code_executor.ts:55, tools/url_context_tool.ts:43,
runner/runner.ts:274) and is a public export
(core/src/common.ts:285, re-exported by core/src/index.ts). Of those three,
adk-python routes only the first two through its EAP-aware predicate; its
CFC gate in runners.py uses a bare startswith('gemini-2') and would
reject an EAP id. Folding the EAP branch into isGemini2OrAbove would
therefore (a) silently change the CFC gate — a call site this task is told not
to touch — introducing a new JS/Python divergence, and (b) leave a public
predicate whose name no longer describes what it does. Adding a second
predicate is three lines, breaks no public API, and mirrors adk-python
one-for-one.

Concretely:

  • isGemini1Model now tests /^gemini-1\.\d+/ against the extracted name.
  • isGemini2OrAbove is restored to numeric-version semantics (the EAP test added in the
    stack base is removed from it).
  • New internal isGeminiEapModel + exported-from-module isGeminiEapOr2OrAbove, mirroring
    _is_gemini_eap_model / is_gemini_eap_or_2_or_above.
  • BuiltInCodeExecutor and UrlContextTool — the two call sites adk-python routes
    through is_gemini_eap_or_2_or_above — switch to the new predicate. Their thrown
    messages are byte-identical to before.
  • runner/runner.ts (CFC) deliberately keeps calling isGemini2OrAbove, matching
    runners.py.

Cross-language parity, which rule applied where: parity wins for the observable
classification of every model id (the expectation tables are adk-python's outputs).
Local JS/TS convention wins for naming and module layout only — isGeminiEapModel stays a
module-private function rather than a _-prefixed one, per the repo style guide.

Behavioural narrowing of isGemini1Model, and what it changes at four call sites this
PR does not otherwise touch.
isGemini1Model now returns false for gemini-1,
gemini-1-pro, gemini-1. and gemini-10.*. That is the fix, and it matches
adk-python — but because four built-in tools branch on this predicate, tightening it also
loosens their Gemini 1.x guards for those undotted ids. Called out explicitly:

  • google_search_tool.ts:40gemini-1 / gemini-1-pro now take the googleSearch
    branch instead of googleSearchRetrieval, and no longer throw
    "Google search tool can not be used with other tools in Gemini 1.x."
  • google_maps_grounding_tool.ts:29 — stops throwing "cannot be used with Gemini 1.x
    models" for those ids.
  • vertex_ai_search_tool.ts:144 and enterprise_web_search_tool.ts:35 shift the same way.

No such model exists today, so this is latent behaviour rather than a live regression, and
every existing test at those four consumers uses a dotted gemini-1.5-* id — meaning
nothing in the suite pinned it. It is now pinned: google_search_tool_test.ts gains cases
asserting that gemini-1, gemini-1-pro and gemini-10.0-pro take the googleSearch
branch and coexist with other tools. Those cases fail against the old predicate (see
mutation 1 below).

Public API surface: unchanged. No export was added, renamed, or removed.
isGeminiEapOr2OrAbove is exported from the module but deliberately not added to
core/src/common.ts / core/src/index.ts; it is internal to the package and tests import
it by relative path, as 67 other files in core/test already do.

Modified existing tests, and why. Restoring isGemini2OrAbove means the EAP cases the
stack base added under describe('isGemini2OrAbove') now encode behaviour this change
deliberately moves. They are not deleted: every input and expectation is carried over
verbatim into the new describe('isGeminiEapOr2OrAbove') block, with only the function
under test changed, in its own commit (refactor(utils): split the EAP check out into isGeminiEapOr2OrAbove). A new describe('isGemini2OrAbove') > describe('EAP models') block
pins the other half — isGemini2OrAbove('gemini-flash-early-exp') === false — so the split
cannot be silently undone, and two new Runner CFC model gate tests assert the CFC gate
still rejects an EAP id at the real call site.

No suppressions. The diff adds no @ts-expect-error, @ts-ignore, eslint-disable,
any, as any, as never, or coverage-ignore comment (verified by grepping the diff).

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.

Added:

  • core/test/utils/model_name_test.tsdescribe('isGemini1Model') > describe('version boundary') (the GAP 3 fix, including gemini-10.0-pro, gemini-10-flash, gemini-1,
    gemini-1-pro, gemini-1.); describe('isGeminiEapOr2OrAbove') (EAP ids, EAP ids in
    every path form, EAP regex boundaries, numeric versions, empty string);
    describe('isGemini2OrAbove') > describe('EAP models') pinning false;
    describe('classification consistency') mirroring Python's
    TestModelNameUtilsIntegration (mutual exclusivity, version ⇒ Gemini, bare form and
    projects/.../models/ form classify identically).
  • core/test/tools/url_context_tool_test.ts — extended model-id forms accepted
    (models/, gemini/, apigee/.../, models/gemini-flash-early-exp) and two rejection
    paths with their exact error messages (openrouter/google/gemini-1.5-pro:online
    "requires Gemini 2 or above"; a malformed projects/ path → "is not supported for
    model").
  • core/test/runner/runner_test.tsdescribe('Runner CFC model gate'): the CFC gate
    rejects gemini-flash-early-exp and gemini-1.5-pro. This is the test that pins the
    design decision at the real call site.

Coverage. core/src/utils/model_name.ts measures 97.36% stmts / 96.96% branch under
--coverage.include='core/src/utils/model_name.ts'. The only uncovered lines are 180-181,
the pre-existing isGeminiModelIdCheckDisabled env-var helper, which this change does not
touch. Every line and branch of the new code is covered. No v8 ignore / istanbul ignore
was added and no threshold in vitest.config.ts was changed.

Proof the new tests can fail. Each mutation was applied to core/src/utils/model_name.ts,
the affected suites re-run, then reverted:

  1. isGemini1Model reverted to extractModelName(modelString).startsWith('gemini-1')
    5 failed in model_name_test.ts: isGemini1Model > version boundary > should return false for model: gemini-10.0-pro, gemini-10-flash, gemini-1, gemini-1-pro,
    gemini-1. (expected true to be false); and 6 failed at the call site in
    google_search_tool_test.tsadds googleSearch, not googleSearchRetrieval, for model: gemini-1 (expected [ { googleSearchRetrieval: {} } ] to deeply equal [ { googleSearch: {} } ]) and does not reject other tools alongside model: gemini-1
    (Google search tool can not be used with other tools in Gemini 1.x.), each for all
    three ids.
  2. isGeminiEapOr2OrAbove made to delegate straight to isGemini2OrAbove (i.e. the EAP
    branch dropped) → 13 failed: all 7 isGeminiEapOr2OrAbove > EAP models rows, all 3
    EAP model in path form rows, BuiltInCodeExecutor > processLlmRequest should attach codeExecution for an EAP model, and both UrlContextTool EAP rows.
  3. The EAP test folded back into isGemini2OrAbove (i.e. the split undone) → 5 failed:
    the four isGemini2OrAbove > EAP models > should return false rows and
    Runner CFC model gate > rejects an EAP model, matching the adk-python bare gemini-2 prefix gate — which is exactly the parity regression the split exists to prevent.
  4. \d* changed to \d+ in the EAP pattern → 11 failed, including
    should return true for EAP model: gemini-flash-early-exp and both call-site EAP rows.

Mutations of the stack base's extractModelName (dropping the apigee pattern, the
projects/ bail-out, or the startsWith('gemini-') guard in the provider-prefix branch)
belong to #471 and were not re-run here.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

This module is pure, synchronous and has no I/O, transport, credential or live-model
dependency, so there is nothing to exercise against a real service; the behavioural
consumers are covered by the call-site regression tests above. To reproduce locally:

npm ci
npm run build

npx vitest run --project unit:core core/test/utils/model_name_test.ts
npx vitest run --project unit:core core/test/code_executors/built_in_code_executor_test.ts
npx vitest run --project unit:core core/test/tools/url_context_tool_test.ts
npx vitest run --project unit:core core/test/runner/runner_test.ts
# broader sweep over every consumer of the changed predicates:
npx vitest run --project unit:core core/test/tools core/test/code_executors \
  core/test/runner core/test/utils
npm run lint
npm run format:check

Results on the pushed commit:

  • core/test/{utils,tools,code_executors,runner} sweep — 69 files, 968 tests, all passed.
  • npm run build — clean.
  • npm run lint — clean, no warnings.
  • npm run format:check — "All matched files use Prettier code style!".
  • npm run ts:check — the repo has 284 pre-existing tsc --noEmit errors on this branch's
    base (unrelated files such as core/test/a2a/agent_card_test.ts). Comparing the error
    list before and after this diff, with line numbers normalised, the two are identical:
    this change introduces zero new type errors.

Reference for every expectation in the new tables: adk-python
src/google/adk/utils/model_name_utils.py and
tests/unittests/utils/test_model_name_utils.py.

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 status: absent (validated locally instead)

.github/workflows/validation.yaml triggers on pull_request: branches: [main]. This is a
stacked PR based on fix/extract-model-name-parity, so the run-tests job never fires;
only auto-assign ran, and that is not validation. The following were therefore run locally
against the exact pushed commit 915659382e0f09c24d8dac45288f4244f5a04f9b, with a clean
working tree:

Command Result
npm run build clean, no errors
npx vitest run --project unit:core core/test/tools core/test/code_executors core/test/runner core/test/utils 69 files, 974 tests, 0 failures
npm run lint clean
npm run format:check "All matched files use Prettier code style!"
npm run ts:check zero new type errors; the diff against the base's error list is removals only (this PR clears a pre-existing TS2322 in google_search_tool_test.ts)

Once #372 and #471 merge and this PR is retargeted at main, run-tests will run normally.

Amaad Martin added 4 commits August 3, 2026 21:25
isGemini1Model used a `startsWith('gemini-1')` prefix test, so `gemini-1`,
`gemini-1-pro`, `gemini-1.` and a future double-digit major such as
`gemini-10.0-pro` were all classified as Gemini 1.x. adk-python matches
`^gemini-1\.\d+` (src/google/adk/utils/model_name_utils.py), which requires the
dotted minor version.

Four built-in tools branch on this predicate, so a `gemini-10` model would have
been routed down the legacy Gemini 1.x path.
isGemini2OrAbove had the EAP naming test folded into it, which widened all
three of its call sites at once. adk-python keeps the two concerns apart: the
built-in code executor and the URL context tool call
is_gemini_eap_or_2_or_above, while the CFC gate in runners.py uses a bare
startswith('gemini-2') and rejects EAP ids.

Restore isGemini2OrAbove to numeric-version semantics, add
isGeminiEapOr2OrAbove alongside it, and migrate only the two call sites Python
routes through the EAP-aware predicate. The runner CFC gate keeps calling
isGemini2OrAbove, so it now matches Python again.

The thrown messages at both migrated call sites are unchanged. No public
export is added, renamed, or removed: isGeminiEapOr2OrAbove is internal to the
package.

The EAP test cases move from the isGemini2OrAbove describe block to the new
predicate with their inputs and expectations intact, and a new block pins
isGemini2OrAbove returning false for EAP ids so the split cannot be silently
undone. New Runner CFC tests assert the gate rejects an EAP id.
…rowing

Review follow-ups:

- Inline the single-caller isGeminiEapModel into isGeminiEapOr2OrAbove and
  drop its docblock, which restated EAP_MODEL_NAME_PATTERN's own. Mirroring
  adk-python's _is_gemini_eap_model is structural parity, not behavioural.
- Drop the duplicated "EAP ids carry no numeric version" sentence from
  isGeminiEapOr2OrAbove; the constant's docblock and the isGemini2OrAbove
  pointer already carry it.
- Drop the "no `g` flag" clause from both pattern docblocks: it explains the
  absence of a flag nobody wrote.
- Inline MODELS_PREFIX, used twice on adjacent lines while the two path
  regexes beside it are not similarly hoisted.
- Pin the isGemini1Model narrowing at a consumer outside this module.
  Tightening the predicate to /^gemini-1\.\d+/ also loosens the Gemini 1.x
  branch of GoogleSearchTool for the undotted ids: `gemini-1` and
  `gemini-1-pro` now take the googleSearch branch instead of
  googleSearchRetrieval, and no longer throw when other tools are present.
  Every existing test there uses a dotted gemini-1.5-* id, so nothing pinned
  it. Type makeRequest's tools parameter so the new cases typecheck, which
  also clears a pre-existing TS2322 on the same helper.
The new undotted-Gemini-1 cases were passing `{} as never` for toolContext,
matching the file's older tests. That is an unchecked cast standing in for a
type the tool's signature genuinely requires, so construct a real Context the
way url_context_tool_test.ts does. The pre-existing cases keep their own
convention rather than being rewritten here.
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