ENG-1182 extend findDiscourseNode to accept the title as an argument #637
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR refactors the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/roam/src/utils/findDiscourseNode.ts (1)
1-1: Remove unused import.The
getPageUidByPageTitleimport is not used anywhere in this file.🔎 Proposed fix
-import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import getDiscourseNodes, { type DiscourseNode } from "./getDiscourseNodes"; import matchDiscourseNode from "./matchDiscourseNode";
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/components/Export.tsxapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/findDiscourseNode.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 559
File: apps/roam/src/utils/findDiscourseNode.ts:37-39
Timestamp: 2025-12-07T20:54:20.007Z
Learning: In apps/roam/src/utils/findDiscourseNode.ts, the function findDiscourseNodeByTitleAndUid accepts both uid and title parameters where uid is primarily used for cache access (as the cache key) while title is used for the actual matching via matchDiscourseNode. This design reflects the pattern where downstream, the uid is mostly used to fetch the title, so the function caches by uid but matches by title.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `node.type` serves as the UID field rather than having a conventional `node.uid` field. This is an unusual naming convention where the type field actually contains the unique identifier.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `type` serves as the unique identifier field, not a type classification field. The interface has no `uid` or `id` field, making `node.type` the correct field to use for UID-related operations.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 534
File: apps/roam/src/utils/createReifiedBlock.ts:40-48
Timestamp: 2025-11-05T21:57:14.909Z
Learning: In the discourse-graph repository, the function `getPageUidByPageTitle` from `roamjs-components/queries/getPageUidByPageTitle` is a synchronous function that returns a string directly (the page UID or an empty string if not found), not a Promise. It should be called without `await`.
📚 Learning: 2025-12-07T20:54:20.007Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 559
File: apps/roam/src/utils/findDiscourseNode.ts:37-39
Timestamp: 2025-12-07T20:54:20.007Z
Learning: In apps/roam/src/utils/findDiscourseNode.ts, the function findDiscourseNodeByTitleAndUid accepts both uid and title parameters where uid is primarily used for cache access (as the cache key) while title is used for the actual matching via matchDiscourseNode. This design reflects the pattern where downstream, the uid is mostly used to fetch the title, so the function caches by uid but matches by title.
Applied to files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
📚 Learning: 2025-06-17T23:37:45.289Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `type` serves as the unique identifier field, not a type classification field. The interface has no `uid` or `id` field, making `node.type` the correct field to use for UID-related operations.
Applied to files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
📚 Learning: 2025-06-17T23:37:45.289Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `node.type` serves as the UID field rather than having a conventional `node.uid` field. This is an unusual naming convention where the type field actually contains the unique identifier.
Applied to files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
📚 Learning: 2025-06-22T10:40:52.752Z
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Applied to files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/Clipboard.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
📚 Learning: 2025-11-05T21:57:14.909Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 534
File: apps/roam/src/utils/createReifiedBlock.ts:40-48
Timestamp: 2025-11-05T21:57:14.909Z
Learning: In the discourse-graph repository, the function `getPageUidByPageTitle` from `roamjs-components/queries/getPageUidByPageTitle` is a synchronous function that returns a string directly (the page UID or an empty string if not found), not a Promise. It should be called without `await`.
Applied to files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/components/Export.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
📚 Learning: 2025-06-22T10:40:21.679Z
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:15-16
Timestamp: 2025-06-22T10:40:21.679Z
Learning: In the getAllDiscourseNodesSince function in apps/roam/src/utils/getAllDiscourseNodesSince.ts, date validation is performed before the function is called, so additional date validation within the function is not needed.
Applied to files:
apps/roam/src/utils/getDiscourseContextResults.tsapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/deriveDiscourseNodeAttribute.tsapps/roam/src/utils/formatUtils.tsapps/roam/src/components/Export.tsxapps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/utils/hyde.tsapps/roam/src/utils/isDiscourseNode.tsapps/roam/src/utils/findDiscourseNode.ts
📚 Learning: 2025-08-25T15:53:21.799Z
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 372
File: apps/roam/src/components/DiscourseNodeMenu.tsx:116-116
Timestamp: 2025-08-25T15:53:21.799Z
Learning: In apps/roam/src/components/DiscourseNodeMenu.tsx, when handling tag insertion, multiple leading hashtags (like ##foo) should be preserved as they represent user intent, not normalized to a single hashtag. The current regex /^#/ is correct as it only removes one leading # before adding one back, maintaining any additional hashtags the user intended.
Applied to files:
apps/roam/src/components/SuggestionsBody.tsxapps/roam/src/components/DiscourseContextOverlay.tsxapps/roam/src/utils/hyde.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/** : Implement the Discourse Graph protocol in the Roam Research extension
Applied to files:
apps/roam/src/utils/hyde.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Applied to files:
apps/roam/src/components/CreateRelationDialog.tsx
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{js,ts,tsx,jsx,json} : Prefer existing dependencies from package.json when working on the Roam Research extension
Applied to files:
apps/roam/src/components/CreateRelationDialog.tsx
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Applied to files:
apps/roam/src/components/CreateRelationDialog.tsx
📚 Learning: 2025-06-17T23:42:29.279Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 220
File: apps/roam/src/utils/conceptConversion.ts:95-104
Timestamp: 2025-06-17T23:42:29.279Z
Learning: In the DiscourseGraphs/discourse-graph codebase, DiscourseRelation type properties are either string or Triple[], and the STANDARD_ROLES filter in conceptConversion.ts excludes Triple[] properties, so only string values remain after filtering.
Applied to files:
apps/roam/src/components/CreateRelationDialog.tsx
📚 Learning: 2025-06-25T22:56:17.522Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-06-25T22:56:17.522Z
Learning: In the Roam discourse-graph system, the existence of the configuration page (identified by DISCOURSE_CONFIG_PAGE_TITLE) and its corresponding UID is a system invariant. The code can safely assume this page will always exist, so defensive null checks are not needed when using `getPageUidByPageTitle(DISCOURSE_CONFIG_PAGE_TITLE)`.
Applied to files:
apps/roam/src/components/CreateRelationDialog.tsxapps/roam/src/utils/findDiscourseNode.ts
🧬 Code graph analysis (1)
apps/roam/src/utils/findDiscourseNode.ts (1)
apps/roam/src/utils/getDiscourseNodes.ts (1)
DiscourseNode(14-35)
🔇 Additional comments (11)
apps/roam/src/components/SuggestionsBody.tsx (1)
218-221: LGTM! Consistent with the new API signature.The call site has been correctly updated to pass an object with the
uidproperty, aligning with the refactoredfindDiscourseNodeAPI. The memoization dependency remains correct.apps/roam/src/utils/getDiscourseContextResults.ts (1)
185-185: LGTM! Correctly updated to the new API.The call site has been properly updated to use the object parameter format.
apps/roam/src/utils/formatUtils.ts (1)
139-139: LGTM! Properly updated with ES6 shorthand syntax.The call site correctly uses the object parameter format with ES6 shorthand property notation.
apps/roam/src/components/Export.tsx (1)
331-331: LGTM! Correctly updated to the new API.The call site has been properly updated to use the object parameter format. The fallback logic for
nodeTyperemains intact.apps/roam/src/components/DiscourseContextOverlay.tsx (1)
115-115: LGTM! Correctly updated to the new API.The call site has been properly updated to use the object parameter format. The null check on line 116 ensures safe access to the result.
apps/roam/src/utils/isDiscourseNode.ts (1)
6-6: LGTM! Correctly updated to the new API.The call site has been properly updated to pass an object with both
uidandnodesproperties using ES6 shorthand syntax. This maintains the same functionality as the previous two-parameter call.apps/roam/src/utils/deriveDiscourseNodeAttribute.ts (1)
52-52: LGTM! Correctly updated to the new API.The call site has been properly updated to pass an object with both
uidandnodesproperties. The early return on line 53 ensures safe handling when no discourse node is found.apps/roam/src/components/canvas/Clipboard.tsx (3)
592-592: LGTM! Correctly updated to the new API.The call site has been properly updated to use the object parameter format. The error handling on lines 593-600 ensures graceful failure when the node type is not found.
731-731: LGTM! Correctly updated to the new API.The call site has been properly updated to use the object parameter format. The early return on line 732 ensures safe access to the node type.
592-592: Verify all call sites have been updated.Since this PR changes the
findDiscourseNodeAPI signature from positional parameters to an object parameter, it's important to ensure all call sites across the codebase have been updated consistently.Run the following script to verify that no call sites are using the old signature:
#!/bin/bash # Description: Search for any remaining call sites that might use the old findDiscourseNode signature. # We're looking for patterns like findDiscourseNode(uid) or findDiscourseNode(uid, nodes) # that are NOT inside an object literal. echo "=== Searching for potential old-style findDiscourseNode calls ===" # Search for findDiscourseNode calls that don't use object parameter syntax # This looks for calls with direct arguments rather than object literals rg -n --type=ts --type=tsx 'findDiscourseNode\s*\([^{]' apps/roam/ echo "" echo "=== Searching for findDiscourseNode import statements ===" # Verify the import statements to understand the expected signature rg -n --type=ts --type=tsx 'from.*findDiscourseNode' apps/roam/ echo "" echo "=== Searching for all findDiscourseNode function calls ===" # Get all calls to review manually rg -n --type=ts --type=tsx -A2 'findDiscourseNode\s*\(' apps/roam/Also applies to: 731-731
apps/roam/src/utils/findDiscourseNode.ts (1)
17-28: Cache behavior is intentional and safe per current usage patterns.The function caches by
uidwhile matching bytitleonly when provided. Analysis of all 15 call sites shows that whentitleis passed (3 cases inCreateRelationDialog.tsx), bothuidandtitlecome from the same source object, maintaining the design contract that they correspond to the same entity. The 13 other call sites pass onlyuid. No evidence of the sameuidbeing called with differenttitlevalues exists in the codebase.
| const findDiscourseNode = ({ | ||
| uid, | ||
| title, | ||
| nodes, | ||
| nodes = getDiscourseNodes(), | ||
| }: { | ||
| uid: string; | ||
| title: string; | ||
| uid?: string; | ||
| title?: string; | ||
| nodes?: DiscourseNode[]; | ||
| }): DiscourseNode | false => { | ||
| nodes = nodes || getDiscourseNodes(); | ||
| if (uid === undefined) return false; |
There was a problem hiding this comment.
Early return breaks title-only lookups.
The guard at line 16 returns false immediately when uid is undefined, preventing title-only lookups. This contradicts the PR objective to "extend findDiscourseNode to accept the title as an argument."
If a caller provides only title (without uid), the function will return false without attempting to match by title. Consider revising the guard to allow title-only matching.
🔎 Proposed fix
const findDiscourseNode = ({
uid,
title,
nodes = getDiscourseNodes(),
}: {
uid?: string;
title?: string;
nodes?: DiscourseNode[];
}): DiscourseNode | false => {
- if (uid === undefined) return false;
+ if (uid === undefined && title === undefined) return false;
+
+ // For title-only lookups, skip cache and match directly
+ if (uid === undefined && title !== undefined) {
+ return nodes.find((node) => matchDiscourseNode({ ...node, title })) || false;
+ }
+
if (typeof discourseNodeTypeCache[uid] !== "undefined") {
return discourseNodeTypeCache[uid];
}
const matchingNode =
nodes.find((node) =>
title === undefined
? matchDiscourseNode({ ...node, uid: uid })
: matchDiscourseNode({ ...node, title }),
) || false;
discourseNodeTypeCache[uid] = matchingNode;
return matchingNode;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const findDiscourseNode = ({ | |
| uid, | |
| title, | |
| nodes, | |
| nodes = getDiscourseNodes(), | |
| }: { | |
| uid: string; | |
| title: string; | |
| uid?: string; | |
| title?: string; | |
| nodes?: DiscourseNode[]; | |
| }): DiscourseNode | false => { | |
| nodes = nodes || getDiscourseNodes(); | |
| if (uid === undefined) return false; | |
| const findDiscourseNode = ({ | |
| uid, | |
| title, | |
| nodes = getDiscourseNodes(), | |
| }: { | |
| uid?: string; | |
| title?: string; | |
| nodes?: DiscourseNode[]; | |
| }): DiscourseNode | false => { | |
| if (uid === undefined && title === undefined) return false; | |
| // For title-only lookups, skip cache and match directly | |
| if (uid === undefined && title !== undefined) { | |
| return nodes.find((node) => matchDiscourseNode({ ...node, title })) || false; | |
| } | |
| if (typeof discourseNodeTypeCache[uid] !== "undefined") { | |
| return discourseNodeTypeCache[uid]; | |
| } | |
| const matchingNode = | |
| nodes.find((node) => | |
| title === undefined | |
| ? matchDiscourseNode({ ...node, uid: uid }) | |
| : matchDiscourseNode({ ...node, title }), | |
| ) || false; | |
| discourseNodeTypeCache[uid] = matchingNode; | |
| return matchingNode; | |
| }; |
🤖 Prompt for AI Agents
In apps/roam/src/utils/findDiscourseNode.ts around lines 7 to 16, the early
return `if (uid === undefined) return false;` prevents title-only lookups;
remove or change this guard so the function continues when uid is undefined and
instead attempts to match by title (and only skip uid-based matching when uid is
absent). Update logic to: if uid is present try uid match first; if not found
and title is provided try title match; return false only if neither lookup
succeeds.
There was a problem hiding this comment.
I thought of doing that, and decided it may be interpreted as scope creep. (But I'm all for it of course.)
Btw if we decide to do this, we also have to get the uid from getPageUidByPageTitle so we can populate the cache.
Leaving the decision to @mdroidian .
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
https://linear.app/discourse-graphs/issue/ENG-1182/extend-finddiscoursenode-to-accept-the-title-as-an-argument
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.