Repository navigation
check_decode: add --format pxf-pb and --format pb-text - #49
Merged
Merged
Conversation
The spec's write gate (cross_write_check.sh) and text gate (cross_text_check.sh) list python as having no harness. pxf-pb decodes with unmarshal_full and writes the PB bytes; pb-text reads PB with the stock runtime and writes pxf.marshal's text. Both match the Go reference's harness. The write gate compares map entries in key order, as Go's deterministic writer puts them. upb (protobuf 7.36.2) ignores deterministic=True for map order: int32 keys -1, 0, 10, 9 came out -1, 10, 9, 0. So the harness writes each map entry itself, sorted by key and recursing into sub-messages, and leaves every other field to the runtime. Measured against the spec at dd8e221 and protowire-cpp v2.1.0, python alone: write gate 87/87 pairs, text gate 60/60, no exclusions. The security gate still passes 136/136 for python and cpp. This port has no struct-binding API of its own (callers use protobuf message classes), so #35's values.tsv twin has nothing to cover.
This was referenced Nov 7, 2026
Collaborator
Author
|
CI on 5274043: all 18 checks pass (12 test legs, 3 cibuildwheel smoke builds, CodeQL). The repo's tests don't run |
trendvidia
pushed a commit
to trendvidia/protowire
that referenced
this pull request
Nov 8, 2026
protowire-python's check_decode has pxf-pb and pb-text (trendvidia/protowire-python#49). The write and text scripts run it from the port's .venv, as cross_security_check.sh does, and python leaves NOT_YET in both. The three workflows build protowire-python against the protowire-cpp checkout beside it and require python. Measured locally against protowire-python #49 and protowire-cpp v2.1.0: write 87/87 and text 60/60 for python, no exclusions; security 136/136 for python and cpp.
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.
Adds
--format pxf-pband--format pb-texttoscripts/check_decode.py, so the spec's write gate and text gate can check this port. It advances #35 and #37. Both close with the spec PR that adds Python to those gates, which needs this merged first, because spec CI checks out this repo's main.What the modes do
Both work the way protowire-go's harness does:
pxf-pbdecodes withunmarshal_full, which applies(pxf.default)and checks(pxf.required). It writes the PB bytes on stdout, or exits 1 on a reject.pb-textreads the PB with the stock protobuf runtime and writespxf.marshal's text on stdout.Map order. The write gate compares map entries in key order. The protobuf runtime here (upb, protobuf 7.36.2) ignores
deterministic=Truefor maps: int32 keys -1, 0, 10, 9 come out as -1, 10, 9, 0. So the harness writes each map entry itself, sorted by key and recursing into sub-messages, and leaves every other field to the runtime. Before that, 6 map cases failed.The descriptor set comes from
text.binpbnext to--proto text.proto, which the spec now ships. No change was needed there.#35's struct-binding table (
values.tsv) has nothing to cover here. This port has no struct API of its own; callers use protobuf message classes.Evidence
Run from the spec at dd8e221, with a scripts change adding Python to the gates (the spec PR), local build against protowire-cpp v2.1.0 (= cpp main, a4977b2):
cross_write_check.sh)Ran 87 … 0 excluded. ✓Ran 174 … 0 excluded. ✓cross_text_check.sh)Ran 60 … 0 excluded. ✓Ran 120 … 0 excluded. ✓cross_security_check.sh)Ran 136 … ✓The four corpus rows #1 tracks (
deep-nesting-200,-1000,-100000,invalid-utf8-string) each exit 1 with a clean reject. #1 closes when the spec PR makes Python a required port in the security workflow.The repo's own tests don't exercise
check_decode.py. The gates above are its test.