Skip to content

ci: pass GEMINI_MODEL repository variable to update-skill workflow #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dkhawk wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ci: pass GEMINI_MODEL repository variable to update-skill workflow #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e49d8d2
ci: pass GEMINI_MODEL repository variable to update-skill workflow
dkhawk Sep 9, 2026
6a90fe8
test: support GEMINI_MODEL in GeminiVisualTestHelper with default gem…
dkhawk Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/update-skill.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ jobs:
- name: Update Skill via Gemini
env:
GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}
GEMINI_MODEL: ${{ vars.GEMINI_MODEL }}

@kikoso kikoso Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When the GEMINI_MODEL repository variable is not set, ${{ vars.GEMINI_MODEL }} expands to the empty string, and Actions still defines the env var. .github/scripts/update_skill.py:21 uses os.getenv("GEMINI_MODEL", "gemini-3.8-flash"), whose default only applies when the name is absent, not when it is empty. So it gets "", and line 22 builds .../v1beta/models/:generateContent?key=....

Net effect: adding this line breaks update-skill on every repo that has not set the variable, where before it used a hardcoded model and worked.

The fix belongs in the script rather than here, os.getenv("GEMINI_MODEL") or "gemini-3.8-flash", but since this PR is what introduces the empty value, it should go in together.

SKILL_FILE: .gemini/skills/android-maps3d-sdk/SKILL.md
run: |
python .github/scripts/update_skill.py
Expand Down
5 changes: 3 additions & 2 deletions visual-testing/src/main/java/com/google/maps/android/visualtesting/GeminiVisualTestHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class GeminiVisualTestHelper {

val fullPrompt = "$systemPrompt\n\nCommand: \"$prompt\"\n\nUI Hierarchy:\n$hierarchyXml"

val modelName = "gemini-3.5-flash"
val modelName = System.getenv("GEMINI_MODEL") ?: "gemini-3.8-flash"

@kikoso kikoso Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This one will silently never do anything. GeminiVisualTestHelper is consumed from androidTest sources (maps3d-compose-demo, Maps3DSamples/ApiDemos/kotlin-app, Maps3DSamples/ComposeDemos/app all instantiate it in their BaseVisualTest), so it runs inside the app process on the device. That process does not inherit the Gradle or CI runner environment, so System.getenv("GEMINI_MODEL") is always null there and the elvis branch always wins. The model stays pinned to gemini-3.8-flash no matter what the repo variable says, which is the opposite of what the PR title promises.

The class already shows the right pattern: every public entry point (performActionFromPrompt, analyzeImage, analyzeImageBlocking) takes apiKey: String as a parameter rather than reading the environment, and BaseVisualTest sources it from BuildConfig.GEMINI_API_KEY via the secrets plugin, exactly because env vars do not survive the hop onto the device.

I would follow that: thread the model in the same way it is already done for the key, either as a defaulted parameter or a second BuildConfig field, so secrets.properties and CI both drive it through one mechanism.

suspend fun analyzeImage(
    bitmap: Bitmap,
    prompt: String,
    apiKey: String,
    model: String = BuildConfig.GEMINI_MODEL,
)

If you prefer to keep the signatures untouched, InstrumentationRegistry.getArguments().getString("GEMINI_MODEL") also works and can be passed with -Pandroid.testInstrumentationRunnerArguments.GEMINI_MODEL=.... Either is fine. System.getenv is the one option that cannot work.


val requestJson = JSONObject().apply {
put("contents", JSONArray().apply {
Expand Down Expand Up @@ -207,7 +207,8 @@ class GeminiVisualTestHelper {
})
}

val response: HttpResponse = client.post("https://generativelanguage.googleapis.com/v1beta/models/gemini-3.5-flash:generateContent?key=$apiKey") {
val model = System.getenv("GEMINI_MODEL") ?: "gemini-3.8-flash"

@kikoso kikoso Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same as line 85, and it also drifts from the neighbouring code: listAvailableModels on line 166 is called from analyzeImage on line 190 and still hits v1/models unversioned. Not asking you to fix that here, just noting that once the model becomes configurable, that debug listing call on every analyzeImage invocation is an extra round trip per screenshot. Might be worth dropping in a follow-up.

val response: HttpResponse = client.post("https://generativelanguage.googleapis.com/v1beta/models/$model:generateContent?key=$apiKey") {
contentType(ContentType.Application.Json)
setBody(requestJson.toString())
}
Expand Down
Loading