fix(event_loop): handle MetadataEvents without optional usage and metrics #1187
+42
−2
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.
Description
This PR resolves issue #1158 where MetadataEvent falsely claimed that metrics and usage fields were optional through type hints, but the runtime implementation required them to be present, causing KeyError exceptions.
The MetadataEvent TypedDict was defined with total=False, making all fields appear optional in type annotations:
However, the
extract_usage_metricsfunction assumed both usage and metrics fields were always present, causing runtime failures when custom model implementations omitted these fields.Solution
We modified the
extract_usage_metricsfunction to handle optional fields gracefully by providing sensible defaults for the required fields in both Usage and Metrics types:Design Decision
We chose to make the runtime behavior match the type hints rather than changing the type definitions because this approach maintains backward compatibility while enabling the flexibility that custom model implementations need. This allows models that don't have access to certain metrics (like latency information) to omit those fields without causing runtime errors.
In general, the defaulting is not ideal compared to None. However, the Usage and Metrics being present is assumed throughout the SDK so we cannot make that backwards incompatible change.
Related Issues
#1158
Documentation PR
N/A
Type of Change
Bug fix
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepareAlso ran
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.