Skip to content

test(bigframes): Use gemini-flash-2.5 in place of deprecated 2.0#16822

Merged
TrevorBergeron merged 3 commits intomainfrom
fix_notebooks
Apr 27, 2026
Merged

test(bigframes): Use gemini-flash-2.5 in place of deprecated 2.0#16822
TrevorBergeron merged 3 commits intomainfrom
fix_notebooks

Conversation

@TrevorBergeron
Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request migrates Gemini model references from version 2.0 to 2.5 across several notebooks and system tests, while also resetting notebook execution counts. The review feedback identifies inconsistencies in model naming conventions (versioned vs. unversioned) and suggests expanding test coverage for the gemini-2.5-pro model, particularly where test function names imply its inclusion.

Comment on lines 30 to 32
"gemini-2.5-pro",
"gemini-2.5-flash",
"gemini-2.5-flash-lite",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

There is an inconsistency in model naming conventions within this file. This test uses unversioned aliases (e.g., gemini-2.5-flash), while other tests (e.g., test_llm_gemini_score at line 169) use specific versioned snapshots (e.g., gemini-2.5-flash-001). It is recommended to use a consistent naming scheme across the test suite to ensure clarity and reproducibility. This applies to the parametrizations in the subsequent tests (Diffs 2-5) as well.

        "gemini-2.5-pro",
        "gemini-2.5-flash",
        "gemini-2.5-flash-lite",
        "gemini-2.5-flash-001",
        "gemini-2.5-flash-lite-001"

Comment on lines +169 to +170
"gemini-2.5-flash-001",
"gemini-2.5-flash-lite-001",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This test is missing coverage for the gemini-2.5-pro model, which is included in other LLM tests in this file. Adding it here would ensure consistent validation of the scoring functionality across all supported Gemini models.

        "gemini-2.5-pro",
        "gemini-2.5-flash-001",
        "gemini-2.5-flash-lite-001",

Comment on lines +198 to +199
"gemini-2.5-flash-001",
"gemini-2.5-flash-lite-001",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The function name test_llm_gemini_pro_score_params explicitly mentions pro, but the parametrization currently only includes flash models. This is misleading and suggests missing test coverage for the Pro model. Adding gemini-2.5-pro would align the test with its name and improve coverage.

        "gemini-2.5-pro",
        "gemini-2.5-flash-001",
        "gemini-2.5-flash-lite-001",

@TrevorBergeron TrevorBergeron marked this pull request as ready for review April 27, 2026 19:59
@TrevorBergeron TrevorBergeron requested review from a team as code owners April 27, 2026 19:59
@TrevorBergeron TrevorBergeron requested review from GarrettWu and mpovoa and removed request for a team April 27, 2026 19:59
@TrevorBergeron TrevorBergeron merged commit 68c3016 into main Apr 27, 2026
30 checks passed
@TrevorBergeron TrevorBergeron deleted the fix_notebooks branch April 27, 2026 22:12
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.

3 participants