Repository navigation
Implementation plan: format-specific bin/ext/str decoding (#13) #62
Closed
SeanTAllen
started this conversation in
Research
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Implementation plan for #13: adding format-specific decode methods for the str, bin, and ext format families. The encoder already has all format-specific methods; the decoder currently only has compact (auto-dispatching) methods for these families.
Context
MessagePackDecoderprovides two styles of methods:ifchain. Examples:uint(),int(),str(),byte_array(),ext().u8(),u16(),fixstr(),array_16().The decoder is missing format-specific methods for three families: str (except
fixstrwhich exists), bin, and ext. The encoder has all of these already.Methods to Add
14 methods on
MessagePackDecoder:str family (3 methods)
str_8(b: Reader): String iso^ ?0xD9String iso^str_16(b: Reader): String iso^ ?0xDAString iso^str_32(b: Reader): String iso^ ?0xDBString iso^bin family (3 methods)
bin_8(b: Reader): Array[U8] iso^ ?0xC4Array[U8] iso^bin_16(b: Reader): Array[U8] iso^ ?0xC5Array[U8] iso^bin_32(b: Reader): Array[U8] iso^ ?0xC6Array[U8] iso^fixext family (5 methods)
fixext_1(b: Reader): (U8, Array[U8] val) ?0xD4(U8, Array[U8] val)fixext_2(b: Reader): (U8, Array[U8] val) ?0xD5(U8, Array[U8] val)fixext_4(b: Reader): (U8, Array[U8] val) ?0xD6(U8, Array[U8] val)fixext_8(b: Reader): (U8, Array[U8] val) ?0xD7(U8, Array[U8] val)fixext_16(b: Reader): (U8, Array[U8] val) ?0xD8(U8, Array[U8] val)ext family (3 methods)
ext_8(b: Reader): (U8, Array[U8] val) ?0xC7(U8, Array[U8] val)ext_16(b: Reader): (U8, Array[U8] val) ?0xC8(U8, Array[U8] val)ext_32(b: Reader): (U8, Array[U8] val) ?0xC9(U8, Array[U8] val)Design Decisions
1. All format-specific methods validate the format byte
Each method calls
_read_type(b)?and checks it matches the expected format byte, erroring on mismatch. This matches the pattern used by existing format-specific methods likeu8(),array_16(),map_32().The "fix" variants that embed data in the format byte (
fixstr,fixarray,fixmap,positive_fixint) skip validation because consuming the format byte is reading the data. The new methods all have separate format bytes and data, so validation applies.The cost is one comparison — negligible relative to the data read, and it catches misuse (calling
str_8on str_16-encoded data).2. ext methods return
(U8, Array[U8] val)— same as compactext()The ext wire format is:
[format byte] [length] [ext type byte] [data bytes]. The ext type byte is user-defined metadata identifying what kind of extension this is. It's part of the value, not something to discard or split off.Returning
(ext_type, data)keeps the methods self-contained — one call decodes the complete ext value. This matches the existing compactext()method, giving a consistent API regardless of which method the caller chooses.The alternative (returning just the data array) would either lose the ext type byte or require the caller to read it separately, splitting the decode across two calls with no benefit.
3. Compact methods remain independent — no delegation
The compact methods (
str(),byte_array(),ext()) cannot delegate to the new format-specific methods. The reason is structural: compact methods call_read_type(b)?to consume and inspect the format byte, then dispatch. But format-specific methods also call_read_type(b)?and validate it. By the time the compact method knows which format-specific method to call, the format byte is already consumed.This differs from the encoder, where compact methods can delegate because the dispatch is based on value size (which doesn't consume the writer), not on reading a format byte.
Both API surfaces coexist as alternatives: callers who don't know the wire format use compact methods; callers who do use format-specific methods.
4. Streaming decoder updated to use format-specific methods
MessagePackStreamingDecoderalready knows the exact wire format before calling intoMessagePackDecoder(it peeks the format byte for dispatch and size checking). Currently it calls compact methods, which redundantly re-dispatch on the format byte. After this change, it will call format-specific methods instead, skipping the redundantifchain.The coupling comments ("Note: MessagePackStreamingDecoder pre-validates total size before calling this method...") move from the compact methods to the format-specific methods, following the principle of documenting coupling at the point of breakage — a future maintainer changing
str_8()'s read sequence needs to know the streaming decoder depends on it.The compact methods' coupling comments are updated or removed since the streaming decoder no longer calls them for str/bin/ext.
5. Parameter style:
Reader(implicit ref)The decoder has an existing inconsistency: integer/compact methods use explicit
Reader ref, while str/bin/ext/array/map format-specific methods use bareReader(which defaults toreffor classes). The new methods go in the str/bin/ext sections, so they match those sections' existing style:Reader.6.
byte_arraynaming left as-isThe compact bin decoder is called
byte_array()while the encoder's equivalent isbin(). This naming asymmetry predates this work. A separate issue should be filed to track renamingbyte_array()tobin()for encoder/decoder symmetry. The new format-specific methods are namedbin_8/bin_16/bin_32(matching the encoder and the MessagePack spec), sitting alongside the existingbyte_array().7. Scope boundary — timestamps excluded
During planning we identified that
timestamp_32/timestamp_64/timestamp_96decode methods are also missing (the encoder has them, the decoder only has compacttimestamp()). We kicked this out to a separate discussion (#61) because timestamps are a higher-level concept layered on ext wire formats, and the performance argument may differ.Implementation Steps
Step 1: Add format-specific methods to
MessagePackDecoderFile:
msgpack/message_pack_decoder.ponyAdd the 14 methods in their respective format-family sections:
str()):str_8,str_16,str_32byte_array()):bin_8,bin_16,bin_32ext(), beforetimestamp()):fixext_1throughfixext_16, thenext_8,ext_16,ext_32Each method follows the same pattern:
Add docstrings to each method. Follow the existing docstring style from the encoder's format-specific methods — describe the wire format, note that the format byte is validated, and describe the return value. For ext methods, document the
(ext_type, data)tuple. Example:Step 2: Update coupling comments
In
message_pack_decoder.pony:str(),byte_array(), andext()that reference the streaming decoder. These comments document a coupling that no longer exists after the refactoring — the streaming decoder will call format-specific methods instead. Leaving stale coupling comments is worse than having none.Step 3: Refactor streaming decoder
File:
msgpack/message_pack_streaming_decoder.ponyUpdate four methods to call format-specific methods instead of compact ones:
_decode_str(fb): ReplaceMessagePackDecoder.str(_reader)?with a dispatch toMessagePackDecoder.str_8/str_16/str_32based onfb._decode_bin(fb): ReplaceMessagePackDecoder.byte_array(_reader)?withMessagePackDecoder.bin_8/bin_16/bin_32._decode_fixext(fb): ReplaceMessagePackDecoder.ext(_reader)?withMessagePackDecoder.fixext_1/2/4/8/16. Note: this method has a timestamp guard that checks for ext_type0xFFon fixext_4 and fixext_8 and routes to_decode_timestamp()before reaching the ext decode. The timestamp guard stays in place — only the ext decode after it changes._decode_ext_variable(fb): ReplaceMessagePackDecoder.ext(_reader)?withMessagePackDecoder.ext_8/16/32. Note: same timestamp guard applies here — ext_8 with data_len=12 and ext_type0xFFroutes to_decode_timestamp()before reaching the ext decode. The guard stays in place.The streaming decoder already dispatches on
fb, so this adds a second dispatch within each method. This is intentional — it matches the existing pattern in_decode_uintand_decode_int, which already dispatch to format-specific methods (u8,u16, etc.) rather than calling compactuint().Step 4: Update
MessagePackDecoderdocstringUpdate the primitive-level docstring on
MessagePackDecoderto mention that format-specific methods are now available for all format families (str, bin, ext), not just integers and arrays/maps.Step 5: Add tests
File:
msgpack/_test_decoder.ponyExisting test classes to update — test classes named
_TestDecodeStr8,_TestDecodeStr16,_TestDecodeStr32,_TestDecodeBin8,_TestDecodeBin16,_TestDecodeBin32,_TestDecodeFixext1through_TestDecodeFixext16,_TestDecodeExt8,_TestDecodeExt16, and_TestDecodeExt32already exist. They currently encode with the format-specific encoder method and decode with the compact decoder method (e.g.,MessagePackDecoder.str()). Update these 14 tests to decode with the new format-specific methods instead (e.g.,MessagePackDecoder.str_8()). This is a simple method name substitution — the return types are identical (String iso^,Array[U8] iso^, and(U8, Array[U8] val)respectively), so the rest of each test body (assertions, destructuring) remains unchanged. Compact decode coverage for these wire formats is already provided by the_TestDecodeCompact*tests added in PR #60. These existing tests are already registered in_TestDecoder.tests(), so no registration changes needed for them.Property-based roundtrip tests — one property per sub-family. All property tests must use the format-specific encoder method (e.g.,
MessagePackEncoder.str_8), not the compact encoder (MessagePackEncoder.str). The compact encoder selects the smallest format, so short strings would be encoded as fixstr, causingMessagePackDecoder.str_8()to error on the mismatched format byte._PropertyStr8Roundtrip: generate strings 0–255 bytes, roundtrip throughstr_8encode/decode._PropertyStr16Roundtrip: generate strings 0–65535 bytes (biased toward smaller for speed)._PropertyBin8Roundtrip,_PropertyBin16Roundtrip: same pattern withArray[U8]._PropertyFixext1Roundtripthrough_PropertyFixext16Roundtrip: generate ext_type (U8, filtered to exclude0xFFto avoid timestamp interpretation) and data of exact fixed size._PropertyExt8Roundtrip,_PropertyExt16Roundtrip: generate ext_type (U8, filtered to exclude0xFF) and variable-length data.For
str_32,bin_32, andext_32, property tests with realistic data sizes are impractical. Example-based roundtrip tests with small payloads are sufficient.Rejection tests — verify each method errors on wrong format bytes. Every new method should appear in at least one rejection test:
str_8should error when given str_16 data, and vice versa.bin_8should error when given bin_16 data.fixext_1should error when given fixext_2 data.ext_8should error when given ext_16 data.These can be grouped into a small number of test classes that exercise cross-format rejection systematically.
Test registration — all new test classes (property-based and rejection) must be registered in
_TestDecoder.tests(). Property tests use thetest(Property1UnitTest[T](...))pattern (see existing registrations at the top of_test_decoder.ponyfor examples). Rejection tests use the standardtest(...)pattern.Streaming decoder — existing tests should pass without modification (internal refactoring, same external behavior). No new streaming decoder tests needed unless the refactoring changes observable behavior, which it should not.
Build and run tests with:
make testStep 6: Update documentation
README.md: The existing line "Format-specific methods (uint_8,uint_32,fixstr,str_8, etc.) are available when you need explicit control over the wire format." is already accurate once the methods exist. Consider expanding with a short example showing format-specific decode usage — this helps users understand when and why they'd choose format-specific over compact methods. For example:CLAUDE.md: Update the Architecture section'sMessagePackDecoderdescription to reflect that format-specific methods are now available for str, bin, and ext families.Step 7: Add release notes
Create a file in
.release-notes/(e.g.,format-specific-decode.md):All reactions