fix(document-api): return NodeAddress from find and getNode instead of SDAddress (SD-2168)#2342
Conversation
…f SDAddress SD-2168: find and getNode now return NodeAddress directly, making their results compatible with mutation targets like create.paragraph. Removes the toSDAddress conversion layer that stripped nodeType and changed kind from 'block' to 'content', which made find results unusable for positional inserts. Also adds docs for the query.match → create.paragraph workflow.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8b1974438
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export interface SDFindInput { | ||
| select: SDSelector; | ||
| within?: SDAddress; | ||
| within?: SDAddress | NodeAddress; |
There was a problem hiding this comment.
Update find input contract schema for NodeAddress scopes
This change allows SDFindInput.within to be NodeAddress, but the contract schema still defines within as sdAddressSchema (packages/document-api/src/contract/schemas.ts, sdFindInputSchema), so CLI input validation (which is derived from those schemas in apps/cli/src/lib/operation-args.ts via validateOperationInputData) rejects valid { kind: 'block', nodeType, nodeId } scopes for doc.find.
Useful? React with 👍 / 👎.
| export interface SDNodeResult { | ||
| node: SDContentNode | SDInlineNode; | ||
| address: SDAddress; | ||
| address: NodeAddress; |
There was a problem hiding this comment.
Sync SDNodeResult schema with NodeAddress return shape
After switching SDNodeResult.address to NodeAddress, the operation schemas still model result addresses as legacy SDAddress (e.g. sdNodeResultSchema.address = sdAddressSchema in packages/document-api/src/contract/schemas.ts), and CLI response validation consumes that schema (validateOperationResponseData in apps/cli/src/lib/operation-args.ts), so successful find/getNode/getNodeById responses with block addresses can fail validation because required legacy fields like stability are missing.
Useful? React with 👍 / 👎.
| return { | ||
| node: projectContentNode(candidate.node), | ||
| address: buildBlockAddress(nodeId), | ||
| address: { kind: 'block', nodeType: resolvedType, nodeId } as NodeAddress, |
There was a problem hiding this comment.
Return canonical nodeId from getNodeById results
The adapter now returns a NodeAddress, but it populates nodeId from user input instead of the resolved candidate; when lookup succeeds through alias entries, the returned address can carry a non-primary ID, and downstream insertion resolution (resolveBlockInsertionPos in plan-engine/create-insertion.ts) matches only primary candidate.nodeId, so chaining getNodeById(...).address into create-before/after calls can still hit TARGET_NOT_FOUND for aliased blocks.
Useful? React with 👍 / 👎.
find and getNode now return NodeAddress directly, making their results compatible with mutation targets like create.paragraph. Removes the toSDAddress conversion layer that stripped nodeType and changed kind from 'block' to 'content', which made find results unusable for positional inserts. Also adds docs for the query.match → create.paragraph workflow.