Add the instruction display orchestrator and public API#1016
Open
lorisleiva wants to merge 1 commit into
Conversation
This PR completes the clear-signing display-text feature (sRFC 39) by assembling the value and render-mode layers into a public API. getInstructionDisplay parses a concrete instruction and resolves it into a human-readable display — an intent label, an interpolated sentence, and a structured fallback list — returning null when the instruction cannot be parsed; getInstructionDisplayFromParsedInstruction does the same from an already-parsed instruction. buildDisplayContext bridges a parsed instruction and its root into the single context threaded through the layer, resolving defined-type links by path against a linkable dictionary (correct even for links nested in types from other programs) and computing which members were surfaced through the provide/inject graph so whenInjected fields hide correctly when their value is presented elsewhere. @codama/dynamic-parsers now exports ParsedInstruction. The whole display layer is exported from the package, and a changeset records the minor bumps.
🦋 Changeset detectedLatest commit: dda515d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
This was referenced Jun 25, 2026
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mikhd
reviewed
Jun 26, 2026
| * resolve from the correct location. | ||
| */ | ||
| export function resolveDisplayType(type: TypeNode, displayContext: DisplayContext): TypeNode { | ||
| export function resolveDisplayType( |
Contributor
There was a problem hiding this comment.
Should we move resolveDisplayType from format-argument-value.ts file, since it's not connected to formatting? May be its own resolve-display-type.ts
What do you think?
There was a problem hiding this comment.
Pull request overview
This PR finalizes the clear-signing “display text” layer for dynamic instructions by adding an orchestrator that turns a concrete instruction (raw or already-parsed) into a human-readable presentation (intent label, optional interpolated sentence, and a structured fallback list). It also expands the display context to support correct cross-program defined-type link resolution and correct whenInjected hiding via a consumed-member computation.
Changes:
- Added a public display orchestration API:
getInstructionDisplayandgetInstructionDisplayFromParsedInstruction, plusbuildDisplayContext. - Implemented
consumedMemberNamescomputation to makewhenInjectedhiding depend on resolved provide/inject usage (online vs offline behavior). - Exported
ParsedInstructionfrom@codama/dynamic-parsersand exported the full display layer from@codama/dynamic-instructions(with changeset + dependency updates).
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace link for @codama/dynamic-parsers dependency usage. |
| packages/dynamic-parsers/src/parsers.ts | Exports ParsedInstruction(+accounts) types for downstream API usage. |
| packages/dynamic-instructions/package.json | Adds @codama/dynamic-parsers dependency required by the new API. |
| packages/dynamic-instructions/src/index.ts | Re-exports the full display layer as part of the public package API. |
| packages/dynamic-instructions/src/display/index.ts | Adds new exports for orchestrator/context and consumed-member resolution. |
| packages/dynamic-instructions/src/display/types.ts | Introduces InstructionDisplay, options, consumedMemberNames, and path-based defined-type resolution. |
| packages/dynamic-instructions/src/display/get-instruction-display.ts | New public API entrypoints for generating instruction display output. |
| packages/dynamic-instructions/src/display/build-display-context.ts | New context builder bridging parsed instruction + root, including link resolution + consumed-member computation. |
| packages/dynamic-instructions/src/display/resolve-consumed-members.ts | New logic to compute which members were surfaced through provide/inject (for whenInjected). |
| packages/dynamic-instructions/src/display/list-fallback.ts | Switches from instruction to instructionPath and from provides.has to consumedMemberNames.has for hiding. |
| packages/dynamic-instructions/src/display/interpolate-intent.ts | Switches from instruction to instructionPath and passes correct owner paths to formatting. |
| packages/dynamic-instructions/src/display/format-argument-value.ts | Adds owner-path-aware link following so nested links resolve against the correct program. |
| packages/dynamic-instructions/src/display/format-value.ts | Adjusts context typing to allow pre-consumed-member context usage. |
| packages/dynamic-instructions/src/display/resolve-injected-value.ts | Adjusts context typing to allow pre-consumed-member context usage. |
| packages/dynamic-instructions/test/test-utils.ts | Updates test helpers for instructionPath, consumed-member plumbing, and new resolver/fetch helpers. |
| packages/dynamic-instructions/test/display/resolve-consumed-members.test.ts | New test coverage for consumed-member computation across online/offline cases. |
| packages/dynamic-instructions/test/display/get-instruction-display.test.ts | New end-to-end tests for the orchestrator across presentation tiers + cross-program link resolution. |
| packages/dynamic-instructions/test/display/build-display-context.test.ts | New tests for context assembly (data/accounts/provides/link resolution/options). |
| packages/dynamic-instructions/test/display/list-fallback.test.ts | Updates tests to use path-based context + consumed-member hiding semantics. |
| packages/dynamic-instructions/test/display/interpolate-intent.test.ts | Updates tests to use path-based context. |
| packages/dynamic-instructions/test/display/format-argument-value.test.ts | Updates tests for new formatArgumentValue(type, ownerPath, value, ctx) signature and resolver helper. |
| .changeset/every-bikes-rhyme.md | Records minor bumps + feature description for the new clear-signing display API. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+64
| /** | ||
| * Builds a {@link ResolveDefinedTypeFn} backed by a `LinkableDictionary` populated from the root. | ||
| * The caller supplies the full path to the link, which carries the program context the dictionary | ||
| * needs — including for links reached after following other links. | ||
| */ | ||
| function createDefinedTypeResolver(root: RootNode): ResolveDefinedTypeFn { | ||
| const linkables = new LinkableDictionary(); | ||
| visit(root, getRecordLinkablesVisitor(linkables)); | ||
| return linkPath => linkables.getPath(linkPath); | ||
| } |
Comment on lines
+58
to
+64
| /** Builds a {@link ResolveDefinedTypeFn} that resolves the given defined types by name. */ | ||
| export function mockResolveDefinedType(...definedTypes: DefinedTypeNode[]): ResolveDefinedTypeFn { | ||
| const byName = new Map(definedTypes.map(definedType => [definedType.name, definedType])); | ||
| return linkPath => { | ||
| const definedType = byName.get(getLastNodeFromPath(linkPath).name); | ||
| return definedType ? ([definedType] as NodePath<DefinedTypeNode>) : undefined; | ||
| }; |
mikhd
reviewed
Jun 26, 2026
| [...displayContext.instructionPath, argument], | ||
| displayContext, | ||
| ); | ||
| if (isNode(type, 'numberTypeNode') && type.display?.kind === 'amountNumberDisplayNode') { |
Contributor
There was a problem hiding this comment.
question: Is there a real-world scenario when we could have something like this? (numberTypeNode with display inside the structFieldType)
structTypeNode([
structFieldTypeNode({
name: 'amount',
type: numberTypeNode('u64', 'le', {
display: amountNumberDisplayNode({ decimals: injectedValueNode({ key: 'decimals' }) }),
}),
}),
]);
And we would need to traverse each key? Something like this:
if (argument.display?.flatten && isNode(resolved.type, 'structTypeNode')) {
resolved.type.fields.forEach(field => {
const fieldType = resolveDisplayType(field.type, [...resolved.ownerPath, field], ctx).type;
addAmountKeys(fieldType, keys);
});
return;
}
mikhd
approved these changes
Jun 29, 2026
mikhd
left a comment
mikhd
left a comment
Contributor
There was a problem hiding this comment.
Do we want to add new display methods to README.md?
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.
This PR completes the clear-signing display-text feature (sRFC 39) by assembling the value and render-mode layers into a public API. getInstructionDisplay parses a concrete instruction and resolves it into a human-readable display — an intent label, an interpolated sentence, and a structured fallback list — returning null when the instruction cannot be parsed; getInstructionDisplayFromParsedInstruction does the same from an already-parsed instruction. buildDisplayContext bridges a parsed instruction and its root into the single context threaded through the layer, resolving defined-type links by path against a linkable dictionary (correct even for links nested in types from other programs) and computing which members were surfaced through the provide/inject graph so whenInjected fields hide correctly when their value is presented elsewhere. @codama/dynamic-parsers now exports ParsedInstruction. The whole display layer is exported from the package, and a changeset records the minor bumps.