Skip to content

Commit 0feec27

Browse files
Jainish-Sclaude
andcommitted
fix(utils): Fix optional chaining error handling in inject_session_state
The previous implementation incorrectly treated entire paths as optional if any segment contained '?', violating per-segment optional chaining semantics. For example, {user.profile?} with missing 'user' would incorrectly return empty string instead of raising an error. Root cause: The catch block checked 'if '?' in full_path' to decide whether to suppress KeyError. This was wrong because _get_nested_value() already handles optional segments correctly by returning None for missing optional parts. Any KeyError that propagates means a required segment was missing and should be fatal. Fix: Removed the conditional suppression logic. Now all KeyErrors from _get_nested_value() are properly re-raised, maintaining correct per-segment optional chaining semantics. Examples: - {user?.profile} with user missing → empty string (correct: user? is optional) - {user.profile?} with user missing → KeyError (correct: user is required) - {user?.profile?.role} with user missing → empty string (correct: chain stops at user?) Added test case to verify required parent with optional child raises error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent ecf09a6 commit 0feec27

File tree

2 files changed

+20
-9
lines changed

2 files changed

+20
-9
lines changed

src/google/adk/utils/instructions_utils.py

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,15 +156,8 @@ async def _replace_match(match) -> str:
156156
if value is None:
157157
return ''
158158
return str(value)
159-
except KeyError:
160-
if '?' in full_path:
161-
logger.debug(
162-
'Context variable %s not found (safe navigation), replacing with'
163-
' empty string',
164-
full_path,
165-
)
166-
return ''
167-
raise KeyError(f'Context variable not found: `{full_path}`.')
159+
except KeyError as e:
160+
raise KeyError(f'Context variable not found: `{full_path}`.') from e
168161

169162
return await _async_sub(r'{+[^{}]*}+', _replace_match, template)
170163

tests/unittests/utils/test_instructions_utils.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,24 @@ async def test_inject_session_state_with_missing_nested_key_raises_error():
391391
)
392392

393393

394+
@pytest.mark.asyncio
395+
async def test_inject_session_state_with_required_parent_missing_raises_error():
396+
"""Test that {user.profile?} raises error when 'user' (required) is missing.
397+
398+
This verifies that optional chaining is per-segment, not for the whole path.
399+
Even though 'profile?' is optional, 'user' is required and should raise error.
400+
"""
401+
instruction_template = "Value: {user.profile?}"
402+
invocation_context = await _create_test_readonly_context(state={})
403+
404+
with pytest.raises(
405+
KeyError, match="Context variable not found: `user.profile\\?`"
406+
):
407+
await instructions_utils.inject_session_state(
408+
instruction_template, invocation_context
409+
)
410+
411+
394412
@pytest.mark.asyncio
395413
async def test_inject_session_state_with_nested_and_prefixed_state():
396414
instruction_template = "User: {app:user.name} Temp: {temp:session.id}"

0 commit comments

Comments
 (0)