fix(response): size the probability-sum tolerance to the option count - #6
Conversation
The OpenRouter System One endpoint rounds each probability to two decimals, so a 21-option Choice legitimately answers with probabilities summing to 0.99 and the fixed 1e-6 tolerance rejected 3-8% of many-option searches. Allow half a unit in the second decimal per option, floored at the original tolerance so two-option answers are held as before, and name the sum and option count in the error. Co-authored-by: Medulla <medulla@tinyhumans.ai>
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. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches📝 Generate docstrings
Comment |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 2 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Before merge
How this fits togetherflowchart LR
n0["validate_distribution<br/>changed<br/>2 findings"]:::blocking
n1["accepts_the_exact_score_rounding_boundary<br/>changed"]:::changed
n2["response"]:::impacted
n3["validate_pair"]:::impacted
n4["request"]:::impacted
n5["validate_for"]:::impacted
n6["openrouter_accepts_resolved_jev_models_only"]:::impacted
n7["validates_all_three_answer_types"]:::impacted
n1 -->|calls| n2
n1 -->|tests| n2
n1 -->|calls| n4
n1 -->|tests| n4
n1 -->|calls| n5
n1 -->|tests| n5
n3 -->|calls| n0
n6 -->|calls| n2
n6 -->|tests| n2
n6 -->|calls| n4
n6 -->|tests| n4
n7 -->|calls| n2
n7 -->|tests| n2
n7 -->|calls| n4
n7 -->|tests| n4
n7 -->|calls| n5
n7 -->|tests| n5
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc2d2e50bc
ℹ️ 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 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 address that feedback".
| fn distribution_tolerance(options: usize) -> f64 { | ||
| // A Choice holds at most 255 options, so the cast is exact. | ||
| let options = f64::from(u32::try_from(options).unwrap_or(u32::MAX)); | ||
| PROBABILITY_TOLERANCE.max(options * 0.005) |
There was a problem hiding this comment.
Bound the scaled tolerance before validation becomes vacuous
For valid 200–255-option Choice requests, options * 0.005 is at least 1.0, so a response containing 0.0 for every matching option passes both this sum check and the highest-probability-choice check. The relaxation also starts immediately—a two-option distribution summing to 0.995 is now accepted despite the stated intent to preserve the old strict behavior. Rework or cap the allowance so rounding tolerance cannot admit completely non-normalized distributions.
AGENTS.md reference: AGENTS.md:L66-L68
Useful? React with 👍 / 👎.
| } | ||
| let sum: f64 = probabilities.values().sum(); | ||
| if (sum - 1.0).abs() > PROBABILITY_TOLERANCE { | ||
| if (sum - 1.0).abs() > distribution_tolerance(probabilities.len()) { |
There was a problem hiding this comment.
Update the specified probability-sum contract
This changes accepted response behavior to an option-count-dependent tolerance, but docs/specs/system-one-client.md still requires every distribution to differ from 1.0 by at most 0.000001. Since callers can no longer rely on the implemented spec—and the repository explicitly treats specs as accepted behavior—update the specification alongside this validation change.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Requesting changes: 1 lane(s) blocking, worst finding is high.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0033 · 49,236 in / 5,737 out · 18,929 cached (38%) · flash, ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash · 233 embedded
critique: $0.0010 · 15,930 in / 1,217 out · 2,548 cached (16%) · gpt-5.6-luna, deepseek/deepseek-v4-flash
security: $0.0009 · 12,859 in / 1,150 out · 0 cached (0%) · gpt-5.6-luna
tests: $0.0006 · 12,419 in / 1,088 out · 8,459 cached (68%) · deepseek/deepseek-v4-flash
description: $0.0001 · 4,151 in / 194 out · 4,151 cached (100%) · deepseek/deepseek-v4-flash
| fn distribution_tolerance(options: usize) -> f64 { | ||
| // A Choice holds at most 255 options, so the cast is exact. | ||
| let options = f64::from(u32::try_from(options).unwrap_or(u32::MAX)); | ||
| PROBABILITY_TOLERANCE.max(options * 0.005) |
There was a problem hiding this comment.
Keep the distribution tolerance from accepting arbitrary sums
For a 255-option Choice, this tolerance is 1.275, so a response with every probability equal to 0.0 has sum difference 1.0 and is accepted. That contradicts the comment that distributions farther from one are rejected and allows a completely uninformative response through validation. Bound the tolerance so it cannot make materially invalid sums pass, or validate the rounded distribution with a stricter invariant.
Additional security observation
Bound the size-based distribution tolerance
[RULE] unbounded-validation-tolerance
With the maximum 255 options, this returns a tolerance of 1.275. Because each probability is only checked to be within [0, 1], a distribution containing all zeroes has a sum difference of 1.0 and is therefore accepted. That can make an invalid response pass validation, and downstream choice validation may accept any label as tying for the highest probability. Keep the rounding allowance bounded so it cannot accept materially non-normalized distributions; the tolerance should account for serialization rounding without exceeding a meaningful distribution error bound.
[RULE] invalid-distribution-acceptance ·
Summary
The OpenRouter System One endpoint rounds each probability to two decimals, so a 21-option
Choicelegitimately answers with probabilities summing to 0.99 — and the fixed1e-6tolerance rejected it asinvalid response: choice probabilities must sum to one. Measured while ranking a 1,215-tool catalogue withtinytools-jev(tinyhumansai/tinytools#17): 5 of 160 searches at 20 options, 13 of 160 at 120 options.options * 0.005), floored at the original tolerance so two-option answers are held exactly as before.Test plan
cargo test --workspace(new: 21-option sum 0.99 accepted; a two-option sum 0.99 still rejected)cargo clippy --workspace --all-targets -- -D warningsCo-authored-by: Medulla medulla@tinyhumans.ai
Summary by CodeRabbit
Bug Fixes
Tests