Repository navigation
fix(pydantic-ai): record audio duration usage - #815
Abhijeet Prasad (AbhiPrasad) merged 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfae3b4550
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| audio_seconds = getattr(usage, "audio_seconds", None) | ||
| if audio_seconds is not None: | ||
| metrics["audio_seconds"] = float(audio_seconds) |
There was a problem hiding this comment.
For every response that supplies RequestUsage.audio_seconds, this introduces audio_seconds as a new top-level metric even though it is not part of the standardized Braintrust integration metric schema. Downstream usage reporting will not recognize it consistently; preserve this provider-specific value in deliberately named metadata, or first add a sanctioned normalized metric to the instrumentation specification.
Useful? React with 👍 / 👎.
| audio_seconds = getattr(usage, "audio_seconds", None) | ||
| if audio_seconds is not None: | ||
| metrics["audio_seconds"] = float(audio_seconds) |
There was a problem hiding this comment.
This new response-shaping branch has no test proving that a current RequestUsage records the intended value while older supported Pydantic AI versions remain unaffected, so a provider model or attribute-shape change can silently drop the metric or make float(audio_seconds) fail without CI detecting it. Add a reproducing integration test before relying on this compatibility path, as required for bug fixes.
AGENTS.md reference: AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
Luca Forstner (lforst)
left a comment
Luca Forstner (lforst)
left a comment
There was a problem hiding this comment.
Would love if we put this into the spec too!
Summary
audio_secondsundermetadata.pydantic_ai_usage.audio_seconds, outside canonical metrics.Validation
mise exec -- nox -s 'test_pydantic_ai_integration(latest)' -- -k test_audio_seconds_is_recorded_as_metadata— passed with Pydantic AI 2.51.0.mise exec -- nox -s 'test_pydantic_ai_integration(1.10.0)' -- -k test_audio_seconds_is_recorded_as_metadata— passed; confirms older usage objects remain unaffected.RequestUsageandModelResponsetypes. It does not make a provider request or use VCR.