From d0912caff82b85adb9aaa7ba07de8f588673e0de Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Wed, 23 Sep 2026 18:17:31 -0400 Subject: [PATCH 1/4] ENG-2272 Show candidate nodes as results in advanced node search Add a "Show candidate nodes" toggle to a new Display options menu. When on, lines tagged with a node type's tag are collected in one pass over the metadata cache and ranked alongside nodes, with nodes winning ties. Candidate rows use the node type's badge outlined instead of filled, in a fixed-width column so every title lines up, and open at the tagged line. Insert link is disabled for candidates since they are not nodes yet. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M385GX3NP1QPTJ9K30QK20TG --- .../src/components/NodeDisplayOptionsMenu.tsx | 51 ++++ .../src/components/NodeSearchFooter.tsx | 4 +- .../src/components/NodeSearchModal.tsx | 113 +++++-- apps/obsidian/src/components/NodeSortMenu.tsx | 2 +- .../src/components/SearchDropdown.tsx | 6 +- .../components/canvas/utils/openFileUtils.ts | 12 +- apps/obsidian/src/services/QueryEngine.ts | 64 +++- .../services/__tests__/QueryEngine.test.ts | 278 +++++++++++++++++- apps/obsidian/src/utils/tagNodeHandler.ts | 16 +- apps/obsidian/src/utils/taggedLine.ts | 18 ++ 10 files changed, 519 insertions(+), 45 deletions(-) create mode 100644 apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx create mode 100644 apps/obsidian/src/utils/taggedLine.ts diff --git a/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx b/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx new file mode 100644 index 0000000000..c1a0e670ed --- /dev/null +++ b/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx @@ -0,0 +1,51 @@ +import type { App } from "obsidian"; +import type { ReactElement } from "react"; +import { activateOnKey } from "~/components/NodeSortMenu"; +import { SearchDropdown } from "~/components/SearchDropdown"; + +export const NodeDisplayOptionsMenu = ({ + app, + isOpen, + onOpenChange, + onShowCandidatesChange, + showCandidates, +}: { + app: App; + isOpen: boolean; + onOpenChange: (isOpen: boolean) => void; + onShowCandidatesChange: (showCandidates: boolean) => void; + showCandidates: boolean; +}): ReactElement => { + const toggle = () => onShowCandidatesChange(!showCandidates); + + return ( + +
activateOnKey(event, toggle)} + onMouseDown={(event) => event.preventDefault()} + className="text-normal hover:bg-modifier-hover flex cursor-pointer items-center justify-between gap-3 px-3 py-2 text-sm" + > + Show candidate nodes + {/* Obsidian's own toggle chrome, so it matches the settings tab. */} +
+
+ + ); +}; diff --git a/apps/obsidian/src/components/NodeSearchFooter.tsx b/apps/obsidian/src/components/NodeSearchFooter.tsx index 176aa1eddf..afa9a0acb6 100644 --- a/apps/obsidian/src/components/NodeSearchFooter.tsx +++ b/apps/obsidian/src/components/NodeSearchFooter.tsx @@ -4,6 +4,7 @@ import { getHintKeys, type HintKey } from "~/utils/keyboardHints"; type NodeSearchFooterProps = { canAct: boolean; canInsertLink: boolean; + isActiveResultLinkable: boolean; onClose: () => void; onInsertLink: () => void; onOpenInNewTab: () => void; @@ -53,6 +54,7 @@ const FooterAction = ({ export const NodeSearchFooter = ({ canAct, canInsertLink, + isActiveResultLinkable, onClose, onInsertLink, onOpenInNewTab, @@ -62,7 +64,7 @@ export const NodeSearchFooter = ({ {/* Absent, not disabled: with no cursor there is nothing to insert into. */} {canInsertLink && ( ( + + {badge.text} + +); + const ResultList = ({ results, activeIndex, @@ -228,7 +252,7 @@ const ResultList = ({ > {results.map((result, index) => (
hasPointerMoved(event) && onActivate(index)} @@ -236,23 +260,28 @@ const ResultList = ({ // Keeps focus in the search input, so the keyboard path stays live // after a click. onMouseDown={(event) => event.preventDefault()} - className={`border-modifier-border flex cursor-pointer items-center gap-2 border-b px-3 py-2 ${ + className={`border-modifier-border flex cursor-pointer items-start gap-2 border-b px-3 py-2 ${ index === activeIndex ? "bg-modifier-hover" : "" }`} > - {result.nodeType.badge && ( - - {result.nodeType.badge.text} - - )} - + {/* Fixed-width column, so every title starts at the same x. */} + + {result.nodeType.badge && ( + + )} + +
+ + {result.tagLine && ( +
+ {`#${result.tagLine.tag} · ${result.file.basename} · L${result.tagLine.line + 1}`} +
+ )} +
))}
@@ -280,6 +309,10 @@ const NodeSearch = ({ // One value per toolbar, so two panels can never be open at once. const [openDropdown, setOpenDropdown] = useState(null); const [sortKey, setSortKey] = useState(DEFAULT_SORT_KEY); + const [showCandidates, setShowCandidates] = useState(false); + const [tagCandidates, setTagCandidates] = useState( + [], + ); const [sortDirection, setSortDirection] = useState( DEFAULT_SORT_DIRECTION, ); @@ -322,6 +355,30 @@ const NodeSearch = ({ } }, [app]); + // Rescans on every toggle-on, so node type edits made meanwhile are picked up. + useEffect(() => { + if (!showCandidates) { + setTagCandidates([]); + return; + } + let cancelled = false; + const load = async () => { + try { + const candidates = await new QueryEngine(app).getCandidateNodes( + plugin.settings.nodeTypes, + ); + if (!cancelled) setTagCandidates(candidates); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + new Notice(`Could not load candidate nodes: ${message}`); + } + }; + void load(); + return () => { + cancelled = true; + }; + }, [app, plugin.settings.nodeTypes, showCandidates]); + useEffect(() => { const timeout = window.setTimeout( () => setDebouncedQuery(query), @@ -334,7 +391,7 @@ const NodeSearch = ({ const results = useMemo(() => { if (candidateState.status !== "ready") return []; const ranked = rankDiscourseNodesByTitle({ - candidates: candidateState.candidates, + candidates: [...candidateState.candidates, ...tagCandidates], query: debouncedQuery, nodeTypeIds: selectedNodeTypeIds, }); @@ -368,6 +425,7 @@ const NodeSearch = ({ selectedNodeTypeIds, sortDirection, sortKey, + tagCandidates, userNames, ]); @@ -403,12 +461,12 @@ const NodeSearch = ({ // Closes before opening: `close()` unmounts this React root, so the file and // app are read first and nothing touches state afterwards. const openActiveResult = ( - open: (app: App, file: TFile) => Promise, + open: (app: App, file: TFile, line?: number) => Promise, ): void => { if (!activeResult) return; - const { file } = activeResult; + const { file, tagLine } = activeResult; onClose(); - void open(app, file).catch((error: unknown) => { + void open(app, file, tagLine?.line).catch((error: unknown) => { const message = error instanceof Error ? error.message : String(error); new Notice(`Could not open ${file.basename}: ${message}`); }); @@ -426,9 +484,12 @@ const NodeSearch = ({ if (!isOpen) inputRef.current?.focus(); }; + // A candidate is a line, not a node yet, so there is nothing to link to. + const isActiveResultLinkable = !!activeResult && !activeResult.tagLine; + // Closes before inserting, like `openActiveResult`. const insertLinkToActiveResult = (): void => { - if (!activeResult || !insertTarget) return; + if (!activeResult || !insertTarget || !isActiveResultLinkable) return; const { file } = activeResult; onClose(); try { @@ -456,7 +517,7 @@ const NodeSearch = ({ (event.metaKey || event.ctrlKey) && !event.altKey && insertTarget && - activeResult + isActiveResultLinkable ) { event.preventDefault(); insertLinkToActiveResult(); @@ -515,6 +576,15 @@ const NodeSearch = ({ sortDirection={sortDirection} sortKey={sortKey} /> + + handleDropdownOpenChange({ id: "display-options", isOpen }) + } + onShowCandidatesChange={setShowCandidates} + showCandidates={showCandidates} + />
@@ -542,6 +612,7 @@ const NodeSearch = ({ openActiveResult(openFileInNewTab)} diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx index ade1326ec5..14ce15671f 100644 --- a/apps/obsidian/src/components/NodeSortMenu.tsx +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -17,7 +17,7 @@ const DIRECTIONS: { direction: SortDirection; label: string }[] = [ ]; // Rows are divs, so Enter and Space have to be wired up the way a button gets them free. -const activateOnKey = ( +export const activateOnKey = ( event: KeyboardEvent, activate: () => void, ): void => { diff --git a/apps/obsidian/src/components/SearchDropdown.tsx b/apps/obsidian/src/components/SearchDropdown.tsx index d4030164e1..c121961529 100644 --- a/apps/obsidian/src/components/SearchDropdown.tsx +++ b/apps/obsidian/src/components/SearchDropdown.tsx @@ -2,7 +2,11 @@ import { App, Scope, setIcon } from "obsidian"; import { useEffect, useRef, type ReactElement, type ReactNode } from "react"; /** Which toolbar panel is open, so two can never be open at once. */ -export type SearchDropdownId = "type-filter" | "sort" | null; +export type SearchDropdownId = + | "type-filter" + | "sort" + | "display-options" + | null; export const SearchDropdown = ({ app, diff --git a/apps/obsidian/src/components/canvas/utils/openFileUtils.ts b/apps/obsidian/src/components/canvas/utils/openFileUtils.ts index 4dbe521800..fcb513d3a0 100644 --- a/apps/obsidian/src/components/canvas/utils/openFileUtils.ts +++ b/apps/obsidian/src/components/canvas/utils/openFileUtils.ts @@ -88,17 +88,25 @@ export const openFileInSidebar = async ( export const openFileInNewTab = async ( app: App, file: TFile, + line?: number, ): Promise => { const leaf = app.workspace.getLeaf("tab"); - await leaf.openFile(file); + await leaf.openFile( + file, + line === undefined ? undefined : { eState: { line } }, + ); app.workspace.setActiveLeaf(leaf); }; export const openFileInNewLeaf = async ( app: App, file: TFile, + line?: number, ): Promise => { const leaf = app.workspace.getLeaf("split"); - await leaf.openFile(file); + await leaf.openFile( + file, + line === undefined ? undefined : { eState: { line } }, + ); app.workspace.setActiveLeaf(leaf); }; diff --git a/apps/obsidian/src/services/QueryEngine.ts b/apps/obsidian/src/services/QueryEngine.ts index e817459b11..a947a3fc03 100644 --- a/apps/obsidian/src/services/QueryEngine.ts +++ b/apps/obsidian/src/services/QueryEngine.ts @@ -10,6 +10,7 @@ import { BulkImportPattern, BulkImportCandidate, DiscourseNode } from "~/types"; import { getDiscourseNodeFormatExpression } from "~/utils/getDiscourseNodeFormatExpression"; import { extractContentFromTitle } from "~/utils/extractContentFromTitle"; import { AppWithUnofficialApis } from "~/utils/obsidianUnofficialTypes"; +import { titleFromTaggedLine } from "~/utils/taggedLine"; // This is a workaround to get the datacore API. // TODO: Remove once we can use datacore npm package @@ -45,6 +46,8 @@ export type DiscourseNodeCandidate = { */ title: string; nodeTypeId: string; + /** Set for candidate nodes: an inline line tagged with a node type's tag. */ + tagLine?: { line: number; tag: string }; }; export type RankedDiscourseNode = DiscourseNodeCandidate & { @@ -87,6 +90,57 @@ export class QueryEngine { return candidates; }; + /** One pass over the metadata cache's tag index; only files with a hit are read. */ + getCandidateNodes = async ( + nodeTypes: DiscourseNode[], + ): Promise => { + const nodeTypeByTag = new Map< + string, + { nodeType: DiscourseNode; tag: string } + >(); + for (const nodeType of nodeTypes) { + if (nodeType.tag) { + nodeTypeByTag.set(nodeType.tag.toLowerCase(), { + nodeType, + tag: nodeType.tag, + }); + } + } + if (!nodeTypeByTag.size) return []; + + type TagHit = { line: number; nodeType: DiscourseNode; tag: string }; + const hitsByFile: { file: TFile; hits: TagHit[] }[] = []; + for (const file of this.app.vault.getMarkdownFiles()) { + const hits: TagHit[] = []; + const seen = new Set(); + for (const tagCache of this.app.metadataCache.getFileCache(file)?.tags ?? + []) { + const match = nodeTypeByTag.get(tagCache.tag.slice(1).toLowerCase()); + const line = tagCache.position.start.line; + const key = `${line}:${match?.nodeType.id}`; + if (!match || seen.has(key)) continue; + seen.add(key); + hits.push({ line, ...match }); + } + if (hits.length) hitsByFile.push({ file, hits }); + } + + const perFile = await Promise.all( + hitsByFile.map(async ({ file, hits }) => { + // A file deleted or locked mid-scan shouldn't sink the other candidates. + const content = await this.app.vault.cachedRead(file).catch(() => ""); + const lines = content.split("\n"); + return hits.flatMap(({ line, nodeType, tag }) => { + const title = titleFromTaggedLine(lines[line] ?? ""); + return title + ? [{ file, title, nodeTypeId: nodeType.id, tagLine: { line, tag } }] + : []; + }); + }), + ); + return perFile.flat(); + }; + /** * Search across all discourse nodes (files that have frontmatter nodeTypeId) */ @@ -744,6 +798,9 @@ const filterCandidatesByNodeTypeIds = ( return candidates.filter((candidate) => selected.has(candidate.nodeTypeId)); }; +const nodesFirst = (a: DiscourseNodeCandidate, b: DiscourseNodeCandidate) => + Number(!!a.tagLine) - Number(!!b.tagLine); + /** * Best match first, uncapped — capping is the caller's, so a later re-sort orders the * whole set rather than a top slice. Filters before scoring: same results, less work. @@ -763,7 +820,7 @@ export const rankDiscourseNodesByTitle = ({ // Filter-only searches still need a list, so an empty query is not an empty result. if (!trimmedQuery) { return [...filtered] - .sort((a, b) => a.title.localeCompare(b.title)) + .sort((a, b) => a.title.localeCompare(b.title) || nodesFirst(a, b)) .map((candidate) => ({ ...candidate, match: { score: 0, matches: [] }, @@ -778,8 +835,9 @@ export const rankDiscourseNodesByTitle = ({ if (match) ranked.push({ ...candidate, match }); } - // Sort is stable, so equal scores keep candidate order. - return ranked.sort((a, b) => b.match.score - a.match.score); + return ranked.sort( + (a, b) => b.match.score - a.match.score || nodesFirst(a, b), + ); }; /** diff --git a/apps/obsidian/src/services/__tests__/QueryEngine.test.ts b/apps/obsidian/src/services/__tests__/QueryEngine.test.ts index 18d30e9c09..ed56c78e35 100644 --- a/apps/obsidian/src/services/__tests__/QueryEngine.test.ts +++ b/apps/obsidian/src/services/__tests__/QueryEngine.test.ts @@ -1,6 +1,22 @@ import { TFile, type App } from "obsidian"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { QueryEngine } from "~/services/QueryEngine"; +import { + QueryEngine, + rankDiscourseNodesByTitle, + type DiscourseNodeCandidate, +} from "~/services/QueryEngine"; +import type { DiscourseNode } from "~/types"; + +// Stand-in scorer: earlier substring hits score higher, mirroring fuzzy search's ordering. +vi.mock("obsidian", async (importOriginal) => ({ + ...(await importOriginal()), + prepareFuzzySearch: (query: string) => (text: string) => { + const index = text.toLowerCase().indexOf(query.toLowerCase()); + return index < 0 + ? null + : { score: -index, matches: [[index, index + query.length]] }; + }, +})); type Frontmatter = Record; @@ -175,3 +191,263 @@ describe("QueryEngine Datacore readiness", () => { expect(datacoreQuery).not.toHaveBeenCalled(); }); }); + +type VaultNote = { + path: string; + content: string; + frontmatter?: Frontmatter; +}; + +const CLAIM: DiscourseNode = { + id: "claim", + name: "Claim", + format: "CLM - {content}", + tag: "clm-candidate", + created: 0, + modified: 0, +}; + +const EVIDENCE: DiscourseNode = { + id: "evidence", + name: "Evidence", + format: "EVD - {content}", + tag: "evd-candidate", + created: 0, + modified: 0, +}; + +// Mirrors Obsidian's metadataCache: tags are indexed per line, case preserved. +const createVaultApp = (notes: VaultNote[]) => { + const files = notes.map((note) => createFile(note.path)); + const noteByPath = new Map(notes.map((note) => [note.path, note])); + const cachedRead = vi.fn((file: TFile) => + Promise.resolve(noteByPath.get(file.path)?.content ?? ""), + ); + const app = { + metadataCache: { + getFileCache: (file: TFile) => { + const note = noteByPath.get(file.path); + if (!note) return null; + const tags = note.content.split("\n").flatMap((text, line) => + [...text.matchAll(/#[\w-]+/g)].map((match) => ({ + tag: match[0], + position: { start: { line, col: match.index, offset: 0 } }, + })), + ); + return { frontmatter: note.frontmatter, tags }; + }, + }, + plugins: { plugins: {} }, + vault: { getMarkdownFiles: () => files, cachedRead }, + } as unknown as App; + return { app, cachedRead }; +}; + +describe("QueryEngine.getCandidateNodes", () => { + it("returns only the tagged line of a paragraph as the candidate title", async () => { + const { app } = createVaultApp([ + { + path: "Meeting notes.md", + content: "line 1: blah blah blah blah\nchange line here #clm-candidate", + }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(candidates).toEqual([ + expect.objectContaining({ + title: "change line here", + nodeTypeId: "claim", + tagLine: { line: 1, tag: "clm-candidate" }, + }), + ]); + expect(candidates[0]?.file.path).toBe("Meeting notes.md"); + }); + + it("strips list, task, heading and quote markers from candidate titles", async () => { + const { app } = createVaultApp([ + { + path: "Shapes.md", + content: [ + "## Heading claim #clm-candidate", + "- list claim #clm-candidate", + " - nested claim #clm-candidate", + "- [ ] task claim #clm-candidate", + "1. numbered claim #clm-candidate", + "> quoted claim #clm-candidate", + "# Top heading claim #clm-candidate", + ].join("\n"), + }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(candidates.map((c) => [c.title, c.tagLine?.line])).toEqual([ + ["Heading claim", 0], + ["list claim", 1], + ["nested claim", 2], + ["task claim", 3], + ["numbered claim", 4], + ["quoted claim", 5], + ["Top heading claim", 6], + ]); + }); + + it("matches node tags case-insensitively and ignores other tags", async () => { + const { app } = createVaultApp([ + { + path: "Journal.md", + content: [ + "Mixed case claim #CLM-Candidate", + "Just a todo #todo", + "Evidence line #evd-candidate", + ].join("\n"), + }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(candidates.map((c) => [c.title, c.nodeTypeId])).toEqual([ + ["Mixed case claim", "claim"], + ]); + }); + + it("returns one candidate per node type when a line carries two node tags", async () => { + const { app } = createVaultApp([ + { + path: "Journal.md", + content: "Dual line #clm-candidate #evd-candidate", + }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([ + CLAIM, + EVIDENCE, + ]); + + expect(candidates.map((c) => [c.title, c.nodeTypeId])).toEqual([ + ["Dual line", "claim"], + ["Dual line", "evidence"], + ]); + }); + + it("collapses a node tag repeated on the same line into one candidate", async () => { + const { app } = createVaultApp([ + { + path: "Journal.md", + content: "Repeated #clm-candidate and again #clm-candidate", + }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(candidates).toHaveLength(1); + }); + + it("drops tagged lines with no text besides markers and tags", async () => { + const { app } = createVaultApp([ + { + path: "Journal.md", + content: ["- #clm-candidate", "Real claim #clm-candidate"].join("\n"), + }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(candidates.map((c) => c.title)).toEqual(["Real claim"]); + }); + + it("skips a file that fails to read and still returns the others", async () => { + const { app, cachedRead } = createVaultApp([ + { path: "Broken.md", content: "Lost claim #clm-candidate" }, + { path: "Fine.md", content: "Kept claim #clm-candidate" }, + ]); + cachedRead.mockImplementationOnce(() => + Promise.reject(new Error("read failed")), + ); + + const candidates = await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(candidates.map((c) => c.title)).toEqual(["Kept claim"]); + }); + + it("reads only files that contain a node tag", async () => { + const { app, cachedRead } = createVaultApp([ + { path: "Tagged.md", content: "A claim #clm-candidate" }, + { path: "Untagged.md", content: "Nothing here #todo" }, + ]); + + await new QueryEngine(app).getCandidateNodes([CLAIM]); + + expect(cachedRead.mock.calls.map(([file]) => file.path)).toEqual([ + "Tagged.md", + ]); + }); +}); + +describe("rankDiscourseNodesByTitle with candidate nodes", () => { + const node = ( + title: string, + nodeTypeId = "claim", + ): DiscourseNodeCandidate => ({ + file: createFile(`${title}.md`), + title, + nodeTypeId, + }); + const candidate = ( + title: string, + nodeTypeId = "claim", + ): DiscourseNodeCandidate => ({ + file: createFile("Journal.md"), + title, + nodeTypeId, + tagLine: { line: 3, tag: `${nodeTypeId}-candidate` }, + }); + + it("filters candidates by node type like nodes", () => { + const ranked = rankDiscourseNodesByTitle({ + candidates: [ + node("Sky claim"), + candidate("Sky evidence", "evidence"), + candidate("Sky candidate claim"), + ], + query: "", + nodeTypeIds: ["claim"], + }); + + expect(ranked.map((r) => r.title)).toEqual([ + "Sky candidate claim", + "Sky claim", + ]); + }); + + it("ranks a better-matching candidate above a weaker node", () => { + const ranked = rankDiscourseNodesByTitle({ + candidates: [node("Why the sky is blue"), candidate("Sky varies")], + query: "sky", + }); + + expect(ranked.map((r) => r.title)).toEqual([ + "Sky varies", + "Why the sky is blue", + ]); + }); + + it("puts nodes before candidates on equal scores", () => { + const ranked = rankDiscourseNodesByTitle({ + candidates: [candidate("Sky is blue"), node("Sky is blue")], + query: "sky", + }); + + expect(ranked.map((r) => Boolean(r.tagLine))).toEqual([false, true]); + }); + + it("puts nodes before candidates on equal titles when the query is empty", () => { + const ranked = rankDiscourseNodesByTitle({ + candidates: [candidate("Sky is blue"), node("Sky is blue")], + query: "", + }); + + expect(ranked.map((r) => Boolean(r.tagLine))).toEqual([false, true]); + }); +}); diff --git a/apps/obsidian/src/utils/tagNodeHandler.ts b/apps/obsidian/src/utils/tagNodeHandler.ts index 8048437d0a..b207542a7c 100644 --- a/apps/obsidian/src/utils/tagNodeHandler.ts +++ b/apps/obsidian/src/utils/tagNodeHandler.ts @@ -14,6 +14,7 @@ import ModifyNodeModal from "~/components/ModifyNodeModal"; import { addRelationIfRequested } from "~/components/canvas/utils/relationJsonUtils"; import { getNodeTagColors } from "./colorUtils"; import { createDiscourseNodeFile, formatNodeName } from "./createNode"; +import { extractListPrefix, titleFromTaggedLine } from "./taggedLine"; const HOVER_DELAY = 200; const HIDE_DELAY = 100; @@ -22,23 +23,8 @@ const STYLE_ELEMENT_ID = "dg-discourse-tag-colors"; const DISCOURSE_TAG_CLASS = "dg-discourse-tag"; const NODE_ID_ATTR = "data-dg-discourse-tag-node"; -const LIST_INDICATOR_REGEX = /^(\s*)(\d+[.)]\s+|[-*+]\s+(?:\[[ xX]\]\s+)?)/; - const TAG_SEGMENT_PREFIX = "tag-"; -const sanitizeTitle = (title: string): string => - title - .replace(LIST_INDICATOR_REGEX, "") - .replace(/[\\/:]/g, "") - .replace(/\s+/g, " ") - .trim(); - -const extractListPrefix = (line: string): string => - line.match(LIST_INDICATOR_REGEX)?.[0] ?? ""; - -const titleFromTaggedLine = (lineText: string): string => - sanitizeTitle(lineText.replace(/#[^\s]+/g, "")); - // Nodes are named like `hashtag_hashtag-end_meta_tag-clm-candidate`; reading the tag // from the tree inherits Obsidian's rules for code blocks, URLs and headings. const tagNameFromSyntaxNode = (nodeName: string): string | null => { diff --git a/apps/obsidian/src/utils/taggedLine.ts b/apps/obsidian/src/utils/taggedLine.ts new file mode 100644 index 0000000000..c3b2824863 --- /dev/null +++ b/apps/obsidian/src/utils/taggedLine.ts @@ -0,0 +1,18 @@ +const LIST_INDICATOR_REGEX = /^(\s*)(\d+[.)]\s+|[-*+]\s+(?:\[[ xX]\]\s+)?)/; + +const BLOCK_MARKER_REGEX = /^\s*(?:#{1,6}\s+|>\s*)/; + +const sanitizeTitle = (title: string): string => + title + .replace(LIST_INDICATOR_REGEX, "") + .replace(/[\\/:]/g, "") + .replace(/\s+/g, " ") + .trim(); + +export const extractListPrefix = (line: string): string => + line.match(LIST_INDICATOR_REGEX)?.[0] ?? ""; + +export const titleFromTaggedLine = (lineText: string): string => + sanitizeTitle( + lineText.replace(BLOCK_MARKER_REGEX, "").replace(/#[^\s]+/g, ""), + ); From 6b6e08aedfdf9071449f6776744ff4df74b9347b Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 14:41:04 -0400 Subject: [PATCH 2/4] ENG-2272 Apply adherence review fixes Pass the open line as a named option, move activateOnKey to utils/keyboardHints, add explicit return types, and style the candidate pill text with a class instead of an inline style. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J2QDY0GGKKK7WM2A69YA6X --- .../src/components/NodeDisplayOptionsMenu.tsx | 4 ++-- apps/obsidian/src/components/NodeSearchModal.tsx | 12 +++++++----- apps/obsidian/src/components/NodeSortMenu.tsx | 13 ++----------- .../src/components/canvas/utils/openFileUtils.ts | 4 ++-- apps/obsidian/src/services/QueryEngine.ts | 11 +++++++---- apps/obsidian/src/utils/keyboardHints.ts | 11 +++++++++++ 6 files changed, 31 insertions(+), 24 deletions(-) diff --git a/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx b/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx index c1a0e670ed..628441ffcc 100644 --- a/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx +++ b/apps/obsidian/src/components/NodeDisplayOptionsMenu.tsx @@ -1,7 +1,7 @@ import type { App } from "obsidian"; import type { ReactElement } from "react"; -import { activateOnKey } from "~/components/NodeSortMenu"; import { SearchDropdown } from "~/components/SearchDropdown"; +import { activateOnKey } from "~/utils/keyboardHints"; export const NodeDisplayOptionsMenu = ({ app, @@ -16,7 +16,7 @@ export const NodeDisplayOptionsMenu = ({ onShowCandidatesChange: (showCandidates: boolean) => void; showCandidates: boolean; }): ReactElement => { - const toggle = () => onShowCandidatesChange(!showCandidates); + const toggle = (): void => onShowCandidatesChange(!showCandidates); return ( {badge.text} @@ -362,7 +364,7 @@ const NodeSearch = ({ return; } let cancelled = false; - const load = async () => { + const load = async (): Promise => { try { const candidates = await new QueryEngine(app).getCandidateNodes( plugin.settings.nodeTypes, @@ -461,12 +463,12 @@ const NodeSearch = ({ // Closes before opening: `close()` unmounts this React root, so the file and // app are read first and nothing touches state afterwards. const openActiveResult = ( - open: (app: App, file: TFile, line?: number) => Promise, + open: (app: App, file: TFile, options: { line?: number }) => Promise, ): void => { if (!activeResult) return; const { file, tagLine } = activeResult; onClose(); - void open(app, file, tagLine?.line).catch((error: unknown) => { + void open(app, file, { line: tagLine?.line }).catch((error: unknown) => { const message = error instanceof Error ? error.message : String(error); new Notice(`Could not open ${file.basename}: ${message}`); }); diff --git a/apps/obsidian/src/components/NodeSortMenu.tsx b/apps/obsidian/src/components/NodeSortMenu.tsx index 14ce15671f..6840d56f9b 100644 --- a/apps/obsidian/src/components/NodeSortMenu.tsx +++ b/apps/obsidian/src/components/NodeSortMenu.tsx @@ -1,6 +1,7 @@ import { App, setIcon } from "obsidian"; -import type { KeyboardEvent, ReactElement } from "react"; +import type { ReactElement } from "react"; import { SearchDropdown } from "~/components/SearchDropdown"; +import { activateOnKey } from "~/utils/keyboardHints"; import { SORT_OPTIONS, getDefaultDirectionForKey, @@ -16,16 +17,6 @@ const DIRECTIONS: { direction: SortDirection; label: string }[] = [ { direction: "desc", label: "Desc" }, ]; -// Rows are divs, so Enter and Space have to be wired up the way a button gets them free. -export const activateOnKey = ( - event: KeyboardEvent, - activate: () => void, -): void => { - if (event.key !== "Enter" && event.key !== " ") return; - event.preventDefault(); - activate(); -}; - const getDirectionIconName = (direction: SortDirection): string => direction === "asc" ? "arrow-up-narrow-wide" : "arrow-down-wide-narrow"; diff --git a/apps/obsidian/src/components/canvas/utils/openFileUtils.ts b/apps/obsidian/src/components/canvas/utils/openFileUtils.ts index fcb513d3a0..f754f78d07 100644 --- a/apps/obsidian/src/components/canvas/utils/openFileUtils.ts +++ b/apps/obsidian/src/components/canvas/utils/openFileUtils.ts @@ -88,7 +88,7 @@ export const openFileInSidebar = async ( export const openFileInNewTab = async ( app: App, file: TFile, - line?: number, + { line }: { line?: number } = {}, ): Promise => { const leaf = app.workspace.getLeaf("tab"); await leaf.openFile( @@ -101,7 +101,7 @@ export const openFileInNewTab = async ( export const openFileInNewLeaf = async ( app: App, file: TFile, - line?: number, + { line }: { line?: number } = {}, ): Promise => { const leaf = app.workspace.getLeaf("split"); await leaf.openFile( diff --git a/apps/obsidian/src/services/QueryEngine.ts b/apps/obsidian/src/services/QueryEngine.ts index a947a3fc03..883fbb0107 100644 --- a/apps/obsidian/src/services/QueryEngine.ts +++ b/apps/obsidian/src/services/QueryEngine.ts @@ -116,9 +116,10 @@ export class QueryEngine { for (const tagCache of this.app.metadataCache.getFileCache(file)?.tags ?? []) { const match = nodeTypeByTag.get(tagCache.tag.slice(1).toLowerCase()); + if (!match) continue; const line = tagCache.position.start.line; - const key = `${line}:${match?.nodeType.id}`; - if (!match || seen.has(key)) continue; + const key = `${line}:${match.nodeType.id}`; + if (seen.has(key)) continue; seen.add(key); hits.push({ line, ...match }); } @@ -798,8 +799,10 @@ const filterCandidatesByNodeTypeIds = ( return candidates.filter((candidate) => selected.has(candidate.nodeTypeId)); }; -const nodesFirst = (a: DiscourseNodeCandidate, b: DiscourseNodeCandidate) => - Number(!!a.tagLine) - Number(!!b.tagLine); +const nodesFirst = ( + a: DiscourseNodeCandidate, + b: DiscourseNodeCandidate, +): number => Number(!!a.tagLine) - Number(!!b.tagLine); /** * Best match first, uncapped — capping is the caller's, so a later re-sort orders the diff --git a/apps/obsidian/src/utils/keyboardHints.ts b/apps/obsidian/src/utils/keyboardHints.ts index ec38800a83..291da19e7f 100644 --- a/apps/obsidian/src/utils/keyboardHints.ts +++ b/apps/obsidian/src/utils/keyboardHints.ts @@ -1,4 +1,5 @@ import { Platform } from "obsidian"; +import type { KeyboardEvent } from "react"; export type HintKey = "Mod" | "Alt" | "Shift" | "Enter" | "Escape" | "Tab"; @@ -33,3 +34,13 @@ export const formatHintKeys = ({ export const getHintKeys = (keys: HintKey[]): string[] => formatHintKeys({ keys, isMacOS: Platform.isMacOS }); + +// Rows are divs, so Enter and Space have to be wired up the way a button gets them free. +export const activateOnKey = ( + event: KeyboardEvent, + activate: () => void, +): void => { + if (event.key !== "Enter" && event.key !== " ") return; + event.preventDefault(); + activate(); +}; From eff43493807e3f7a793f281fcc7baed406daf8ab Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 15:00:23 -0400 Subject: [PATCH 3/4] ENG-2272 Keep every node type that shares a candidate tag Node type settings don't enforce unique tags, so the tag lookup now maps each tag to all matching types instead of keeping only the last one. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M3J3TS9ZG3TPS0TXYVGAGKY0 --- apps/obsidian/src/services/QueryEngine.ts | 33 ++++++++++--------- .../services/__tests__/QueryEngine.test.ts | 13 ++++++++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/apps/obsidian/src/services/QueryEngine.ts b/apps/obsidian/src/services/QueryEngine.ts index 883fbb0107..7b5a824d8c 100644 --- a/apps/obsidian/src/services/QueryEngine.ts +++ b/apps/obsidian/src/services/QueryEngine.ts @@ -94,19 +94,20 @@ export class QueryEngine { getCandidateNodes = async ( nodeTypes: DiscourseNode[], ): Promise => { - const nodeTypeByTag = new Map< + // Settings don't enforce unique tags, so one tag can map to several types. + const nodeTypesByTag = new Map< string, - { nodeType: DiscourseNode; tag: string } + { nodeType: DiscourseNode; tag: string }[] >(); for (const nodeType of nodeTypes) { - if (nodeType.tag) { - nodeTypeByTag.set(nodeType.tag.toLowerCase(), { - nodeType, - tag: nodeType.tag, - }); - } + if (!nodeType.tag) continue; + const key = nodeType.tag.toLowerCase(); + nodeTypesByTag.set(key, [ + ...(nodeTypesByTag.get(key) ?? []), + { nodeType, tag: nodeType.tag }, + ]); } - if (!nodeTypeByTag.size) return []; + if (!nodeTypesByTag.size) return []; type TagHit = { line: number; nodeType: DiscourseNode; tag: string }; const hitsByFile: { file: TFile; hits: TagHit[] }[] = []; @@ -115,13 +116,15 @@ export class QueryEngine { const seen = new Set(); for (const tagCache of this.app.metadataCache.getFileCache(file)?.tags ?? []) { - const match = nodeTypeByTag.get(tagCache.tag.slice(1).toLowerCase()); - if (!match) continue; const line = tagCache.position.start.line; - const key = `${line}:${match.nodeType.id}`; - if (seen.has(key)) continue; - seen.add(key); - hits.push({ line, ...match }); + const matches = + nodeTypesByTag.get(tagCache.tag.slice(1).toLowerCase()) ?? []; + for (const match of matches) { + const key = `${line}:${match.nodeType.id}`; + if (seen.has(key)) continue; + seen.add(key); + hits.push({ line, ...match }); + } } if (hits.length) hitsByFile.push({ file, hits }); } diff --git a/apps/obsidian/src/services/__tests__/QueryEngine.test.ts b/apps/obsidian/src/services/__tests__/QueryEngine.test.ts index ed56c78e35..4f24d1beca 100644 --- a/apps/obsidian/src/services/__tests__/QueryEngine.test.ts +++ b/apps/obsidian/src/services/__tests__/QueryEngine.test.ts @@ -331,6 +331,19 @@ describe("QueryEngine.getCandidateNodes", () => { ]); }); + it("returns a candidate for every node type that shares a tag", async () => { + const { app } = createVaultApp([ + { path: "Journal.md", content: "Shared line #shared-candidate" }, + ]); + + const candidates = await new QueryEngine(app).getCandidateNodes([ + { ...CLAIM, tag: "shared-candidate" }, + { ...EVIDENCE, tag: "Shared-Candidate" }, + ]); + + expect(candidates.map((c) => c.nodeTypeId)).toEqual(["claim", "evidence"]); + }); + it("collapses a node tag repeated on the same line into one candidate", async () => { const { app } = createVaultApp([ { From ed66545efcfbbff169f7f124d68bd41f8f38df22 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 27 Sep 2026 15:01:43 -0400 Subject: [PATCH 4/4] Revert "ENG-2272 Keep every node type that shares a candidate tag" Duplicate node tags should be blocked in node type settings rather than handled in candidate search; tracked in ENG-2327. Co-Authored-By: Claude Opus 5.5 --- apps/obsidian/src/services/QueryEngine.ts | 33 +++++++++---------- .../services/__tests__/QueryEngine.test.ts | 13 -------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/apps/obsidian/src/services/QueryEngine.ts b/apps/obsidian/src/services/QueryEngine.ts index 7b5a824d8c..883fbb0107 100644 --- a/apps/obsidian/src/services/QueryEngine.ts +++ b/apps/obsidian/src/services/QueryEngine.ts @@ -94,20 +94,19 @@ export class QueryEngine { getCandidateNodes = async ( nodeTypes: DiscourseNode[], ): Promise => { - // Settings don't enforce unique tags, so one tag can map to several types. - const nodeTypesByTag = new Map< + const nodeTypeByTag = new Map< string, - { nodeType: DiscourseNode; tag: string }[] + { nodeType: DiscourseNode; tag: string } >(); for (const nodeType of nodeTypes) { - if (!nodeType.tag) continue; - const key = nodeType.tag.toLowerCase(); - nodeTypesByTag.set(key, [ - ...(nodeTypesByTag.get(key) ?? []), - { nodeType, tag: nodeType.tag }, - ]); + if (nodeType.tag) { + nodeTypeByTag.set(nodeType.tag.toLowerCase(), { + nodeType, + tag: nodeType.tag, + }); + } } - if (!nodeTypesByTag.size) return []; + if (!nodeTypeByTag.size) return []; type TagHit = { line: number; nodeType: DiscourseNode; tag: string }; const hitsByFile: { file: TFile; hits: TagHit[] }[] = []; @@ -116,15 +115,13 @@ export class QueryEngine { const seen = new Set(); for (const tagCache of this.app.metadataCache.getFileCache(file)?.tags ?? []) { + const match = nodeTypeByTag.get(tagCache.tag.slice(1).toLowerCase()); + if (!match) continue; const line = tagCache.position.start.line; - const matches = - nodeTypesByTag.get(tagCache.tag.slice(1).toLowerCase()) ?? []; - for (const match of matches) { - const key = `${line}:${match.nodeType.id}`; - if (seen.has(key)) continue; - seen.add(key); - hits.push({ line, ...match }); - } + const key = `${line}:${match.nodeType.id}`; + if (seen.has(key)) continue; + seen.add(key); + hits.push({ line, ...match }); } if (hits.length) hitsByFile.push({ file, hits }); } diff --git a/apps/obsidian/src/services/__tests__/QueryEngine.test.ts b/apps/obsidian/src/services/__tests__/QueryEngine.test.ts index 4f24d1beca..ed56c78e35 100644 --- a/apps/obsidian/src/services/__tests__/QueryEngine.test.ts +++ b/apps/obsidian/src/services/__tests__/QueryEngine.test.ts @@ -331,19 +331,6 @@ describe("QueryEngine.getCandidateNodes", () => { ]); }); - it("returns a candidate for every node type that shares a tag", async () => { - const { app } = createVaultApp([ - { path: "Journal.md", content: "Shared line #shared-candidate" }, - ]); - - const candidates = await new QueryEngine(app).getCandidateNodes([ - { ...CLAIM, tag: "shared-candidate" }, - { ...EVIDENCE, tag: "Shared-Candidate" }, - ]); - - expect(candidates.map((c) => c.nodeTypeId)).toEqual(["claim", "evidence"]); - }); - it("collapses a node tag repeated on the same line into one candidate", async () => { const { app } = createVaultApp([ {