Skip to content

fix(response): size the probability-sum tolerance to the option count - #6

Merged
senamakel merged 1 commit into
mainfrom
probability-tolerance
Sep 22, 2026
Merged

senamakel merged 1 commit into
mainfrom
probability-tolerance

Conversation

@senamakel

@senamakel senamakel commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Summary

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 it as invalid response: choice probabilities must sum to one. Measured while ranking a 1,215-tool catalogue with tinytools-jev (tinyhumansai/tinytools#17): 5 of 160 searches at 20 options, 13 of 160 at 120 options.

  • Tolerance is now half a unit in the second decimal per option (options * 0.005), floored at the original tolerance so two-option answers are held exactly as before.
  • The error names the observed sum and option count.

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 warnings

Co-authored-by: Medulla medulla@tinyhumans.ai

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation of probability distributions with many options by allowing expected rounding differences that scale with distribution size.
    • Continued rejecting distributions whose probability totals exceed the calculated tolerance.
    • Enhanced validation errors with the probability sum and number of options.
  • Tests

    • Added coverage for large distributions affected by per-option rounding and smaller distributions that remain invalid.

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>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-22T01:59:51.842673Z dc2d2e5 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d66910f7-d585-4c4b-8764-48fdfcddc1d7

📥 Commits

Reviewing files that changed from the base of the PR and between e53d5f0 and dc2d2e5.

📒 Files selected for processing (2)
  • crates/tinyjevclient/src/response/mod.rs
  • crates/tinyjevclient/src/response/test.rs
 _________________________________
< My GPU is bigger than your GPU. >
 ---------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@senamakel
senamakel merged commit 84b3983 into main Sep 22, 2026
8 of 9 checks passed
@tinysweeper

tinysweeper Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny 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
Priority: high
Reviewed head: dc2d2e50bc6e
Updated: 1790042610 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 2
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • high · critique · 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 (crates/tinyjevclient/src/response/mod\.rs:191)
  • medium · security · Bound the size-based distribution 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 di (crates/tinyjevclient/src/response/mod\.rs:191)

Before merge

  • Address Keep the distribution tolerance from accepting arbitrary sums (crates/tinyjevclient/src/response/mod\.rs).

How this fits together

flowchart 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
Loading
Agent review details

critique

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: The size-aware tolerance accommodates the reported 21-option rounding case, but it grows beyond 1 for large distributions and can accept probabilities whose total is zero. This breaks the response validation invariant and should not merge unchanged. _The code index is behind this pull request (indexed at `66db2de72ac0`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
  • Evidence: crates/tinyjevclient/src/response/mod\.rs — Keep the distribution tolerance from accepting arbitrary sums

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The size-aware tolerance accommodates the reported provider rounding case but becomes too permissive for large distributions, allowing materially malformed responses through. This should be fixed before merging. (1 observation(s) grouped into shared inline comments) _The code index is behind this pull request (indexed at `66db2de72ac0`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
  • Evidence: crates/tinyjevclient/src/response/mod\.rs — Bound the size-based distribution tolerance

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This change tightens the public API surface of the library: it expands a response enum with a new variant, widens the accepted input payload to cover all wire fields, replaces `unwrap`-based assertions, and hardens the documented MSRV/CI/lints/README concerns. It should not be merged as-is: the missing test coverage for the new serde variant and the unreachable-clone error path mean the repository's own 90% coverage and serde-wire-test rules are not met, and the previously reported behavior cannot be called verified. (4 findings discarded for not matching a changed line) _The code index is behind this pull request (indexed at `66db2de72ac0`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: This change makes the probability-sum tolerance grow with the option count, fixing false rejections of many-option responses whose per-option rounding sums to 0.99, and keeps small distributions strict. The code, tests, and documentation hang together and I found no defects. _The code index is behind this pull request (indexed at `66db2de72ac0`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: flash, ladder/vectors, gpt-5.6-luna, deepseek/deepseek-v4-flash
  • Spend: $0.003318
  • Tokens: 49236 input · 5737 output · 18929 cached · 233 embedding
Head State Pass summary
dc2d2e50bc6e changes requested 2 active finding(s), 0 resolved finding(s) (at 1790042610)

tinysweeper 0.1.0

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high critique confident

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

priority medium confident

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 ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant