Fix 0083 zero-bound to exceed per section 5.5.15#236
Merged
Conversation
A declared token budget bound of 0 was skipped for both metrics and emitted no exceeded signal. Per observability section 5.5.15 the exceeded test is a strict actual > max, so a 0 bound is exceeded by any positive usage: the exceeded span attribute and the exceeded counter must fire. Only the utilization histogram sample is skipped, since actual / 0 has no defined ratio. Narrow the evaluation helper to include a 0 bound (gate on is-not-None, not truthy) and move the divide-by-zero guard to the utilization record site alone.
There was a problem hiding this comment.
Pull request overview
This PR updates OpenArmature’s OTel token-budget observability to treat a declared token-budget bound of 0 as a real bound (not “unset”), aligning exceeded evaluation with observability §5.5.15 while avoiding an undefined utilization ratio.
Changes:
- Adjust token-budget evaluation gating to include bounds where
max_tokens == 0(usingis not Noneinstead of truthiness). - Skip only the utilization histogram sample when the denominator is
0, while still emitting exceeded signals/counters per strictactual > max. - Add unit tests covering both
input_max_tokens=0andtotal_max_tokens=0behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/openarmature/observability/otel/observer.py |
Includes 0 bounds in exceeded evaluation while guarding utilization recording against division by zero. |
tests/unit/test_observability_otel.py |
Adds regression tests asserting exceeded fires for 0 bounds and utilization samples are omitted. |
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.
Follow-up to #235 (0083 completion path). Corrects the handling of a declared token-budget bound of
0to match observability §5.5.15, per spec's coordination response.The gap
PR A treated a declared bound of
0as degenerate and skipped it for both §11.2 instruments (and emitted noexceededsignal). But §5.5.15's exceeded evaluation is a strictusage.prompt_tokens > input_max_tokens, so a0bound is exceeded by any positive usage. Skipping the exceeded signal diverged from the spec.The fix
0bound (gates each bound onis not Nonerather than truthiness, which had conflated a declared0with an undeclaredNone).openarmature.llm.token_budget.exceeded) and the exceeded counter fire per the strict>formula, so a0bound reads as exceeded.0denominator, sinceactual / 0has no defined ratio -- an omitted sample rather than a fabricated sentinel bucket.Behavior is unchanged for every positive-bound case (fixtures 126-129 and the existing budget unit tests pass). Adds unit tests for both the input and total
0-bound paths.