-
Notifications
You must be signed in to change notification settings - Fork 1
fix(response): size the probability-sum tolerance to the option count #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -166,14 +166,31 @@ fn validate_distribution( | |
| validate_probability(*probability, name)?; | ||
| } | ||
| let sum: f64 = probabilities.values().sum(); | ||
| if (sum - 1.0).abs() > PROBABILITY_TOLERANCE { | ||
| if (sum - 1.0).abs() > distribution_tolerance(probabilities.len()) { | ||
| return Err(Error::invalid_response(format!( | ||
| "{name} probabilities must sum to one" | ||
| "{name} probabilities must sum to one (sum {sum:.6} over {} options)", | ||
| probabilities.len() | ||
| ))); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// How far a distribution's sum may stray from one. | ||
| /// | ||
| /// Providers serialise each probability with a fixed number of decimals, so | ||
| /// the rounding error grows with the number of options. Measured on the | ||
| /// `OpenRouter` System One endpoint (2026-09): two decimals per option, and a | ||
| /// 21-option Choice answered with probabilities summing to 0.99. The | ||
| /// tolerance is therefore half a unit in the second decimal per option, | ||
| /// floored at `PROBABILITY_TOLERANCE` so a two-option answer is held as | ||
| /// tightly as before. It is a bound on rounding, not on the model: a | ||
| /// distribution that is off by more than that is still rejected. | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For valid 200–255-option Choice requests, AGENTS.md reference: AGENTS.md:L66-L68 Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the distribution tolerance from accepting arbitrary sums For a 255-option Choice, this tolerance is Additional
|
||
| } | ||
|
|
||
| fn validate_probability(value: f64, name: &str) -> Result<()> { | ||
| if !value.is_finite() || !(0.0..=1.0).contains(&value) { | ||
| return Err(Error::invalid_response(format!( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes accepted response behavior to an option-count-dependent tolerance, but
docs/specs/system-one-client.mdstill requires every distribution to differ from1.0by at most0.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 👍 / 👎.