Repository navigation
fix(perf): report every out-of-range protocol number on every Python - #451
Merged
Merged
Conversation
Follow-up to #377 (HACK-1333) for HACK-1389. Four gaps in the application benchmark protocol checker: - An out-of-range summary number is now reported alongside summary_revision_mismatch and summary_workload_unknown, as the page says, instead of being hidden by their early return. - A repeated object key is refused as protocol_key_duplicate or summary_key_duplicate. json.loads kept only the last value, so a NaN under a shadowed key was never scanned and the protocol validated. - An integer literal longer than MAX_JSON_NUMBER's 16 digits parses as just out of range, so a 5000-digit literal is protocol_number_out_of_range on Python 3.9 and 3.11+ alike (3.11+ refused the conversion, which surfaced as protocol_unreadable). Exact identity checks treat an unserializable value as different instead of raising. - A test pins the page's ±(2⁵³ − 1) bound and the code names to MAX_JSON_NUMBER and DIAGNOSTICS. Refs HACK-1389 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
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 #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". Butvalidate_summaryreturned early onsummary_revision_mismatchandsummary_workload_unknownbefore 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 withprotocol_revision: 2and aNaNnow 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.loadskeeps only the last value of a repeated key. The frozen protocol with, "notes": NaN, "notes": 1appended was reported valid (exit 0) because theNaNnever reached the scan._loadnow uses anobject_pairs_hookthat refuses any repeated key at any depth, with two new closed codes:protocol_key_duplicateandsummary_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_rangeon 3.9 and asprotocol_unreadableon 3.11 and later. Since 3.11, Python refuses to convert an integer of more than 4300 digits, and the resultingValueErrorwas caught as an unreadable file. Any literal with more digits thanMAX_JSON_NUMBER(16) is out of range whatever its value, so aparse_inthook now parses it as ±(MAX_JSON_NUMBER+ 1). The result is still anint, so every type check behaves as it does for the exact value, and the code is nowprotocol_number_out_of_rangeon every version. The same limit also crashed the in-process API on 3.11+:validate_protocolraisedValueErrorfrom_same(json.dumps) for a 5000-digithost.logical_cpus._samenow 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 equalsMAX_JSON_NUMBER. It also checks that the page contains no±(written any other way, and that each related code name is both inDIAGNOSTICSand 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: coversNaN,2**53and-10**400, each with a revision or workload refusal, both in process and through the CLI. A2**53revision gets both codes.test_a_repeated_key_is_refused_so_no_number_is_shadowed: the exact repro from the issue, a nested repeatedidin 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,0and-0stay 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_processtest_document_states_the_number_bound_and_its_codesEvidence
valid/exit 0; the 5000-digit literal wasprotocol_number_out_of_rangeon 3.9.6 andprotocol_unreadableon 3.14.8; a revision-2 summary withNaNwas["summary_revision_mismatch"]only. After the fix, both interpreters reportprotocol_key_duplicate,protocol_number_out_of_rangeand["summary_number_out_of_range", "summary_revision_mismatch"]./usr/bin/python3(3.9.6) and the mise Python (3.14.8). Hosted CI runs this step on 3.12.parse_inthook, setting the digit bound one too small,_sameno longer catchingValueError,MAX_JSON_NUMBERchanging to2**54 - 1, the page's bound changing to2⁵⁴, the page's bound written as2^53 - 1, and a code name losing its backticks on the page. Theparse_intand_samemutants 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.bun run check(includesprivacy:check) passes;bun run --cwd packages/cli typecheckhas 0 errors; gitleaks 8.30.1 overorigin/next..HEADfinds no leaks.Runtime state models) passed, as did every docker-e2e shard,Candidate core (ubuntu-latest), the config compilers,secret-scan,runtime-imagesandlinux-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 wastests/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 unhandledE_NATIVE_COMPOSE_STATEbetween 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::managedtestsauthority_startup_waits_for_inspection_before_reading_pool_state(provider_busy) andauthority_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 inpackages/runtime-core/src/provider/hostname_authority/managed.rsispid + SystemTime nanoswith no counter, and macOSSystemTimehas microsecond resolution, so two parallel tests can share one root. The same job passed onnextat 7d9c89f and on every other recent CI run checked./usr/bin/python3(3.9.6) and the mise Python (3.14.8). Through the CLI, the repeated-key repro givesprotocol_key_duplicateand the 5000-digit literal givesprotocol_number_out_of_rangeon both, each with exit 2.Gaps
*_key_duplicate. That is deliberate: the parsed document is ambiguous.protocol_not_object/summary_not_object, without a number code. The page's guarantee covers numbers inside a protocol or summary object.validate_protocol,evaluate) takes already-parsed values, so it cannot see repeated keys. Only the CLI's_loadrefuses them.docs:cli-referenceandgenerate:agent-pluginsare not needed.Refs HACK-1389
🤖 Generated with Claude Code