Repository navigation
🎯 fix: Accept Two-Decimal Rounding and Quota Failures in System One Decisions - #624
Conversation
|
@codex review |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d92467537
ℹ️ 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".
| if ( | ||
| !Number.isFinite(weightedScore) || | ||
| Math.abs(record.score - weightedScore) > roundingTolerance | ||
| Math.abs(record.score - weightedScore) > scoreTolerance(levels) |
There was a problem hiding this comment.
The level-count-only tolerance does not ensure that the score and probabilities could originate from the same unit-sum distribution. For example, with reported probabilities {0: 0, 1: 0, 2: 0.5, 3: 0.5}, any compatible unrounded distribution has an expectation of at least 2.48, yet this check accepts a reported score of 2.47 because it permits the full 0.035 global bound. This allows internally inconsistent measured scores to pass parsing; compute the feasible expectation interval from each reported probability's rounding interval and the unit-sum constraint, then apply the score's own rounding allowance.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7e7e2e0.
The level-count tolerance is gone. For a rounded systemone score, expectationRange now computes the exact range of expectations over measured distributions that sum to one and round to the reported probabilities. Each level starts at its clipped lower bound, and the leftover mass fills the lowest levels first for the minimum and the highest first for the maximum. The score must then lie within one half-step of that range. For {0, 0, 0.5, 0.5} the range is [2.48, 2.505], so 2.47 and 2.52 are now rejected, and 2.48 through 2.51 are accepted.
Tests in src/decisions/wire.test.ts:
accepts a score only where the expectation of its distribution can round to itcovers2.47,2.52and the off-grid2.475, plus 10 levels with all mass on level 9:8.95is accepted and8.94rejected.caps rounding drift at what values summing to one can producenow also rejects1.51and2on the 300-level rubric, and accepts1.49.
Both tests fail with the previous dialect.ts restored and pass with the fix. All 120 recorded jev-1.13.0 score answers in the fixture still parse.
| return roundsFromUnitSum(Object.values(probabilities)) | ||
| ? probabilities | ||
| : undefined; |
There was a problem hiding this comment.
This tolerance is applied to both dialects because probabilitiesOf is not given the dialect, although the new two-decimal behavior is documented for hosts in front of System One. Consequently, the default port dialect now accepts responses that its previous validation rejected—for example, a 100-option choice distribution containing 0.015 for every option (sum 1.5) passes because some hypothetical values within 0.005 could sum to one. Keep the broader rounding model on the systemone path so malformed generic HTTP responses do not silently become valid.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7e7e2e0.
readAnswer now passes dialect into probabilitiesOf and into the score check. The two-decimal model (roundsFromUnitSum / roundsFromExpectation) applies only when the dialect is systemone and every reported probability (and, for a score, the score itself) sits on the 0.01 grid. Every port answer keeps the pre-PR four-decimal bounds (sumsToOne / nearExpectation, the same formulas as before this PR).
The grid gate also covers the same class on the systemone path: 0.015 is not a two-decimal value, so it never gets the wide bound, and a self-hosted model that does not round keeps the strict bounds.
Test: keeps the four-decimal bounds for the port dialect and for unrounded numbers in src/decisions/wire.test.ts.
- Under both dialects it rejects 100 options of
0.015(sum 1.5), 10 of0.105, and a uniform 4-level score of1.53, and it accepts unrounded four-decimal answers. - It shows that a rounded 5-option sum of
1.02and a uniform score of1.52are accepted undersystemoneand rejected underport.
The test fails with the previous dialect.ts restored and passes with the fix.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e7e2e0730
ℹ️ 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".
| const consistent = | ||
| dialect === 'systemone' && values.every(isTwoDecimal) | ||
| ? roundsFromUnitSum(values) | ||
| : sumsToOne(values); |
There was a problem hiding this comment.
When a systemone score is off the two-decimal grid but all probabilities are on it, this branch validates the distribution with the permissive two-decimal model, while the later score check uses nearExpectation. For example, probabilities {0: 0.25, 1: 0.25, 2: 0.25, 3: 0.27} and score 1.5601 are accepted even though the probabilities total 1.02 and fail the four-decimal sum bound. Select the rounding model once for the entire score answer, including both its score and probabilities, so an off-grid score forces the distribution through sumsToOne as documented.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eac8f5f. probabilitiesOf no longer checks the sum; readAnswer now chooses the rounding model once per answer (rounded = systemone && every probability on the grid && the score, for a score answer, on the grid) and applies it to both the distribution sum (roundsFromUnitSum vs sumsToOne) and the score (roundsFromExpectation vs nearExpectation). The JSDoc and README now say "probabilities and score" and that one off-grid number puts the whole answer under the four-decimal bounds.
Test: wire.test.ts › "reads a score and its distribution under one rounding model": with {0: 0.25, 1: 0.25, 2: 0.25, 3: 0.27} (sum 1.02), score 1.53 is accepted, and the off-grid 1.5599 and 1.5601 are rejected. It fails on 7e7e2e0 (1.5599 was accepted) and passes with the fix. Checks: jest src/decisions src/rerank 144/144, tsc, eslint, prettier, build + test:light-entries 8/8, check:circular-deps.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
A distribution is now accepted only when some measured distribution summing to exactly one rounds to it, so drift never exceeds one however many values there are, and the score bound counts at most the 200 highest levels. Restores the 2000-key guard test and documents that a 401 is re-sent once after a refresh.
The two-decimal rounding model applied to every dialect, so generic port responses that summed to 1.5 or missed their own expectation started parsing. It now applies only to systemone answers whose numbers all sit on the 0.01 grid; port answers and unrounded numbers keep the four-decimal bounds.
A rounded score was checked against a tolerance from the level count alone, which accepted scores no unit-sum distribution behind the reported probabilities could produce (2.47 for {0, 0, 0.5, 0.5}). The score must now lie within a half-step of the exact range of expectations over measured distributions that sum to one and round to the reported ones.
…g Model The distribution sum check chose its model from the probabilities alone while the score check also looked at the score, so an off-grid score paired the four-decimal score bound with the loose two-decimal sum bound. readAnswer now chooses the model once per answer and applies it to both.
eac8f5f to
40620d3
Compare
1 participant
What this unblocks
codegraph, a separate code-intelligence service, is moving its decision client off its own copy and onto this SDK's
./decisionssubpath (#623). It can't do that yet, because the SDK throws on most real System One answers that pass through a hosted gateway. The same failure hits any LibreChat backend that calls System One through the ClickHouse gateway or OpenRouter.What breaks today
Rounded answers are rejected. Hosts in front of System One round every number they report (score and each probability) to two decimals.
dialect.tschecked those numbers with four-decimal tolerances:min(0.02, 0.00005*(1+L(L-1)/2) + 0.001), which is0.00135for a 4-level rubricmin(0.02, max(0.01, n*0.00005 + 0.001))A legitimately rounded score therefore fails the consistency check,
readAnswerreturnsnull, and the wholedecide()call throws. codegraph measured that 58% of real jev-1.13.0 score answers were rejected. On the fixture recorded for this PR, the old bounds reject 43 of the 120 score answers. One bad answer fails the whole call, so 30 of the 40 calls threw.An out-of-credits key looks like a malformed request.
failureForStatusmapped HTTP 402 tobad_request, so a caller couldn't tell a broken request from an exhausted credential. A breaker needs that difference: an exhausted credential should stop all further calls.What changes
Rounding bounds (
src/decisions/dialect.ts). The rounding model applies only to asystemoneanswer whose probabilities and score all sit on the0.01grid. The model is chosen once per answer: everyportanswer, and anysystemoneanswer with a probability or score off that grid (for example a self-hosted model that does not round), keeps the four-decimal bounds above unchanged for its distribution and its score alike.Under the rounding model, each reported value is treated as within
0.005of its measured value and inside[0, 1].0.005of an expectation that such a measured distribution can have.expectationRangecomputes that range exactly: every level starts at its lowest measured value, and the mass left over fills the lowest levels first for the minimum and the highest levels first for the maximum. For{0, 0, 0.5, 0.5}the expectation lies in[2.48, 2.505], so the scores accepted are2.48through2.51.Anything outside these bounds is rejected as inconsistent.
Quota failure (
transport.ts,types.ts). HTTP 402 maps to a newDecisionFailurevalue,'quota'. Likebad_request, it is never retried.unauthorizedis still re-sent once after a 401 when the credential is a function. Backoff retries still apply only torate_limited,server_errorandnetwork.Fixture (
src/decisions/fixtures/jev-1.13.0.json). This file holds 40 real jev-1.13.0 calls recorded through the ClickHouse gateway on 2026-10-18. The question and state text is synthetic code, with no private data and no credentials. Each call asks 7 questions, for 280 answers in total: 80 boolean (noul), 120 score (3- and 4-level) and 80 choice (including a 5-option rubric).How it was tested
npx jest src/decisions: 5 suites, 119 tests pass. The newwire.test.ts:{0, 0, 0.5, 0.5}, scores2.48to2.51are accepted, and2.47,2.52and the off-grid2.475are rejected. With 10 levels and all mass on level 9,8.95is accepted and8.94is rejected;0.015, 10 values of0.105, and a uniform 4-level score of1.53are rejected, and unrounded four-decimal answers are accepted. A rounded 5-option sum of1.02and a uniform score of1.52are accepted undersystemoneand rejected underport;{0.25, 0.25, 0.25, 0.27}(sum1.02), score1.53is accepted, and the off-grid scores1.5599and1.5601are rejected because their distribution must then sum to one within four decimals;1.02is accepted and1.03is rejected;{0.5, 0.52, 0, 0}are rejected, and 200 values of0.01among 250 (a sum of 2) are accepted. For a 300-level score whose mass sits on level 0,1.49is accepted and1.51,2and210are rejected.failure: 'quota'. The response body is not echoed into the error.{k0: 1, k1: 0.05}over 2000 keys is still rejected.dialect.ts(5d92467) restored, the score-range, dialect-gate and large-rubric tests fail (3 failed, 3 passed inwire.test.ts). With the fix restored, all 119 tests pass.npx jest src/decisions src/rerank(143 tests),npx tsc --noEmit,npm run build,npm run test:light-entries(8 of 8) andnpm run check:circular-depsall pass.Stack
This PR is 2 of 4. Together they let codegraph depend on the SDK's decisions and rerank modules:
./decisionsand./reranksubpaths (basemain). This PR's base.quotafailure. This PR.Merge #623 first.