Conversation
…or non-whole floats Add non-integer float guard to AssistantMessageData._sanitize_non_numeric_tokens so that values like 1.5 are mapped to 0, matching the behavior of _extract_output_tokens which returns None for such values. Previously Pydantic would lax-coerce 1.5 to int(1), causing a divergence between the summary view (0 tokens) and the detail view (1 token). Closes #874 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a divergence between the parser “fast path” and the Pydantic model path when handling fractional (non-whole) float outputTokens values, ensuring session summaries and detail timelines report consistent token counts.
Changes:
- Clamp non-whole float
outputTokens(e.g.1.5) to0inAssistantMessageData._sanitize_non_numeric_tokens, and coerce whole positive floats (e.g.100.0) toint. - Update
_extract_output_tokensdocstring to match the unified “positive whole-number only” behavior. - Extend tests to cover fractional/whole float cases and verify summary/detail consistency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/copilot_usage/models.py |
Updates outputTokens validator to reject fractional floats and coerce whole floats, aligning with parser behavior. |
src/copilot_usage/parser.py |
Docstring updated to reflect that both token-extraction paths now agree without validation caveats. |
tests/copilot_usage/test_models.py |
Adds unit tests for fractional float clamping and whole-float coercion. |
tests/copilot_usage/test_parser.py |
Simplifies equivalence test (no longer expects ValidationError) and adds an end-to-end consistency regression test. |
Contributor
There was a problem hiding this comment.
Quality Gate: APPROVED (Medium impact, good code quality)
What was evaluated:
_sanitize_non_numeric_tokensvalidator inmodels.pynow correctly handles non-whole floats (e.g.1.5→0), aligning with the existing_extract_output_tokensfast path inparser.py- Docstring updates in
parser.pyreflect the tightened contract - 4 new tests: 3 unit tests for the validator edge cases + 1 end-to-end regression test verifying both code paths agree
- Existing cross-check test simplified (try/except removed since
ValidationErroris no longer raised for non-whole floats)
Impact: Medium — touches a Pydantic field validator in the data model, but the change is a narrow bugfix for a specific edge case (non-integer float outputTokens). No API contract changes, no new dependencies. All 8 CI checks pass.
Auto-approving for merge.
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.
Closes #874
Problem
Two parallel token-counting code paths diverged for non-integer positive float
outputTokensvalues (e.g.1.5):outputTokens: 1.5_extract_output_tokens(parser fast path)None→ 0 tokens in summaryAssistantMessageData.outputTokens(Pydantic model)1(truncated via lax coercion)This caused a session event with
outputTokens: 1.5to show 0 tokens in summary/cost views but 1 token in the detail timeline.Fix
Added a non-integer float guard to
_sanitize_non_numeric_tokensinmodels.py:1.5,2.3) →0100.0) → explicitly coerced toint(100)before Pydantic0Updated the
_extract_output_tokensdocstring inparser.pyto reflect that both paths now fully agree without caveats.Updated the cross-check equivalence test in
test_parser.pyto remove theValidationErrortry/except (no longer needed since the validator now handles non-whole floats cleanly).Tests
test_non_whole_float_maps_to_zero—AssistantMessageData(outputTokens=1.5).outputTokens == 0test_non_whole_float_large_maps_to_zero—AssistantMessageData(outputTokens=2.3).outputTokens == 0test_whole_positive_float_coerced_to_int—AssistantMessageData(outputTokens=100.0).outputTokens == 100test_fractional_float_consistent_summary_and_detail— end-to-end: both session summary and detail view show 0 foroutputTokens: 1.5All existing tests continue to pass.
make checkpasses cleanly (lint, typecheck, security, unit + e2e tests, 99% coverage).