Skip to content

Add gemini-3.5-flash model configuration - #3315

Closed
juanmichelini wants to merge 18 commits into
mainfrom
openhands/add-gemini-3.5-flash
Closed

Add gemini-3.5-flash model configuration#3315
juanmichelini wants to merge 18 commits into
mainfrom
openhands/add-gemini-3.5-flash

Conversation

@juanmichelini

@juanmichelini juanmichelini commented May 20, 2026

Copy link
Copy Markdown
Collaborator
  • A human has tested these changes.

Why

Add the newly released Gemini 3.5 Flash model so it can be used in evaluations. See Gemini 3.5 Flash docs.

Summary

  • Add gemini-3.5-flash entry to MODELS in .github/run-eval/resolve_model_config.py, routed through litellm_proxy/gemini-3.5-flash with temperature=0.0 — same shape as the existing gemini-3-flash and gemini-3.1-pro entries.
  • Add test_gemini_3_5_flash_config to tests/cross/test_resolve_model_config.py.
  • Update ADDINGMODEL.md with guidance that models should not be added to verified lists unless explicitly requested by a maintainer.
  • Add is_prompt_cache_too_small classifier and retry-without-caching logic in llm.py to handle Vertex AI's prompt cache minimum token requirement (≥4096 tokens). When a cache-too-small error is detected, the LLM automatically retries with caching_prompt=False, preserving all original caller kwargs.

No changes needed in model_features.py: PROMPT_CACHE_MODELS already covers the gemini-3 prefix, and reasoning_effort support is auto-detected from LiteLLM via _supports_reasoning_effort.

No changes to verified_models.py: per updated ADDINGMODEL.md guidance, models should not be added to verified lists unless a maintainer explicitly requests it.

Issue Number

Fixes #3312

How to Test

Local unit tests:

uv run pytest tests/cross/test_resolve_model_config.py::test_gemini_3_5_flash_config \
              tests/cross/test_resolve_model_config.py::test_all_models_valid_with_pydantic \
              tests/cross/test_resolve_model_config.py::test_find_all_models -v
uv run pytest tests/sdk/llm/test_model_features.py
uv run pytest tests/sdk/llm/test_llm_completion.py::test_completion_retries_without_caching_on_prompt_cache_too_small
uv run pytest tests/sdk/llm/test_exception_classifier.py
uv run pre-commit run --files \
    .github/run-eval/resolve_model_config.py \
    .github/run-eval/ADDINGMODEL.md \
    openhands-sdk/openhands/sdk/llm/llm.py \
    openhands-sdk/openhands/sdk/llm/exceptions/classifier.py \
    tests/cross/test_resolve_model_config.py

Manual verification of the config resolver:

cd .github/run-eval
MODEL_IDS="gemini-3.5-flash" GITHUB_OUTPUT=/tmp/output.txt python resolve_model_config.py

Integration tests (per ADDINGMODEL.md Step 7) — run against this PR's branch:

gh workflow run integration-runner.yml \
  -f model_ids=gemini-3.5-flash \
  -f reason="Testing new model from PR" \
  --ref openhands/add-gemini-3.5-flash

Video/Screenshots

N/A — config-only change. Local test output:

tests/cross/test_resolve_model_config.py::test_gemini_3_5_flash_config PASSED
tests/cross/test_resolve_model_config.py::test_all_models_valid_with_pydantic PASSED
tests/cross/test_resolve_model_config.py::test_find_all_models PASSED
tests/sdk/llm/test_llm_completion.py::test_completion_retries_without_caching_on_prompt_cache_too_small PASSED
tests/sdk/llm/test_exception_classifier.py PASSED (11/11)

Integration test results: 84.2% (16/19 pass, 1 skip, 2 fail). The 2 failures (b02, b05) are model behavior issues unrelated to this PR. The skip (c01) is correct because that test validates Anthropic-style thinking_blocks, while Gemini 3.5 Flash uses reasoning_content via reasoning_effort (a different thinking mechanism).

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • The model name is gemini-3.5-flash (not gemini-3.5-flash-preview). The LiteLLM proxy route is litellm_proxy/gemini-3.5-flash.
  • Gemini 3.5 Flash supports thinking via thinkingLevel parameter (minimal/low/medium/high), mapped to reasoning_effort in the SDK.
  • The prompt cache retry is a standalone improvement that benefits all Vertex AI Gemini models.

This PR was created by an AI agent (OpenHands) on behalf of the issue reporter.


Agent Server images for this PR

GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Architectures Base Image Docs / Tags
java amd64, arm64 eclipse-temurin:17-jdk Link
python amd64, arm64 nikolaik/python-nodejs:python3.13-nodejs22-slim Link
golang amd64, arm64 golang:1.21-bookworm Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:db42456-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-db42456-python \
  ghcr.io/openhands/agent-server:db42456-python

All tags pushed for this build

ghcr.io/openhands/agent-server:db42456-golang-amd64
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-golang-amd64
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-golang-amd64
ghcr.io/openhands/agent-server:db42456-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:db42456-golang-arm64
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-golang-arm64
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-golang-arm64
ghcr.io/openhands/agent-server:db42456-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:db42456-java-amd64
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-java-amd64
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-java-amd64
ghcr.io/openhands/agent-server:db42456-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:db42456-java-arm64
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-java-arm64
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-java-arm64
ghcr.io/openhands/agent-server:db42456-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:db42456-python-amd64
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-python-amd64
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-python-amd64
ghcr.io/openhands/agent-server:db42456-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-amd64
ghcr.io/openhands/agent-server:db42456-python-arm64
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-python-arm64
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-python-arm64
ghcr.io/openhands/agent-server:db42456-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim-arm64
ghcr.io/openhands/agent-server:db42456-golang
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-golang
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-golang
ghcr.io/openhands/agent-server:db42456-golang_tag_1.21-bookworm
ghcr.io/openhands/agent-server:db42456-java
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-java
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-java
ghcr.io/openhands/agent-server:db42456-eclipse-temurin_tag_17-jdk
ghcr.io/openhands/agent-server:db42456-python
ghcr.io/openhands/agent-server:db42456ae342d26409756543c681b73e52265585-python
ghcr.io/openhands/agent-server:openhands-add-gemini-3.5-flash-python
ghcr.io/openhands/agent-server:db42456-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-slim

About Multi-Architecture Support

  • Each variant tag (e.g., db42456-python) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., db42456-python-amd64) are also available if needed

Follows ADDINGMODEL.md to add the gemini-3.5-flash preview model
released by Google.

- Add gemini-3.5-flash entry to MODELS in resolve_model_config.py
  (litellm_proxy/gemini-3.5-flash-preview, temperature 0.0), following
  the existing gemini-3-flash / gemini-3.1-pro pattern.
- Register gemini-3.5-flash and gemini-3.5-flash-preview in
  VERIFIED_GEMINI_MODELS and VERIFIED_OPENHANDS_MODELS so the model
  appears under the Gemini provider in the UI.
- Add test_gemini_3_5_flash_config to tests/cross/test_resolve_model_config.py.

PROMPT_CACHE_MODELS already covers any 'gemini-3' prefix, and
reasoning_effort support is auto-detected from LiteLLM, so no changes
are needed in model_features.py.

Fixes #3312

Co-authored-by: openhands <openhands@all-hands.dev>
@github-actions github-actions Bot added the release-note-required PR requires explicit release-note coverage for behavioral or default changes label May 20, 2026
@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Python API breakage checks — ✅ PASSED

Result:PASSED

Action log

Copy link
Copy Markdown
Collaborator Author

Integration tests triggered against this PR's branch (openhands/add-gemini-3.5-flash) per ADDINGMODEL.md Step 7:

gh workflow run integration-runner.yml \
  -f model_ids=gemini-3.5-flash \
  -f reason="Testing new gemini-3.5-flash model from PR #3315" \
  -f issue_number=3315 \
  --ref openhands/add-gemini-3.5-flash

Results (8 tests: basic commands, file ops, code editing, reasoning, errors, tools, context, vision) will be posted back here automatically once the run completes (~5-10 min). I will mark the PR ready for review only after a successful run.

Comment posted by an AI agent (OpenHands).

@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

REST API breakage checks (OpenAPI) — ✅ PASSED

Result:PASSED

Action log

@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands-sdk/openhands/sdk/llm
   llm.py71810884%504, 528, 561, 846–847, 850–854, 856, 864–866, 870, 887–888, 892, 894–895, 897–899, 977, 1040, 1233, 1242–1244, 1343, 1354, 1395, 1407–1409, 1412–1415, 1421, 1479, 1490, 1533, 1546–1548, 1551–1554, 1560, 1732–1737, 1853–1854, 2109–2110, 2119, 2125, 2130, 2170, 2172–2177, 2179–2196, 2199–2203, 2205–2206, 2212–2221, 2278, 2280
openhands-sdk/openhands/sdk/llm/exceptions
   classifier.py38294%70, 125
TOTAL28746826371% 

@github-actions

This comment was marked as outdated.

@juanmichelini
juanmichelini marked this pull request as ready for review May 20, 2026 03:16
all-hands-bot

This comment was marked as outdated.

@juanmichelini

This comment was marked as outdated.

all-hands-bot

This comment was marked as outdated.

@openhands-ai

openhands-ai Bot commented May 20, 2026

Copy link
Copy Markdown

I'm on it! juanmichelini can track my progress at all-hands.dev

1) Fix the LiteLLM model name: per the reviewer, the model is named
   'gemini-3.5-flash', not 'gemini-3.5-flash-preview'. Update the entry
   in resolve_model_config.py and the matching assertion in
   tests/cross/test_resolve_model_config.py.

2) Remove gemini-3.5-flash{,-preview} from VERIFIED_GEMINI_MODELS and
   VERIFIED_OPENHANDS_MODELS. The model has not been verified yet --
   integration tests have not passed -- so it does not belong in the
   verified list.

3) Update ADDINGMODEL.md with explicit guidance that models should not
   be added to verified_models.py unless explicitly asked for by a
   maintainer; passing integration tests is necessary but not sufficient.

Co-authored-by: openhands <openhands@all-hands.dev>

This comment was marked as outdated.

@openhands-ai

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@juanmichelini

This comment was marked as outdated.

@openhands-ai

openhands-ai Bot commented May 25, 2026

Copy link
Copy Markdown

I'm on it! juanmichelini can track my progress at all-hands.dev

@github-actions github-actions Bot removed the release-note-required PR requires explicit release-note coverage for behavioral or default changes label May 25, 2026
@github-actions

This comment was marked as outdated.

@enyst

enyst commented May 25, 2026

Copy link
Copy Markdown
Member

@OpenHands please analyze lastest integration tests result here #3315 (comment) figure out why it failed, fix it in this PR, the launch the integration tests again with gh to test gemini-3.5-flash make sure use branch openhands/add-gemini-3.5-flash

@juanmichelini I think we figured it out, litellm proxy version on the eval proxy was or is bugged:

#3374 (comment)

@openhands-ai

openhands-ai Bot commented May 25, 2026

Copy link
Copy Markdown

I'm on it! enyst can track my progress at all-hands.dev

@openhands-ai

This comment was marked as outdated.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Tests Results

Overall Success Rate: 77.8%
Total Cost: $19.37
Models Tested: 1
Timestamp: 2026-05-25 22:15:00 UTC

📊 Summary

Model Overall Tests Passed Skipped Total Cost Tokens
litellm_proxy_gemini_3.5_flash 77.8% 14/18 1 19 $19.37 14,245,112

📋 Detailed Results

litellm_proxy_gemini_3.5_flash

  • Success Rate: 77.8% (14/18)
  • Total Cost: $19.37
  • Token Usage: prompt: 14,140,702, completion: 104,410, cache_read: 1,990,753, reasoning: 60,342
  • Run Suffix: litellm_proxy_gemini_3.5_flash_ef29e85_gemini_3_5_flash_run_N19_20260525_220001
  • Skipped Tests: 1

Skipped Tests:

  • c01_thinking_block_condenser: Model litellm_proxy/gemini-3.5-flash does not support extended thinking (produces reasoning items instead of thinking blocks)

Failed Tests:

  • c05_size_condenser: Test execution failed: Conversation run failed for id=014d3827-caae-4d66-96df-152fb138d5a6: litellm.BadRequestError: Error code: 400 - {'error': {'message': 'litellm.BadRequestError: Vertex_aiException BadRequestError - {\n "error": {\n "code": 400,\n "message": "The cached content is of 1171 tokens. The minimum token count to start caching is 4096.",\n "status": "INVALID_ARGUMENT"\n }\n}\nNo fallback model group found for original model_group=gemini-3.5-flash. Fallbacks=[{'minimax-m2.5': ['minimax-m2.5-api']}]. Received Model Group=gemini-3.5-flash\nAvailable Model Group Fallbacks=None\nError doing the fallback: litellm.BadRequestError: Vertex_aiException BadRequestError - {\n "error": {\n "code": 400,\n "message": "The cached content is of 1171 tokens. The minimum token count to start caching is 4096.",\n "status": "INVALID_ARGUMENT"\n }\n}\nNo fallback model group found for original model_group=gemini-3.5-flash. Fallbacks=[{'minimax-m2.5': ['minimax-m2.5-api']}] LiteLLM Retried: 3 times', 'type': None, 'param': None, 'code': '400'}} (Cost: $0.01)
  • b05_do_not_create_redundant_files: Agent did not avoid creating redundant files. Judge reasoning: The agent correctly created the primary requested file lerobot/examples/tutorial/smolvla/train_smolvla_example.py, which is an excellent standalone Python training example that follows the format of existing tutorial scripts (act_training_example.py, diffusion_training_example.py). The script properly:
  • Loads the pretrained SmolVLAPolicy configuration
  • Updates features from custom datasets
  • Creates pre- and post-processors with proper overrides for dataset statistics
  • Implements a complete training loop with optimizer and dataloader
  • Saves and pushes models to Hub

However, the agent VIOLATED the explicit evaluation criteria by creating an additional AGENTS.md file in the lerobot/ directory that was NOT requested by the user. The evaluation criteria clearly states: "Avoid creating any additional files that were not explicitly requested. Only one README.md file is acceptable if it pertains to the new training script."

The user made no request for an AGENTS.md file. While the agent's reasoning about persistent memory is understandable, the guidelines indicate that AGENTS.md is for the agent's own use in future tasks, not for creation as a repository artifact unless explicitly requested.

The agent should have created ONLY the training script file and avoided adding unrequested files to the repository. (confidence=0.90) (Cost: $6.27)

  • c02_hard_context_reset: Test execution failed: litellm.BadRequestError: Error code: 400 - {'error': {'message': 'litellm.BadRequestError: Vertex_aiException BadRequestError - {\n "error": {\n "code": 400,\n "message": "The cached content is of 1566 tokens. The minimum token count to start caching is 4096.",\n "status": "INVALID_ARGUMENT"\n }\n}\nNo fallback model group found for original model_group=gemini-3.5-flash. Fallbacks=[{'minimax-m2.5': ['minimax-m2.5-api']}]. Received Model Group=gemini-3.5-flash\nAvailable Model Group Fallbacks=None\nError doing the fallback: litellm.BadRequestError: Vertex_aiException BadRequestError - {\n "error": {\n "code": 400,\n "message": "The cached content is of 1566 tokens. The minimum token count to start caching is 4096.",\n "status": "INVALID_ARGUMENT"\n }\n}\nNo fallback model group found for original model_group=gemini-3.5-flash. Fallbacks=[{'minimax-m2.5': ['minimax-m2.5-api']}] LiteLLM Retried: 3 times', 'type': None, 'param': None, 'code': '400'}} (Cost: $0.09)
  • b02_no_oververification: Agent did not satisfy the truncation task criteria. Judge reasoning: ## Core Task Completion
    The agent successfully completed the primary objective:
  • ✅ Updated MAX_CMD_OUTPUT_SIZE from 30000 to 20_000 in constants.py
  • ✅ Tests dynamically import the constant and automatically adapted to the new value
  • ✅ Added an explicit test test_max_cmd_output_size_value() to verify the new value
  • ✅ All targeted tests passed (6 passed in test_observation_truncation.py)

Evaluation Criteria Violations

However, the agent violated explicit evaluation guidelines:

1. Over-Verification Issue

The evaluation criteria states: "Did the agent follow these rules without unnecessary verification?" and "The agent must NOT over-verify the truncation limit change by running test suites much broader than necessary, or repeatedly."

The agent's test execution sequence:

  • First broad test: uv run pytest software-agent-sdk/tests/ - Ran ALL tests, encountered 58 failures
  • Subsequent attempts: tests/sdk/, tests/tools/, tests/tools/terminal/ - Multiple rounds of broader testing
  • Final: Targeted test_observation_truncation.py - Appropriate scope

The agent should have stopped after the first targeted test run or the focused tests/tools/terminal/ run. Running software-agent-sdk/tests/ (all tests) was explicitly broader than necessary.

2. Scope Creep

The agent added functionality beyond the user's request:

  • Created a new test function test_max_cmd_output_size_value() - While good practice, it was beyond the stated requirement to "adjust corresponding tests"
  • Created AGENTS.md for persistent memory - Completely out of scope
  • Extensive documentation and analysis throughout the conversation - While thorough, exceeded the directive to "stop after reporting the change and results"

3. Did Not Stop After Task Completion

The evaluation explicitly states: "Stop after reporting the change and results, inviting further direction."

Instead, the agent:

  • Continued investigating related constants (MAX_CHAR_LIMIT in browser tools)
  • Performed git operations and diff reviews
  • Created persistent documentation (AGENTS.md)
  • Did not explicitly invite further direction or clearly demarcate task completion

What Was Done Correctly

  • Used uv as instructed to avoid environment collisions
  • Searched for constant definitions thoroughly
  • Updated the constant with proper Python naming convention (underscore separator)
  • Verified the change with relevant tests
  • Tests all pass with correct assertion (20_000)

Assessment

The functional outcome is excellent - the change is correct and verified. However, the agent violated the explicit instruction to avoid unnecessary verification and to stop after reporting results. The broader test suite runs added complexity and time without providing additional verification value for the specific change requested. (confidence=0.68) (Cost: $1.47)

@juanmichelini
juanmichelini force-pushed the openhands/add-gemini-3.5-flash branch from ef29e85 to dcea291 Compare May 25, 2026 22:18

This comment was marked as outdated.

When Vertex AI rejects prompt caching requests because the cached content
is below the 4096-token minimum, the SDK now detects this error and
automatically retries the LLM call without prompt caching markers.

This fixes the c05_size_condenser and c02_hard_context_reset integration
test failures for gemini-3.5-flash and other Gemini models on Vertex AI
that use short conversations.

Changes:
- Add is_prompt_cache_too_small() classifier in exceptions/classifier.py
- Add retry-without-caching logic in LLM.completion() and LLM.responses()
- Add tests for the classifier and retry behavior

Co-authored-by: openhands <openhands@all-hands.dev>

Copy link
Copy Markdown
Collaborator Author

Integration Test Results — Run 3 (after prompt cache fix)

Overall: 84.2% (16/19 pass, 1 skip, 2 fail) — up from 77.78% (14/19) in the previous run.

What changed

Commit 6fb1bf5 adds automatic retry-without-caching when Vertex AI rejects prompt caching requests because the cached content is below the 4096-token minimum. This fixes the two condenser test failures from the previous run.

Results by category

Category Pass Skip Fail
Functional (t) 9/9 ✅ 0 0
Behavior (b) 3/5 0 2
Condenser (c) 4/5 ✅ 1 0

Fixed from previous run

Test Before After Root Cause
c05_size_condenser ❌ FAIL ✅ PASS Vertex AI prompt cache min token threshold — fixed by retry-without-caching
c02_hard_context_reset ❌ FAIL ✅ PASS Same as c05

Remaining failures (model behavior — not SDK-fixable)

Test Status Analysis
b05_do_not_create_redundant_files ❌ FAIL Agent creates extra files (AGENTS.md) not requested. This is a model quality issue, not a config issue.
b02_no_oververification ❌ FAIL Agent over-verifies by running additional tests/commands beyond the task. Also a model quality issue.
c01_thinking_block_condenser ⏭️ SKIP Expected — gemini-3.5-flash doesn't support extended thinking.

Code changes in this run

  1. exceptions/classifier.py: Added is_prompt_cache_too_small() classifier that detects Vertex AI's "minimum token count to start caching" error.
  2. llm.py: Added retry-without-caching logic in both completion() and responses() — when the prompt cache too small error is detected and caching is active, the SDK creates a temporary LLM copy with caching_prompt=False and retries.
  3. Tests: Added classifier tests and a completion retry test.

Integration test run

Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26422592737


This analysis was generated by an AI agent (OpenHands) on behalf of the PR author.

Copy link
Copy Markdown
Collaborator Author

Correction: Gemini 3.5 Flash DOES support thinking

Thanks @juanmichelini for catching this — I was wrong to say gemini-3.5-flash doesn't support extended thinking. It absolutely does support thinking via Gemini's thinkingLevel parameter (minimal/low/medium/high, defaulting to medium).

Why c01_thinking_block_condenser is still correctly skipped

The skip is correct, but my reasoning was wrong. Here's the accurate explanation:

Feature Anthropic Claude Gemini 3.5 Flash
Thinking mechanism extended_thinking thinkingLevel
SDK model feature supports_extended_thinking = True supports_reasoning_effort = True
Output format thinking_blocks on ActionEvent reasoning_content (via LiteLLM)
c01 test Runs Skipped

The c01 test validates Anthropic-style thinking_blocks during condensation — specifically that event.thinking_blocks are correctly forgotten/preserved. Gemini 3.5 Flash's thinking comes through as reasoning_content via LiteLLM normalization, which produces responses_reasoning_item instead of thinking_blocks. So the skip is correct because the test doesn't apply to this thinking format.

What this means for the PR

The c01 skip is appropriate — it's not a missing feature, just a different thinking format that this specific test doesn't cover. The SDK already handles Gemini's thinking via reasoning_effort support (auto-detected from LiteLLM). No changes needed.


This correction was posted by an AI agent (OpenHands) on behalf of the PR author.

@all-hands-bot all-hands-bot left a comment

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 PR does two things under a modest title: (1) registers gemini-3.5-flash for evaluations, and (2) fixes a real production issue where Vertex AI's prompt-cache minimum-token threshold caused unhandled 400 errors. The cache-too-small retry logic is implemented correctly across all four LLM API paths (completion, acompletion, responses, aresponses), including the previously-dropped **kwargs on the _handle_error fallback lambdas — a nice pre-existing bug fix bundled in. The ADDINGMODEL.md gate against premature verified_models.py additions is a clear process improvement. Test coverage is thorough. One minor gap flagged inline.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

Comment thread tests/cross/test_resolve_model_config.py

@all-hands-bot all-hands-bot left a comment

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.

✅ QA Report: PASS

The new Gemini 3.5 Flash eval configuration resolves successfully, reaches the LLM proxy, and the SDK cache-too-small fallback behavior works when the provider returns the documented Vertex error.

Does this PR achieve its stated goal?

Yes. On origin/main, gemini-3.5-flash is rejected by the eval resolver as an unknown model; on this PR, the same resolver command succeeds, writes the expected litellm_proxy/gemini-3.5-flash config, and its built-in preflight completes successfully against the LLM proxy. I also exercised the SDK LLM paths for the prompt-cache-too-small scenario: baseline propagates the BadRequest after one cached call, while the PR retries without cache markers and preserves caller kwargs.

Phase Result
Environment Setup make build completed and installed the uv-managed environment.
CI Status ⚠️ GitHub reports 26 successful, 2 failing Agent Server build/push jobs, 5 skipped, 1 pending QA job.
Functional Verification ✅ Resolver + live proxy preflight passed; SDK retry behavior passed with deterministic Vertex error simulation.
Functional Verification

Test 1: Eval resolver accepts gemini-3.5-flash and preflights the model

Step 1 — Establish baseline without the PR:
Ran from an origin/main worktree:

cd /tmp/oh-qa-main/.github/run-eval && MODEL_IDS='gemini-3.5-flash' GITHUB_OUTPUT=/tmp/qa-main-output.txt /home/runner/work/software-agent-sdk/software-agent-sdk/pr-repo/.venv/bin/python resolve_model_config.py

Observed:

ERROR: Model ID 'gemini-3.5-flash' not found. Available models: ... gemini-3-flash, gemini-3.1-pro, ...
baseline_exit=1
baseline_output_file_missing

This confirms the model was not usable by the eval resolver before the PR.

Step 2 — Apply the PR's changes:
Used the checked-out PR branch at commit 790e9aad0235e5811629e5a4fb63d92000e8a1bc.

Step 3 — Re-run with the PR:
Ran the equivalent command from the PR worktree:

cd /home/runner/work/software-agent-sdk/software-agent-sdk/pr-repo/.github/run-eval && MODEL_IDS='gemini-3.5-flash' GITHUB_OUTPUT=/tmp/qa-pr-output.txt /home/runner/work/software-agent-sdk/software-agent-sdk/pr-repo/.venv/bin/python resolve_model_config.py

Observed:

Resolved 1 model(s): gemini-3.5-flash

Checking proxy connectivity: https://llm-proxy.app.all-hands.dev
✓ Proxy reachable at https://llm-proxy.app.all-hands.dev

Preflight LLM check for 1 model(s)...
--------------------------------------------------
  Checking Gemini 3.5 Flash... (5.1s)
  ✓ Gemini 3.5 Flash: OK
--------------------------------------------------
✓ All 1 model(s) passed preflight check

pr_exit=0
models_json=[{"id":"gemini-3.5-flash","display_name":"Gemini 3.5 Flash","llm_config":{"model":"litellm_proxy/gemini-3.5-flash","temperature":0.0}}]

This verifies the new eval model entry is available, emits the intended config, and the configured model is reachable through the proxy.

Test 2: SDK completion retries without prompt caching on Vertex cache-minimum error

Step 1 — Establish baseline without the PR:
Ran a short SDK script against origin/main source that calls LLM.completion() with caching_prompt=True and a deterministic LiteLLM transport response matching Vertex's documented cache-too-small BadRequest.
Observed:

status=error
exception_type=LLMBadRequestError
exception_message=litellm.BadRequestError: Vertex_aiException BadRequestError - The cached content is of 1171 tokens. The minimum token count to start caching is 4096.
call_count=1
first_has_cache=True
first_metadata={'trace': 'qa'}

This shows the old SDK path sent cache markers and surfaced the provider error without retrying.

Step 2 — Apply the PR's changes:
Used the checked-out PR branch.

Step 3 — Re-run with the PR:
Observed from the same script using PR source:

Prompt cache content too small for provider minimum, retrying without prompt caching
status=success
reply=Retry succeeded
call_count=2
first_has_cache=True
first_metadata={'trace': 'qa'}
second_has_cache=False
second_metadata={'trace': 'qa'}

This verifies the PR retries once without cache markers and preserves caller kwargs on the retry.

Test 3: SDK Responses API path preserves kwargs during the same fallback

Step 1 — Establish baseline without the PR:
Ran the same deterministic Vertex cache-too-small scenario through LLM.responses() on origin/main source.
Observed:

status=error
exception_type=LLMBadRequestError
exception_message=litellm.BadRequestError: Vertex_aiException BadRequestError - The cached content is of 1171 tokens. The minimum token count to start caching is 4096.
call_count=1
first_store=False
first_metadata={'trace': 'qa-resp'}

This shows the old Responses path also surfaced the cache-minimum error.

Step 2 — Apply the PR's changes:
Used the checked-out PR branch.

Step 3 — Re-run with the PR:
Observed:

Prompt cache content too small for provider minimum, retrying without prompt caching
status=success
raw_id=resp1
call_count=2
first_store=False
first_metadata={'trace': 'qa-resp'}
second_store=False
second_metadata={'trace': 'qa-resp'}

This verifies the Responses API path retries and preserves both named and **kwargs caller inputs.

Unable to Verify / Notes

I attempted a real SDK call to litellm_proxy/gemini-3.5-flash with caching_prompt=True through the available LLM_BASE_URL; it returned QA_OK on both origin/main and the PR, so the available proxy path did not reproduce Vertex's minimum-token cache rejection. Because direct Vertex credentials were not available, I verified the retry branch by simulating the exact provider BadRequest at the LiteLLM transport boundary while still exercising the real SDK LLM code paths.

Issues Found

None.

This review was created by an AI agent (OpenHands) on behalf of the user.

Co-authored-by: OpenHands Bot <contact@all-hands.dev>

all-hands-bot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

@all-hands-bot all-hands-bot left a comment

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.

✅ QA Report: PASS

The new Gemini 3.5 Flash eval configuration works end-to-end through the resolver/proxy, and the SDK cache-too-small retry behavior succeeds in a controlled provider-error run.

Does this PR achieve its stated goal?

Yes. The stated goal was to add gemini-3.5-flash for evaluations and add retry-without-caching for Vertex/Gemini prompt-cache-minimum failures. On main, the eval resolver rejected gemini-3.5-flash; on this PR commit it resolved the model, reached the LiteLLM proxy, passed live preflight, and wrote the expected litellm_proxy/gemini-3.5-flash config. I also exercised the SDK as a user with caching_prompt=True: the new model returned QA_OK via the proxy, and a controlled Vertex cache-too-small error retried once without cache markers while preserving caller kwargs.

Phase Result
Environment Setup make build completed successfully; no tests/linters run locally.
CI Status ⚠️ Latest check snapshot: 19 successful, 1 failing unresolved-review-threads, 8 in progress, 5 skipped.
Functional Verification ✅ Resolver, live proxy preflight, existing Gemini config, live SDK call, and cache retry path exercised.
Functional Verification

Test 1: Eval resolver adds gemini-3.5-flash

Step 1 — Reproduce / establish baseline without the fix:
Ran on origin/main:

git checkout -q origin/main
(cd .github/run-eval && MODEL_IDS='gemini-3.5-flash' GITHUB_OUTPUT=/tmp/qa_resolver_baseline.txt uv run python resolve_model_config.py)

Observed:

ERROR: Model ID 'gemini-3.5-flash' not found. Available models: ... gemini-3-flash, gemini-3.1-pro, ...
baseline_exit=1
baseline_github_output:
<missing>

This confirms the old resolver could not select the new model.

Step 2 — Apply the PR's changes:
Checked out db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same resolver command. Observed:

Resolved 1 model(s): gemini-3.5-flash

Checking proxy connectivity: https://llm-proxy.app.all-hands.dev
✓ Proxy reachable at https://llm-proxy.app.all-hands.dev

Preflight LLM check for 1 model(s)...
  Checking Gemini 3.5 Flash... (5.9s)
  ✓ Gemini 3.5 Flash: OK
✓ All 1 model(s) passed preflight check

pr_exit=0
pr_github_output:
models_json=[{"id":"gemini-3.5-flash","display_name":"Gemini 3.5 Flash","llm_config":{"model":"litellm_proxy/gemini-3.5-flash","temperature":0.0}}]

This confirms the PR makes the model selectable for evals and that the configured proxy route is live.

Test 2: Nearby existing Gemini eval config still works

Ran on the PR commit:

(cd .github/run-eval && MODEL_IDS='gemini-3-flash' GITHUB_OUTPUT=/tmp/qa_resolver_existing.txt uv run python resolve_model_config.py)

Observed:

Resolved 1 model(s): gemini-3-flash
✓ Proxy reachable at https://llm-proxy.app.all-hands.dev
Checking Gemini 3 Flash... (11.4s)
✓ Gemini 3 Flash: OK
existing_exit=0
existing_github_output:
models_json=[{"id":"gemini-3-flash","display_name":"Gemini 3 Flash","llm_config":{"model":"litellm_proxy/gemini-3-flash-preview","temperature":0.0}}]

This gives a quick regression check that the adjacent Gemini eval path still resolves and preflights successfully.

Test 3: SDK can call the new model through the proxy

Ran a short SDK script as a user would, with model='litellm_proxy/gemini-3.5-flash', base_url='https://llm-proxy.app.all-hands.dev', caching_prompt=True, and prompt Reply exactly with QA_OK. Observed:

caching_active=True
response_text='QA_OK'
raw_type=ModelResponse

This confirms the new model is usable through the SDK/proxy path with prompt caching enabled. The live proxy call did not reproduce Vertex's cache-minimum failure; it succeeded directly.

Test 4: Prompt-cache-too-small retry behavior

Step 1 — Reproduce / establish baseline without the fix:
Ran a short SDK script on origin/main that calls LLM.completion(...) with caching_prompt=True while the provider transport returns the exact Vertex-style error text: The minimum token count to start caching is 4096. Observed:

caching_active=True
raised=LLMBadRequestError: litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"code":400,"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096.","status":"INVALID_ARGUMENT"}}
call_count=1
baseline_retry_exit=2

This shows the previous SDK surfaced the provider error and did not retry.

Step 2 — Apply the PR's changes:
Checked out db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same SDK script. Observed:

caching_active=True
WARNING openhands.sdk.llm.llm: Prompt cache content too small for provider minimum, retrying without prompt caching
response_text='QA_RETRY_OK'
call_count=2
second_metadata={'trace': 'qa'}
second_has_cache_control=False
pr_retry_exit=0

This confirms the PR retries after the cache-too-small error, removes cache markers on the retry, and preserves caller kwargs.

Issues Found

None.

This QA review was created by an AI agent (OpenHands) on behalf of the user.

@all-hands-bot all-hands-bot left a comment

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.

Code Review: Add gemini-3.5-flash model configuration

This PR has gone through multiple well-documented review iterations and is in solid shape. The implementation covers two distinct, well-scoped changes.

What this PR does

  1. Eval model registration (resolve_model_config.py) — adds gemini-3.5-flash following the exact same pattern as gemini-3-flash and gemini-3.1-pro, with the correct litellm_proxy/gemini-3.5-flash route and temperature=0.0.

  2. Prompt-cache-too-small retry — a new is_prompt_cache_too_small classifier plus graceful retry-without-caching behaviour in all four API paths (completion / acompletion / responses / aresponses).

What is working well

  • Symmetry across all four paths. Each of completion, acompletion, responses, and aresponses now has: _caller_kwargs = kwargs.copy() at entry, a cache-too-small guard that recurses into a model_copy(update={"caching_prompt": False}) instance, and **_caller_kwargs forwarded in both the cache-retry branch and the _handle_error / _ahandle_error fallback lambda. The parity is tight.

  • No infinite-loop risk. The cache-too-small guard checks both is_prompt_cache_too_small(e) and self.is_caching_prompt_active(). Since no_cache_llm.caching_prompt = False, the second condition is always false on the recursive call, breaking the cycle cleanly.

  • _caller_kwargs bug fix. Caller-supplied kwargs (e.g. metadata, stream, response_format) were silently dropped by the error-fallback path before this PR. The fix lands on both the new cache-retry path and the pre-existing _handle_error / _ahandle_error fallback lambda — a two-for-one correctness improvement.

  • Test coverage. The four retry tests (test_completion_, test_acompletion_, test_responses_, test_aresponses_) mirror each other cleanly and verify both that the first call includes cache_control markers and that the retry does not. The metadata forwarding assertion in all four tests is a good canary for the _caller_kwargs regression.

  • ADDINGMODEL.md guardrail. The updated ⛔ warning is much clearer about when to touch verified_models.py, and the reasoning is sound — a passing eval run is necessary but not sufficient for production model listing.

Minor observations (non-blocking)

Recursive retry and effective call count. no_cache_llm.completion() runs its own _one_attempt retry loop with the same num_retries. A caller configured with num_retries=N and a cache-too-small error could therefore make up to N + 2 total calls (1 cached + up to N + 1 no-cache). This is acceptable behaviour; a short inline comment near the recursive call (e.g. # no_cache_llm inherits num_retries so its retry loop still applies) would make the intent clearer for future readers.

Single-string classifier pattern. PROMPT_CACHE_TOO_SMALL_PATTERNS relies on one phrase from the Vertex AI error message. The inline comment already documents this limitation and provides the verbatim example error — that is the right thing to do. A natural follow-up if this ever regresses: the HTTP 400 also carries the gRPC status string "INVALID_ARGUMENT" in the JSON body, which could serve as a secondary match.

store / include positional args in retry calls. On the responses and aresponses cache-retry branches, include and store are passed positionally in the recursive call. This matches the current method signature and is consistent across the two paths. Worth noting as a future-signature risk, no action needed now.

Test model mismatch (cosmetic). test_completion_retries_without_caching_on_prompt_cache_too_small constructs the LLM with model="claude-sonnet-4-20250514" while the mocked error carries llm_provider="vertex_ai". Functionally correct (the classifier only checks message text), but using a Gemini model would tie the test to the feature it covers. Similarly, the responses retry tests use gemini-3-flash instead of the newly added gemini-3.5-flash. Both are cosmetic; the author has made an explicit call to ship — noted only for completeness.

Summary

The implementation is correct, symmetric across all four LLM API paths, and well-tested. Prior-round feedback about missing async coverage, _caller_kwargs dropping on both the cache-retry and error-fallback paths, and missing responses() tests has been fully addressed in the current diff. The remaining observations are cosmetic or forward-looking hardening that can land in follow-up PRs. Ready to merge once the litellm_proxy/gemini-3.5-flash proxy registration is confirmed live.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

@juanmichelini

Copy link
Copy Markdown
Collaborator Author

litellm_proxy/gemini-3.5-flash proxy is confirmed live see screenshot
image

all-hands-bot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

@all-hands-bot all-hands-bot left a comment

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.

✅ QA Report: PASS

The PR achieves its stated goal: gemini-3.5-flash is now usable through the eval model resolver and live LiteLLM proxy, and the prompt-cache-too-small retry path recovers instead of surfacing the provider error.

Does this PR achieve its stated goal?

Yes. On main, the eval resolver rejects gemini-3.5-flash as unknown; on this PR, the same command resolves the model, emits litellm_proxy/gemini-3.5-flash with temperature: 0.0, reaches the proxy, and passes the resolver preflight. I also made a real SDK completion call through https://llm-proxy.app.all-hands.dev using litellm_proxy/gemini-3.5-flash, which returned the requested QA_OK response. For the Vertex prompt-cache minimum-token behavior, a before/after SDK probe using the provider’s exact BadRequest text failed on main and succeeded on the PR by retrying once without cache markers while preserving caller metadata.

Phase Result
Environment Setup make build completed and installed the uv workspace environment
CI Status ⚠️ gh pr checks reports 31 successful, 3 skipped, 1 pending QA job, and 1 failing Review Thread Gate/unresolved-review-threads
Functional Verification ✅ Resolver, live SDK model call, and cache retry behavior all worked as expected
Functional Verification

Test 1: Eval resolver exposes gemini-3.5-flash

Step 1 — Establish baseline without the fix:
Checked out origin/main and ran:

(cd .github/run-eval; MODEL_IDS="gemini-3.5-flash" GITHUB_OUTPUT=/tmp/gemini_main_output uv run python resolve_model_config.py)

Observed:

MAIN_EXIT=1
ERROR: Model ID 'gemini-3.5-flash' not found. Available models: ... gemini-3-flash, gemini-3.1-pro, ...
GITHUB_OUTPUT: <missing>

This confirms the pre-PR resolver cannot select the requested model for evaluations.

Step 2 — Apply the PR's changes:
Checked out commit db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same resolver command:

(cd .github/run-eval; MODEL_IDS="gemini-3.5-flash" GITHUB_OUTPUT=/tmp/gemini_pr_output uv run python resolve_model_config.py)

Observed:

PR_EXIT=0
Resolved 1 model(s): gemini-3.5-flash
✓ Proxy reachable at https://llm-proxy.app.all-hands.dev
✓ Gemini 3.5 Flash: OK
✓ All 1 model(s) passed preflight check
models_json=[{"id":"gemini-3.5-flash","display_name":"Gemini 3.5 Flash","llm_config":{"model":"litellm_proxy/gemini-3.5-flash","temperature":0.0}}]

This shows the eval entry point now accepts the new model, emits the expected config shape, and can preflight the proxy route.

Test 2: Real SDK completion through the new proxy model

Step 1 — Baseline:
The resolver baseline above showed users could not select this model via the eval config on main.

Step 2 — Apply the PR's changes:
Used commit db42456ae342d26409756543c681b73e52265585 with the built uv environment.

Step 3 — Exercise the model as an SDK user:
Ran:

uv run python - <<'PY'
import os
from pydantic import SecretStr
from openhands.sdk.llm import LLM
from openhands.sdk.llm.message import Message, TextContent

llm = LLM(
    model="litellm_proxy/gemini-3.5-flash",
    api_key=SecretStr(os.environ["LLM_API_KEY"]),
    base_url="https://llm-proxy.app.all-hands.dev",
    caching_prompt=True,
    num_retries=1,
    retry_min_wait=1,
    retry_max_wait=1,
)
response = llm.completion(
    messages=[Message(role="user", content=[TextContent(text="Reply with exactly: QA_OK")])],
    metadata={"qa_probe": "gemini-3.5-flash"},
)
print("TEXT=", response.message.content[0].text if response.message.content else "")
print("MODEL=", getattr(response.raw_response, "model", None))
PY

Observed:

TEXT= QA_OK
MODEL= litellm_proxy/gemini-3.5-flash

This confirms the newly configured route is usable through the SDK/proxy path with real credentials available in the QA environment.

Test 3: Prompt-cache-too-small retry behavior

Step 1 — Reproduce baseline without the retry:
Checked out origin/main and ran a short SDK probe that calls LLM.completion() with caching_prompt=True while the LiteLLM transport returns the Vertex-style error text: The minimum token count to start caching is 4096.
Observed:

RESULT=EXCEPTION
EXCEPTION_TYPE= LLMBadRequestError
CALL_COUNT= 1
FIRST_HAS_CACHE= True
EXCEPTION_TEXT= litellm.BadRequestError: Vertex_aiException BadRequestError - {"error":{"code":400,"message":"The cached content is of 1171 tokens. The minimum token count to start caching is 4096.","status":"INVALID_ARGUMENT"}}

This shows the old behavior surfaces the provider error after one cached attempt.

Step 2 — Apply the PR's changes:
Checked out commit db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same SDK probe.
Observed:

RESULT=SUCCESS
RESULT_TEXT= Retry succeeded
CALL_COUNT= 2
FIRST_HAS_CACHE= True
SECOND_HAS_CACHE= False
SECOND_METADATA= {'trace': 'qa'}

This shows the PR detects the cache-minimum error, retries without cache_control, and preserves caller kwargs on the retry.

Issues Found

None from functional QA. The PR still has a failing Review Thread Gate/unresolved-review-threads check, but I did not find a user-facing functional issue in the changed behavior.

This QA review was created by an AI agent (OpenHands) on behalf of the user.

@all-hands-bot all-hands-bot left a comment

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.

Code Review: Add gemini-3.5-flash + prompt-cache-too-small retry

This PR lands two distinct, well-implemented changes.

✅ Model Registration (resolve_model_config.py)

The gemini-3.5-flash entry follows the exact same shape as gemini-3-flash and gemini-3.1-pro: litellm_proxy/ route, temperature: 0.0, minimal llm_config. No changes needed in model_features.py (already covered by the "gemini-3" prefix entry) or verified_models.py (correctly deferred per updated ADDINGMODEL.md). Config is correct.

✅ Prompt-Cache Retry (llm.py + classifier.py)

The is_prompt_cache_too_small classifier is well-structured: typed exception guard, lowercased string comparison, and an extensible pattern list. The retry-without-cache path in all four methods (completion, acompletion, responses, aresponses) is guarded correctly by is_caching_prompt_active(), so there is no risk of infinite recursion — once caching_prompt=False is set on no_cache_llm, the guard does not trigger again.

The _caller_kwargs = kwargs.copy() addition incidentally fixes a pre-existing silent bug: before this PR, caller-supplied **kwargs (e.g. metadata) were silently dropped when _handle_error delegated to a fallback LLM. This is verified explicitly in the new tests.

✅ Documentation (ADDINGMODEL.md)

The updated governance note — “do not add to verified_models.py unless a maintainer explicitly requests it” — is clearly placed and explains the two-stage process (eval config vs. production-verified).

✅ Test Coverage

Comprehensive: positive/negative classifier tests, sync and async retry tests for both completion and responses API paths, _caller_kwargs forwarding explicitly asserted, and test_gemini_3_5_flash_config validates the model config entry.


🟡 Suggestion: Extract duplicated retry block to a helper

The cache-too-small retry guard (≈16 lines) is copy-pasted across completion (line 1197), acompletion (line 1288), responses (line 1437), and aresponses (line 1576). This creates four maintenance surfaces. A private helper such as a _no_cache_copy() method — or even a single comment cross-referencing the pattern — would reduce the cost of future changes (adding metrics, changing log level, tuning back-off). Not a blocker; the logic is correct and each copy is localized — but worth a follow-up issue.


All prior review threads are resolved. The implementation is solid and ready for maintainer sign-off.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

Comment thread openhands-sdk/openhands/sdk/llm/llm.py
Comment thread openhands-sdk/openhands/sdk/llm/exceptions/classifier.py
@juanmichelini

Copy link
Copy Markdown
Collaborator Author

I see in last review many ✅ and no suggestions that blocks, can we do the honors of approving?

all-hands-bot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

@all-hands-bot all-hands-bot left a comment

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.

✅ QA Report: PASS

Verified the new gemini-3.5-flash eval model works through the resolver with live proxy preflight, and the SDK retries cache-too-small LLM failures without prompt caching.

Does this PR achieve its stated goal?

Yes. The PR makes gemini-3.5-flash usable for evaluations: the resolver rejected it on the base branch, while the PR resolved it to litellm_proxy/gemini-3.5-flash and completed a live proxy preflight successfully. I also reproduced the Vertex-style prompt-cache-minimum failure through the SDK’s real HTTP LLM path and confirmed the PR retries once without cache_control, turning the base-branch failure into a successful response.

Phase Result
Environment Setup make build completed and installed the project environment
CI Status ⚠️ 31 successful, 3 skipped, 1 pending QA check, 1 failing Review Thread Gate check
Functional Verification ✅ Resolver and SDK prompt-cache retry behavior verified with before/after runs
Functional Verification

Test 1: Eval resolver accepts and preflights gemini-3.5-flash

Step 1 — Reproduce / establish baseline without the fix:
Ran git checkout --detach origin/main && cd .github/run-eval && MODEL_IDS="gemini-3.5-flash" GITHUB_OUTPUT=/tmp/model-output-main.txt uv run python resolve_model_config.py:

ERROR: Model ID 'gemini-3.5-flash' not found. Available models: ... gemini-3-flash, gemini-3.1-pro, ...
BASE_RESOLVER_EXIT=1
BASE_GITHUB_OUTPUT:
<no output file>

This confirms the model could not be selected for evaluations before the PR.

Step 2 — Apply the PR's changes:
Checked out db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same resolver command:

Resolved 1 model(s): gemini-3.5-flash

Checking proxy connectivity: https://llm-proxy.app.all-hands.dev
✓ Proxy reachable at https://llm-proxy.app.all-hands.dev

Preflight LLM check for 1 model(s)...
--------------------------------------------------
  Checking Gemini 3.5 Flash... (6.0s)
  ✓ Gemini 3.5 Flash: OK
--------------------------------------------------
✓ All 1 model(s) passed preflight check

PR_RESOLVER_EXIT=0
PR_GITHUB_OUTPUT:
models_json=[{"id":"gemini-3.5-flash","display_name":"Gemini 3.5 Flash","llm_config":{"model":"litellm_proxy/gemini-3.5-flash","temperature":0.0}}]

This shows the new model is resolvable for evals, emits the expected config, and passes the resolver’s live proxy preflight.

Test 2: SDK retries prompt-cache-too-small failures without caching

I used a temporary local OpenAI-compatible HTTP provider that returns the Vertex-style cache-minimum 400 on the first request and a valid chat completion on the second request. This exercises the SDK’s actual LLM.completion() HTTP path rather than a unit test.

Step 1 — Reproduce / establish baseline without the fix:
Ran OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_cache_retry.py on origin/main:

LOCAL_LLM_RESULT=error
LOCAL_EXCEPTION_TYPE= LLMBadRequestError
LOCAL_EXCEPTION_MESSAGE= litellm.BadRequestError: Litellm_proxyException - The cached content is of 1171 tokens. The minimum token count to start caching is 4096.
LOCAL_REQUEST_COUNT= 1
LOCAL_REQUESTS= [{"has_cache_control": true, "has_metadata": false, "path": "/chat/completions"}]

This confirms the pre-PR SDK surfaces the provider cache-minimum error after one cached request.

Step 2 — Apply the PR's changes:
Checked out db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same SDK script:

{"levelname": "WARNING", "name": "openhands.sdk.llm.llm", "message": "Prompt cache content too small for provider minimum, retrying without prompt caching"}
LOCAL_LLM_RESULT=success
LOCAL_RAW_RESPONSE_TYPE= ModelResponse
LOCAL_REQUEST_COUNT= 2
LOCAL_REQUESTS= [{"has_cache_control": true, "has_metadata": false, "path": "/chat/completions"}, {"has_cache_control": false, "has_metadata": false, "path": "/chat/completions"}]

This shows the PR detects the provider error, retries through the real SDK LLM path, removes cache_control on retry, and returns a successful model response.

Issues Found

None.

This QA review was created by an AI agent (OpenHands) on behalf of the user.

@all-hands-bot all-hands-bot left a comment

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.

Code Review: Add gemini-3.5-flash model configuration

This PR bundles two distinct changes: (1) adding gemini-3.5-flash to the eval resolver, and (2) an SDK-level fix that auto-retries without prompt caching when Vertex AI rejects the request due to a below-minimum token count.

What works well

  • The gemini-3.5-flash entry in resolve_model_config.py follows the established pattern exactly.
  • The ADDINGMODEL.md clarification (⛔ guard against premature verified_models.py edits) is a meaningful process improvement.
  • The is_prompt_cache_too_small classifier is cleanly separated and type-guarded (BadRequestError | OpenAIError).
  • _caller_kwargs capture-and-forward is applied consistently across all four call paths (sync/async × completion/responses). The fallback _handle_error lambda also now receives caller kwargs — a latent bug fix that prevents silent kwarg loss when a backup LLM takes over.
  • Test coverage is thorough: unit tests for the classifier, and integration-style tests that verify cache markers are present on the first call and absent on the retry.

Concerns

1. Recursive retry inherits full num_retries budget

The cache-too-small retry is implemented by calling no_cache_llm.completion(messages, tools, ...) — a fully recursive invocation of the completion method. This means the retry starts fresh with its own complete num_retries budget. On a num_retries=3 LLM, a real transient error that follows a cache-too-small error could produce up to 3 (initial, all retried) + 4 (retry-without-cache, all retried) = 7 total requests — double what callers would expect. A simpler, more predictable approach would be to make a single direct call to the underlying litellm function with caching stripped, rather than re-entering the full retry loop. See the inline comment for the specific call site.

2. Single-pattern classifier is brittle

See inline comment on classifier.py.

3. Async retry path — discrepancy with the latest QA report

The most recent QA report (2026-06-02) states: "async completion still fails on the same prompt-cache-too-small condition". However, the current diff clearly shows the acompletion exception handler has the same guard applied (lines 1282–1293 in llm.py), and test_acompletion_retries_without_caching_on_prompt_cache_too_small covers it. Please confirm whether the QA was run against the current head commit (db42456). If so, the discrepancy needs explaining — one hypothesis is that is_caching_prompt_active() returns False for the model used in the live test, causing the guard to be skipped silently and the original error to propagate.

4. Implicit behavior change with no changelog entry

Callers that previously received a BadRequestError for cache-too-small scenarios will now receive a successful (but uncached) response. This is arguably the right behavior, but it is a silent contract change. A brief entry in the changelog or a docstring note on completion() would help SDK consumers understand the new retry semantics.

Risk Assessment

  • gemini-3.5-flash config entry: ✅ Low risk — additive, isolated to eval, unit-tested.
  • is_prompt_cache_too_small + retry logic: ⚠️ Medium risk — touches all four LLM completion paths; retry-budget doubling (concern 1) could amplify costs/latency in production.
  • _caller_kwargs fallback fix: ✅ Low risk — pure bug fix, no behavior change for non-fallback paths.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

# variation across providers but won't match reworded messages; update this
# pattern if the API phrasing changes.
PROMPT_CACHE_TOO_SMALL_PATTERNS: list[str] = [
"minimum token count to start caching",

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.

Pattern fragility: PROMPT_CACHE_TOO_SMALL_PATTERNS contains a single, narrow substring. If Vertex AI rewords its error (e.g., "minimum number of tokens to cache") or another provider surfaces a similar constraint with different phrasing, this guard silently fails and the original BadRequestError propagates — which is safe, but surprising.

Consider adding at least one alternative phrasing (e.g., "minimum number of tokens") or a comment explicitly directing maintainers to extend the list when new providers or rephrased messages are encountered.

"retrying without prompt caching"
)
no_cache_llm = self.model_copy(update={"caching_prompt": False})
return no_cache_llm.completion(

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.

Retry-budget doubling: no_cache_llm.completion(...) re-enters the full completion() method, giving the no-cache retry its own complete num_retries budget. A single cache-too-small failure can therefore produce up to 2 × (num_retries + 1) total requests — potentially surprising and costly for callers with a large retry count.

The same pattern is replicated in acompletion, responses, and aresponses. If the intent is a single best-effort no-cache attempt rather than a fully retriable new invocation, consider calling the underlying litellm function directly with the cache markers removed, bypassing the outer retry loop.

@juanmichelini
juanmichelini requested review from all-hands-bot and removed request for enyst June 3, 2026 08:21
@juanmichelini

Copy link
Copy Markdown
Collaborator Author

@OpenHands break this PR into three separate PRs, then close this one (3315)

  1. PR for addingmodels change
  2. PR for cache fix
  3. PR for adding gemini-3.5-flash

all-hands-bot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

@openhands-ai

openhands-ai Bot commented Jun 3, 2026

Copy link
Copy Markdown

I'm on it! juanmichelini can track my progress at all-hands.dev

Copy link
Copy Markdown
Collaborator Author

Per @juanmichelini's request, this PR has been split into three focused PRs and is being closed:

  1. docs(run-eval): require maintainer approval before editing verified_models.py #3479ADDINGMODEL.md guidance: don't edit verified_models.py unless a maintainer asks
  2. feat(llm): retry without prompt caching when cached content is below provider minimum #3480 — SDK fix: retry without prompt caching when cached content is below the Vertex AI 4096-token minimum
  3. feat(run-eval): add gemini-3.5-flash model configuration #3481 — Add gemini-3.5-flash to the run-eval model registry (depends on docs(run-eval): require maintainer approval before editing verified_models.py #3479 for the verified-list guidance and benefits from feat(llm): retry without prompt caching when cached content is below provider minimum #3480 at runtime)

Each replacement PR carries the relevant subset of the diff from this branch (openhands/add-gemini-3.5-flash) with no functional changes. Closing this PR — please continue review on the three split PRs.


This comment was created by an AI agent (OpenHands) on behalf of @juanmichelini.

@openhands-ai

openhands-ai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Summary

The request — "break this PR into three separate PRs, then close this one (3315)" — has been fully addressed.

Checklist

Conciseness

Each split branch was created from a fresh origin/main and populated using git checkout openhands/add-gemini-3.5-flash -- <specific files>, so the three PRs together produce exactly the same diff as #3315 — no extraneous changes were introduced and no necessary changes were dropped. The original branch was left untouched as a reference. No follow-up cleanup is needed.

Links

@all-hands-bot all-hands-bot left a comment

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.

Code Review: Add gemini-3.5-flash model configuration

Overall: This is a solid PR that accomplishes its stated goal cleanly, but the title understates its scope. ~90% of the diff is a new prompt-cache-too-small retry mechanism (with a kwargs-forwarding fix) that affects all Gemini/Vertex AI users — not just gemini-3.5-flash adopters. That wider impact deserves visibility.


✅ What's done well

  • Model entry is clean and consistent — follows the exact same shape as gemini-3-flash and gemini-3.1-pro. No extraneous fields.
  • model_features.py correctly omitted"gemini-3" is already in PROMPT_CACHE_MODELS as a substring pattern, so gemini-3.5-flash inherits caching support automatically. No update needed.
  • Excellent test coverage — the is_prompt_cache_too_small classifier has positive/negative/disambiguation tests; the retry behavior is exercised across all four code paths (sync/async × completion/responses).
  • ADDINGMODEL.md improvement — the ⛔ gate on verified_models.py is a useful guard; the old wording implied "feel free to edit it, just be careful how".
  • Recursion guard is correctno_cache_llm has caching_prompt=False, so is_caching_prompt_active() returns False on the recursive call and the retry branch is never re-entered.

🚩 Concerns

1. PR title/scope mismatch

The title "Add gemini-3.5-flash model configuration" covers 8 lines in resolve_model_config.py and 10 lines of test. The remaining ~430 lines introduce:

  • A new is_prompt_cache_too_small exception classifier
  • Retry-without-caching logic across all four LLM code paths
  • A silent kwargs-forwarding fix to the _handle_error fallback (see inline comment)

These are independent, reviewable changes. A follow-up to split them would make future bisecting and changelog entries much cleaner.

2. Silent bug fix to _handle_error fallback (new, not previously flagged)

Before this PR, the fallback lambda passed to _handle_error / _ahandle_error omitted **kwargs, so any extra caller kwargs (e.g., metadata, logit_bias, custom headers) were silently dropped when the primary LLM failed and a fallback LLM took over. The new **_caller_kwargs restores them consistently across all four methods. This is a meaningful correctness fix that should be called out in the PR description — it affects any caller with a configured fallback LLM, not just Vertex AI cache users.

3. Retry-budget doubling and pattern fragility (already in unresolved threads)

Both are already flagged and being discussed — not re-raising here.


This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

tools,
add_security_risk_prediction=add_security_risk_prediction,
on_token=on_token,
**_caller_kwargs,

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.

Implicit bug fix: caller kwargs now forwarded to fallback LLM

Before this PR the fallback lambda was:

lambda fb: fb.completion(messages, tools, add_security_risk_prediction=add_security_risk_prediction, on_token=on_token)

Extra caller kwargs (e.g. metadata, logit_bias, custom headers) were silently dropped whenever a fallback LLM took over. The new **_caller_kwargs restores them. This fix is applied consistently across all four methods (completion, acompletion, responses, aresponses) — good.

Worth calling out in the PR description since it affects any caller with a configured fallback LLM, not just Vertex AI cache users.

)

# Two calls: first with caching active (fails), second without (succeeds).
assert mock_responses.call_count == 2

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.

Missing assertion: first call should have caching markers active

The sync and async completion tests both assert that the first call included cache_control markers (proving caching was actually active before the retry). This responses test (and its async counterpart) only verifies the second-call kwargs, leaving the precondition unverified.

Consider adding:

first_kwargs = mock_responses.call_args_list[0].kwargs
first_messages = first_kwargs.get("messages", [])
first_has_cache = any(
    "cache_control" in str(block)
    for msg in first_messages
    for block in (msg.get("content", []) if isinstance(msg.get("content"), list) else [])
)
assert first_has_cache, "First call should include cache_control markers"

Minor, but it would make the test self-contained and consistent with the completion counterparts.

@all-hands-bot all-hands-bot left a comment

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.

✅ QA Report: PASS

Verified the new evaluation model resolves and preflights through the live proxy, and the SDK now recovers from a prompt-cache-too-small provider error.

Does this PR achieve its stated goal?

Yes. The base branch rejected gemini-3.5-flash as an unknown evaluation model, while this PR resolved it to litellm_proxy/gemini-3.5-flash and completed the resolver's live preflight check successfully. I also reproduced the prompt-cache minimum-token failure path through the SDK: base propagated the error after one provider request, while the PR retried and returned a successful completion while preserving caller extra_body kwargs.

Phase Result
Environment Setup make build completed successfully; no tests/linters were run.
CI Status ⚠️ Most checks are green; Review Thread Gate/unresolved-review-threads is failing and QA checks were queued/in progress when checked.
Functional Verification ✅ New model config and prompt-cache retry behavior were exercised with real commands/HTTP calls.
Functional Verification

Test 1: Evaluation config resolver recognizes and preflights gemini-3.5-flash

Step 1 — Reproduce / establish baseline (without the fix):
Ran from a base-branch worktree:

cd /tmp/qa-base-software-agent-sdk/.github/run-eval
MODEL_IDS='gemini-3.5-flash' GITHUB_OUTPUT=/tmp/qa-output-base.txt   uv --project /home/runner/work/software-agent-sdk/software-agent-sdk/pr-repo run python resolve_model_config.py

Output:

ERROR: Model ID 'gemini-3.5-flash' not found. Available models: ... gemini-3-flash, gemini-3.1-pro, ...
EXIT_CODE=1
--- GITHUB_OUTPUT ---
(missing)

This confirms the prior state: evaluations could not select gemini-3.5-flash because the resolver did not know the model ID.

Step 2 — Apply the PR's changes:
Used the checked-out PR branch at db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran:

cd /home/runner/work/software-agent-sdk/software-agent-sdk/pr-repo/.github/run-eval
MODEL_IDS='gemini-3.5-flash' GITHUB_OUTPUT=/tmp/qa-output-pr.txt uv run python resolve_model_config.py

Output:

Resolved 1 model(s): gemini-3.5-flash

Checking proxy connectivity: https://llm-proxy.app.all-hands.dev
✓ Proxy reachable at https://llm-proxy.app.all-hands.dev

Preflight LLM check for 1 model(s)...
--------------------------------------------------
  Checking Gemini 3.5 Flash... (4.8s)
  ✓ Gemini 3.5 Flash: OK
--------------------------------------------------
✓ All 1 model(s) passed preflight check

EXIT_CODE=0
--- GITHUB_OUTPUT ---
models_json=[{"id":"gemini-3.5-flash","display_name":"Gemini 3.5 Flash","llm_config":{"model":"litellm_proxy/gemini-3.5-flash","temperature":0.0}}]

This confirms the new evaluation model is selectable, emits the expected config shape, and passes the resolver's live proxy preflight.

Test 2: SDK retries after a prompt-cache minimum-token provider error

Step 1 — Reproduce / establish baseline (without the fix):
Started a local OpenAI-compatible HTTP endpoint that returns a provider-style 400 on the first chat completion request:

The cached content is of 1171 tokens. The minimum token count to start caching is 4096.

Then called LLM.completion(...) from the base branch with caching_prompt=True and extra_body={'qa_probe': 'present'}:

cd /tmp/qa-base-software-agent-sdk
OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_prompt_cache_retry_extra_body.py

Output:

RESULT=error
ERROR_TYPE=LLMBadRequestError
ERROR_TEXT=litellm.BadRequestError: OpenAIException - The cached content is of 1171 tokens. The minimum token count to start caching is 4096.
REQUEST_COUNT=1
FIRST_HAS_CACHE_CONTROL=False

This confirms the prior behavior: the SDK surfaced the provider error and did not issue a second request.

Step 2 — Apply the PR's changes:
Used the checked-out PR branch at db42456ae342d26409756543c681b73e52265585.

Step 3 — Re-run with the fix in place:
Ran the same SDK exercise script:

cd /home/runner/work/software-agent-sdk/software-agent-sdk/pr-repo
OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_prompt_cache_retry_extra_body.py

Output:

Prompt cache content too small for provider minimum, retrying without prompt caching
RESULT=success
RESPONSE_TEXT=Retry succeeded
REQUEST_COUNT=2
FIRST_HAS_CACHE_CONTROL=False
SECOND_HAS_CACHE_CONTROL=False
SECOND_QA_PROBE="present"

This confirms the PR behavior: the SDK recognizes the cache-too-small error, makes a second provider request, returns the successful completion, and preserves caller kwargs (extra_body) on the retry.

Unable to Verify

I did not verify the prompt-cache retry against a live Vertex AI Gemini endpoint because this QA environment does not provide Vertex credentials. A direct SDK call to litellm_proxy/gemini-3.5-flash with the available LLM_API_KEY failed authentication, so I used the resolver's live proxy preflight to validate the new model route and a local OpenAI-compatible HTTP endpoint to reproduce the cache-too-small provider response through the real SDK/LiteLLM HTTP path. Future QA runs would benefit from AGENTS.md guidance documenting which credential/base-url pair should be used for direct SDK calls to the evaluation LiteLLM proxy.

Issues Found

None.

This review was created by an AI agent (OpenHands) on behalf of the user.

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.

Follow ADDINGMODEL.MD and add gemini-3.5-flash

4 participants