fix: use average O(1) ordinary-record key lookup - #9
Merged
Merged
Conversation
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
1 participant
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.
Intent
Release msgspec-toon 0.3.0b3 from a branch based on public origin/main. Fix issue 11 by making ordinary-record key lookup average O(1) in distinct-key count without changing canonical bytes or weakening conformance, G2, G3, G5, or payload-safe errors. Keep the permanent 32-key and 512-key ten-worker A/B guard, publish the full fixed-size 4-to-1024-key curve and exact source-bound evidence, preserve the lazy cache layout that avoids typed-path regressions, include the public benchmark module required by ab.py, and open a reviewable PR. Do not include private development files or unrelated user changes. Do not tag or publish PyPI until PR CI and the verified wheel workflow are green.
What Changed
msgspec-toonto 0.3.0b3 and refresh the changelog, conformance report, benchmark evidence, assets, and documentation.Risk Assessment
🚨 High: The implementation appears sound, but a required release-performance guard is not executed with the explicitly required worker count.
Testing
On Python 3.13.1, focused behavioral and release tests plus the pinned conformance corpus succeeded; live ten-worker and same-session A/B artifacts demonstrate average-O(1)-style cardinality scaling while G2, G3, G5, canonical bytes, safe errors, and publication controls remain satisfied. An initial default-Python setup was superseded by the complete release-Python rerun, and all temporary worktree artifacts were cleaned.
Evidence: 10-worker 4–1,024-key cardinality curve
Evidence: 32-key base-versus-target A/B
Evidence: 512-key base-versus-target A/B
Evidence: G2 allocation proof
Evidence: G3 typed-path benchmark
/var/folders/pt/19wwr80d5bs38zmww5rm2vsc0000gn/T/no-mistakes-evidence/01KZYBEM8QNRQXN69HCXN8F1GA/g5-codec-4096.json)Evidence: Commit, runtime, and evidence SHA-256 binding
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
README.md:242- Intent requires “exact source-bound evidence.” This line claims 36.8% and 87.9%, but the referenced final report records improvements of 35.8309% and 87.4457% (35.8% and 87.4% rounded); CHANGELOG.md:13 repeats the stale values. Synchronize both claims with the final guard, or regenerate evidence that supports the claimed figures.🔧 Fix: Synchronize benchmark claims with release guard evidence
1 error still open:
benches/ab.py:184- Intent requires a “32-key and 512-key ten-worker A/B guard,” but these new guard cells use the existing default--rounds=2, producing only four baseline and four current worker observations per metric; the committed guard JSON confirms four values per side. Either make ten workers the enforced guard default and regenerate the evidence, or confirm that “ten-worker” was intended to describe only the standalone cardinality curve.✅ **Test** - passed
✅ No issues found.
uv sync --group bench --locked(initial environment setup, superseded by the release-Python run)uv sync --python 3.13 --group bench --lockeduv run --no-sync maturin develop --releaseuv run --no-sync pytest -q tests/test_benchmark_shapes.py tests/test_efficiency_lock.py tests/test_containment.py tests/test_errors.py tests/test_control_patterns.py tests/test_release_workflows.py tests/test_release_report.pyuv run --no-sync python conformance/run.pyuv run --no-sync python benches/bench_key_cardinality.pyBuilt base commit6cb36cb50fd7ab2ded0e298c27574357c676bda1as a Python 3.13 release wheel fromgit archive, then installed it into an isolated.venv-base.uv run --no-sync python benches/ab.py --baseline-venv .venv-base --current-venv .venv --rounds 4 --only 'untyped distinct-32-key decode' --output …/ab-distinct-32.jsonuv run --no-sync python benches/ab.py --baseline-venv .venv-base --current-venv .venv --rounds 4 --only 'untyped distinct-512-key decode' --output …/ab-distinct-512.jsonmake g2uv run --no-sync python -c '… bench_typed.run(4096) …'uv run --no-sync python -c '… bench_codecs.run(4096, shape="uniform-records") …'Validated evidence contracts withjq -e, recorded SHA-256 digests, removed transient environments/build outputs, and confirmedgit status --shortis empty.🔧 **Document** - 1 issue found → auto-fixed ✅
scripts/release-report.py:324- Generated evidence metadata still says the A/B harness runs sixteen metrics, while the checked-in guard contains 100 metric-size points. Updating the authoritative generator and regenerating conformance/report.json requires a benchmark-authorized phase.🔧 Fix: Correct release guard point count
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.