Repository navigation
fix(perf): fsum window means and bound every protocol number - #377
Merged
Merged
Conversation
The evaluator computed a window mean with built-in sum(), which rounds after every addition and changed its algorithm in Python 3.12. Ten idle samples of 0.01 averaged to 0.009999999999999998 on the macOS system Python 3.9, so a mean exactly at the exclusive 1% idle ceiling passed there and failed on 3.12+. Use math.fsum so every interpreter judges the same correctly rounded mean, and add a regression test that fails with plain sum() on both 3.9 and 3.12+. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The protocol page says every number in a protocol or summary must be finite and within +/-(2^53 - 1), but the checker only bounded the fields it judged with _number. A count read with _count (p95_min_samples, expected) and any field the checker never reads accepted 2^53, 10^400, NaN or Infinity: a protocol with p95_min_samples = 2^53 validated, and a summary with expected = 10^400 was judged inconclusive instead of being refused. Scan every number in the whole document iteratively and add the closed codes protocol_number_out_of_range and summary_number_out_of_range, reported alongside any field-specific code. The page now names both codes and says the bound covers unread fields. Refs: HACK-1333, HACK-1169 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Full-suite and re-check evidence at exact head Full Serial reruns on the M3 with
Hosted CI on this head: every check is green, including Local checks on this head: the protocol tests pass under 🤖 Generated with Claude Code |
This was referenced Oct 23, 2026
roodboi
marked this pull request as ready for review
October 23, 2026 07:12
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
…451) Follow-up to #377 (HACK-1333, merged as 6b2bd73) for HACK-1389. #377 made the application benchmark protocol checker refuse any number outside ±(2⁵³ − 1). Review found four gaps in that work. ## Root cause and fix **1. The documentation and the code disagreed about early refusals.** The page says an out-of- range number is refused as `summary_number_out_of_range` "alongside any code for the field that holds it". But `validate_summary` returned early on `summary_revision_mismatch` and `summary_workload_unknown` before it scanned any numbers, so the number code never appeared next to those codes. I changed the code to match the page: the range scan now runs first, and both early returns include its result. For example, a summary with `protocol_revision: 2` and a `NaN` now reports `["summary_number_out_of_range", "summary_revision_mismatch"]`. The page now names this case explicitly. **2. A repeated key hid a number from the scan.** `json.loads` keeps only the last value of a repeated key. The frozen protocol with `, "notes": NaN, "notes": 1` appended was reported valid (exit 0) because the `NaN` never reached the scan. `_load` now uses an `object_pairs_hook` that refuses any repeated key at any depth, with two new closed codes: `protocol_key_duplicate` and `summary_key_duplicate` (exit 2, before any other check). I-JSON (RFC 7493), which the page already cites for the number range, forbids repeated member names too. **3. One input gave a different code depending on the Python version.** A 5000-digit integer literal was refused as `protocol_number_out_of_range` on 3.9 and as `protocol_unreadable` on 3.11 and later. Since 3.11, Python refuses to convert an integer of more than 4300 digits, and the resulting `ValueError` was caught as an unreadable file. Any literal with more digits than `MAX_JSON_NUMBER` (16) is out of range whatever its value, so a `parse_int` hook now parses it as ±(`MAX_JSON_NUMBER` + 1). The result is still an `int`, so every type check behaves as it does for the exact value, and the code is now `protocol_number_out_of_range` on every version. The same limit also crashed the in-process API on 3.11+: `validate_protocol` raised `ValueError` from `_same` (`json.dumps`) for a 5000-digit `host.logical_cpus`. `_same` now treats a value that cannot be serialized as different, which keeps the docstring's "never raises on any JSON value" true. **4. No test tied the page to the code.** A new test reads every `±(2ⁿ − 1)` bound on the page and checks that it equals `MAX_JSON_NUMBER`. It also checks that the page contains no `±(` written any other way, and that each related code name is both in `DIAGNOSTICS` and on the page. Stdlib only, as before. ## Tests New tests in `tests/python/test_application_benchmark_protocol.py`: - `test_an_out_of_range_number_is_reported_with_an_early_refusal`: covers `NaN`, `2**53` and `-10**400`, each with a revision or workload refusal, both in process and through the CLI. A `2**53` revision gets both codes. - `test_a_repeated_key_is_refused_so_no_number_is_shadowed`: the exact repro from the issue, a nested repeated `id` in a lane, and a repeated key in a summary. Each has a passing control that has no repeated key. - `test_integer_literals_of_any_length_get_the_range_code_on_every_interpreter`: integer literals of 5000 digits (both signs), 2⁵³ and 10¹⁶ in a protocol and in a summary. `±MAX_JSON_NUMBER`, `0` and `-0` stay valid. A 5000-digit literal in a field the checker reads gets the same codes as an exact out-of-range value there. - `test_an_integer_beyond_the_conversion_limit_never_raises_in_process` - `test_document_states_the_number_bound_and_its_codes` ## Evidence - The repro on the base commit 7d9c89f, on both interpreters: the repeated-key protocol was `valid`/exit 0; the 5000-digit literal was `protocol_number_out_of_range` on 3.9.6 and `protocol_unreadable` on 3.14.8; a revision-2 summary with `NaN` was `["summary_revision_mismatch"]` only. After the fix, both interpreters report `protocol_key_duplicate`, `protocol_number_out_of_range` and `["summary_number_out_of_range", "summary_revision_mismatch"]`. - The checker validates the frozen protocol, and all 45 tests in the file pass, on `/usr/bin/python3` (3.9.6) and the mise Python (3.14.8). Hosted CI runs this step on 3.12. - Mutation proof: I applied ten mutants one at a time, and each was killed by the new tests. They cover each early return dropping the number code, removing the repeated-key hook, removing the `parse_int` hook, setting the digit bound one too small, `_same` no longer catching `ValueError`, `MAX_JSON_NUMBER` changing to `2**54 - 1`, the page's bound changing to `2⁵⁴`, the page's bound written as `2^53 - 1`, and a code name losing its backticks on the page. The `parse_int` and `_same` mutants fail only on 3.11 and later, which is expected because 3.9 has no conversion limit. Setting the digit bound one too large survives, but that mutant is equivalent: 17-digit integers are still out of range when parsed exactly. - Gates: root `bun run check` (includes `privacy:check`) passes; `bun run --cwd packages/cli typecheck` has 0 errors; gitleaks 8.30.1 over `origin/next..HEAD` finds no leaks. - Hosted CI at 8465ff7: the step for this checker ("Check application benchmark protocol controls", Python 3.12, in `Runtime state models`) passed, as did every docker-e2e shard, `Candidate core (ubuntu-latest)`, the config compilers, `secret-scan`, `runtime-images` and `linux-process-lifetime`. - `test` (full Bun suite): **passed on run attempt 2 at the exact head 8465ff7** (job 120494444599, 54m48s). On attempt 1, 8211 of 8212 passed. The one failure was `tests/native- compose-storage-witness-xattr-owner.test.ts`, "same-birth root replacement while awaiting the final reader cannot publish ready from its pinned old directory", which timed out at 5000 ms with an unhandled `E_NATIVE_COMPOSE_STATE` between tests. That test is not in the known-flake list, and this change does not touch it. The rerun was not started by this workflow unit. - `Candidate core (macos-latest)`, "Test candidate optional feature contracts", **still fails** (not rerun by this unit, because it is not a listed known flake): `hostname_authority::managed` tests `authority_startup_waits_for_inspection_before_reading_pool_state` (`provider_busy`) and `authority_startup_contention_deadline_admits_no_pool_or_socket_effect` (`NotFound`). This PR changes no Rust. The likely cause is a fixture race: the temp root in `packages/runtime- core/src/provider/hostname_authority/managed.rs` is `pid + SystemTime nanos` with no counter, and macOS `SystemTime` has microsecond resolution, so two parallel tests can share one root. The same job passed on `next` at 7d9c89f and on every other recent CI run checked. - Re-verified locally at 8465ff7: all 45 tests pass on `/usr/bin/python3` (3.9.6) and the mise Python (3.14.8). Through the CLI, the repeated-key repro gives `protocol_key_duplicate` and the 5000-digit literal gives `protocol_number_out_of_range` on both, each with exit 2. ## Gaps - A repeated key is refused on its own, before the number scan and other checks run, so a file with both problems reports only `*_key_duplicate`. That is deliberate: the parsed document is ambiguous. - A top-level non-object still reports only `protocol_not_object` / `summary_not_object`, without a number code. The page's guarantee covers numbers inside a protocol or summary object. - The in-process API (`validate_protocol`, `evaluate`) takes already-parsed values, so it cannot see repeated keys. Only the CLI's `_load` refuses them. - No CLI surface or consumer agent guidance changed, so `docs:cli-reference` and `generate:agent-plugins` are not needed. Refs HACK-1389 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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 #334 (merged as a388ce2) for HACK-1333 / HACK-1169. Two fixes to the application benchmark protocol checker.
1. Window means use a correctly rounded sum
window_meanused the built-insum(), which rounds after every addition. Python 3.12 also changed howsum()adds floats. On the macOS system Python 3.9, ten idle samples of0.01averaged to0.009999999999999998. That mean is exactly at the exclusive 1% idle ceiling, so it passed on 3.9 and failed on 3.12 and later. The checker now usesmath.fsum, so every interpreter computes the same correctly rounded mean. This is a cherry-pick of 485d46fa; it applied cleanly onnext.2. The ±(2⁵³ − 1) bound now covers every number
The protocol page says every number in a protocol or summary must be within ±(2⁵³ − 1). The checker enforced this only for fields it read with
_number. Before this change, it accepted these out-of-range values:p95_min_samples = 2**53: the protocol validated.expected = 10**400: the summary was judgedinconclusive_coverageinstead of being refused.2**53,10**400,NaNorInfinityin a field the checker does not read: accepted in both a protocol and a summary.The checker now scans every number in the whole document, iteratively so deep nesting cannot raise
RecursionError. It adds two closed codes,protocol_number_out_of_rangeandsummary_number_out_of_range, which are reported alongside any code for the field that holds the number. The page now names both codes and says the bound also covers fields the checker does not read. Three existing assertions that expected exactly one code now expect both codes. They still check that the input is refused.Validation
test_absolute_ceilings_respect_inclusivityandtest_window_mean_is_correctly_rounded_on_every_interpreterpass under/usr/bin/python3(3.9.6) and the repo'spython3(3.14.8).fsumreverted tosum(), both tests fail on 3.9.6 (['pass'] != ['fail']and0.009999999999999998 != 0.01). On 3.14.8 the window-mean test fails.test_application_benchmark_protocol.pysuite passes on both interpreters (40 tests), andscripts/check-application-benchmark-protocol.pyreports the frozen protocol as valid.2**53.origin/next..HEAD.privacy:checkpasses.packages/clitypecheck and check report 0 errors (no TypeScript changes).bun run teston the M3 at the exact head: see the comment below.Remaining gaps
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.