fix(cli): raise a clear error when conformance replay recordings are missing - #7129
Open
chelsealong wants to merge 1 commit into
Open
chelsealong wants to merge 1 commit into
chelsealong wants to merge 1 commit into
Conversation
…missing adk conformance test crashed with AttributeError: 'NoneType' object has no attribute 'recordings' when the ADK web server it talks to over HTTP wasn't started with ReplayPlugin loaded (e.g. only RecordingsPlugin was passed via --extra_plugins, which is what record mode needs but replay does not use). _ConformanceTestGemini now raises a ReplayVerificationError that names the missing plugin and the --extra_plugins flag needed to fix it, instead of crashing on an unrelated attribute access. Fixes google#7128
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7128
Root cause
adk conformance testtalks to an already-runningadk webserver overHTTP (
AdkWebServerClient, defaulthttp://127.0.0.1:8000). It neverstarts a server itself, so whichever plugins the server was started with
determine what conformance operations actually work.
RecordingsPluginloaded server-side (viaadk web --extra_plugins=google.adk.cli.plugins.recordings_plugin.RecordingsPlugin)to capture LLM/tool interactions into
generated-recordings.yaml.adk conformance test) needs the differentReplayPlugin(google.adk.cli.plugins.replay_plugin.ReplayPlugin) toload those recordings into session state (
_adk_replay_recordings) so_ConformanceTestGeminican mock the model.ReplayPluginis never referenced anywhere outside its own module (onlycovered by its own unit tests), so there is no built-in or documented way
to discover that it's required. The issue's repro follows the only
plugin hinted at in the workflow —
RecordingsPlugin— and reuses thesame running server for both the record and test phases. Because that
server never loaded
ReplayPlugin,_ConformanceTestGemini.__init__read
config.get('_adk_replay_recordings')asNoneand crashed with anunrelated
AttributeErrorwhen indexing into it:Fix
_ConformanceTestGemini.__init__now checks for the missing recordingsand raises a
ReplayVerificationErrorthat names the actual problem andhow to fix it (start the server with
ReplayPluginloaded), instead ofcrashing on an unrelated attribute access a few lines later. This follows
the same "turn a cryptic runtime error into an actionable one" pattern
already used elsewhere in this codebase (e.g. the
ImportErrorraisedfor
VertexAiSearchTool's GCP bypass).I did not change the server/plugin wiring itself (e.g. auto-registering
ReplayPlugin/RecordingsPluginon everyadk webserver) since thatchanges production request-path behavior for all
adk webusers and isa larger design call than a minimal bug fix should make unilaterally.
Test plan
Added
tests/unittests/cli/conformance/test_conformance_test_google_llm.py:test_missing_recordings_raises_actionable_error— constructs_ConformanceTestGeminiwith a config missing_adk_replay_recordings(the exact state produced when
ReplayPluginisn't loaded) and assertsa
ReplayVerificationErrormentioningReplayPluginis raised.test_present_recordings_does_not_raise— sanity check that aproperly populated config still constructs fine.
Verified the first test reproduces the issue's exact
AttributeErroragainst the pre-fix code (via
git checkout HEAD~1 -- <file>, confirmedfailing, then restored the fix):
With the fix applied:
Also ran the full
tests/unittests/clisuite (includes existingconformance and replay-plugin tests) to check for regressions:
isort/pyinkreport no formatting changes needed on the touched files.AI assistance disclosure
This change was developed with the help of Claude Code (Anthropic).
🤖 Generated with Claude Code