[ENG-658] Add existing node flow

[trangdoan982]

https://www.loom.com/share/826ab70ee976490f940ed41b7b428af2

Summary by CodeRabbit

• New Features
  • Added an “Existing Node” search in the canvas to find notes by title and drop them as discourse-node shapes with automatic linking and metadata.
• Style
  • Search bar now supports custom CSS classes for easier theming.
• Refactor
  • Improved reliability and consistency of node search results.
  • Provided plugin context to the canvas view for better extensibility (no user-facing changes).

[linear]

ENG-658 Add existing node flow

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[trangdoan982]

• [ENG-658] Add existing node flow #389 👈 (View in Graphite)
• [ENG-604] Create node flow #387
• tldraw-obsidian

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

[trangdoan982]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors QueryEngine.searchCompatibleNodeByTitle to use an options object and adds searchDiscourseNodesByTitle. Updates RelationshipSection to the new API. Extends SearchBar with an optional className. Adds ExistingNodeSearch and integrates it into DiscourseNodePanel. Wraps TldrawPreviewComponent with PluginProvider in TldrawView.

Changes

Cohort / File(s)	Summary
QueryEngine additions and refactor apps/obsidian/src/services/QueryEngine.ts	Added searchDiscourseNodesByTitle(query, nodeTypeIds?). Refactored searchCompatibleNodeByTitle to accept a single options object and added Datacore availability guard; aligned error handling and filtering.
Caller update to new search API apps/obsidian/src/components/RelationshipSection.tsx	Updated call to QueryEngine.searchCompatibleNodeByTitle from positional args to an options object with compatibleNodeTypeIds.
SearchBar API extension apps/obsidian/src/components/SearchBar.tsx	Added optional className?: string prop; appended to input’s className without changing behavior.
Canvas: search and placement of existing nodes apps/obsidian/src/components/canvas/DiscourseNodePanel.tsx, apps/obsidian/src/components/canvas/ExistingNodeSearch.tsx, apps/obsidian/src/components/canvas/TldrawView.tsx	Introduced ExistingNodeSearch component for querying nodes and placing shapes on canvas; integrated alongside Node Types panel. Wrapped TldrawPreviewComponent with PluginProvider for context propagation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant ENS as ExistingNodeSearch
  participant SB as SearchBar
  participant QE as QueryEngine
  participant DC as Datacore
  participant Obs as Obsidian (Vault)
  participant TL as tldraw Editor
  participant FS as Canvas File

  User->>SB: Type query
  SB-->>ENS: onSearch(query)
  ENS->>QE: searchDiscourseNodesByTitle(query, nodeTypeIds)
  QE->>DC: Execute query (filter by nodeTypeId)
  DC-->>QE: Results
  QE-->>ENS: TFile[] (filtered/fuzzy)
  ENS-->>SB: results

  User->>SB: Select item
  SB-->>ENS: onSelect(TFile)
  ENS->>TL: getEditor()
  alt Editor available
    ENS->>FS: addWikilinkBlockrefForFile(selected)
    ENS->>Obs: getFrontmatterForFile(selected)
    ENS->>TL: create "discourse-node" shape (src, title, nodeTypeId)
    ENS->>TL: select shape, commit history
  else No editor
    ENS-->>User: No-op
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

• [ENG-198] Filtered out related file in search #125 — Modifies QueryEngine.searchCompatibleNodeByTitle API (adds selectedRelationType), closely related to this PR’s signature refactor to an options object.
• [ENG-598] Data persistence for tldraw #303 — Changes TldrawView to adjust plugin/file context handling; related to wrapping with PluginProvider here.
• ENG-96 Create new relationship between nodes #115 — Introduces/updates SearchBar and QueryEngine types; this PR extends SearchBar’s API and updates QueryEngine signatures accordingly.

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

apps/obsidian/src/components/SearchBar.tsx (1)

38-53: Debounce can leave previous Promises unresolved

Clearing the timeout cancels earlier searches but their returned Promises never resolve, which can leak if the caller holds them. Track the prior resolver and resolve to [] on cancel.

Apply:

 class GenericSuggest<T> extends AbstractInputSuggest<T> {
   private getItemTextFn: (item: T) => string;
   private renderItemFn: (item: T, el: HTMLElement) => void;
   private onSelectCallback: (item: T) => void;
   private asyncSearchFn: (query: string) => Promise<T[]>;
   private minQueryLength: number;
   private debounceTimeout: number | null = null;
+  private pendingResolve: ((items: T[]) => void) | null = null;

   async getSuggestions(inputStr: string): Promise<T[]> {
@@
-    return new Promise((resolve) => {
+    return new Promise((resolve) => {
       if (this.debounceTimeout) {
-        clearTimeout(this.debounceTimeout);
+        clearTimeout(this.debounceTimeout);
+        // Resolve the previous promise to an empty result to avoid dangling promises.
+        this.pendingResolve?.([]);
       }
+      this.pendingResolve = resolve;

       this.debounceTimeout = window.setTimeout(async () => {
         try {
           const results = await this.asyncSearchFn(query);
-          resolve(results);
+          this.pendingResolve?.(results);
         } catch (error) {
           console.error(`[GenericSuggest] Error in async search:`, error);
-          resolve([]);
+          this.pendingResolve?.([]);
         }
-      }, 250);
+        this.pendingResolve = null;
+      }, 250);
     });
   }

apps/obsidian/src/services/QueryEngine.ts (3)

99-106: Fix query precedence, escaping, and missing await.

• Parentheses are needed around the OR group to avoid precedence issues.
• Escape IDs via JSON.stringify.
• dc.query is awaited in the other method; use await here too for consistency and correctness.

Apply this diff:

-      const dcQuery = `@page and exists(nodeTypeId) and ${compatibleNodeTypeIds
-        .map((id) => `nodeTypeId = "${id}"`)
-        .join(" or ")}`;
-
-      const potentialNodes = this.dc.query(dcQuery);
+      const dcQuery = `@page and exists(nodeTypeId) and (${compatibleNodeTypeIds
+        .map((id) => `nodeTypeId = ${JSON.stringify(id)}`)
+        .join(" or ")})`;
+
+      const potentialNodes = await this.dc.query(dcQuery);

---

111-119: Normalize frontmatter relation value to an array.

Frontmatter may be a single string or an array. Mapping over a string will throw.

Apply this diff:

-      if (selectedRelationType) {
-        const fileCache = this.app.metadataCache.getFileCache(activeFile);
-        const existingRelations =
-          fileCache?.frontmatter?.[selectedRelationType] || [];
-
-        existingRelatedFiles = existingRelations.map((relation: string) => {
+      if (selectedRelationType) {
+        const fileCache = this.app.metadataCache.getFileCache(activeFile);
+        const rawRelations = fileCache?.frontmatter?.[selectedRelationType];
+        const existingRelations: string[] = Array.isArray(rawRelations)
+          ? rawRelations
+          : rawRelations
+          ? [rawRelations]
+          : [];
+
+        existingRelatedFiles = existingRelations
+          .filter((r) => typeof r === "string" && r.trim().length > 0)
+          .map((relation: string) => {
           const match = relation.match(/\[\[(.*?)(?:\|.*?)?\]\]/);
           return match ? match[1] : relation.replace(/^\[\[|\]\]$/g, "");
-        });
+        });

---

121-130: Same as above: return only TFile instances (avoid unsafe cast).

Align mapping with the stricter approach to prevent runtime errors accessing TFile props.

Apply this diff:

-      const finalResults = searchResults
-        .map((dcFile: any) => {
-          if (dcFile && dcFile.$path) {
-            const realFile = this.app.vault.getAbstractFileByPath(dcFile.$path);
-            if (realFile && realFile instanceof TFile) {
-              return realFile;
-            }
-          }
-          return dcFile as TFile;
-        })
+      const finalResults = searchResults
+        .map((dcFile: any) => {
+          if (dcFile?.$path) {
+            const real = this.app.vault.getAbstractFileByPath(dcFile.$path);
+            if (real instanceof TFile) return real;
+          }
+          return null;
+        })
+        .filter((f): f is TFile => f instanceof TFile)
         .filter((file: TFile) => {

🧹 Nitpick comments (5)

apps/obsidian/src/components/SearchBar.tsx (1)

125-130: Avoid dynamic class fragments that JIT/Purge might miss

If using a utility-CSS JIT/purge (e.g., Tailwind), bg-${...} can fail to be included in the build. Prefer enumerating both tokens explicitly.

Apply:

-        className={`w-full p-2 ${
-          selected ? "pr-9" : ""
-        } border-modifier-border rounded border bg-${
-          selected || disabled ? "secondary" : "primary"
-        } ${disabled ? "cursor-not-allowed opacity-70" : "cursor-text"} ${className}`}
+        className={[
+          "w-full p-2",
+          selected ? "pr-9" : "",
+          "border-modifier-border rounded border",
+          selected || disabled ? "bg-secondary" : "bg-primary",
+          disabled ? "cursor-not-allowed opacity-70" : "cursor-text",
+          className,
+        ].filter(Boolean).join(" ")}

apps/obsidian/src/components/canvas/ExistingNodeSearch.tsx (1)

82-91: SearchBar styling hook works; consider min query length

Optional: expose and pass a minQueryLength={2} to avoid unnecessary queries on short inputs.

-      <SearchBar<TFile>
+      <SearchBar<TFile>
         onSelect={handleSelect}
         placeholder="Node search"
         getItemText={getItemText}
         renderItem={renderItem}
         asyncSearch={search}
         className="!bg-[var(--color-panel)] !text-[var(--color-text)]"
+        // minQueryLength prop would need to be added to SearchBar and forwarded to GenericSuggest
+        // minQueryLength={2}
       />

apps/obsidian/src/services/QueryEngine.ts (3)

40-45: Consider a lightweight vault fallback when Datacore isn’t available.

Right now search returns [] without Datacore. Optionally fall back to scanning vault markdown files and filtering by frontmatter nodeTypeId + fuzzy title match to keep UX functional.

I can draft a private fallbackSearchDiscourseNodesByTitle() that mirrors fallbackScanVault if you want.

---

149-151: Correct the log message to the actual method name.

Apply this diff:

-      console.error("Error in searchNodeByTitle:", error);
+      console.error("Error in searchCompatibleNodeByTitle:", error);

---

78-153: DRY: Reuse searchDiscourseNodesByTitle within searchCompatibleNodeByTitle.

Call searchDiscourseNodesByTitle(query, compatibleNodeTypeIds) to build the base set, then apply active-file and existing-relations filters. Reduces duplicate query-building and result mapping logic.

I can provide a concrete refactor if you want this consolidated.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6e25a2b and ebe51c0.

📒 Files selected for processing (6)

• apps/obsidian/src/components/RelationshipSection.tsx (1 hunks)
• apps/obsidian/src/components/SearchBar.tsx (2 hunks)
• apps/obsidian/src/components/canvas/DiscourseNodePanel.tsx (2 hunks)
• apps/obsidian/src/components/canvas/ExistingNodeSearch.tsx (1 hunks)
• apps/obsidian/src/components/canvas/TldrawView.tsx (2 hunks)
• apps/obsidian/src/services/QueryEngine.ts (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-06-22T10:40:52.752Z

Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#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/obsidian/src/services/QueryEngine.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check
• GitHub Check: Graphite / mergeability_check

🔇 Additional comments (5)

apps/obsidian/src/components/canvas/TldrawView.tsx (1)

10-10: Wrap with PluginProvider looks correct; ensure single provider

Import path and provider usage LGTM. This enables usePlugin() in descendants rendered within this view.

Also applies to: 151-159

apps/obsidian/src/components/SearchBar.tsx (1)

70-86: Public API extension (className) is fine

Adding className?: string and threading it through is clean and backwards-compatible.

apps/obsidian/src/components/canvas/DiscourseNodePanel.tsx (1)

35-41: Good integration of ExistingNodeSearch

Props passed (plugin, canvasFile, getEditor) are sufficient, and colocating with node types improves UX.

apps/obsidian/src/components/canvas/ExistingNodeSearch.tsx (1)

25-29: Keep node type ids in sync if settings mutate in place

If plugin.settings.nodeTypes is mutated in place, this effect may not re-run. Consider deriving ids on-demand for each search or listening to a settings change event if available.

Would you like me to update this to read directly in search() (and memoize) so it stays in sync without relying on reference changes?

apps/obsidian/src/services/QueryEngine.ts (1)

78-88: Nice: object-arg signature for searchCompatibleNodeByTitle improves call-site clarity.

Early guards for query length and Datacore availability are also good.

[mdroidian]

Good stuff so far!

[mdroidian]

The x in the searchbar is off for me