fix(dynamic-codecs): honor variant.discriminator on the wire

[plutohan]

Implements what we agreed in #1112: the wire honors variant.discriminator, and decoded enums carry the discriminator so the round trip is lossless.

• Each variant's codec is now the payload behind a constant prefix of variant.discriminator ?? index, assembled with getUnionCodec (value lookup by __kind, byte lookup by reading the size prefix and matching declared discriminants). For IDLs without custom discriminants the wire bytes are unchanged.
• Decoded objects include __discriminator (your naming), e.g. { __kind: 'Info', __discriminator: 10 }. Encoding accepts and ignores the field, and the running example from the issue now round-trips: encode({ __kind: 'Critical' }) writes 0x1e and decode(0x1e) returns { __kind: 'Critical', __discriminator: 30 }.
• enumValueNode resolution includes the field too, and the argument validator plus the codec input transformer strip it before payload validation and re-encoding, so decode-then-build keeps working.

Tests: the issue's repro as a codec test (custom and gapped discriminants, struct variant payload, re-encoding decoded output byte-for-byte), updated decode expectations across the dynamic packages, and the client fixture assertions. All five dynamic package suites pass.

Changesets: @codama/dynamic-codecs minor with a breaking note for the decoded shape, patches for the two input-path packages.

Fixes #1112

[changeset-bot]

🦋 Changeset detected

Latest commit: 030379f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@codama/dynamic-codecs	Minor
@codama/dynamic-address-resolution	Patch
@codama/dynamic-instructions	Patch
@codama/dynamic-client	Patch
@codama/dynamic-parsers	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[lorisleiva]

Thanks! nit: I'd rather we did something like:

const { __kind, __discriminator, ...rest } = input;

I think you've already done it like that in the rest of the code. Btw, I'll do another thorough review soon after the AI review.

[lorisleiva]

@trevor-cortex

[trevor-cortex]

Implements the #1112 agreement: enum variant codecs become constant-prefixed payloads assembled with getUnionCodec, so the wire writes variant.discriminator ?? index, and decoded objects carry __discriminator for a lossless round trip. The input-path packages (dynamic-instructions validator, dynamic-address-resolution input transformer) strip the field before payload validation/re-encoding.

The core codec logic checks out: the union's byte-side lookup reads the size prefix and matches declared discriminants, the hidden-prefix constant then consumes/validates it, and the value-side lookup by __kind preserves prior encode semantics. Wire bytes are unchanged for default-discriminant IDLs, and the re-encode-decoded-output test locks in the round trip.

Things I'd like your take on (details inline):

1. Mixed explicit/implicit discriminants — variant.discriminator ?? index uses the position for unannotated variants, but Rust continues from the last explicit value (enum { A = 5, B } → B is 6, not 1). If the spec defines the default as the index and IDL producers (nodes-from-anchor etc.) always materialize explicit values in the mixed case, this is fine — worth confirming.
2. Error DX — unknown __kind on encode and unknown wire discriminant on decode now surface as getUnionCodec's generic out-of-range error instead of the discriminated-union codec's error that names the valid variants.
3. Changeset framing — the **Breaking** label on a minor bump sits oddly next to CONTRIBUTING's "breaking changes to individual packages wait for the next major cut" policy.

One design note, not inline since it was agreed in #1112: encode silently ignores __discriminator even when it contradicts __kind (e.g. { __kind: 'Info', __discriminator: 20 } on a variant declared as 10 encodes 0x0a). That's the agreed contract, but a mismatch check would catch stale decoded objects being re-encoded after an IDL's discriminants change. Fine to punt.

For subsequent reviewers: worth a quick grep across the other dynamic packages for anything that consumes decoded enum shapes via exact equality or by iterating payload fields — only the validator and the input transformer were updated here, and the test fixture churn in dynamic-client shows how easily exact-shape assertions pick this up.

[trevor-cortex]

variant.discriminator ?? index treats an unannotated variant's discriminant as its position. Rust (and Anchor) semantics continue from the last explicit value instead: enum Status { A = 5, B } gives B = 6, whereas this yields 1. If the Codama spec defines the default as the index and IDL producers always emit explicit discriminators once any variant deviates, this is correct as-is — but it would be good to confirm that's guaranteed, and perhaps add a mixed explicit/implicit case to the new test to pin the intended semantics either way.

[trevor-cortex]

Minor DX regression vs getDiscriminatedUnionCodec: both lookup callbacks return -1 for unknown inputs, so an unrecognized __kind on encode or an undeclared wire discriminant on decode now throws the union codec's generic variant-out-of-range error, whereas the discriminated-union codec's error names the valid variants. Consider throwing a more descriptive error (listing valid kinds / declared discriminants) before returning -1.

Two smaller notes on the byte-side lookup: discriminators.indexOf(...) silently resolves duplicate declared discriminants to the first match — probably an invalid IDL upstream, but a cheap construction-time duplicate check would make the failure loud. And Number(discriminator) would lose precision for u64-sized discriminants above MAX_SAFE_INTEGER — vanishingly unlikely, just noting it's a known bound.

[trevor-cortex]

Nit: this could fold into the existing destructuring on the line above instead of a cast + delete:

const { __kind, __discriminator: _discriminator, ...rest } = input as Record<string, unknown> & { __kind: unknown };

(matching the pattern used in dynamic-instructions/src/validators.ts).

[lorisleiva]

@trevor-cortex For your design note I think you're on to something. Since technically the variants are known at the time of creating the codec, are you saying we could bake the variants into that codec such that the actual byte value would be respected even when the decoded output is simple { __kind: "whatever" }? We could still have __discriminator (which I actually think could be better described as __value now since this is the numerical value assigned to the byte array) on the decoded value but it would be ignored when encoded since __kind would be the source of truth. Wdyt?

[trevor-cortex]

Yes — and the good news is the PR already does exactly that. The discriminator values are baked into each variant's codec at creation time (the constant prefix of variant.discriminator ?? index), and encode-side lookup goes by __kind alone. So encode({ __kind: 'Critical' }) writes 0x1e today without __discriminator being present at all — __kind is already the source of truth, and the field is already ignored on encode.

My design note was about a narrower case: when the field is present and contradicts the baked-in value, e.g. { __kind: 'Info', __discriminator: 99 } where Info is declared as 10. Right now we silently write 10. That's the correct byte output, but the silent part can mask a bug upstream — typically someone decoded with an old IDL, the discriminants changed, and they're re-encoding stale data. An encode-time check like "if the field is present and doesn't match the variant's declared value, throw" is cheap (one comparison against the value you already have baked in) and turns that silent drift into a loud failure. Ignoring an absent field stays as-is, so plain { __kind: ... } inputs are unaffected.

On the rename: agreed that __value is arguably more accurate now that it's "the numeric value assigned to this variant" rather than something the caller supplies. Two small considerations before committing:

1. __discriminator is more self-describing at a glance — __value reads a bit generic next to variant payload fields, especially for tuple/struct variants that might themselves have a value field (no actual collision thanks to the prefix, just visual ambiguity).
2. Whichever name wins, it needs to change in all the strip/ignore sites in one sweep — the argument validator, the codec-input transformer, the enumValueNode resolution, and the changeset's decoded-shape note — otherwise decode-then-build breaks in the gap.

No strong preference from me; if __value matches how you talk about it in the enum spec/docs, go with that. Happy to re-review once the mismatch check and/or rename lands.

[lorisleiva]

@trevor-cortex Oh I see in this case I think the current implementation is correct to ignore invalid values since the source of the truth is __kind. On the naming, my issue with __discriminator is the codec API actually called __kind the discriminator property because, as we've established, it is the source of truth that identifies the variant. Therefore the discriminator is more __kind than the actual value that gets stored in the byte array. As such I have a preference towards __value unless you can think of a better alternative?

[trevor-cortex]

Agreed on ignoring mismatches — once __kind is the sole source of truth on encode, throwing on a stale numeric field would just add a failure mode for data the codec never reads. Consistent as-is.

On naming: your rationale is sound — getDiscriminatedUnionCodec already uses "discriminator" for the __kind property, so reusing that word for the numeric value overloads the term within the same API. Two alternatives I considered:

• __discriminant — this is Rust's exact term for the numeric value (enum Severity { Critical = 30 } → 30 is the discriminant), so it's technically the most precise and keeps "discriminator" (the kind) and "discriminant" (the value) as distinct concepts. The downside is real, though: one letter apart from __discriminator, it's a typo/confusion magnet in docs, reviews, and conversation.
• __code — short and unambiguous, but it's not established terminology anywhere in the stack, so it would need explaining.

__value's only weakness is mild ambiguity next to variant payload fields ({ __kind: 'Info', __value: 10, message: '...' } — a reader could momentarily parse __value as payload), but the __ prefix already signals "codec metadata, not payload", so I don't think it's a blocker.

One thing worth a line in the README either way: the Codama node field is still variant.discriminator, so whatever the runtime field is called, the docs should explicitly say "__value carries the variant's declared discriminator (or its index)" to link the two names.

Net: I'd go with __value. __discriminant is the pedantically correct choice, but the near-collision with "discriminator" costs more than the precision buys.

[lorisleiva]

@plutohan Ignore my conversation with Trevor haha. I'm happy to keep __discriminator as the property. If you just tackle my nit comment I think we should be good to merge.

[plutohan]

Done in 030379f, switched to the destructuring pattern matching validators.ts (with the same lint pragma, since the config flags the unused binding either way).

On the two open notes from the review, for the record:

• The ?? index default follows the node docs, which define an omitted discriminator as "inferred from the variant position". Rust's continue-from-last-explicit semantics would be a spec-level change; happy to raise that separately if you think producers will hit it.
• The generic out-of-range errors for an unknown __kind or an undeclared wire byte are indeed less helpful than the discriminated-union ones. I'd take that as a small follow-up (dedicated error codes listing the valid kinds and declared discriminators) rather than growing this PR, if that works for you.

[lorisleiva]

Thanks! Don't worry about these follow up items. Would love @mikhd's approval on that one as well since it touches the dynamic packages.