Repository navigation
[2.x] Decode Codama nodes into decoded node trees - #1214
Conversation
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adds getNodeCodec, a codec that decodes bytes into a DecodedNode tree (path + value + absolute byte range + named children) and re-derives getNodeValueCodec / getNodeValueCodecVisitor from it via mapVisitor(..., getValueCodec). The visitor now builds every node as an "unranged" decoded node through getLeafCodec / getCompositeCodec, and the intercept layer stamps offset/size on top with withRange. Byte-slicing transforms (fixedSize, sizePrefix, sentinel) go through withSlicedBytes, which maintains a baseOffsets stack so inner ranges stay absolute even when Kit hands the inner codec a copy of the bytes. Links are transparent (decode as the linked definition's type, reported at the definition's path), recursive types work via the existing lazy codec, and decoded.ts adds one Decoded…Node type per kind plus isDecodedNode / assertIsDecodedNode.
The design is sound and the implementation is careful. I verified the withSlicedBytes slice-start logic against Kit's fixDecoderSize / addDecoderSizePrefix / addDecoderSentinel (including the fixBytes pad/truncate path and the "no slice when offset is 0 and length matches" shortcut, both handled by the bytes !== outer.bytes check), and the value-codec behaviour matches the previous implementation line by line (tuple missing items, option undefined → None, struct undefined → {}, map keys as strings, enum string shorthand). Nested size prefixes, recursive types, cross-program links and link transforms are all covered by tests.
Things to watch
- Changeset.
@codama/dynamic-codecsis public and this adds new API, but I don't see a.changeset/*.mdin the changed files. CONTRIBUTING asks for one per user-facing change (npx changeset add --empty). If it lives elsewhere in the stack, ignore this. - Byte ranges under non-padded offset transforms (inline on
withRange).offset/sizeare the cursor span, which diverges from the "first byte read / bytes read" wording inDecodedNodeBaseforabsolute/relative/preOffsetstrategies and can producesize: 0or a negativesize. Only thepaddedstrategy is tested. inProgress.codecis the unranged composite (inline invisitDefinedType). Harmless today because the recursive link path only ever reads.type, but it's a latent trap.NodeCodecisn't exported although it's the return type of the publicgetNodeCodecVisitor(inline).- Link transforms are attributed to the linked type's node.
it includes the transforms of links in the byte range of their typemakes this deliberate, and it's consistent with "transforms included", but note the consumer-facing asymmetry: the reportedpathends at the definition'stype, which doesn't carry the link's transforms, so an explorer can't explain the extra bytes from the path alone. Might be worth a sentence in the README's "Links are transparent" paragraph.
Notes for subsequent reviewers
baseOffsetsis shared by every codec produced by one visitor instance. That's fine because reads are synchronous and nested (push/pop intry/finally), but a codec from visitor A nested inside a codec from visitor B wouldn't see B's base offset. Not a realistic path through the public API.- Kit's sentinel-count array strategy compares at item boundaries without slicing, so sentinel-counted collections don't need
withSlicedBytes. The existing test only exercises one at offset 0; a quick case inside a struct would make that explicit. - The
TupleTypeNodetest change isn't a behaviour change: a 2-item tuple encoded from[42]already failed in the string codec withactualType: 'undefined'; the test just pins it now thatgetCompositeCodecpassesinput?.valuethrough.
343c9c9 to
2e1a5ca
Compare
|
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
Re-fetched everything. All the points from my first pass are addressed:
- Cursor positions.
offset/sizebecamepreOffset/postOffset, documented as cursor positions in Kit's vocabulary rather than "bytes read", which removes the ambiguity under non-padded offset transforms. The three new tests (relativepre-offset moving back,absolutepost-offset rewinding,absolutepre-offset on a composite whose children land elsewhere) pin the semantics, andpostOffset < preOffsetis now a documented possibility rather than a surprise. inProgress.codecis nowwithRange(codec)with a comment explaining why, so the lazy recursive codec hands back rangedDecodedDefinedTypeNodes. The outer interceptwithRangestill wraps the returned unrangedcodec, so there's no double application on the direct path and the recursive path gets exactly one. I checked the linked-list test's expected offsets against this and they're consistent.DecodedNodeCodecis exported fromdecoded.tsand used as the visitor's return type.- README now states that a link's transforms are included in the cursor positions of the type it decodes as, even though that type's node doesn't carry them.
- Sentinel-counted array inside a struct is covered by
it keeps cursor positions absolute within arrays ending with a sentinel.
Nothing new to flag in the updated codecs.ts; the Unranged<T> distributive Omit and the satisfies checks per visitor are a nice way to keep each decoded node shape honest without widening the public types.
The only item still outstanding is the changeset: the file list is unchanged (9 files, no .changeset/*.md). Since @codama/dynamic-codecs is public and this adds getNodeCodec, getNodeCodecVisitor and the Decoded… types, it'll need one before release unless it's being handled elsewhere in the stack. Not blocking the approval.
2e1a5ca to
ce304e7
Compare
03b5d2b to
f3f5bbb
Compare
361b8ef to
9b0fbca
Compare
f3f5bbb to
73c32ec
Compare
9b0fbca to
f143ca4
Compare
3a89937 to
86485ca
Compare
f143ca4 to
fa8eac0
Compare
86485ca to
a9b7be2
Compare
fa8eac0 to
12782e9
Compare
a9b7be2 to
7e05b3c
Compare
1248d65 to
fd9f4ec
Compare
572de5a to
51c3f8b
Compare
0ad1db3 to
fcc31ac
Compare
51c3f8b to
2acf133
Compare
fcc31ac to
123c3f8
Compare

This PR adds
getNodeCodec, which decodes bytes into a tree of decoded nodes rather than plain values. Each decoded node keeps the path of the node that decoded it, so consumers such as explorers and the upcoming display code can read values from their types rather than guess them from JavaScript shapes.path, avaluein the format ofgetNodeValueCodec, and the absolute cursor positions before and after reading it (preOffsetandpostOffset, transforms included, as in Kit's offset codecs). Children are named after their node's attributes:data,type,fields,variant,item,itemsandentries. Map keys keep their decoded values, for example7n.Decoded…Nodetype per node kind, plusDecodedNode<TNode>,GetDecodedNodeFromKind,DecodedNodeCodec, and theisDecodedNode/assertIsDecodedNodeguards.getNodeValueCodecand its visitor are now derived fromgetNodeCodecVisitor, and their behaviour is unchanged. On mpl-token-metadata'screatedata, value decoding goes from 12 to 22 µs and value encoding from 16 to 20 µs. That cost was accepted to avoid maintaining two codec paths.