[2.x] Adapt visitors-core to Codama v2

[lorisleiva]

This PR adapts @codama/visitors-core to the v2 node model. The package is fully green: it type-checks, builds, tree-shakes, and 2095/2095 tests pass.

src

Reworked for v2:

• getResolvedInstructionInputsVisitor — rewritten. New signature (linkables, { stack?, scope?, includeDataValueNodes? }). Resolves account defaults and instructionNode.data struct fields (following definedTypeLinkNode), matches dataValueNode dependencies by longest path prefix, and returns a wrapper discriminated on node.kind ({ node, dependsOn, resolvedDefaultValue?, … }) instead of spreading the input node.
• getByteSizeVisitor / getMaxByteSizeVisitor — integerTypeNode/floatTypeNode split, enumVariantTypeNode, instructionNode.data, and flat transforms applied on top of each type node's own size via a shared internal applyByteSizeTransforms helper. Also fixes bytesTypeNode sizing to null (it merged to 0).
• LinkableDictionary, getDebugStringVisitor, identityVisitor, NodeSelector — v2 node kinds and attributes. NodeSelector compares identifiers as-is. identityVisitor drops the v1 empty-variant and empty-transform normalisations; only the conditionalValueNode rule remains.

Added:

• ProvidedScope + recordProvidedScopeVisitor — lexical scope of providedNodes used to resolve injectedValueNodes. scope.resolve(node, { kinds }) replaces every injection within node (nested ones included, e.g. a PDA seed), so the result never contains one. The innermost frame wins, outer frames stay visible (a sub-instruction can shadow its parent), a provided node resolves against the frames outside the one providing it, the consumer's fallback applies when no frame provides the key or the provided chain dead-ends, and resolution is all-or-nothing: if any injection within the value is unresolvable, the whole value resolves to undefined rather than a partially resolved one (e.g. a PDA missing a seed). A value containing no injection is returned as-is. A provided node of an unexpected kind throws CODAMA_ERROR__VISITORS__INVALID_PROVIDED_VALUE. Frames are independent of NodeStack, so following links keeps the consuming instruction's providers in scope.
• interceptVisitor — the interceptor now also receives the running visitor as self, typed over the wrapped visitor's node kinds (additive).

Dependencies:

• @codama/errors — new CODAMA_ERROR__VISITORS__INVALID_PROVIDED_VALUE ({ expectedKinds, key, providedKind, provider }).
• @codama/nodes — new getTextNodeContent(string | TextNode), used to render TextNode messages in debug strings.

test

• Reconciled the per-node fixtures with the generated NODE_TEST_PATHS coverage gate: deleted 24 fixtures for removed kinds, added 19 for new kinds.
• Migrated every remaining test to v2 (identifier, numeric split, transforms, options-bag constructors, inject/provide).
• Added ProvidedScope and recordProvidedScopeVisitor tests, plus resolver tests for provided/fallback injections, bump-derived isPda, sub-instruction shadowing and data as a defined type link.

Follow-ups

• getResolvedInstructionInputsVisitor's new signature breaks its consumers (validators here; renderers-js and renderers-js-umi downstream), updated as those packages are adapted.
• Identifier fold (lowercase, strip underscores) is no longer used for matching; the validators PR will use it only to warn about identifiers that collide after folding.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9c63d57

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[lorisleiva]

• [2.x] Adapt the dynamic-instructions builder to Codama v2 #1194
• [2.x] Reject values of the wrong type when encoding dynamic codecs #1193
• [2.x] Adapt dynamic-address-resolution to Codama v2 #1192
• [2.x] Accept bytes, enum shorthands and struct defaults when encoding dynamic codecs #1191
• [2.x] Add resolveTypePath helper to visitors-core #1190
• [2.x] Adapt dynamic-parsers to Codama v2 #1187
• [2.x] Add withPath and visitPath helpers to NodeStack #1186
• [2.x] Adapt dynamic-codecs to Codama v2 #1185
• [2.x] Add relations, nested argument seeds and legacy PDAs to nodes-from-anchor #1181
• [2.x] Adapt nodes-from-anchor to Codama v2 #1180
• [2.x] Adapt instruction visitors to Codama v2 #1179
• [2.x] Adapt update visitors to Codama v2 #1178
• [2.x] Adapt structure visitors to Codama v2 #1175
• [2.x] Move casing helpers to @codama/fragments #1174
• [2.x] Adapt validators to Codama v2 #1173
• [2.x] Adapt visitors-core to Codama v2 #1172 👈 (View in Graphite)
• [2.x] Adapt nodes to Codama v2 #1171
• [2.x] Adapt errors to Codama v2 #1167
• [2.x] Rename node name attribute to identifier #1162
• [2.x] Regenerate the node types from the v2 spec #1158
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[lorisleiva]

@trevor-cortex

[trevor-cortex]

Summary

Migrates @codama/visitors-core to the v2 node model: the instructionArgumentNode family is gone in favour of instructionNode.data struct fields addressed by PathString, the numeric split (integerTypeNode/floatTypeNode) and flat transforms are threaded through the byte-size visitors via a shared applyByteSizeTransforms interceptor layer, and the inject/provide model gets a first-class ProvidedScope + recordProvidedScopeVisitor. getResolvedInstructionInputsVisitor is rebuilt around a { node, dependsOn, resolvedDefaultValue, … } wrapper and now follows definedTypeLinkNode data. Fixtures are reconciled with the NODE_TEST_PATHS gate.

I read all src changes in full (plus the full post-change getResolvedInstructionInputsVisitor.ts, getByteSizeVisitor.ts, LinkableDictionary.ts, NodeSelector.ts) and the substantive tests; the per-node fixtures I spot-checked are mechanically correct. The code is clean and the test coverage for the new scope semantics is good. Nothing here is a blocker — I'm leaving this as COMMENT rather than APPROVE only because of the first design question below, which I'd like a decision on (even if the decision is "top-level only, by design").

Things to decide / watch

1. Injections are only resolved at the top of a default value. InjectedValueNode is a ValueNode, so it can legally sit anywhere a ValueNode can — a pdaValueNode seed value, a conditionalValueNode branch, a structValueNode field, pdaValueNode.programId, etc. resolveDefaultValue only unwraps the outermost injection, so for e.g. pdaValueNode('foo', { seeds: [pdaSeedValueNode('authority', injectedValueNode({ key: 'authority' }))] }) on a reusable data struct: (a) getDependencies sees an injectedValueNode seed and records no dependency, so resolution order / cycle detection miss it, and (b) resolvedDefaultValue still contains an unresolved injection that renderers would have to resolve themselves — but they won't have the scope, since the visitor consumed it. If the spec intends injections to be top-level-only on defaults, a one-line note on resolveDefaultValue (and ideally a validator rule later) would settle it. If not, a small bottom-up transformer over the default value that replaces every injectedValueNode via the scope would make resolvedDefaultValue genuinely "resolved" and let getDependencies stay oblivious to injections.

2. Fallback semantics when a provider chain dead-ends. In ProvidedScope.resolveFrom, if the consumer is inject('x', { fallback: A }) and an enclosing frame provides x = inject('y') with no y anywhere and no fallback, the result is undefined — the consumer's own A is never consulted. That's defensible ("the key was provided, just badly"), but the spec text ("a value used when no provider supplies the key") could be read either way. Worth a deliberate choice + a test either way.

3. Link following silently requires the program on the stack. collectDataFields resolves definedTypeLinkNode via linkables.getPath([...stack.getPath(), type]), and LinkableDictionary.getProgramDictionary needs a programNode ancestor on that path (or an explicit program on the link). With the default new NodeStack(), a linked data type just yields zero fields — no error, the fields simply aren't there, and isPda will quietly be false. That matches getByteSizeVisitor's "unresolvable link → null" convention, so I'm not asking to throw, but the options.stack JSDoc should say the stack must contain the instruction's program ancestry for definedTypeLinkNode data to be followed. Relatedly, unlike getByteSizeVisitor this visitor doesn't wrap itself in recordNodeStackVisitor(v, stack), so the instruction itself is never on the stack when called standalone — probably fine today since getProgramDictionary only needs the program, but worth being consistent.

4. self in VisitorInterceptor is typed wider than it is. See inline — it claims Visitor<TReturn> over all kinds while the runtime object only has the wrapped visitor's keys.

5. Changeset. No .changeset/ entry in the file list. Per CONTRIBUTING this is fine if the seeded major changeset on this branch covers all public packages during the 2.x candidacy — just flagging so it's a conscious skip.

Notes for other reviewers

• visitDefinedType caches by identifier and the link's transforms are layered outside it by the interceptor, so the cache correctly holds the untransformed size. Worth keeping in mind if anyone later moves the interceptor inside extendVisitor.
• bytesTypeNode now sizes to null instead of merging to 0 — this is a behaviour change that also affects constantValueNode sizing (it now falls through to the base16 bytesValueNode branch, which is the intended path).
• identityVisitor deliberately drops the v1 "empty variant / empty hidden-prefix collapses" — the HiddenPrefixTransformNode fixture asserts hiddenPrefixTransformNode([]) survives deletion, so that's tested, not accidental.
• Follow-up PRs (validators, renderers-js, renderers-js-umi) will break on the getResolvedInstructionInputsVisitor signature — the description already calls this out.

[trevor-cortex]

Re-review

Re-fetched everything that moved since my first pass (ProvidedScope.ts, getResolvedInstructionInputsVisitor.ts, interceptVisitor.ts, interceptFirstVisitVisitor.ts, recordProvidedScopeVisitor.ts, both test files, plus bottomUpTransformerVisitor/recordNodeStackVisitor for context). All five points from the previous review are addressed:

1. Nested injections — ProvidedScope.resolve now does a deep pass via resolveWithin (bottom-up transformer), so resolvedDefaultValue never carries an injection and getDependencies stays oblivious to them. The provider-depth tracking in Resolution is the right shape: a provided node's own nested injections resolve strictly below the frame that provided it, and a fallback's nested injections resolve at the consumer's depth. Tested for a PDA seed and for a provided struct re-injecting a shadowed key.
2. Dead-end fallback — decided (consumer's fallback applies), documented, tested.
3. Stack requirement — JSDoc added, and the visitor is now wrapped in recordNodeStackVisitor so the instruction is on the stack when following links. The defined-type-link test uses new NodeStack([program]) as the doc prescribes.
4. VisitorInterceptor.self — generic over TNodeKind, interceptFirstVisitVisitor follows suit, cast dropped.
5. Per-visit hoisting — dataFields, dataDefaults, bumpAccounts computed once per instructionNode visit.

Approving. One design question below that I'd like you to weigh, but it's a documented, tested choice rather than a defect, so I'm not holding the PR on it.

One thing to weigh

Pruning an unresolvable nested injection can produce a well-formed but wrong value. resolveWithin returns null for a nested injection nobody provides, and the identity visitor then removes the node that held it. For a structFieldValueNode that's a reasonable "optional hook not filled". For a pdaSeedValueNode it means pdaValueNode.seeds silently loses an entry — the resolved default is still a valid pdaValueNode, just one that derives the wrong address. Same story for pdaValueNode.programId (drops to "current program") or a conditionalValueNode branch. The spec's "IDLs must be self-contained" clause plus the upcoming validator rule should catch this upstream, but this visitor is also what renderers consume directly, and a mangled default is harder to notice than a missing one.

An alternative that keeps the "missing provider is not an error" stance: if any nested injection is unresolvable, treat the whole root as unresolvable (return undefined) instead of pruning. That's the same outcome the root-level case already has, it's a one-flag change inside resolveWithin, and it means a renderer either gets a fully resolved default or none — never a partial one. If you'd rather keep pruning for the struct-field case specifically, it might be worth restricting it to that (or to a known-safe set of parent kinds) rather than applying it uniformly.

Small nits (inline)

• lookup: when a provided injection's chain ends in its own fallback, the returned Resolution has no provider, so a wrong-kind fallback surfaces as CODAMA_ERROR__UNEXPECTED_NODE_KIND rather than INVALID_PROVIDED_VALUE and loses the provider attribution.
• resolve runs the identity rebuild on every default, so every resolvedDefaultValue is a fresh deep copy even when there was nothing to resolve. Not a problem (tests use toEqual, and the cost is negligible), just noting it in case a downstream consumer ever expects resolvedDefaultValue === node.defaultValue for the no-injection case.

Notes for other reviewers

• Unchanged from the last pass: bytesTypeNode → null sizing is a deliberate behaviour change; identityVisitor intentionally drops the v1 empty-variant/empty-transform collapses; the getResolvedInstructionInputsVisitor signature change breaks validators/renderers-js/renderers-js-umi, to be fixed as those are adapted.
• recordProvidedScopeVisitor still hard-codes instructionNode; the ProvidedScope doc now says it's the only provides host today, so the two are consistent.

[trevor-cortex]

Re-review (3rd pass)

Re-fetched ProvidedScope.ts, ProvidedScope.test.ts, getResolvedInstructionInputsVisitor.ts and its test diff. All three follow-ups from the last pass are in:

• All-or-nothing resolution — resolveWithin now tracks an unresolvable flag and returns null for the whole subtree, so resolve yields undefined instead of a pruned value. Covered by the new "unresolvable nested injection" test, and the doc on resolve states the contract. This closes the "valid-looking PDA with a missing seed" concern.
• Provider attribution — lookup now carries chained.provider ?? provider through a provided injection's fallback, so a wrong-kind fallback raises INVALID_PROVIDED_VALUE naming the offending providedNode. Tested.
• Identity for injection-free values — resolveWithin returns the original node when nothing was replaced (asserted with toBe), so resolvedDefaultValue === node.defaultValue holds in the common case.

I also re-checked the interplay: the transformer short-circuits once unresolvable is set, resolveWithin is never handed a bare injectedValueNode (lookup always unwraps to a concrete node before recursing), and nested injections inside a provided node still resolve strictly below the providing frame. No new issues.

Approving — nothing further from me. Prior notes for other reviewers still apply (bytesTypeNode → null, dropped v1 identity normalisations, downstream signature break for validators/renderers).