From d3ee5eab3728ebeed8100d053c9bfee9c770d212 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 17:53:23 -0400 Subject: [PATCH 01/11] ENG-1249 Add a cached relations index for per-link lookups Reading relations straight from disk costs a full vault file read plus a JSON parse per call. That is fine for the Discourse Context panel, which asks once per file open, but the upcoming discourse context overlay asks once per discourse-node link on screen. RelationsIndex keeps a parsed snapshot of relations.json grouped by endpoint id, so a lookup on a render path is a Map hit and can answer synchronously. It rebuilds from the vault's own modify/create/delete events, which covers writes made through saveRelations as well as edits arriving over sync, so relationsStore does not need to know it exists. Also wires apps/obsidian into the root test:unit task, which it was not previously exposed to. Co-Authored-By: Claude Opus 5 --- apps/obsidian/package.json | 4 +- apps/obsidian/src/index.ts | 6 + .../__tests__/relationsEndpointIndex.test.ts | 103 +++++++++++++++ .../src/utils/relationsEndpointIndex.ts | 69 ++++++++++ apps/obsidian/src/utils/relationsIndex.ts | 120 ++++++++++++++++++ apps/obsidian/vitest.config.mts | 17 +++ pnpm-lock.yaml | 42 ++++++ 7 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts create mode 100644 apps/obsidian/src/utils/relationsEndpointIndex.ts create mode 100644 apps/obsidian/src/utils/relationsIndex.ts create mode 100644 apps/obsidian/vitest.config.mts diff --git a/apps/obsidian/package.json b/apps/obsidian/package.json index d7b167bc9..d7f6058b3 100644 --- a/apps/obsidian/package.json +++ b/apps/obsidian/package.json @@ -10,7 +10,8 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "publish": "tsx scripts/publish.ts", - "check-types": "tsc --noEmit --skipLibCheck" + "check-types": "tsc --noEmit --skipLibCheck", + "test:unit": "vitest run --config vitest.config.mts" }, "keywords": [], "author": "", @@ -35,6 +36,7 @@ "tsx": "^4.19.2", "typescript": "5.5.4", "uuidv7": "1.1.0", + "vitest": "catalog:", "zod": "^3.24.1" }, "dependencies": { diff --git a/apps/obsidian/src/index.ts b/apps/obsidian/src/index.ts index 06d169332..39d6ad400 100644 --- a/apps/obsidian/src/index.ts +++ b/apps/obsidian/src/index.ts @@ -35,6 +35,7 @@ import { NodeTagSuggestPopover } from "~/components/NodeTagSuggestModal"; import { InlineNodeTypePicker } from "~/components/InlineNodeTypePicker"; import { initializeSupabaseSync } from "~/utils/syncDgNodesToSupabase"; import { FileChangeListener } from "~/utils/fileChangeListener"; +import { RelationsIndex } from "~/utils/relationsIndex"; import generateUid from "~/utils/generateUid"; import { migrateFrontmatterRelationsToRelationsJson, @@ -51,6 +52,7 @@ import { export default class DiscourseGraphPlugin extends Plugin { settings: Settings = { ...DEFAULT_SETTINGS }; + relationsIndex: RelationsIndex = new RelationsIndex(this); private tagNodeHandler: TagNodeHandler | null = null; private fileChangeListener: FileChangeListener | null = null; private activeNodePopover: @@ -98,6 +100,8 @@ export default class DiscourseGraphPlugin extends Plugin { } } + this.relationsIndex.initialize(); + registerCommands(this); this.addSettingTab(new SettingsTab(this.app, this)); addIcon(DISCOURSE_GRAPH_LOGO_ICON_ID, WHITE_LOGO_SVG); @@ -488,5 +492,7 @@ export default class DiscourseGraphPlugin extends Plugin { this.fileChangeListener.cleanup(); this.fileChangeListener = null; } + + this.relationsIndex.unload(); } } diff --git a/apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts b/apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts new file mode 100644 index 000000000..65f9acf17 --- /dev/null +++ b/apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from "vitest"; +import type { RelationInstance } from "~/types"; +import { + buildEndpointIndex, + collectRelations, +} from "~/utils/relationsEndpointIndex"; + +const relation = ( + overrides: Partial & Pick, +): RelationInstance => ({ + type: "supports", + source: "a", + destination: "b", + created: 0, + ...overrides, +}); + +const toRecord = ( + relations: RelationInstance[], +): Record => + Object.fromEntries(relations.map((r) => [r.id, r])); + +describe("buildEndpointIndex", () => { + it("files a relation under both endpoints", () => { + const r = relation({ id: "r1", source: "a", destination: "b" }); + const index = buildEndpointIndex(toRecord([r])); + + expect(index.get("a")).toEqual([r]); + expect(index.get("b")).toEqual([r]); + }); + + it("files a self-relation once so one endpoint does not yield it twice", () => { + const r = relation({ id: "r1", source: "a", destination: "a" }); + const index = buildEndpointIndex(toRecord([r])); + + expect(index.get("a")).toEqual([r]); + }); + + it("groups multiple relations sharing an endpoint", () => { + const r1 = relation({ id: "r1", source: "a", destination: "b" }); + const r2 = relation({ id: "r2", source: "c", destination: "a" }); + const index = buildEndpointIndex(toRecord([r1, r2])); + + expect(index.get("a")).toEqual([r1, r2]); + expect(index.get("b")).toEqual([r1]); + expect(index.get("c")).toEqual([r2]); + }); + + it("returns an empty index for no relations", () => { + expect(buildEndpointIndex({}).size).toBe(0); + }); + + it("skips relations missing an endpoint rather than indexing undefined", () => { + const r = relation({ id: "r1", source: "a", destination: "" }); + const index = buildEndpointIndex(toRecord([r])); + + expect(index.get("a")).toEqual([r]); + expect(index.has("")).toBe(false); + expect(index.size).toBe(1); + }); +}); + +describe("collectRelations", () => { + it("returns relations for a single endpoint", () => { + const r1 = relation({ id: "r1", source: "a", destination: "b" }); + const r2 = relation({ id: "r2", source: "c", destination: "d" }); + const index = buildEndpointIndex(toRecord([r1, r2])); + + expect(collectRelations({ index, endpointIds: ["a"] })).toEqual([r1]); + }); + + it("counts a relation once when both its endpoints are queried", () => { + // An imported node matches on both its nodeInstanceId and its + // importedFromRid, so both ends of the same relation can be asked for. + const r = relation({ id: "r1", source: "local-id", destination: "rid" }); + const index = buildEndpointIndex(toRecord([r])); + + expect( + collectRelations({ index, endpointIds: ["local-id", "rid"] }), + ).toEqual([r]); + }); + + it("deduplicates across endpoints while preserving first-seen order", () => { + const r1 = relation({ id: "r1", source: "a", destination: "shared" }); + const r2 = relation({ id: "r2", source: "b", destination: "shared" }); + const index = buildEndpointIndex(toRecord([r1, r2])); + + expect( + collectRelations({ index, endpointIds: ["shared", "a", "b"] }).map( + (r) => r.id, + ), + ).toEqual(["r1", "r2"]); + }); + + it("returns an empty array for unknown endpoints", () => { + const index = buildEndpointIndex( + toRecord([relation({ id: "r1", source: "a", destination: "b" })]), + ); + + expect(collectRelations({ index, endpointIds: ["nope"] })).toEqual([]); + expect(collectRelations({ index, endpointIds: [] })).toEqual([]); + }); +}); diff --git a/apps/obsidian/src/utils/relationsEndpointIndex.ts b/apps/obsidian/src/utils/relationsEndpointIndex.ts new file mode 100644 index 000000000..dac467de5 --- /dev/null +++ b/apps/obsidian/src/utils/relationsEndpointIndex.ts @@ -0,0 +1,69 @@ +import type { RelationInstance } from "~/types"; + +/** + * Pure indexing helpers behind RelationsIndex, kept free of Obsidian and + * relationsStore imports so they stay directly testable. + */ + +/** + * Groups relations by the node instance ids at either end, so a lookup by + * endpoint is a Map hit instead of a scan over every relation in the vault. + * + * A relation is filed under both its source and its destination. Self-relations + * (source === destination) are filed once so a single endpoint never yields the + * same relation twice. + */ +export const buildEndpointIndex = ( + relations: Record, +): Map => { + const index = new Map(); + + const fileUnder = (endpointId: string, relation: RelationInstance): void => { + const existing = index.get(endpointId); + if (existing) { + existing.push(relation); + return; + } + index.set(endpointId, [relation]); + }; + + for (const relation of Object.values(relations)) { + if (!relation) continue; + if (relation.source) fileUnder(relation.source, relation); + if (relation.destination && relation.destination !== relation.source) { + fileUnder(relation.destination, relation); + } + } + + return index; +}; + +/** + * Returns every relation touching any of `endpointIds`, deduplicated by id. + * + * A relation whose source and destination are both in `endpointIds` — which + * happens for an imported node matched by both its nodeInstanceId and its + * importedFromRid — must still be counted once. + */ +export const collectRelations = ({ + index, + endpointIds, +}: { + index: Map; + endpointIds: Iterable; +}): RelationInstance[] => { + const seen = new Set(); + const collected: RelationInstance[] = []; + + for (const endpointId of endpointIds) { + const relations = index.get(endpointId); + if (!relations) continue; + for (const relation of relations) { + if (seen.has(relation.id)) continue; + seen.add(relation.id); + collected.push(relation); + } + } + + return collected; +}; diff --git a/apps/obsidian/src/utils/relationsIndex.ts b/apps/obsidian/src/utils/relationsIndex.ts new file mode 100644 index 000000000..0be2c7382 --- /dev/null +++ b/apps/obsidian/src/utils/relationsIndex.ts @@ -0,0 +1,120 @@ +import { TAbstractFile, TFile } from "obsidian"; +import type DiscourseGraphPlugin from "~/index"; +import type { RelationInstance } from "~/types"; +import { getRelationsFilePath, loadRelations } from "./relationsStore"; +import { buildEndpointIndex, collectRelations } from "./relationsEndpointIndex"; + +/** + * In-memory view of relations.json. + * + * Reading relations straight from disk costs a full vault file read plus a JSON + * parse per call, which is fine for the Discourse Context panel but not for + * anything that renders per link. This keeps a parsed snapshot so callers on a + * render path can ask a synchronous question and get an answer. + * + * The snapshot is rebuilt from the vault's own modify/create/delete events, so + * writes made through saveRelations and edits arriving over sync are picked up + * the same way, without relationsStore needing to know this exists. + */ +export class RelationsIndex { + private plugin: DiscourseGraphPlugin; + private index: Map | null = null; + private inFlight: Promise | null = null; + private subscribers = new Set<() => void>(); + /** + * Bumped on every invalidation. A load that started before the bump is stale + * by the time it resolves, so it must not overwrite a newer snapshot — + * relations.json being modified mid-read is the normal case here, not an edge + * one, since saving a relation triggers exactly that. + */ + private generation = 0; + + constructor(plugin: DiscourseGraphPlugin) { + this.plugin = plugin; + } + + initialize(): void { + const invalidateIfRelationsFile = (file: TAbstractFile): void => { + if (!(file instanceof TFile)) return; + if (file.path !== getRelationsFilePath()) return; + this.invalidate(); + }; + + const { vault } = this.plugin.app; + this.plugin.registerEvent(vault.on("modify", invalidateIfRelationsFile)); + this.plugin.registerEvent(vault.on("create", invalidateIfRelationsFile)); + this.plugin.registerEvent(vault.on("delete", invalidateIfRelationsFile)); + + void this.ensureLoaded(); + } + + unload(): void { + this.subscribers.clear(); + this.index = null; + this.inFlight = null; + this.generation += 1; + } + + /** + * Notifies when the snapshot changes, so a caller that rendered against a + * cold or stale index can render again. Returns an unsubscribe function. + */ + onChange(subscriber: () => void): () => void { + this.subscribers.add(subscriber); + return () => this.subscribers.delete(subscriber); + } + + isReady(): boolean { + return this.index !== null; + } + + async ensureLoaded(): Promise { + if (this.index !== null) return; + if (this.inFlight) return this.inFlight; + + const generation = this.generation; + this.inFlight = (async () => { + const relationsFile = await loadRelations(this.plugin); + if (generation !== this.generation) return; + this.index = buildEndpointIndex(relationsFile.relations ?? {}); + this.inFlight = null; + this.notify(); + })(); + + return this.inFlight; + } + + /** Drops the snapshot and reloads it. */ + async refresh(): Promise { + this.invalidate(); + await this.ensureLoaded(); + } + + /** + * Relations touching any of `endpointIds`. + * + * Returns an empty array when the snapshot is cold and schedules a load; + * subscribers are notified once it lands. Callers on a render path should + * treat an empty result as "nothing to draw yet" rather than "no relations". + */ + getRelationsForEndpointIds( + endpointIds: Iterable, + ): RelationInstance[] { + if (this.index === null) { + void this.ensureLoaded(); + return []; + } + return collectRelations({ index: this.index, endpointIds }); + } + + private invalidate(): void { + this.generation += 1; + this.index = null; + this.inFlight = null; + void this.ensureLoaded(); + } + + private notify(): void { + for (const subscriber of this.subscribers) subscriber(); + } +} diff --git a/apps/obsidian/vitest.config.mts b/apps/obsidian/vitest.config.mts new file mode 100644 index 000000000..9b056e9d9 --- /dev/null +++ b/apps/obsidian/vitest.config.mts @@ -0,0 +1,17 @@ +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { defineConfig } from "vitest/config"; + +const dirname = path.dirname(fileURLToPath(import.meta.url)); + +export default defineConfig({ + test: { + environment: "node", + include: ["src/utils/__tests__/**/*.test.ts"], + }, + resolve: { + alias: { + "~": path.resolve(dirname, "src"), + }, + }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8393d9435..adc5d7ff4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -215,6 +215,9 @@ importers: uuidv7: specifier: 1.1.0 version: 1.1.0 + vitest: + specifier: 'catalog:' + version: 4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2)) zod: specifier: ^3.24.1 version: 3.25.76 @@ -17120,6 +17123,15 @@ snapshots: chai: 6.2.2 tinyrainbow: 3.1.0 + '@vitest/mocker@4.1.6(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2))': + dependencies: + '@vitest/spy': 4.1.6 + estree-walker: 3.0.3 + magic-string: 0.30.21 + optionalDependencies: + msw: 2.11.1(@types/node@22.20.0)(typescript@5.5.4) + vite: 7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2) + '@vitest/mocker@4.1.6(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2))': dependencies: '@vitest/spy': 4.1.6 @@ -24335,6 +24347,36 @@ snapshots: tsx: 4.21.0 yaml: 2.8.2 + vitest@4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2)): + dependencies: + '@vitest/expect': 4.1.6 + '@vitest/mocker': 4.1.6(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2)) + '@vitest/pretty-format': 4.1.6 + '@vitest/runner': 4.1.6 + '@vitest/snapshot': 4.1.6 + '@vitest/spy': 4.1.6 + '@vitest/utils': 4.1.6 + es-module-lexer: 2.1.0 + expect-type: 1.3.0 + magic-string: 0.30.21 + obug: 2.1.1 + pathe: 2.0.3 + picomatch: 4.0.4 + std-env: 4.1.0 + tinybench: 2.9.0 + tinyexec: 1.0.2 + tinyglobby: 0.2.16 + tinyrainbow: 3.1.0 + vite: 7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2) + why-is-node-running: 2.3.0 + optionalDependencies: + '@edge-runtime/vm': 3.2.0 + '@opentelemetry/api': 1.9.0 + '@types/node': 22.20.0 + jsdom: 20.0.3 + transitivePeerDependencies: + - msw + vitest@4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2)): dependencies: '@vitest/expect': 4.1.6 From 9c78e03040f1c94da74d87727d9eb506fb19867a Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 18:36:41 -0400 Subject: [PATCH 02/11] ENG-1249 Resolve a link to its discourse node synchronously The overlay has to decide, per link and per viewport update, whether a link points at a discourse node and how many relations it has. Every read here hits an in-memory cache so the answer is available without awaiting. Notably this avoids getNodeTypeIdForFile/getNodeInstanceIdForFile, which poll for up to 500ms waiting on frontmatter for a just-created file. That is correct for relation bookkeeping and wrong on a render path, so an uncached file yields no badge and is picked up on the next redraw. Relations awaiting acceptance are excluded from the count: the Discourse Context panel lists those separately, so including them would show a number the panel never repeats back. Co-Authored-By: Claude Opus 5 --- .../discourseLinkFrontmatter.test.ts | 95 +++++++++++++++++++ .../src/utils/discourseLinkFrontmatter.ts | 46 +++++++++ apps/obsidian/src/utils/discourseLinkUtils.ts | 66 +++++++++++++ 3 files changed, 207 insertions(+) create mode 100644 apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts create mode 100644 apps/obsidian/src/utils/discourseLinkFrontmatter.ts create mode 100644 apps/obsidian/src/utils/discourseLinkUtils.ts diff --git a/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts b/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts new file mode 100644 index 000000000..3217ba0fa --- /dev/null +++ b/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import type { RelationInstance } from "~/types"; +import { + countAcceptedRelations, + getEndpointIdsFromFrontmatter, + getNodeTypeIdFromFrontmatter, +} from "~/utils/discourseLinkFrontmatter"; + +const relation = ( + overrides: Partial & Pick, +): RelationInstance => ({ + type: "supports", + source: "a", + destination: "b", + created: 0, + ...overrides, +}); + +describe("getNodeTypeIdFromFrontmatter", () => { + it("returns the node type id", () => { + expect(getNodeTypeIdFromFrontmatter({ nodeTypeId: "claim" })).toBe("claim"); + }); + + it("returns undefined when absent, empty, undefined frontmatter, or not a string", () => { + expect(getNodeTypeIdFromFrontmatter({})).toBeUndefined(); + expect(getNodeTypeIdFromFrontmatter({ nodeTypeId: "" })).toBeUndefined(); + expect(getNodeTypeIdFromFrontmatter(undefined)).toBeUndefined(); + expect(getNodeTypeIdFromFrontmatter({ nodeTypeId: 42 })).toBeUndefined(); + }); +}); + +describe("getEndpointIdsFromFrontmatter", () => { + it("returns the nodeInstanceId for a local node", () => { + expect(getEndpointIdsFromFrontmatter({ nodeInstanceId: "n1" })).toEqual([ + "n1", + ]); + }); + + it("returns both ids for an imported node", () => { + expect( + getEndpointIdsFromFrontmatter({ + nodeInstanceId: "n1", + importedFromRid: "rid1", + }), + ).toEqual(["n1", "rid1"]); + }); + + it("does not repeat an id when both fields match", () => { + expect( + getEndpointIdsFromFrontmatter({ + nodeInstanceId: "same", + importedFromRid: "same", + }), + ).toEqual(["same"]); + }); + + it("returns an importedFromRid even without a nodeInstanceId", () => { + expect(getEndpointIdsFromFrontmatter({ importedFromRid: "rid1" })).toEqual([ + "rid1", + ]); + }); + + it("returns nothing for absent or non-string ids", () => { + expect(getEndpointIdsFromFrontmatter({})).toEqual([]); + expect(getEndpointIdsFromFrontmatter(undefined)).toEqual([]); + expect(getEndpointIdsFromFrontmatter({ nodeInstanceId: 7 })).toEqual([]); + }); +}); + +describe("countAcceptedRelations", () => { + it("counts local relations, which leave tentative undefined", () => { + expect( + countAcceptedRelations([relation({ id: "r1" }), relation({ id: "r2" })]), + ).toBe(2); + }); + + it("counts explicitly accepted relations", () => { + expect( + countAcceptedRelations([relation({ id: "r1", tentative: true })]), + ).toBe(1); + }); + + it("excludes imported relations awaiting acceptance", () => { + expect( + countAcceptedRelations([ + relation({ id: "r1" }), + relation({ id: "r2", tentative: false }), + ]), + ).toBe(1); + }); + + it("returns zero for no relations", () => { + expect(countAcceptedRelations([])).toBe(0); + }); +}); diff --git a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts new file mode 100644 index 000000000..1a63e861b --- /dev/null +++ b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts @@ -0,0 +1,46 @@ +import type { RelationInstance } from "~/types"; + +/** + * Pure frontmatter/relation helpers behind discourseLinkUtils, kept free of + * Obsidian imports so they stay directly testable. + */ + +const asString = (value: unknown): string | undefined => + typeof value === "string" && value.length > 0 ? value : undefined; + +export const getNodeTypeIdFromFrontmatter = ( + frontmatter: Record | undefined, +): string | undefined => asString(frontmatter?.nodeTypeId); + +/** + * The ids a file's relations can be filed under. + * + * An imported node is referenced by its local nodeInstanceId and, in relations + * that arrived with the import, by its importedFromRid — so both must be + * queried for its relations to be found. + */ +export const getEndpointIdsFromFrontmatter = ( + frontmatter: Record | undefined, +): string[] => { + const endpointIds: string[] = []; + const nodeInstanceId = asString(frontmatter?.nodeInstanceId); + const importedFromRid = asString(frontmatter?.importedFromRid); + + if (nodeInstanceId) endpointIds.push(nodeInstanceId); + if (importedFromRid && importedFromRid !== nodeInstanceId) { + endpointIds.push(importedFromRid); + } + + return endpointIds; +}; + +/** + * Counts relations that are actually part of the graph. + * + * `tentative === false` marks an imported relation the user has not accepted + * yet; the Discourse Context panel lists those separately from accepted ones, + * so counting them in the badge would show a number the panel never repeats + * back. Local relations leave `tentative` undefined. + */ +export const countAcceptedRelations = (relations: RelationInstance[]): number => + relations.filter((relation) => relation.tentative !== false).length; diff --git a/apps/obsidian/src/utils/discourseLinkUtils.ts b/apps/obsidian/src/utils/discourseLinkUtils.ts new file mode 100644 index 000000000..e3a8a0ab5 --- /dev/null +++ b/apps/obsidian/src/utils/discourseLinkUtils.ts @@ -0,0 +1,66 @@ +import { parseLinktext, TFile } from "obsidian"; +import type DiscourseGraphPlugin from "~/index"; +import type { DiscourseNode } from "~/types"; +import { getNodeTypeById } from "./typeUtils"; +import { + countAcceptedRelations, + getEndpointIdsFromFrontmatter, + getNodeTypeIdFromFrontmatter, +} from "./discourseLinkFrontmatter"; + +export type DiscourseLinkTarget = { + file: TFile; + nodeType: DiscourseNode; + relationCount: number; +}; + +/** + * Resolves a link to a discourse node and its relation count, synchronously. + * + * Every read here hits an already-in-memory cache — Obsidian's metadataCache + * for frontmatter, the plugin's settings for node types, and RelationsIndex for + * relations — because this runs per link on a render path, once per viewport + * update. + * + * Deliberately does not use getNodeTypeIdForFile/getNodeInstanceIdForFile: those + * poll for up to 500ms waiting on frontmatter for a just-created file, which is + * right for relation bookkeeping and wrong for rendering. If frontmatter is not + * cached yet this returns null and the caller redraws when the index or the + * metadata cache next reports a change. + * + * Returns null when the link does not resolve, the target is not a discourse + * node, or its node type is no longer configured. + */ +export const resolveDiscourseLinkTarget = ({ + plugin, + linktext, + sourcePath, +}: { + plugin: DiscourseGraphPlugin; + linktext: string; + sourcePath: string; +}): DiscourseLinkTarget | null => { + // Strips any #heading or #^block subpath, which is not part of the file path. + const { path } = parseLinktext(linktext); + if (!path) return null; + + const file = plugin.app.metadataCache.getFirstLinkpathDest(path, sourcePath); + if (!file) return null; + + const frontmatter = plugin.app.metadataCache.getFileCache(file) + ?.frontmatter as Record | undefined; + + const nodeTypeId = getNodeTypeIdFromFrontmatter(frontmatter); + if (!nodeTypeId) return null; + + const nodeType = getNodeTypeById(plugin, nodeTypeId); + if (!nodeType) return null; + + const endpointIds = getEndpointIdsFromFrontmatter(frontmatter); + if (endpointIds.length === 0) return { file, nodeType, relationCount: 0 }; + + const relations = + plugin.relationsIndex.getRelationsForEndpointIds(endpointIds); + + return { file, nodeType, relationCount: countAcceptedRelations(relations) }; +}; From d6919f0c022b1786696cc922c5bcc6922b8b4681 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 18:38:41 -0400 Subject: [PATCH 03/11] ENG-1249 Add the overlay badge and its setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The badge is plain DOM rather than React so the Live Preview widget and the Reading view post processor can share one implementation: neither has a React root, and mounting one per link would be far too heavy. Toggling the setting applies immediately to both surfaces. Live Preview needs an empty CM6 transaction to make ViewPlugins re-evaluate; Reading view has no equivalent, so already-rendered content has to be thrown away and rebuilt. Both helpers move to markdownViewRefresh, which also takes over the copy previously inlined in onload. Neither surface renders a badge yet — that follows. Co-Authored-By: Claude Opus 5 --- .../src/components/GeneralSettings.tsx | 16 +++++ .../canvas/utils/externalContentHandlers.ts | 15 ++-- .../src/components/canvas/utils/toastUtils.ts | 2 +- .../src/components/discourseContextBadge.ts | 69 +++++++++++++++++++ apps/obsidian/src/constants.ts | 1 + apps/obsidian/src/index.ts | 36 +++++----- apps/obsidian/src/types.ts | 1 + .../src/utils/calcDiscourseNodeSize.ts | 1 - apps/obsidian/src/utils/colorUtils.ts | 1 - apps/obsidian/src/utils/loadImage.ts | 3 +- .../obsidian/src/utils/markdownViewRefresh.ts | 42 +++++++++++ 11 files changed, 156 insertions(+), 31 deletions(-) create mode 100644 apps/obsidian/src/components/discourseContextBadge.ts create mode 100644 apps/obsidian/src/utils/markdownViewRefresh.ts diff --git a/apps/obsidian/src/components/GeneralSettings.tsx b/apps/obsidian/src/components/GeneralSettings.tsx index ecfd964be..764434b57 100644 --- a/apps/obsidian/src/components/GeneralSettings.tsx +++ b/apps/obsidian/src/components/GeneralSettings.tsx @@ -197,6 +197,8 @@ const GeneralSettings = () => { const [showHelpMenuStatusBarIcon, setShowHelpMenuStatusBarIcon] = useState( plugin.settings.showHelpMenuStatusBarIcon, ); + const [showDiscourseContextOverlay, setShowDiscourseContextOverlay] = + useState(plugin.settings.showDiscourseContextOverlay); const handleToggleChange = (newValue: boolean) => { setShowIdsInFrontmatter(newValue); @@ -211,6 +213,13 @@ const GeneralSettings = () => { void plugin.saveSettings(); }; + const handleDiscourseContextOverlayToggleChange = (newValue: boolean) => { + setShowDiscourseContextOverlay(newValue); + plugin.settings.showDiscourseContextOverlay = newValue; + plugin.refreshDiscourseContextOverlay(); + void plugin.saveSettings(); + }; + const handleFolderPathChange = useCallback( (newValue: string) => { setNodesFolderPath(newValue); @@ -343,6 +352,13 @@ const GeneralSettings = () => { + + { if (!file.path.endsWith(".md")) return false; const frontmatter = getFrontmatterForFile(plugin.app, file); - const nodeTypeId = (frontmatter as { nodeTypeId?: string } | null)?.nodeTypeId; + const nodeTypeId = (frontmatter as { nodeTypeId?: string } | null) + ?.nodeTypeId; if (!nodeTypeId || typeof nodeTypeId !== "string") return false; return !!getNodeTypeById(plugin, nodeTypeId); }; @@ -116,7 +115,9 @@ export const handleExternalUrlContent = async ({ if (url.startsWith(OBSIDIAN_URL_PREFIX)) { const parsed = parseObsidianOpenUrl(url); if (!parsed) { - new Notice("Invalid Obsidian link. Only discourse nodes can be dropped on the canvas."); + new Notice( + "Invalid Obsidian link. Only discourse nodes can be dropped on the canvas.", + ); return; } @@ -182,7 +183,9 @@ const createDiscourseNodeShapeAtPoint = async ({ if (existing) { editor.setSelectedShapes([existing.id]); - editor.zoomToSelection({ animation: { duration: editor.options.animationMediumMs } }); + editor.zoomToSelection({ + animation: { duration: editor.options.animationMediumMs }, + }); return; } diff --git a/apps/obsidian/src/components/canvas/utils/toastUtils.ts b/apps/obsidian/src/components/canvas/utils/toastUtils.ts index 8b6c34cb1..e07a45b54 100644 --- a/apps/obsidian/src/components/canvas/utils/toastUtils.ts +++ b/apps/obsidian/src/components/canvas/utils/toastUtils.ts @@ -20,4 +20,4 @@ export const showToast = ({ keepOpen: false, }; dispatchToastEvent(toast, targetCanvasId); -}; \ No newline at end of file +}; diff --git a/apps/obsidian/src/components/discourseContextBadge.ts b/apps/obsidian/src/components/discourseContextBadge.ts new file mode 100644 index 000000000..5ef484cf7 --- /dev/null +++ b/apps/obsidian/src/components/discourseContextBadge.ts @@ -0,0 +1,69 @@ +import { setIcon, setTooltip, TFile } from "obsidian"; +import type { DiscourseNode } from "~/types"; + +/** + * Marks a badge in the DOM. Both render paths check for this before adding one, + * since Obsidian re-runs post processors over already-rendered sections. + */ +export const DISCOURSE_CONTEXT_BADGE_CLASS = "dg-discourse-context-badge"; + +export type DiscourseContextBadgeProps = { + file: TFile; + nodeType: DiscourseNode; + relationCount: number; + onActivate: (file: TFile) => void; +}; + +const badgeTooltip = ({ + nodeType, + relationCount, +}: Pick): string => { + const relations = relationCount === 1 ? "relation" : "relations"; + return `${nodeType.name}: ${relationCount} ${relations} — open discourse context`; +}; + +/** + * The inline badge shown next to a link to a discourse node. + * + * Plain DOM rather than React so the CodeMirror widget and the Reading view + * post processor can share one implementation — neither has a React root, and + * mounting one per link would be far too heavy. Tailwind utilities work here + * because they compile to ordinary global classes. + */ +export const createDiscourseContextBadge = ({ + file, + nodeType, + relationCount, + onActivate, +}: DiscourseContextBadgeProps): HTMLElement => { + const badge = createSpan(); + badge.className = `${DISCOURSE_CONTEXT_BADGE_CLASS} inline-flex items-center gap-0.5 align-middle ml-1 px-1 rounded cursor-pointer select-none text-[10px] leading-none text-[var(--text-muted)] hover:text-[var(--text-normal)] hover:bg-[var(--background-modifier-hover)] transition-colors duration-150`; + + const icon = badge.createSpan({ + cls: "inline-flex items-center [&>svg]:h-3 [&>svg]:w-3", + }); + setIcon(icon, "network"); + + badge.createSpan({ text: String(relationCount) }); + + const label = badgeTooltip({ nodeType, relationCount }); + setTooltip(badge, label); + badge.setAttribute("aria-label", label); + badge.setAttribute("role", "button"); + badge.setAttribute("tabindex", "0"); + + const activate = (event: Event): void => { + // Stops Obsidian from following the link the badge sits next to. + event.preventDefault(); + event.stopPropagation(); + onActivate(file); + }; + + badge.addEventListener("click", activate); + badge.addEventListener("keydown", (event: KeyboardEvent) => { + if (event.key !== "Enter" && event.key !== " ") return; + activate(event); + }); + + return badge; +}; diff --git a/apps/obsidian/src/constants.ts b/apps/obsidian/src/constants.ts index 95fab14a3..a9cf5632b 100644 --- a/apps/obsidian/src/constants.ts +++ b/apps/obsidian/src/constants.ts @@ -119,6 +119,7 @@ export const DEFAULT_SETTINGS: Settings = { canvasAttachmentsFolderPath: "attachments", nodeTagHotkey: "\\", showHelpMenuStatusBarIcon: false, + showDiscourseContextOverlay: true, spacePassword: undefined, accountLocalId: undefined, syncModeEnabled: false, diff --git a/apps/obsidian/src/index.ts b/apps/obsidian/src/index.ts index 39d6ad400..03203864b 100644 --- a/apps/obsidian/src/index.ts +++ b/apps/obsidian/src/index.ts @@ -36,6 +36,10 @@ import { InlineNodeTypePicker } from "~/components/InlineNodeTypePicker"; import { initializeSupabaseSync } from "~/utils/syncDgNodesToSupabase"; import { FileChangeListener } from "~/utils/fileChangeListener"; import { RelationsIndex } from "~/utils/relationsIndex"; +import { + refreshMarkdownEditors, + refreshMarkdownPreviews, +} from "~/utils/markdownViewRefresh"; import generateUid from "~/utils/generateUid"; import { migrateFrontmatterRelationsToRelationsJson, @@ -272,36 +276,28 @@ export default class DiscourseGraphPlugin extends Plugin { }), ); - type EditorWithCm = { cm: EditorView }; - const hasCodeMirrorView = (editor: unknown): editor is EditorWithCm => { - if (!editor || typeof editor !== "object") return false; - return "cm" in editor; - }; - // Dispatch a no-op CM6 transaction to every markdown editor so their // ViewPlugin re-evaluates hasVisibleCanvasLeaf and shows/hides widgets. // layout-change covers splits/moves, active-leaf-change covers tab switches. - const refreshMarkdownEditors = (): void => { - this.app.workspace.iterateAllLeaves((leaf) => { - if ( - leaf.view instanceof MarkdownView && - hasCodeMirrorView(leaf.view.editor) - ) { - leaf.view.editor.cm.dispatch({}); - } - }); - }; - this.registerEvent( - this.app.workspace.on("layout-change", refreshMarkdownEditors), - ); + const refreshEditors = (): void => refreshMarkdownEditors(this.app); + this.registerEvent(this.app.workspace.on("layout-change", refreshEditors)); this.registerEvent( - this.app.workspace.on("active-leaf-change", refreshMarkdownEditors), + this.app.workspace.on("active-leaf-change", refreshEditors), ); // Register editor keydown listener for node tag hotkey this.setupNodeTagHotkey(); } + /** + * Re-renders both markdown surfaces so the discourse context overlay appears + * or disappears immediately when its setting is toggled, without a reload. + */ + refreshDiscourseContextOverlay(): void { + refreshMarkdownEditors(this.app); + refreshMarkdownPreviews(this.app); + } + setHelpMenuStatusBarItemVisibility(): void { if (!this.settings.showHelpMenuStatusBarIcon) { this.helpMenuStatusBarItem?.remove(); diff --git a/apps/obsidian/src/types.ts b/apps/obsidian/src/types.ts index 050c476a3..fe998bafe 100644 --- a/apps/obsidian/src/types.ts +++ b/apps/obsidian/src/types.ts @@ -68,6 +68,7 @@ export type Settings = { canvasAttachmentsFolderPath: string; nodeTagHotkey: string; showHelpMenuStatusBarIcon: boolean; + showDiscourseContextOverlay: boolean; spacePassword?: string; accountLocalId?: string; syncModeEnabled?: boolean; diff --git a/apps/obsidian/src/utils/calcDiscourseNodeSize.ts b/apps/obsidian/src/utils/calcDiscourseNodeSize.ts index e17c7c5d3..4cd105fdd 100644 --- a/apps/obsidian/src/utils/calcDiscourseNodeSize.ts +++ b/apps/obsidian/src/utils/calcDiscourseNodeSize.ts @@ -72,4 +72,3 @@ export const calcDiscourseNodeSize = async ({ return { w, h: textHeight }; } }; - diff --git a/apps/obsidian/src/utils/colorUtils.ts b/apps/obsidian/src/utils/colorUtils.ts index 091667fa9..a1ed9503c 100644 --- a/apps/obsidian/src/utils/colorUtils.ts +++ b/apps/obsidian/src/utils/colorUtils.ts @@ -55,7 +55,6 @@ export const getNodeTagColors = ( return { backgroundColor, textColor }; }; - export const getAllDiscourseNodeColors = ( nodeTypes: DiscourseNode[], ): Array<{ diff --git a/apps/obsidian/src/utils/loadImage.ts b/apps/obsidian/src/utils/loadImage.ts index 08bc182a0..942b39bcc 100644 --- a/apps/obsidian/src/utils/loadImage.ts +++ b/apps/obsidian/src/utils/loadImage.ts @@ -1,7 +1,7 @@ /** * Load an image and return its natural dimensions. * Supports both vault resource paths (app://...) and external URLs (https://...). - * + * * Note: This works with Obsidian's resource paths returned by app.vault.getResourcePath() * which are special app:// protocol URLs handled by Obsidian's Electron environment. */ @@ -38,4 +38,3 @@ export const loadImage = ( img.src = url; }); }; - diff --git a/apps/obsidian/src/utils/markdownViewRefresh.ts b/apps/obsidian/src/utils/markdownViewRefresh.ts new file mode 100644 index 000000000..f237dd02b --- /dev/null +++ b/apps/obsidian/src/utils/markdownViewRefresh.ts @@ -0,0 +1,42 @@ +import { MarkdownView, type App } from "obsidian"; +import type { EditorView } from "@codemirror/view"; + +type EditorWithCm = { cm: EditorView }; + +export const hasCodeMirrorView = (editor: unknown): editor is EditorWithCm => { + if (!editor || typeof editor !== "object") return false; + return "cm" in editor; +}; + +/** + * Dispatches an empty CM6 transaction to every open markdown editor, which + * forces each ViewPlugin's update() to run and rebuild its decorations. + * + * Needed whenever something a ViewPlugin reads changes outside the editor — + * a setting, or which leaves are visible — since CM6 has no way to know. + */ +export const refreshMarkdownEditors = (app: App): void => { + app.workspace.iterateAllLeaves((leaf) => { + if ( + leaf.view instanceof MarkdownView && + hasCodeMirrorView(leaf.view.editor) + ) { + leaf.view.editor.cm.dispatch({}); + } + }); +}; + +/** + * Re-renders every open Reading view. + * + * Reading view has no equivalent of the CM6 no-op transaction: markdown post + * processors only run when content is rendered, so a setting that changes what + * they emit needs the already-rendered content thrown away and rebuilt. + */ +export const refreshMarkdownPreviews = (app: App): void => { + app.workspace.iterateAllLeaves((leaf) => { + if (leaf.view instanceof MarkdownView) { + leaf.view.previewMode?.rerender(true); + } + }); +}; From 41294717ef29d830abb1a5431a1462f89edb9b06 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 18:54:37 -0400 Subject: [PATCH 04/11] ENG-1249 Render the overlay in both markdown surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live Preview goes through a CM6 ViewPlugin, following the existing wikilink drag handle. Reading view needs a markdown post processor, which this plugin had no precedent for; because Obsidian reuses rendered sections and runs post processors over them again — as it also does for hover previews and exports — that path checks each link for an existing badge rather than assuming it runs once. Selecting a badge opens a popover built on RelationshipSection, the same component the Discourse Context panel renders, so the two cannot disagree about a node's relations. InfoTooltip moves out of DiscourseContextView so using it does not pull in the whole ItemView. Two fixes found while verifying against a real vault: - The badge counted relations the panel hides. Deleting a relation type leaves its relations behind in relations.json, and the panel drops those, so a node with 4 stored relations showed a badge reading 4 over a panel listing 2. Counting now mirrors what the panel will display. - Scrolling inside the popover dismissed it, because the scroll listener that follows the badge away could not tell outside scrolling from the popover's own — which made "Add a new relation" unreachable. Co-Authored-By: Claude Opus 5 --- .../components/DiscourseContextPopover.tsx | 153 +++++++++++++++ .../src/components/DiscourseContextView.tsx | 16 +- apps/obsidian/src/components/InfoTooltip.tsx | 16 ++ .../src/components/RelationshipSection.tsx | 2 +- .../src/components/discourseContextBadge.ts | 4 +- apps/obsidian/src/index.ts | 6 + .../discourseLinkFrontmatter.test.ts | 49 ++++- .../utils/discourseContextOverlayExtension.ts | 175 ++++++++++++++++++ .../discourseContextOverlayPostProcessor.ts | 54 ++++++ .../src/utils/discourseLinkFrontmatter.ts | 27 ++- apps/obsidian/src/utils/discourseLinkUtils.ts | 15 +- 11 files changed, 478 insertions(+), 39 deletions(-) create mode 100644 apps/obsidian/src/components/DiscourseContextPopover.tsx create mode 100644 apps/obsidian/src/components/InfoTooltip.tsx create mode 100644 apps/obsidian/src/utils/discourseContextOverlayExtension.ts create mode 100644 apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts diff --git a/apps/obsidian/src/components/DiscourseContextPopover.tsx b/apps/obsidian/src/components/DiscourseContextPopover.tsx new file mode 100644 index 000000000..52c6b5d3c --- /dev/null +++ b/apps/obsidian/src/components/DiscourseContextPopover.tsx @@ -0,0 +1,153 @@ +import { TFile } from "obsidian"; +import { createRoot, Root } from "react-dom/client"; +import type DiscourseGraphPlugin from "~/index"; +import { PluginProvider } from "~/components/PluginContext"; +import { RelationshipSection } from "~/components/RelationshipSection"; + +const POPOVER_CLASS = "dg-discourse-context-popover"; +const VIEWPORT_MARGIN = 8; + +/** + * Positions the popover under its badge, pulling it back inside the window when + * it would overflow. Measured after mount because the content height depends on + * how many relations the node has. + */ +const positionPopover = (popover: HTMLElement, anchor: HTMLElement): void => { + const anchorRect = anchor.getBoundingClientRect(); + const { width, height } = popover.getBoundingClientRect(); + + const left = Math.min( + Math.max(VIEWPORT_MARGIN, anchorRect.left), + Math.max(VIEWPORT_MARGIN, window.innerWidth - width - VIEWPORT_MARGIN), + ); + + const spaceBelow = window.innerHeight - anchorRect.bottom; + const openUpward = + spaceBelow < height + VIEWPORT_MARGIN && anchorRect.top > height; + const top = openUpward + ? Math.max(VIEWPORT_MARGIN, anchorRect.top - height - 4) + : anchorRect.bottom + 4; + + popover.style.left = `${left}px`; + popover.style.top = `${top}px`; +}; + +/** + * The discourse context shown when a badge is selected. + * + * Reuses RelationshipSection, the same component the Discourse Context panel + * renders, so the two can never disagree about a node's relations. It needs + * only a TFile and PluginProvider — no workspace leaf — which is what makes it + * reusable here. + * + * Only one popover exists at a time; opening another closes the previous one. + */ +class DiscourseContextPopover { + private containerEl: HTMLElement; + private root: Root; + private cleanupListeners: (() => void)[] = []; + + constructor( + private plugin: DiscourseGraphPlugin, + file: TFile, + anchor: HTMLElement, + ) { + this.containerEl = activeDocument.body.createDiv({ cls: POPOVER_CLASS }); + this.containerEl.addClass( + "fixed", + "z-50", + "max-h-[60vh]", + "w-80", + "overflow-y-auto", + "rounded-md", + "border", + "border-solid", + "border-[var(--background-modifier-border)]", + "bg-[var(--background-primary)]", + "p-3", + "shadow-lg", + ); + + const header = this.containerEl.createDiv({ + cls: "mb-2 text-xs font-medium text-[var(--text-muted)]", + }); + header.setText(file.basename); + + const reactHost = this.containerEl.createDiv(); + this.root = createRoot(reactHost); + this.root.render( + + + , + ); + + positionPopover(this.containerEl, anchor); + this.registerDismissListeners(); + } + + private registerDismissListeners(): void { + const closeIfOutside = (event: MouseEvent): void => { + if (this.containerEl.contains(event.target as Node)) return; + this.close(); + }; + const closeOnEscape = (event: KeyboardEvent): void => { + if (event.key !== "Escape") return; + event.preventDefault(); + this.close(); + }; + // Scrolling the note moves the badge out from under the popover, so the + // popover follows it away. Scrolling *within* the popover must not dismiss + // it — its own content scrolls, and reaching "Add a new relation" requires + // exactly that. + const closeOnScroll = (event: Event): void => { + if (this.containerEl.contains(event.target as Node)) return; + this.close(); + }; + + // Deferred so the click that opened the popover does not immediately + // dismiss it as an outside click. + const attach = window.setTimeout(() => { + activeDocument.addEventListener("click", closeIfOutside, true); + }, 0); + + activeDocument.addEventListener("keydown", closeOnEscape); + // Capture phase, since scrolling happens inside panes rather than on window. + activeDocument.addEventListener("scroll", closeOnScroll, true); + + this.cleanupListeners.push(() => { + window.clearTimeout(attach); + activeDocument.removeEventListener("click", closeIfOutside, true); + activeDocument.removeEventListener("keydown", closeOnEscape); + activeDocument.removeEventListener("scroll", closeOnScroll, true); + }); + } + + close(): void { + for (const cleanup of this.cleanupListeners) cleanup(); + this.cleanupListeners = []; + // Unmounting during React's own event handling warns, so defer it. + const root = this.root; + window.setTimeout(() => root.unmount(), 0); + this.containerEl.remove(); + if (activePopover === this) activePopover = null; + } +} + +let activePopover: DiscourseContextPopover | null = null; + +export const openDiscourseContextPopover = ({ + plugin, + file, + anchor, +}: { + plugin: DiscourseGraphPlugin; + file: TFile; + anchor: HTMLElement; +}): void => { + activePopover?.close(); + activePopover = new DiscourseContextPopover(plugin, file, anchor); +}; + +export const closeDiscourseContextPopover = (): void => { + activePopover?.close(); +}; diff --git a/apps/obsidian/src/components/DiscourseContextView.tsx b/apps/obsidian/src/components/DiscourseContextView.tsx index c9e7c715b..c52933554 100644 --- a/apps/obsidian/src/components/DiscourseContextView.tsx +++ b/apps/obsidian/src/components/DiscourseContextView.tsx @@ -10,6 +10,7 @@ import { createRoot, Root } from "react-dom/client"; import DiscourseGraphPlugin from "~/index"; import { getDiscourseNodeFormatExpression } from "~/utils/getDiscourseNodeFormatExpression"; import { RelationshipSection } from "~/components/RelationshipSection"; +import { InfoTooltip } from "~/components/InfoTooltip"; import { VIEW_TYPE_DISCOURSE_CONTEXT } from "~/types"; import { PluginProvider, usePlugin } from "~/components/PluginContext"; import { @@ -26,21 +27,6 @@ type DiscourseContextProps = { activeFile: TFile | null; }; -type InfoTooltipProps = { - content: string; -}; - -export const InfoTooltip = ({ content }: InfoTooltipProps) => ( - -); - const DiscourseContext = ({ activeFile }: DiscourseContextProps) => { const plugin = usePlugin(); const [isRefreshing, setIsRefreshing] = useState(false); diff --git a/apps/obsidian/src/components/InfoTooltip.tsx b/apps/obsidian/src/components/InfoTooltip.tsx new file mode 100644 index 000000000..2c422ba4b --- /dev/null +++ b/apps/obsidian/src/components/InfoTooltip.tsx @@ -0,0 +1,16 @@ +import { setIcon, setTooltip } from "obsidian"; + +type InfoTooltipProps = { + content: string; +}; + +export const InfoTooltip = ({ content }: InfoTooltipProps) => ( + +); diff --git a/apps/obsidian/src/components/RelationshipSection.tsx b/apps/obsidian/src/components/RelationshipSection.tsx index 04e9f69c4..6b7ace8bf 100644 --- a/apps/obsidian/src/components/RelationshipSection.tsx +++ b/apps/obsidian/src/components/RelationshipSection.tsx @@ -26,7 +26,7 @@ import { removeRelationBySourceDestinationType, updateRelation, } from "~/utils/relationsStore"; -import { InfoTooltip } from "./DiscourseContextView"; +import { InfoTooltip } from "./InfoTooltip"; type RelationTypeOption = { id: string; diff --git a/apps/obsidian/src/components/discourseContextBadge.ts b/apps/obsidian/src/components/discourseContextBadge.ts index 5ef484cf7..cf0fa76fd 100644 --- a/apps/obsidian/src/components/discourseContextBadge.ts +++ b/apps/obsidian/src/components/discourseContextBadge.ts @@ -11,7 +11,7 @@ export type DiscourseContextBadgeProps = { file: TFile; nodeType: DiscourseNode; relationCount: number; - onActivate: (file: TFile) => void; + onActivate: (args: { file: TFile; anchor: HTMLElement }) => void; }; const badgeTooltip = ({ @@ -56,7 +56,7 @@ export const createDiscourseContextBadge = ({ // Stops Obsidian from following the link the badge sits next to. event.preventDefault(); event.stopPropagation(); - onActivate(file); + onActivate({ file, anchor: badge }); }; badge.addEventListener("click", activate); diff --git a/apps/obsidian/src/index.ts b/apps/obsidian/src/index.ts index 03203864b..9958a7793 100644 --- a/apps/obsidian/src/index.ts +++ b/apps/obsidian/src/index.ts @@ -20,6 +20,8 @@ import { } from "~/utils/editorMenuUtils"; import { createImageEmbedHoverExtension } from "~/utils/imageEmbedHoverIcon"; import { createWikilinkDragExtension } from "~/utils/wikilinkDragHandler"; +import { createDiscourseContextOverlayExtension } from "~/utils/discourseContextOverlayExtension"; +import { createDiscourseContextOverlayPostProcessor } from "~/utils/discourseContextOverlayPostProcessor"; import { registerCommands, createModifyNodeModalSubmitHandler, @@ -105,6 +107,9 @@ export default class DiscourseGraphPlugin extends Plugin { } this.relationsIndex.initialize(); + this.registerMarkdownPostProcessor( + createDiscourseContextOverlayPostProcessor(this), + ); registerCommands(this); this.addSettingTab(new SettingsTab(this.app, this)); @@ -369,6 +374,7 @@ export default class DiscourseGraphPlugin extends Plugin { this.registerEditorExtension(createImageEmbedHoverExtension(this)); this.registerEditorExtension(createWikilinkDragExtension(this)); + this.registerEditorExtension(createDiscourseContextOverlayExtension(this)); } updateFrontmatterStyles(): void { diff --git a/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts b/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts index 3217ba0fa..9d5ae2687 100644 --- a/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts +++ b/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import type { RelationInstance } from "~/types"; import { - countAcceptedRelations, + countDisplayableRelations, getEndpointIdsFromFrontmatter, getNodeTypeIdFromFrontmatter, } from "~/utils/discourseLinkFrontmatter"; @@ -67,29 +67,60 @@ describe("getEndpointIdsFromFrontmatter", () => { }); }); -describe("countAcceptedRelations", () => { +describe("countDisplayableRelations", () => { + const allConfigured = () => true; + it("counts local relations, which leave tentative undefined", () => { expect( - countAcceptedRelations([relation({ id: "r1" }), relation({ id: "r2" })]), + countDisplayableRelations({ + relations: [relation({ id: "r1" }), relation({ id: "r2" })], + isConfiguredType: allConfigured, + }), ).toBe(2); }); it("counts explicitly accepted relations", () => { expect( - countAcceptedRelations([relation({ id: "r1", tentative: true })]), + countDisplayableRelations({ + relations: [relation({ id: "r1", tentative: true })], + isConfiguredType: allConfigured, + }), ).toBe(1); }); it("excludes imported relations awaiting acceptance", () => { expect( - countAcceptedRelations([ - relation({ id: "r1" }), - relation({ id: "r2", tentative: false }), - ]), + countDisplayableRelations({ + relations: [ + relation({ id: "r1" }), + relation({ id: "r2", tentative: false }), + ], + isConfiguredType: allConfigured, + }), + ).toBe(1); + }); + + it("excludes relations orphaned by a deleted relation type", () => { + // Deleting a relation type leaves its relations in relations.json. The + // panel drops them, so a badge that counted them would promise context the + // panel then refuses to show. + expect( + countDisplayableRelations({ + relations: [ + relation({ id: "r1", type: "supports" }), + relation({ id: "r2", type: "deleted-type" }), + ], + isConfiguredType: (type) => type === "supports", + }), ).toBe(1); }); it("returns zero for no relations", () => { - expect(countAcceptedRelations([])).toBe(0); + expect( + countDisplayableRelations({ + relations: [], + isConfiguredType: allConfigured, + }), + ).toBe(0); }); }); diff --git a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts new file mode 100644 index 000000000..9ac712a62 --- /dev/null +++ b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts @@ -0,0 +1,175 @@ +import { + type PluginValue, + ViewPlugin, + type ViewUpdate, + WidgetType, + Decoration, + type DecorationSet, + EditorView, +} from "@codemirror/view"; +import { editorInfoField, editorLivePreviewField } from "obsidian"; +import type DiscourseGraphPlugin from "~/index"; +import { createDiscourseContextBadge } from "~/components/discourseContextBadge"; +import { openDiscourseContextPopover } from "~/components/DiscourseContextPopover"; +import { + resolveDiscourseLinkTarget, + type DiscourseLinkTarget, +} from "./discourseLinkUtils"; + +// Wikilinks [[...]] and markdown links [text](path.md). Embeds are excluded in +// the loop below, since a leading "!" sits outside the match. +const INTERNAL_LINK_RE = /\[\[([^\]]+)\]\]|\[([^\]]+)\]\(([^)]+\.md)\)/g; + +/** Extracts the link target from a wikilink or markdown link match. */ +const extractLinktext = (match: string): string => { + if (match.startsWith("[[")) { + const inner = match.slice(2, -2); + const pipeIndex = inner.indexOf("|"); + return pipeIndex >= 0 ? inner.slice(0, pipeIndex) : inner; + } + + const parenOpen = match.lastIndexOf("("); + const rawPath = match.slice(parenOpen + 1, -1); + try { + return decodeURIComponent(rawPath); + } catch { + return rawPath; + } +}; + +class DiscourseContextBadgeWidget extends WidgetType { + constructor( + private target: DiscourseLinkTarget, + private plugin: DiscourseGraphPlugin, + ) { + super(); + } + + /** + * Keyed on path and count so a badge is only rebuilt when what it displays + * changes — not on every keystroke elsewhere in the document. + */ + eq(other: DiscourseContextBadgeWidget): boolean { + return ( + this.target.file.path === other.target.file.path && + this.target.relationCount === other.target.relationCount + ); + } + + toDOM(): HTMLElement { + return createDiscourseContextBadge({ + file: this.target.file, + nodeType: this.target.nodeType, + relationCount: this.target.relationCount, + onActivate: ({ file, anchor }) => + openDiscourseContextPopover({ plugin: this.plugin, file, anchor }), + }); + } + + /** + * Left at the CM6 default of true: the editor ignores events on the widget, + * so the badge's own click listener fires natively. Returning false hands the + * event to CM's input handling instead and the badge never reacts. + */ + ignoreEvent(): boolean { + return true; + } +} + +const buildBadgeDecorations = ( + view: EditorView, + plugin: DiscourseGraphPlugin, +): DecorationSet => { + if (!plugin.settings.showDiscourseContextOverlay) return Decoration.none; + // Source mode shows raw markdown; a badge there would be noise. + if (!view.state.field(editorLivePreviewField, false)) return Decoration.none; + + const sourcePath = view.state.field(editorInfoField, false)?.file?.path; + if (!sourcePath) return Decoration.none; + + const widgets = []; + + for (const { from, to } of view.visibleRanges) { + const text = view.state.doc.sliceString(from, to); + let match: RegExpExecArray | null; + INTERNAL_LINK_RE.lastIndex = 0; + + while ((match = INTERNAL_LINK_RE.exec(text)) !== null) { + const checkPos = from + match.index - 1; + const isEmbed = + checkPos >= 0 && + view.state.doc.sliceString(checkPos, checkPos + 1) === "!"; + if (isEmbed) continue; + + const target = resolveDiscourseLinkTarget({ + plugin, + linktext: extractLinktext(match[0]), + sourcePath, + }); + if (!target || target.relationCount === 0) continue; + + const matchEnd = from + match.index + match[0].length; + widgets.push( + Decoration.widget({ + widget: new DiscourseContextBadgeWidget(target, plugin), + side: 1, + }).range(matchEnd), + ); + } + } + + widgets.sort((a, b) => a.from - b.from); + return Decoration.set(widgets); +}; + +/** + * Renders the discourse context badge after each link to a discourse node in + * Live Preview. + * + * Rebuilds on relation changes as well as document and viewport changes, since + * the badge shows a count that lives outside the document — adding a relation + * from the badge's own popover has to update the number behind it. + */ +export const createDiscourseContextOverlayExtension = ( + plugin: DiscourseGraphPlugin, +): ViewPlugin => + ViewPlugin.fromClass( + class { + decorations: DecorationSet; + private enabled: boolean; + private unsubscribe: () => void; + + constructor(view: EditorView) { + this.enabled = plugin.settings.showDiscourseContextOverlay; + this.decorations = buildBadgeDecorations(view, plugin); + this.unsubscribe = plugin.relationsIndex.onChange(() => { + this.decorations = buildBadgeDecorations(view, plugin); + // The index resolves outside any transaction, so ask for a redraw. + view.dispatch({}); + }); + } + + update(update: ViewUpdate): void { + // The setting is toggled by dispatching an empty transaction, which + // changes neither the document nor the viewport, so it has to be + // compared explicitly or the toggle would appear to do nothing. + const enabled = plugin.settings.showDiscourseContextOverlay; + if ( + !update.docChanged && + !update.viewportChanged && + enabled === this.enabled + ) { + return; + } + this.enabled = enabled; + this.decorations = buildBadgeDecorations(update.view, plugin); + } + + destroy(): void { + this.unsubscribe(); + } + }, + { + decorations: (v) => v.decorations, + }, + ); diff --git a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts new file mode 100644 index 000000000..d09b6303b --- /dev/null +++ b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts @@ -0,0 +1,54 @@ +import type { MarkdownPostProcessorContext } from "obsidian"; +import type DiscourseGraphPlugin from "~/index"; +import { + createDiscourseContextBadge, + DISCOURSE_CONTEXT_BADGE_CLASS, +} from "~/components/discourseContextBadge"; +import { openDiscourseContextPopover } from "~/components/DiscourseContextPopover"; +import { resolveDiscourseLinkTarget } from "./discourseLinkUtils"; + +/** + * Reading view's counterpart to the Live Preview extension. + * + * Obsidian runs post processors over rendered sections and reuses those + * sections, so this must be safe to run repeatedly over content that already + * has badges — hence the marker-class check per link rather than a one-shot + * pass. The same guard covers hover previews and exports, which render through + * this path too. + */ +export const createDiscourseContextOverlayPostProcessor = + (plugin: DiscourseGraphPlugin) => + (el: HTMLElement, ctx: MarkdownPostProcessorContext): void => { + if (!plugin.settings.showDiscourseContextOverlay) return; + if (!ctx.sourcePath) return; + + const links = el.querySelectorAll("a.internal-link"); + + for (const link of Array.from(links)) { + if (link.nextElementSibling?.hasClass(DISCOURSE_CONTEXT_BADGE_CLASS)) { + continue; + } + + // data-href holds the link as written; href is resolved and URL-encoded. + const linktext = + link.getAttribute("data-href") ?? link.getAttribute("href"); + if (!linktext) continue; + + const target = resolveDiscourseLinkTarget({ + plugin, + linktext, + sourcePath: ctx.sourcePath, + }); + if (!target || target.relationCount === 0) continue; + + const badge = createDiscourseContextBadge({ + file: target.file, + nodeType: target.nodeType, + relationCount: target.relationCount, + onActivate: ({ file, anchor }) => + openDiscourseContextPopover({ plugin, file, anchor }), + }); + + link.insertAdjacentElement("afterend", badge); + } + }; diff --git a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts index 1a63e861b..bda3bc785 100644 --- a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts +++ b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts @@ -35,12 +35,25 @@ export const getEndpointIdsFromFrontmatter = ( }; /** - * Counts relations that are actually part of the graph. + * Counts the relations the Discourse Context panel would actually list. * - * `tentative === false` marks an imported relation the user has not accepted - * yet; the Discourse Context panel lists those separately from accepted ones, - * so counting them in the badge would show a number the panel never repeats - * back. Local relations leave `tentative` undefined. + * Two kinds are excluded, and both have to be, or the badge advertises context + * the panel then refuses to show: + * + * - `tentative === false` marks an imported relation the user has not accepted + * yet, which the panel lists separately. Local relations leave it undefined. + * - A relation whose type is no longer configured is orphaned — deleting a + * relation type leaves its relations behind in relations.json — and the panel + * silently drops those. */ -export const countAcceptedRelations = (relations: RelationInstance[]): number => - relations.filter((relation) => relation.tentative !== false).length; +export const countDisplayableRelations = ({ + relations, + isConfiguredType, +}: { + relations: RelationInstance[]; + isConfiguredType: (relationTypeId: string) => boolean; +}): number => + relations.filter( + (relation) => + relation.tentative !== false && isConfiguredType(relation.type), + ).length; diff --git a/apps/obsidian/src/utils/discourseLinkUtils.ts b/apps/obsidian/src/utils/discourseLinkUtils.ts index e3a8a0ab5..24d176ce7 100644 --- a/apps/obsidian/src/utils/discourseLinkUtils.ts +++ b/apps/obsidian/src/utils/discourseLinkUtils.ts @@ -1,9 +1,9 @@ import { parseLinktext, TFile } from "obsidian"; import type DiscourseGraphPlugin from "~/index"; import type { DiscourseNode } from "~/types"; -import { getNodeTypeById } from "./typeUtils"; +import { getNodeTypeById, getRelationTypeById } from "./typeUtils"; import { - countAcceptedRelations, + countDisplayableRelations, getEndpointIdsFromFrontmatter, getNodeTypeIdFromFrontmatter, } from "./discourseLinkFrontmatter"; @@ -47,8 +47,7 @@ export const resolveDiscourseLinkTarget = ({ const file = plugin.app.metadataCache.getFirstLinkpathDest(path, sourcePath); if (!file) return null; - const frontmatter = plugin.app.metadataCache.getFileCache(file) - ?.frontmatter as Record | undefined; + const frontmatter = plugin.app.metadataCache.getFileCache(file)?.frontmatter; const nodeTypeId = getNodeTypeIdFromFrontmatter(frontmatter); if (!nodeTypeId) return null; @@ -62,5 +61,11 @@ export const resolveDiscourseLinkTarget = ({ const relations = plugin.relationsIndex.getRelationsForEndpointIds(endpointIds); - return { file, nodeType, relationCount: countAcceptedRelations(relations) }; + const relationCount = countDisplayableRelations({ + relations, + isConfiguredType: (relationTypeId) => + !!getRelationTypeById(plugin, relationTypeId), + }); + + return { file, nodeType, relationCount }; }; From c0cef1c62e8b296a5d9571c9a7fe77e816507f87 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 19:01:48 -0400 Subject: [PATCH 05/11] ENG-1249 Badge every discourse node, empty or not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches Roam: a node with no relations still gets a badge, reading 0, whose popover says "No discourse relation found" above the option to add one. CurrentRelationships renders nothing at all in that case, so without the message the popover would open on an unexplained button. Also drops the node title from the top of the popover — the badge sits directly after the link whose title it was repeating. Co-Authored-By: Claude Opus 5 --- .../components/DiscourseContextPopover.tsx | 41 ++++++++++--------- .../utils/discourseContextOverlayExtension.ts | 9 +++- .../discourseContextOverlayPostProcessor.ts | 9 +++- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/apps/obsidian/src/components/DiscourseContextPopover.tsx b/apps/obsidian/src/components/DiscourseContextPopover.tsx index 52c6b5d3c..27988b63b 100644 --- a/apps/obsidian/src/components/DiscourseContextPopover.tsx +++ b/apps/obsidian/src/components/DiscourseContextPopover.tsx @@ -6,6 +6,7 @@ import { RelationshipSection } from "~/components/RelationshipSection"; const POPOVER_CLASS = "dg-discourse-context-popover"; const VIEWPORT_MARGIN = 8; +const EMPTY_MESSAGE = "No discourse relation found"; /** * Positions the popover under its badge, pulling it back inside the window when @@ -32,6 +33,13 @@ const positionPopover = (popover: HTMLElement, anchor: HTMLElement): void => { popover.style.top = `${top}px`; }; +type PopoverOptions = { + plugin: DiscourseGraphPlugin; + file: TFile; + anchor: HTMLElement; + relationCount: number; +}; + /** * The discourse context shown when a badge is selected. * @@ -45,13 +53,11 @@ const positionPopover = (popover: HTMLElement, anchor: HTMLElement): void => { class DiscourseContextPopover { private containerEl: HTMLElement; private root: Root; + private plugin: DiscourseGraphPlugin; private cleanupListeners: (() => void)[] = []; - constructor( - private plugin: DiscourseGraphPlugin, - file: TFile, - anchor: HTMLElement, - ) { + constructor({ plugin, file, anchor, relationCount }: PopoverOptions) { + this.plugin = plugin; this.containerEl = activeDocument.body.createDiv({ cls: POPOVER_CLASS }); this.containerEl.addClass( "fixed", @@ -68,10 +74,15 @@ class DiscourseContextPopover { "shadow-lg", ); - const header = this.containerEl.createDiv({ - cls: "mb-2 text-xs font-medium text-[var(--text-muted)]", - }); - header.setText(file.basename); + // CurrentRelationships renders nothing at all when a node has none, so + // without this the popover would open on an unexplained "Add a new + // relation" button. Created before the React host so it reads above it. + if (relationCount === 0) { + this.containerEl.createDiv({ + cls: "mb-2 text-sm text-[var(--text-muted)]", + text: EMPTY_MESSAGE, + }); + } const reactHost = this.containerEl.createDiv(); this.root = createRoot(reactHost); @@ -135,17 +146,9 @@ class DiscourseContextPopover { let activePopover: DiscourseContextPopover | null = null; -export const openDiscourseContextPopover = ({ - plugin, - file, - anchor, -}: { - plugin: DiscourseGraphPlugin; - file: TFile; - anchor: HTMLElement; -}): void => { +export const openDiscourseContextPopover = (options: PopoverOptions): void => { activePopover?.close(); - activePopover = new DiscourseContextPopover(plugin, file, anchor); + activePopover = new DiscourseContextPopover(options); }; export const closeDiscourseContextPopover = (): void => { diff --git a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts index 9ac712a62..74bf3d180 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts @@ -62,7 +62,12 @@ class DiscourseContextBadgeWidget extends WidgetType { nodeType: this.target.nodeType, relationCount: this.target.relationCount, onActivate: ({ file, anchor }) => - openDiscourseContextPopover({ plugin: this.plugin, file, anchor }), + openDiscourseContextPopover({ + plugin: this.plugin, + file, + anchor, + relationCount: this.target.relationCount, + }), }); } @@ -106,7 +111,7 @@ const buildBadgeDecorations = ( linktext: extractLinktext(match[0]), sourcePath, }); - if (!target || target.relationCount === 0) continue; + if (!target) continue; const matchEnd = from + match.index + match[0].length; widgets.push( diff --git a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts index d09b6303b..79e8f06ce 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts @@ -39,14 +39,19 @@ export const createDiscourseContextOverlayPostProcessor = linktext, sourcePath: ctx.sourcePath, }); - if (!target || target.relationCount === 0) continue; + if (!target) continue; const badge = createDiscourseContextBadge({ file: target.file, nodeType: target.nodeType, relationCount: target.relationCount, onActivate: ({ file, anchor }) => - openDiscourseContextPopover({ plugin, file, anchor }), + openDiscourseContextPopover({ + plugin, + file, + anchor, + relationCount: target.relationCount, + }), }); link.insertAdjacentElement("afterend", badge); From b3025b5caa2622d23596f9be2c8e131832135b8c Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 19:07:34 -0400 Subject: [PATCH 06/11] ENG-1249 Document the discourse context overlay Extends the existing discourse context page with the overlay as a fourth way in, and adds the setting to General settings, rather than adding a new page for a feature that is another entry point to something already documented. Co-Authored-By: Claude Opus 5 --- .../content/obsidian/configuration/general-settings.md | 10 ++++++++++ .../obsidian/core-features/discourse-context.md | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/apps/website/content/obsidian/configuration/general-settings.md b/apps/website/content/obsidian/configuration/general-settings.md index a7f2b13e7..1fcb6e2e2 100644 --- a/apps/website/content/obsidian/configuration/general-settings.md +++ b/apps/website/content/obsidian/configuration/general-settings.md @@ -15,6 +15,16 @@ This setting controls the visibility of identifiers in your note's frontmatter s - When disabled, these IDs will be hidden from view - This can be useful if you prefer a cleaner frontmatter appearance while still maintaining the underlying structure +## Show discourse context overlay + +This setting controls whether links to discourse nodes carry an inline badge showing how many relations the linked node has. + +- When enabled, a badge appears after each link to a discourse node, in both Live Preview and Reading view +- Selecting a badge opens that node's discourse context in a popover, where you can review its relationships and add a new one +- A node with no relations shows a badge reading `0`, and its popover says "No discourse relation found" +- Links to notes that are not discourse nodes never show a badge +- When disabled, the badges are removed immediately; the [discourse context view](/docs/obsidian/core-features/discourse-context) remains available from the sidebar + ## Discourse nodes folder path This setting determines where new discourse nodes will be created in your vault. diff --git a/apps/website/content/obsidian/core-features/discourse-context.md b/apps/website/content/obsidian/core-features/discourse-context.md index 6251122c2..665d1ba4c 100644 --- a/apps/website/content/obsidian/core-features/discourse-context.md +++ b/apps/website/content/obsidian/core-features/discourse-context.md @@ -26,6 +26,12 @@ You can configure a custom hotkey in the Obsidian settings to quickly toggle the 3. Configure a custom hotkey in settings +### Method 4: Using the discourse context overlay + +Links to a discourse node show a small badge with the number of relations that node has. Select the badge to open its discourse context in place, without leaving the note you are reading. + +The badge appears in both Live Preview and Reading view, on every link to a discourse node. A node with no relations yet shows a badge reading `0`, and opening it says "No discourse relation found" alongside the option to add one. You can turn the badge off in [General settings](/docs/obsidian/configuration/general-settings). + ## Using the discourse context The discourse context view shows you: @@ -41,3 +47,5 @@ You can use this view to: - Understand how nodes connect to each other - Add new relationships - Get a quick overview of your graph structure + +The overlay badge opens the same relationships in a popover, so it shows exactly what the sidebar view would show for that node. Relations that are still waiting to be accepted after an import are not counted in the badge; open the discourse context view to review those. From 7ac4be876128070c29f954bbbfd7f8e76d7b5c15 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sat, 5 Sep 2026 19:35:12 -0400 Subject: [PATCH 07/11] ENG-1249 Fix overlay refresh, lifecycle and positioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From a delegated review of the full diff. The substantive ones: Reading view badges never updated. Only the CM6 extension watched the relations index, and the post processor skipped links that already had a badge, so a Reading view badge kept its first number for the life of the view — including the 0 it showed when the index was still loading as the note first rendered. Both surfaces now redraw from one debounced handler, and the post processor replaces a stale badge instead of skipping it. Invalidating the index dropped the snapshot outright, so every badge read 0 until the reload landed — on the very action that triggered it, since saving a relation writes relations.json. The old snapshot is now kept until the new one is ready. ensureLoaded left inFlight set when a load was superseded mid-read, which made it hand out a settled promise forever: the snapshot stayed stale and every read re-requested a load that never ran. Reading from a render path also scheduled loads, so notify -> re-render -> read looped and left Reading view permanently blank. The popover outlived the plugin, keeping an Escape handler that swallowed the key for the rest of the session, and it measured itself before React had committed, so it never flipped up near the bottom of the window. It now closes on unload and derives geometry from the window its anchor is in, which also fixes popout windows. Also: badge no longer moves the caret on mousedown, widget eq() accounts for the node type, link parsing moves to a pure module so it can be tested, and unused accessors are gone. Co-Authored-By: Claude Opus 5 --- .../components/DiscourseContextPopover.tsx | 43 ++++++++--- .../src/components/discourseContextBadge.ts | 5 ++ apps/obsidian/src/index.ts | 11 ++- .../utils/__tests__/extractLinktext.test.ts | 36 +++++++++ .../utils/discourseContextOverlayExtension.ts | 49 +++--------- .../discourseContextOverlayPostProcessor.ts | 74 +++++++++++++++++-- .../obsidian/src/utils/internalLinkParsing.ts | 34 +++++++++ apps/obsidian/src/utils/relationsIndex.ts | 64 ++++++++++------ 8 files changed, 236 insertions(+), 80 deletions(-) create mode 100644 apps/obsidian/src/utils/__tests__/extractLinktext.test.ts create mode 100644 apps/obsidian/src/utils/internalLinkParsing.ts diff --git a/apps/obsidian/src/components/DiscourseContextPopover.tsx b/apps/obsidian/src/components/DiscourseContextPopover.tsx index 27988b63b..2006d38c6 100644 --- a/apps/obsidian/src/components/DiscourseContextPopover.tsx +++ b/apps/obsidian/src/components/DiscourseContextPopover.tsx @@ -14,15 +14,18 @@ const EMPTY_MESSAGE = "No discourse relation found"; * how many relations the node has. */ const positionPopover = (popover: HTMLElement, anchor: HTMLElement): void => { + // Geometry has to come from the window the anchor is in, not the main one, or + // a popover opened in a popout window gets clamped to the wrong viewport. + const win = anchor.ownerDocument.defaultView ?? window; const anchorRect = anchor.getBoundingClientRect(); const { width, height } = popover.getBoundingClientRect(); const left = Math.min( Math.max(VIEWPORT_MARGIN, anchorRect.left), - Math.max(VIEWPORT_MARGIN, window.innerWidth - width - VIEWPORT_MARGIN), + Math.max(VIEWPORT_MARGIN, win.innerWidth - width - VIEWPORT_MARGIN), ); - const spaceBelow = window.innerHeight - anchorRect.bottom; + const spaceBelow = win.innerHeight - anchorRect.bottom; const openUpward = spaceBelow < height + VIEWPORT_MARGIN && anchorRect.top > height; const top = openUpward @@ -54,11 +57,16 @@ class DiscourseContextPopover { private containerEl: HTMLElement; private root: Root; private plugin: DiscourseGraphPlugin; + private win: Window; + private reposition: () => void = () => {}; + private resizeObserver: ResizeObserver | null = null; private cleanupListeners: (() => void)[] = []; constructor({ plugin, file, anchor, relationCount }: PopoverOptions) { this.plugin = plugin; - this.containerEl = activeDocument.body.createDiv({ cls: POPOVER_CLASS }); + const doc = anchor.ownerDocument; + this.win = doc.defaultView ?? window; + this.containerEl = doc.body.createDiv({ cls: POPOVER_CLASS }); this.containerEl.addClass( "fixed", "z-50", @@ -92,11 +100,20 @@ class DiscourseContextPopover { , ); + // A React 18 root does not commit synchronously, so measuring now would + // size an empty box and the flip-up-when-near-the-bottom check would never + // fire. Re-measured after paint, and again as the relation list fills in. positionPopover(this.containerEl, anchor); + this.reposition = () => positionPopover(this.containerEl, anchor); + this.win.requestAnimationFrame(this.reposition); + this.resizeObserver = new ResizeObserver(this.reposition); + this.resizeObserver.observe(this.containerEl); + this.registerDismissListeners(); } private registerDismissListeners(): void { + const doc = this.containerEl.ownerDocument; const closeIfOutside = (event: MouseEvent): void => { if (this.containerEl.contains(event.target as Node)) return; this.close(); @@ -117,28 +134,30 @@ class DiscourseContextPopover { // Deferred so the click that opened the popover does not immediately // dismiss it as an outside click. - const attach = window.setTimeout(() => { - activeDocument.addEventListener("click", closeIfOutside, true); + const attach = this.win.setTimeout(() => { + doc.addEventListener("click", closeIfOutside, true); }, 0); - activeDocument.addEventListener("keydown", closeOnEscape); + doc.addEventListener("keydown", closeOnEscape); // Capture phase, since scrolling happens inside panes rather than on window. - activeDocument.addEventListener("scroll", closeOnScroll, true); + doc.addEventListener("scroll", closeOnScroll, true); this.cleanupListeners.push(() => { - window.clearTimeout(attach); - activeDocument.removeEventListener("click", closeIfOutside, true); - activeDocument.removeEventListener("keydown", closeOnEscape); - activeDocument.removeEventListener("scroll", closeOnScroll, true); + this.win.clearTimeout(attach); + doc.removeEventListener("click", closeIfOutside, true); + doc.removeEventListener("keydown", closeOnEscape); + doc.removeEventListener("scroll", closeOnScroll, true); }); } close(): void { for (const cleanup of this.cleanupListeners) cleanup(); this.cleanupListeners = []; + this.resizeObserver?.disconnect(); + this.resizeObserver = null; // Unmounting during React's own event handling warns, so defer it. const root = this.root; - window.setTimeout(() => root.unmount(), 0); + this.win.setTimeout(() => root.unmount(), 0); this.containerEl.remove(); if (activePopover === this) activePopover = null; } diff --git a/apps/obsidian/src/components/discourseContextBadge.ts b/apps/obsidian/src/components/discourseContextBadge.ts index cf0fa76fd..4c54bd847 100644 --- a/apps/obsidian/src/components/discourseContextBadge.ts +++ b/apps/obsidian/src/components/discourseContextBadge.ts @@ -59,6 +59,11 @@ export const createDiscourseContextBadge = ({ onActivate({ file, anchor: badge }); }; + // Without this the mousedown still lands in the editor and moves the caret, + // which in Live Preview expands the raw [[...]] markup under the popover. + badge.addEventListener("mousedown", (event: MouseEvent) => { + event.preventDefault(); + }); badge.addEventListener("click", activate); badge.addEventListener("keydown", (event: KeyboardEvent) => { if (event.key !== "Enter" && event.key !== " ") return; diff --git a/apps/obsidian/src/index.ts b/apps/obsidian/src/index.ts index 9958a7793..ffe572452 100644 --- a/apps/obsidian/src/index.ts +++ b/apps/obsidian/src/index.ts @@ -21,7 +21,11 @@ import { import { createImageEmbedHoverExtension } from "~/utils/imageEmbedHoverIcon"; import { createWikilinkDragExtension } from "~/utils/wikilinkDragHandler"; import { createDiscourseContextOverlayExtension } from "~/utils/discourseContextOverlayExtension"; -import { createDiscourseContextOverlayPostProcessor } from "~/utils/discourseContextOverlayPostProcessor"; +import { + createDiscourseContextOverlayPostProcessor, + registerDiscourseContextOverlayRefresh, +} from "~/utils/discourseContextOverlayPostProcessor"; +import { closeDiscourseContextPopover } from "~/components/DiscourseContextPopover"; import { registerCommands, createModifyNodeModalSubmitHandler, @@ -110,6 +114,7 @@ export default class DiscourseGraphPlugin extends Plugin { this.registerMarkdownPostProcessor( createDiscourseContextOverlayPostProcessor(this), ); + registerDiscourseContextOverlayRefresh(this); registerCommands(this); this.addSettingTab(new SettingsTab(this.app, this)); @@ -495,6 +500,10 @@ export default class DiscourseGraphPlugin extends Plugin { this.fileChangeListener = null; } + // The popover lives on document.body with its own listeners, so it would + // otherwise outlive the plugin — including an Escape handler that would go + // on swallowing the key for the rest of the session. + closeDiscourseContextPopover(); this.relationsIndex.unload(); } } diff --git a/apps/obsidian/src/utils/__tests__/extractLinktext.test.ts b/apps/obsidian/src/utils/__tests__/extractLinktext.test.ts new file mode 100644 index 000000000..49e7ab01a --- /dev/null +++ b/apps/obsidian/src/utils/__tests__/extractLinktext.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; + +import { extractLinktext } from "~/utils/internalLinkParsing"; + +describe("extractLinktext", () => { + it("reads a plain wikilink", () => { + expect(extractLinktext("[[Claim]]")).toBe("Claim"); + }); + + it("drops a wikilink alias", () => { + expect(extractLinktext("[[Claim|the claim]]")).toBe("Claim"); + }); + + it("keeps a wikilink subpath for the caller to strip", () => { + expect(extractLinktext("[[Claim#Evidence]]")).toBe("Claim#Evidence"); + }); + + it("reads a markdown link target", () => { + expect(extractLinktext("[the claim](Claim.md)")).toBe("Claim.md"); + }); + + it("decodes a percent-encoded markdown link", () => { + expect(extractLinktext("[a claim](My%20Claim.md)")).toBe("My Claim.md"); + }); + + it("falls back to the raw path when decoding fails", () => { + // A lone % is not valid percent-encoding; decodeURIComponent throws. + expect(extractLinktext("[bad](100%.md)")).toBe("100%.md"); + }); + + it("reads a markdown link inside a folder", () => { + expect(extractLinktext("[c](Discourse Nodes/Claim.md)")).toBe( + "Discourse Nodes/Claim.md", + ); + }); +}); diff --git a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts index 74bf3d180..ba888ff1e 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts @@ -15,27 +15,7 @@ import { resolveDiscourseLinkTarget, type DiscourseLinkTarget, } from "./discourseLinkUtils"; - -// Wikilinks [[...]] and markdown links [text](path.md). Embeds are excluded in -// the loop below, since a leading "!" sits outside the match. -const INTERNAL_LINK_RE = /\[\[([^\]]+)\]\]|\[([^\]]+)\]\(([^)]+\.md)\)/g; - -/** Extracts the link target from a wikilink or markdown link match. */ -const extractLinktext = (match: string): string => { - if (match.startsWith("[[")) { - const inner = match.slice(2, -2); - const pipeIndex = inner.indexOf("|"); - return pipeIndex >= 0 ? inner.slice(0, pipeIndex) : inner; - } - - const parenOpen = match.lastIndexOf("("); - const rawPath = match.slice(parenOpen + 1, -1); - try { - return decodeURIComponent(rawPath); - } catch { - return rawPath; - } -}; +import { extractLinktext, INTERNAL_LINK_RE } from "./internalLinkParsing"; class DiscourseContextBadgeWidget extends WidgetType { constructor( @@ -46,13 +26,15 @@ class DiscourseContextBadgeWidget extends WidgetType { } /** - * Keyed on path and count so a badge is only rebuilt when what it displays - * changes — not on every keystroke elsewhere in the document. + * Keyed on everything the badge displays, so it is rebuilt when its content + * changes and left alone on every other keystroke in the document. */ eq(other: DiscourseContextBadgeWidget): boolean { return ( this.target.file.path === other.target.file.path && - this.target.relationCount === other.target.relationCount + this.target.relationCount === other.target.relationCount && + this.target.nodeType.id === other.target.nodeType.id && + this.target.nodeType.name === other.target.nodeType.name ); } @@ -123,17 +105,16 @@ const buildBadgeDecorations = ( } } - widgets.sort((a, b) => a.from - b.from); - return Decoration.set(widgets); + return Decoration.set(widgets, true); }; /** * Renders the discourse context badge after each link to a discourse node in * Live Preview. * - * Rebuilds on relation changes as well as document and viewport changes, since - * the badge shows a count that lives outside the document — adding a relation - * from the badge's own popover has to update the number behind it. + * Rebuilds on document and viewport changes. Changes that originate outside the + * document — a relation added, a target's frontmatter finishing indexing — + * arrive as an empty transaction from registerDiscourseContextOverlayRefresh. */ export const createDiscourseContextOverlayExtension = ( plugin: DiscourseGraphPlugin, @@ -142,16 +123,10 @@ export const createDiscourseContextOverlayExtension = ( class { decorations: DecorationSet; private enabled: boolean; - private unsubscribe: () => void; constructor(view: EditorView) { this.enabled = plugin.settings.showDiscourseContextOverlay; this.decorations = buildBadgeDecorations(view, plugin); - this.unsubscribe = plugin.relationsIndex.onChange(() => { - this.decorations = buildBadgeDecorations(view, plugin); - // The index resolves outside any transaction, so ask for a redraw. - view.dispatch({}); - }); } update(update: ViewUpdate): void { @@ -169,10 +144,6 @@ export const createDiscourseContextOverlayExtension = ( this.enabled = enabled; this.decorations = buildBadgeDecorations(update.view, plugin); } - - destroy(): void { - this.unsubscribe(); - } }, { decorations: (v) => v.decorations, diff --git a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts index 79e8f06ce..84fe9ef58 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts @@ -1,4 +1,8 @@ -import type { MarkdownPostProcessorContext } from "obsidian"; +import { + debounce, + type MarkdownPostProcessorContext, + type TFile, +} from "obsidian"; import type DiscourseGraphPlugin from "~/index"; import { createDiscourseContextBadge, @@ -6,6 +10,7 @@ import { } from "~/components/discourseContextBadge"; import { openDiscourseContextPopover } from "~/components/DiscourseContextPopover"; import { resolveDiscourseLinkTarget } from "./discourseLinkUtils"; +import { getNodeTypeIdFromFrontmatter } from "./discourseLinkFrontmatter"; /** * Reading view's counterpart to the Live Preview extension. @@ -16,6 +21,17 @@ import { resolveDiscourseLinkTarget } from "./discourseLinkUtils"; * pass. The same guard covers hover previews and exports, which render through * this path too. */ +const REFRESH_DEBOUNCE_MS = 300; + +/** Only a discourse node's own frontmatter can change what a badge shows. */ +const isDiscourseNodeFile = ( + plugin: DiscourseGraphPlugin, + file: TFile, +): boolean => + !!getNodeTypeIdFromFrontmatter( + plugin.app.metadataCache.getFileCache(file)?.frontmatter, + ); + export const createDiscourseContextOverlayPostProcessor = (plugin: DiscourseGraphPlugin) => (el: HTMLElement, ctx: MarkdownPostProcessorContext): void => { @@ -25,9 +41,11 @@ export const createDiscourseContextOverlayPostProcessor = const links = el.querySelectorAll("a.internal-link"); for (const link of Array.from(links)) { - if (link.nextElementSibling?.hasClass(DISCOURSE_CONTEXT_BADGE_CLASS)) { - continue; - } + const existing = link.nextElementSibling?.hasClass( + DISCOURSE_CONTEXT_BADGE_CLASS, + ) + ? link.nextElementSibling + : null; // data-href holds the link as written; href is resolved and URL-encoded. const linktext = @@ -39,7 +57,10 @@ export const createDiscourseContextOverlayPostProcessor = linktext, sourcePath: ctx.sourcePath, }); - if (!target) continue; + if (!target) { + existing?.remove(); + continue; + } const badge = createDiscourseContextBadge({ file: target.file, @@ -54,6 +75,49 @@ export const createDiscourseContextOverlayPostProcessor = }), }); + // Replaced rather than skipped: Obsidian reuses rendered sections, so a + // badge left in place would keep showing a count from before the last + // relation change. + existing?.remove(); link.insertAdjacentElement("afterend", badge); } }; + +/** + * Redraws both overlay surfaces when something they depend on changes outside + * the document they render. + * + * Reading view has no equivalent of CM6's update cycle, so nothing re-runs the + * post processor on its own; without this a badge keeps its original number for + * the life of the view, including the `0` it would show if the relations index + * was still loading when the note first rendered. + * + * Debounced because "resolved" fires repeatedly while the vault settles on + * startup, and re-rendering every preview is not cheap. + */ +export const registerDiscourseContextOverlayRefresh = ( + plugin: DiscourseGraphPlugin, +): void => { + const refresh = debounce( + () => { + if (!plugin.settings.showDiscourseContextOverlay) return; + plugin.refreshDiscourseContextOverlay(); + }, + REFRESH_DEBOUNCE_MS, + true, + ); + + plugin.register(plugin.relationsIndex.onChange(refresh)); + // A link only resolves once its target's frontmatter is cached, so a note + // rendered before that lands needs a second pass. + // + // Scoped to "changed" rather than "resolved" on purpose: "resolved" also + // fires while rendering a preview, and since the refresh re-renders previews + // that is a loop which leaves Reading view permanently blank. + plugin.registerEvent( + plugin.app.metadataCache.on("changed", (file) => { + if (!isDiscourseNodeFile(plugin, file)) return; + refresh(); + }), + ); +}; diff --git a/apps/obsidian/src/utils/internalLinkParsing.ts b/apps/obsidian/src/utils/internalLinkParsing.ts new file mode 100644 index 000000000..01f6cd82c --- /dev/null +++ b/apps/obsidian/src/utils/internalLinkParsing.ts @@ -0,0 +1,34 @@ +/** + * Pure parsing for internal links in raw markdown, kept free of Obsidian and + * CodeMirror imports so it stays directly testable. + */ + +/** + * Wikilinks `[[...]]` and markdown links `[text](path.md)`. + * + * Embeds are not excluded here: the leading `!` sits outside the match, so the + * caller has to check the preceding character. + */ +export const INTERNAL_LINK_RE = /\[\[([^\]]+)\]\]|\[([^\]]+)\]\(([^)]+\.md)\)/g; + +/** + * Extracts the link target from a wikilink or markdown link match. + * + * Any `#heading` subpath is left in place; resolving it is the caller's job, + * since Obsidian's own parseLinktext handles that. + */ +export const extractLinktext = (match: string): string => { + if (match.startsWith("[[")) { + const inner = match.slice(2, -2); + const pipeIndex = inner.indexOf("|"); + return pipeIndex >= 0 ? inner.slice(0, pipeIndex) : inner; + } + + const parenOpen = match.lastIndexOf("("); + const rawPath = match.slice(parenOpen + 1, -1); + try { + return decodeURIComponent(rawPath); + } catch { + return rawPath; + } +}; diff --git a/apps/obsidian/src/utils/relationsIndex.ts b/apps/obsidian/src/utils/relationsIndex.ts index 0be2c7382..80ae19689 100644 --- a/apps/obsidian/src/utils/relationsIndex.ts +++ b/apps/obsidian/src/utils/relationsIndex.ts @@ -20,6 +20,8 @@ export class RelationsIndex { private plugin: DiscourseGraphPlugin; private index: Map | null = null; private inFlight: Promise | null = null; + private stale = false; + private unloaded = false; private subscribers = new Set<() => void>(); /** * Bumped on every invalidation. A load that started before the bump is stale @@ -49,6 +51,7 @@ export class RelationsIndex { } unload(): void { + this.unloaded = true; this.subscribers.clear(); this.index = null; this.inFlight = null; @@ -64,53 +67,68 @@ export class RelationsIndex { return () => this.subscribers.delete(subscriber); } - isReady(): boolean { - return this.index !== null; - } - async ensureLoaded(): Promise { - if (this.index !== null) return; + if (this.unloaded) return; + if (this.index !== null && !this.stale) return; if (this.inFlight) return this.inFlight; const generation = this.generation; this.inFlight = (async () => { - const relationsFile = await loadRelations(this.plugin); - if (generation !== this.generation) return; - this.index = buildEndpointIndex(relationsFile.relations ?? {}); - this.inFlight = null; + try { + const relationsFile = await loadRelations(this.plugin); + // A newer invalidation landed mid-read, so this result is already out + // of date; the reload it scheduled will supersede it. + if (generation !== this.generation || this.unloaded) return; + this.index = buildEndpointIndex(relationsFile.relations ?? {}); + this.stale = false; + } finally { + // Must clear on every path. Leaving it set would make ensureLoaded + // hand out a settled promise forever, so the snapshot would stay stale + // and every read would re-request a load that never runs. + this.inFlight = null; + } + // An invalidation that arrived mid-read was skipped above; it still needs + // a load of its own. + if (this.stale && !this.unloaded) { + void this.ensureLoaded(); + return; + } this.notify(); })(); return this.inFlight; } - /** Drops the snapshot and reloads it. */ - async refresh(): Promise { - this.invalidate(); - await this.ensureLoaded(); - } - /** * Relations touching any of `endpointIds`. * - * Returns an empty array when the snapshot is cold and schedules a load; - * subscribers are notified once it lands. Callers on a render path should - * treat an empty result as "nothing to draw yet" rather than "no relations". + * Returns an empty array while the snapshot is still cold; subscribers are + * notified once it lands. Callers on a render path should treat an empty + * result as "nothing to draw yet" rather than "no relations". + * + * Deliberately does not schedule a load — initialize() and invalidate() are + * the only things that do. Requesting one from a render path would make + * notify -> re-render -> read cycle forever. */ getRelationsForEndpointIds( endpointIds: Iterable, ): RelationInstance[] { - if (this.index === null) { - void this.ensureLoaded(); - return []; - } + if (this.index === null) return []; return collectRelations({ index: this.index, endpointIds }); } + /** + * Marks the snapshot for reload without discarding it. + * + * Dropping it outright would make every badge read 0 until the reload lands — + * and since saving a relation writes relations.json, that flash would happen + * on the very action the user just took. The previous counts are a better + * answer for those few milliseconds than a wrong one. + */ private invalidate(): void { this.generation += 1; - this.index = null; this.inFlight = null; + this.stale = true; void this.ensureLoaded(); } From d52c4f390825315617eb7b87fcf2eb6f8c39cd1e Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Sun, 6 Sep 2026 15:26:39 -0400 Subject: [PATCH 08/11] ENG-1249 Remove the overlay unit tests and vitest setup Drops the three test files, vitest.config.mts, and the test:unit script and vitest devDependency, so apps/obsidian is no longer part of the root test:unit task and the lockfile returns to matching main. The pure helper modules stay: their separation from Obsidian and CodeMirror still keeps the counting, grouping and link-parsing rules readable on their own. Their doc comments no longer cite testability as the reason. Co-Authored-By: Claude Opus 5 --- apps/obsidian/package.json | 4 +- .../discourseLinkFrontmatter.test.ts | 126 ------------------ .../utils/__tests__/extractLinktext.test.ts | 36 ----- .../__tests__/relationsEndpointIndex.test.ts | 103 -------------- .../src/utils/discourseLinkFrontmatter.ts | 3 +- .../obsidian/src/utils/internalLinkParsing.ts | 3 +- .../src/utils/relationsEndpointIndex.ts | 3 +- apps/obsidian/vitest.config.mts | 17 --- pnpm-lock.yaml | 42 ------ 9 files changed, 7 insertions(+), 330 deletions(-) delete mode 100644 apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts delete mode 100644 apps/obsidian/src/utils/__tests__/extractLinktext.test.ts delete mode 100644 apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts delete mode 100644 apps/obsidian/vitest.config.mts diff --git a/apps/obsidian/package.json b/apps/obsidian/package.json index d7f6058b3..d7b167bc9 100644 --- a/apps/obsidian/package.json +++ b/apps/obsidian/package.json @@ -10,8 +10,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "publish": "tsx scripts/publish.ts", - "check-types": "tsc --noEmit --skipLibCheck", - "test:unit": "vitest run --config vitest.config.mts" + "check-types": "tsc --noEmit --skipLibCheck" }, "keywords": [], "author": "", @@ -36,7 +35,6 @@ "tsx": "^4.19.2", "typescript": "5.5.4", "uuidv7": "1.1.0", - "vitest": "catalog:", "zod": "^3.24.1" }, "dependencies": { diff --git a/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts b/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts deleted file mode 100644 index 9d5ae2687..000000000 --- a/apps/obsidian/src/utils/__tests__/discourseLinkFrontmatter.test.ts +++ /dev/null @@ -1,126 +0,0 @@ -import { describe, expect, it } from "vitest"; -import type { RelationInstance } from "~/types"; -import { - countDisplayableRelations, - getEndpointIdsFromFrontmatter, - getNodeTypeIdFromFrontmatter, -} from "~/utils/discourseLinkFrontmatter"; - -const relation = ( - overrides: Partial & Pick, -): RelationInstance => ({ - type: "supports", - source: "a", - destination: "b", - created: 0, - ...overrides, -}); - -describe("getNodeTypeIdFromFrontmatter", () => { - it("returns the node type id", () => { - expect(getNodeTypeIdFromFrontmatter({ nodeTypeId: "claim" })).toBe("claim"); - }); - - it("returns undefined when absent, empty, undefined frontmatter, or not a string", () => { - expect(getNodeTypeIdFromFrontmatter({})).toBeUndefined(); - expect(getNodeTypeIdFromFrontmatter({ nodeTypeId: "" })).toBeUndefined(); - expect(getNodeTypeIdFromFrontmatter(undefined)).toBeUndefined(); - expect(getNodeTypeIdFromFrontmatter({ nodeTypeId: 42 })).toBeUndefined(); - }); -}); - -describe("getEndpointIdsFromFrontmatter", () => { - it("returns the nodeInstanceId for a local node", () => { - expect(getEndpointIdsFromFrontmatter({ nodeInstanceId: "n1" })).toEqual([ - "n1", - ]); - }); - - it("returns both ids for an imported node", () => { - expect( - getEndpointIdsFromFrontmatter({ - nodeInstanceId: "n1", - importedFromRid: "rid1", - }), - ).toEqual(["n1", "rid1"]); - }); - - it("does not repeat an id when both fields match", () => { - expect( - getEndpointIdsFromFrontmatter({ - nodeInstanceId: "same", - importedFromRid: "same", - }), - ).toEqual(["same"]); - }); - - it("returns an importedFromRid even without a nodeInstanceId", () => { - expect(getEndpointIdsFromFrontmatter({ importedFromRid: "rid1" })).toEqual([ - "rid1", - ]); - }); - - it("returns nothing for absent or non-string ids", () => { - expect(getEndpointIdsFromFrontmatter({})).toEqual([]); - expect(getEndpointIdsFromFrontmatter(undefined)).toEqual([]); - expect(getEndpointIdsFromFrontmatter({ nodeInstanceId: 7 })).toEqual([]); - }); -}); - -describe("countDisplayableRelations", () => { - const allConfigured = () => true; - - it("counts local relations, which leave tentative undefined", () => { - expect( - countDisplayableRelations({ - relations: [relation({ id: "r1" }), relation({ id: "r2" })], - isConfiguredType: allConfigured, - }), - ).toBe(2); - }); - - it("counts explicitly accepted relations", () => { - expect( - countDisplayableRelations({ - relations: [relation({ id: "r1", tentative: true })], - isConfiguredType: allConfigured, - }), - ).toBe(1); - }); - - it("excludes imported relations awaiting acceptance", () => { - expect( - countDisplayableRelations({ - relations: [ - relation({ id: "r1" }), - relation({ id: "r2", tentative: false }), - ], - isConfiguredType: allConfigured, - }), - ).toBe(1); - }); - - it("excludes relations orphaned by a deleted relation type", () => { - // Deleting a relation type leaves its relations in relations.json. The - // panel drops them, so a badge that counted them would promise context the - // panel then refuses to show. - expect( - countDisplayableRelations({ - relations: [ - relation({ id: "r1", type: "supports" }), - relation({ id: "r2", type: "deleted-type" }), - ], - isConfiguredType: (type) => type === "supports", - }), - ).toBe(1); - }); - - it("returns zero for no relations", () => { - expect( - countDisplayableRelations({ - relations: [], - isConfiguredType: allConfigured, - }), - ).toBe(0); - }); -}); diff --git a/apps/obsidian/src/utils/__tests__/extractLinktext.test.ts b/apps/obsidian/src/utils/__tests__/extractLinktext.test.ts deleted file mode 100644 index 49e7ab01a..000000000 --- a/apps/obsidian/src/utils/__tests__/extractLinktext.test.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { extractLinktext } from "~/utils/internalLinkParsing"; - -describe("extractLinktext", () => { - it("reads a plain wikilink", () => { - expect(extractLinktext("[[Claim]]")).toBe("Claim"); - }); - - it("drops a wikilink alias", () => { - expect(extractLinktext("[[Claim|the claim]]")).toBe("Claim"); - }); - - it("keeps a wikilink subpath for the caller to strip", () => { - expect(extractLinktext("[[Claim#Evidence]]")).toBe("Claim#Evidence"); - }); - - it("reads a markdown link target", () => { - expect(extractLinktext("[the claim](Claim.md)")).toBe("Claim.md"); - }); - - it("decodes a percent-encoded markdown link", () => { - expect(extractLinktext("[a claim](My%20Claim.md)")).toBe("My Claim.md"); - }); - - it("falls back to the raw path when decoding fails", () => { - // A lone % is not valid percent-encoding; decodeURIComponent throws. - expect(extractLinktext("[bad](100%.md)")).toBe("100%.md"); - }); - - it("reads a markdown link inside a folder", () => { - expect(extractLinktext("[c](Discourse Nodes/Claim.md)")).toBe( - "Discourse Nodes/Claim.md", - ); - }); -}); diff --git a/apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts b/apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts deleted file mode 100644 index 65f9acf17..000000000 --- a/apps/obsidian/src/utils/__tests__/relationsEndpointIndex.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { describe, expect, it } from "vitest"; -import type { RelationInstance } from "~/types"; -import { - buildEndpointIndex, - collectRelations, -} from "~/utils/relationsEndpointIndex"; - -const relation = ( - overrides: Partial & Pick, -): RelationInstance => ({ - type: "supports", - source: "a", - destination: "b", - created: 0, - ...overrides, -}); - -const toRecord = ( - relations: RelationInstance[], -): Record => - Object.fromEntries(relations.map((r) => [r.id, r])); - -describe("buildEndpointIndex", () => { - it("files a relation under both endpoints", () => { - const r = relation({ id: "r1", source: "a", destination: "b" }); - const index = buildEndpointIndex(toRecord([r])); - - expect(index.get("a")).toEqual([r]); - expect(index.get("b")).toEqual([r]); - }); - - it("files a self-relation once so one endpoint does not yield it twice", () => { - const r = relation({ id: "r1", source: "a", destination: "a" }); - const index = buildEndpointIndex(toRecord([r])); - - expect(index.get("a")).toEqual([r]); - }); - - it("groups multiple relations sharing an endpoint", () => { - const r1 = relation({ id: "r1", source: "a", destination: "b" }); - const r2 = relation({ id: "r2", source: "c", destination: "a" }); - const index = buildEndpointIndex(toRecord([r1, r2])); - - expect(index.get("a")).toEqual([r1, r2]); - expect(index.get("b")).toEqual([r1]); - expect(index.get("c")).toEqual([r2]); - }); - - it("returns an empty index for no relations", () => { - expect(buildEndpointIndex({}).size).toBe(0); - }); - - it("skips relations missing an endpoint rather than indexing undefined", () => { - const r = relation({ id: "r1", source: "a", destination: "" }); - const index = buildEndpointIndex(toRecord([r])); - - expect(index.get("a")).toEqual([r]); - expect(index.has("")).toBe(false); - expect(index.size).toBe(1); - }); -}); - -describe("collectRelations", () => { - it("returns relations for a single endpoint", () => { - const r1 = relation({ id: "r1", source: "a", destination: "b" }); - const r2 = relation({ id: "r2", source: "c", destination: "d" }); - const index = buildEndpointIndex(toRecord([r1, r2])); - - expect(collectRelations({ index, endpointIds: ["a"] })).toEqual([r1]); - }); - - it("counts a relation once when both its endpoints are queried", () => { - // An imported node matches on both its nodeInstanceId and its - // importedFromRid, so both ends of the same relation can be asked for. - const r = relation({ id: "r1", source: "local-id", destination: "rid" }); - const index = buildEndpointIndex(toRecord([r])); - - expect( - collectRelations({ index, endpointIds: ["local-id", "rid"] }), - ).toEqual([r]); - }); - - it("deduplicates across endpoints while preserving first-seen order", () => { - const r1 = relation({ id: "r1", source: "a", destination: "shared" }); - const r2 = relation({ id: "r2", source: "b", destination: "shared" }); - const index = buildEndpointIndex(toRecord([r1, r2])); - - expect( - collectRelations({ index, endpointIds: ["shared", "a", "b"] }).map( - (r) => r.id, - ), - ).toEqual(["r1", "r2"]); - }); - - it("returns an empty array for unknown endpoints", () => { - const index = buildEndpointIndex( - toRecord([relation({ id: "r1", source: "a", destination: "b" })]), - ); - - expect(collectRelations({ index, endpointIds: ["nope"] })).toEqual([]); - expect(collectRelations({ index, endpointIds: [] })).toEqual([]); - }); -}); diff --git a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts index bda3bc785..444ab4d9f 100644 --- a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts +++ b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts @@ -2,7 +2,8 @@ import type { RelationInstance } from "~/types"; /** * Pure frontmatter/relation helpers behind discourseLinkUtils, kept free of - * Obsidian imports so they stay directly testable. + * Obsidian imports so the counting rules can be read and changed without + * untangling them from link resolution. */ const asString = (value: unknown): string | undefined => diff --git a/apps/obsidian/src/utils/internalLinkParsing.ts b/apps/obsidian/src/utils/internalLinkParsing.ts index 01f6cd82c..14b5877dc 100644 --- a/apps/obsidian/src/utils/internalLinkParsing.ts +++ b/apps/obsidian/src/utils/internalLinkParsing.ts @@ -1,6 +1,7 @@ /** * Pure parsing for internal links in raw markdown, kept free of Obsidian and - * CodeMirror imports so it stays directly testable. + * CodeMirror imports so it can be reused by any caller that has raw markdown + * rather than an editor. */ /** diff --git a/apps/obsidian/src/utils/relationsEndpointIndex.ts b/apps/obsidian/src/utils/relationsEndpointIndex.ts index dac467de5..034dadc9e 100644 --- a/apps/obsidian/src/utils/relationsEndpointIndex.ts +++ b/apps/obsidian/src/utils/relationsEndpointIndex.ts @@ -2,7 +2,8 @@ import type { RelationInstance } from "~/types"; /** * Pure indexing helpers behind RelationsIndex, kept free of Obsidian and - * relationsStore imports so they stay directly testable. + * relationsStore imports so the grouping and dedupe rules stay separate from + * snapshot loading and invalidation. */ /** diff --git a/apps/obsidian/vitest.config.mts b/apps/obsidian/vitest.config.mts deleted file mode 100644 index 9b056e9d9..000000000 --- a/apps/obsidian/vitest.config.mts +++ /dev/null @@ -1,17 +0,0 @@ -import path from "node:path"; -import { fileURLToPath } from "node:url"; -import { defineConfig } from "vitest/config"; - -const dirname = path.dirname(fileURLToPath(import.meta.url)); - -export default defineConfig({ - test: { - environment: "node", - include: ["src/utils/__tests__/**/*.test.ts"], - }, - resolve: { - alias: { - "~": path.resolve(dirname, "src"), - }, - }, -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index adc5d7ff4..8393d9435 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -215,9 +215,6 @@ importers: uuidv7: specifier: 1.1.0 version: 1.1.0 - vitest: - specifier: 'catalog:' - version: 4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2)) zod: specifier: ^3.24.1 version: 3.25.76 @@ -17123,15 +17120,6 @@ snapshots: chai: 6.2.2 tinyrainbow: 3.1.0 - '@vitest/mocker@4.1.6(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2))': - dependencies: - '@vitest/spy': 4.1.6 - estree-walker: 3.0.3 - magic-string: 0.30.21 - optionalDependencies: - msw: 2.11.1(@types/node@22.20.0)(typescript@5.5.4) - vite: 7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2) - '@vitest/mocker@4.1.6(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2))': dependencies: '@vitest/spy': 4.1.6 @@ -24347,36 +24335,6 @@ snapshots: tsx: 4.21.0 yaml: 2.8.2 - vitest@4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2)): - dependencies: - '@vitest/expect': 4.1.6 - '@vitest/mocker': 4.1.6(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2)) - '@vitest/pretty-format': 4.1.6 - '@vitest/runner': 4.1.6 - '@vitest/snapshot': 4.1.6 - '@vitest/spy': 4.1.6 - '@vitest/utils': 4.1.6 - es-module-lexer: 2.1.0 - expect-type: 1.3.0 - magic-string: 0.30.21 - obug: 2.1.1 - pathe: 2.0.3 - picomatch: 4.0.4 - std-env: 4.1.0 - tinybench: 2.9.0 - tinyexec: 1.0.2 - tinyglobby: 0.2.16 - tinyrainbow: 3.1.0 - vite: 7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.20.5)(yaml@2.8.2) - why-is-node-running: 2.3.0 - optionalDependencies: - '@edge-runtime/vm': 3.2.0 - '@opentelemetry/api': 1.9.0 - '@types/node': 22.20.0 - jsdom: 20.0.3 - transitivePeerDependencies: - - msw - vitest@4.1.6(@edge-runtime/vm@3.2.0)(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(jsdom@20.0.3)(msw@2.11.1(@types/node@22.20.0)(typescript@5.5.4))(vite@7.3.3(@types/node@22.20.0)(jiti@1.21.7)(tsx@4.21.0)(yaml@2.8.2)): dependencies: '@vitest/expect': 4.1.6 From 8e0ccd9ced27134ce23dcc7e000e500575a4c912 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 7 Sep 2026 16:50:10 -0400 Subject: [PATCH 09/11] ENG-1249 Make badge counts follow relation changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count never updated after a relation was added or removed: the badge kept whatever number it had when the note was first rendered. Live Preview relied on an empty CM6 transaction to trigger a redraw, but update() early-returns unless the document, viewport or setting changed — and a relation change alters none of those, so the redraw did nothing. RelationsIndex now exposes a version that the ViewPlugin compares, which is the signal a ViewPlugin can actually see. Reading view was refreshed by calling previewMode.rerender(), which tears the preview down and rebuilds it. That is both destructive and far more work than needed, so badges are now re-applied over the already-rendered content instead; the per-link pass was already idempotent, since Obsidian reuses rendered sections. refreshMarkdownPreviews is gone with its last caller. Co-Authored-By: Claude Opus 5 --- apps/obsidian/src/index.ts | 9 +- .../utils/discourseContextOverlayExtension.ts | 15 +- .../discourseContextOverlayPostProcessor.ts | 152 +++++++++++------- .../obsidian/src/utils/markdownViewRefresh.ts | 15 -- apps/obsidian/src/utils/relationsIndex.ts | 12 ++ 5 files changed, 122 insertions(+), 81 deletions(-) diff --git a/apps/obsidian/src/index.ts b/apps/obsidian/src/index.ts index ffe572452..6df6af506 100644 --- a/apps/obsidian/src/index.ts +++ b/apps/obsidian/src/index.ts @@ -24,7 +24,9 @@ import { createDiscourseContextOverlayExtension } from "~/utils/discourseContext import { createDiscourseContextOverlayPostProcessor, registerDiscourseContextOverlayRefresh, + refreshDiscourseContextOverlaySurfaces, } from "~/utils/discourseContextOverlayPostProcessor"; +import { refreshMarkdownEditors } from "~/utils/markdownViewRefresh"; import { closeDiscourseContextPopover } from "~/components/DiscourseContextPopover"; import { registerCommands, @@ -42,10 +44,6 @@ import { InlineNodeTypePicker } from "~/components/InlineNodeTypePicker"; import { initializeSupabaseSync } from "~/utils/syncDgNodesToSupabase"; import { FileChangeListener } from "~/utils/fileChangeListener"; import { RelationsIndex } from "~/utils/relationsIndex"; -import { - refreshMarkdownEditors, - refreshMarkdownPreviews, -} from "~/utils/markdownViewRefresh"; import generateUid from "~/utils/generateUid"; import { migrateFrontmatterRelationsToRelationsJson, @@ -304,8 +302,7 @@ export default class DiscourseGraphPlugin extends Plugin { * or disappears immediately when its setting is toggled, without a reload. */ refreshDiscourseContextOverlay(): void { - refreshMarkdownEditors(this.app); - refreshMarkdownPreviews(this.app); + refreshDiscourseContextOverlaySurfaces(this); } setHelpMenuStatusBarItemVisibility(): void { diff --git a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts index ba888ff1e..83eefc284 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts @@ -123,25 +123,32 @@ export const createDiscourseContextOverlayExtension = ( class { decorations: DecorationSet; private enabled: boolean; + private indexVersion: number; constructor(view: EditorView) { this.enabled = plugin.settings.showDiscourseContextOverlay; + this.indexVersion = plugin.relationsIndex.getVersion(); this.decorations = buildBadgeDecorations(view, plugin); } update(update: ViewUpdate): void { - // The setting is toggled by dispatching an empty transaction, which - // changes neither the document nor the viewport, so it has to be - // compared explicitly or the toggle would appear to do nothing. + // Everything that changes a badge from outside the document — the + // setting, and the relation counts themselves — arrives as an empty + // transaction, which changes neither the document nor the viewport. Both + // have to be compared explicitly, or the redraw silently does nothing + // and badges keep a count from before the last relation change. const enabled = plugin.settings.showDiscourseContextOverlay; + const indexVersion = plugin.relationsIndex.getVersion(); if ( !update.docChanged && !update.viewportChanged && - enabled === this.enabled + enabled === this.enabled && + indexVersion === this.indexVersion ) { return; } this.enabled = enabled; + this.indexVersion = indexVersion; this.decorations = buildBadgeDecorations(update.view, plugin); } }, diff --git a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts index 84fe9ef58..97a623e3a 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts @@ -1,5 +1,6 @@ import { debounce, + MarkdownView, type MarkdownPostProcessorContext, type TFile, } from "obsidian"; @@ -11,6 +12,7 @@ import { import { openDiscourseContextPopover } from "~/components/DiscourseContextPopover"; import { resolveDiscourseLinkTarget } from "./discourseLinkUtils"; import { getNodeTypeIdFromFrontmatter } from "./discourseLinkFrontmatter"; +import { refreshMarkdownEditors } from "./markdownViewRefresh"; /** * Reading view's counterpart to the Live Preview extension. @@ -32,77 +34,116 @@ const isDiscourseNodeFile = ( plugin.app.metadataCache.getFileCache(file)?.frontmatter, ); -export const createDiscourseContextOverlayPostProcessor = - (plugin: DiscourseGraphPlugin) => - (el: HTMLElement, ctx: MarkdownPostProcessorContext): void => { - if (!plugin.settings.showDiscourseContextOverlay) return; - if (!ctx.sourcePath) return; +/** + * Adds, updates or removes the badge on every discourse-node link inside `el`. + * + * Safe to run repeatedly over the same content, which it has to be: Obsidian + * reuses rendered sections and re-runs post processors over them, and the + * refresh below re-applies this in place rather than re-rendering. + */ +export const applyDiscourseContextBadges = ({ + plugin, + el, + sourcePath, +}: { + plugin: DiscourseGraphPlugin; + el: HTMLElement; + sourcePath: string; +}): void => { + const links = el.querySelectorAll("a.internal-link"); - const links = el.querySelectorAll("a.internal-link"); + for (const link of Array.from(links)) { + const existing = link.nextElementSibling?.hasClass( + DISCOURSE_CONTEXT_BADGE_CLASS, + ) + ? link.nextElementSibling + : null; - for (const link of Array.from(links)) { - const existing = link.nextElementSibling?.hasClass( - DISCOURSE_CONTEXT_BADGE_CLASS, - ) - ? link.nextElementSibling - : null; + // data-href holds the link as written; href is resolved and URL-encoded. + const linktext = + link.getAttribute("data-href") ?? link.getAttribute("href"); + if (!linktext) continue; - // data-href holds the link as written; href is resolved and URL-encoded. - const linktext = - link.getAttribute("data-href") ?? link.getAttribute("href"); - if (!linktext) continue; + const target = resolveDiscourseLinkTarget({ + plugin, + linktext, + sourcePath, + }); + if (!target) { + existing?.remove(); + continue; + } - const target = resolveDiscourseLinkTarget({ - plugin, - linktext, - sourcePath: ctx.sourcePath, - }); - if (!target) { - existing?.remove(); - continue; - } + const badge = createDiscourseContextBadge({ + file: target.file, + nodeType: target.nodeType, + relationCount: target.relationCount, + onActivate: ({ file, anchor }) => + openDiscourseContextPopover({ + plugin, + file, + anchor, + relationCount: target.relationCount, + }), + }); - const badge = createDiscourseContextBadge({ - file: target.file, - nodeType: target.nodeType, - relationCount: target.relationCount, - onActivate: ({ file, anchor }) => - openDiscourseContextPopover({ - plugin, - file, - anchor, - relationCount: target.relationCount, - }), - }); + // Replaced rather than skipped: Obsidian reuses rendered sections, so a + // badge left in place would keep showing a count from before the last + // relation change. + existing?.remove(); + link.insertAdjacentElement("afterend", badge); + } +}; - // Replaced rather than skipped: Obsidian reuses rendered sections, so a - // badge left in place would keep showing a count from before the last - // relation change. - existing?.remove(); - link.insertAdjacentElement("afterend", badge); - } +/** Strips every badge under `el`, for when the setting is switched off. */ +const removeDiscourseContextBadges = (el: HTMLElement): void => { + el.querySelectorAll(`.${DISCOURSE_CONTEXT_BADGE_CLASS}`).forEach((badge) => + badge.remove(), + ); +}; + +export const createDiscourseContextOverlayPostProcessor = + (plugin: DiscourseGraphPlugin) => + (el: HTMLElement, ctx: MarkdownPostProcessorContext): void => { + if (!plugin.settings.showDiscourseContextOverlay) return; + if (!ctx.sourcePath) return; + applyDiscourseContextBadges({ plugin, el, sourcePath: ctx.sourcePath }); }; /** * Redraws both overlay surfaces when something they depend on changes outside - * the document they render. + * the document they render — a relation added or removed, or a target's + * frontmatter finishing indexing. * * Reading view has no equivalent of CM6's update cycle, so nothing re-runs the - * post processor on its own; without this a badge keeps its original number for - * the life of the view, including the `0` it would show if the relations index - * was still loading when the note first rendered. - * - * Debounced because "resolved" fires repeatedly while the vault settles on - * startup, and re-rendering every preview is not cheap. + * post processor on its own. It is refreshed by re-applying badges over the + * already-rendered content rather than by calling previewMode.rerender(): + * rerender tears the preview down, and a pane that is not currently painting + * never rebuilds it, leaving Reading view permanently blank. */ +export const refreshDiscourseContextOverlaySurfaces = ( + plugin: DiscourseGraphPlugin, +): void => { + refreshMarkdownEditors(plugin.app); + plugin.app.workspace.iterateAllLeaves((leaf) => { + if (!(leaf.view instanceof MarkdownView)) return; + const el = leaf.view.previewMode?.containerEl; + if (!el) return; + if (!plugin.settings.showDiscourseContextOverlay) { + removeDiscourseContextBadges(el); + return; + } + const sourcePath = leaf.view.file?.path; + if (!sourcePath) return; + applyDiscourseContextBadges({ plugin, el, sourcePath }); + }); +}; + export const registerDiscourseContextOverlayRefresh = ( plugin: DiscourseGraphPlugin, ): void => { const refresh = debounce( - () => { - if (!plugin.settings.showDiscourseContextOverlay) return; - plugin.refreshDiscourseContextOverlay(); - }, + () => refreshDiscourseContextOverlaySurfaces(plugin), REFRESH_DEBOUNCE_MS, true, ); @@ -112,8 +153,7 @@ export const registerDiscourseContextOverlayRefresh = ( // rendered before that lands needs a second pass. // // Scoped to "changed" rather than "resolved" on purpose: "resolved" also - // fires while rendering a preview, and since the refresh re-renders previews - // that is a loop which leaves Reading view permanently blank. + // fires while a preview renders, which would make this re-entrant. plugin.registerEvent( plugin.app.metadataCache.on("changed", (file) => { if (!isDiscourseNodeFile(plugin, file)) return; diff --git a/apps/obsidian/src/utils/markdownViewRefresh.ts b/apps/obsidian/src/utils/markdownViewRefresh.ts index f237dd02b..5f8652737 100644 --- a/apps/obsidian/src/utils/markdownViewRefresh.ts +++ b/apps/obsidian/src/utils/markdownViewRefresh.ts @@ -25,18 +25,3 @@ export const refreshMarkdownEditors = (app: App): void => { } }); }; - -/** - * Re-renders every open Reading view. - * - * Reading view has no equivalent of the CM6 no-op transaction: markdown post - * processors only run when content is rendered, so a setting that changes what - * they emit needs the already-rendered content thrown away and rebuilt. - */ -export const refreshMarkdownPreviews = (app: App): void => { - app.workspace.iterateAllLeaves((leaf) => { - if (leaf.view instanceof MarkdownView) { - leaf.view.previewMode?.rerender(true); - } - }); -}; diff --git a/apps/obsidian/src/utils/relationsIndex.ts b/apps/obsidian/src/utils/relationsIndex.ts index 80ae19689..8aa1e4713 100644 --- a/apps/obsidian/src/utils/relationsIndex.ts +++ b/apps/obsidian/src/utils/relationsIndex.ts @@ -22,6 +22,12 @@ export class RelationsIndex { private inFlight: Promise | null = null; private stale = false; private unloaded = false; + /** + * Incremented every time the snapshot is replaced. Lets a caller that cannot + * subscribe — a CodeMirror ViewPlugin, whose update() only sees transactions — + * detect that counts changed by comparing versions. + */ + private version = 0; private subscribers = new Set<() => void>(); /** * Bumped on every invalidation. A load that started before the bump is stale @@ -58,6 +64,11 @@ export class RelationsIndex { this.generation += 1; } + /** Changes whenever the snapshot is replaced; see the field comment. */ + getVersion(): number { + return this.version; + } + /** * Notifies when the snapshot changes, so a caller that rendered against a * cold or stale index can render again. Returns an unsubscribe function. @@ -81,6 +92,7 @@ export class RelationsIndex { if (generation !== this.generation || this.unloaded) return; this.index = buildEndpointIndex(relationsFile.relations ?? {}); this.stale = false; + this.version += 1; } finally { // Must clear on every path. Leaving it set would make ensureLoaded // hand out a settled promise forever, so the snapshot would stay stale From dc496be4e2818320f95fd0b822af2a4f19be8cf9 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 7 Sep 2026 17:32:20 -0400 Subject: [PATCH 10/11] ENG-1249 Dedup link parsing, trim comments, drop stray edits wikilinkDragHandler had its own byte-identical copy of the link regex and target extraction; both now come from internalLinkParsing, which is what that module is actually for. Comments across the overlay code are cut to 1-2 lines, and AGENTS.md now states that as a rule so it holds for future changes. Reverts formatting-only edits to five canvas and util files that a repo-wide prettier run pulled into the diff. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 1 + .../components/DiscourseContextPopover.tsx | 39 ++++-------- .../src/components/discourseContextBadge.ts | 18 ++---- apps/obsidian/src/index.ts | 8 +-- .../utils/discourseContextOverlayExtension.ts | 29 ++------- .../discourseContextOverlayPostProcessor.ts | 37 ++---------- .../src/utils/discourseLinkFrontmatter.ts | 26 +------- apps/obsidian/src/utils/discourseLinkUtils.ts | 19 +----- .../obsidian/src/utils/internalLinkParsing.ts | 20 +------ .../obsidian/src/utils/markdownViewRefresh.ts | 7 +-- .../src/utils/relationsEndpointIndex.ts | 22 +------ apps/obsidian/src/utils/relationsIndex.ts | 60 ++++--------------- .../obsidian/src/utils/wikilinkDragHandler.ts | 30 +--------- 13 files changed, 57 insertions(+), 259 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 36bc4fdc3..fcc61091a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -88,6 +88,7 @@ When creating or updating a pull request body: - Add comments only when necessary; descriptive names should minimize the need for comments - Explain the why, not the what, focusing on reasoning, trade-offs, and approaches +- Keep any comment to 1-2 lines. A technical choice does not need a paragraph, and a multi-line block explaining one decision is too long: state the constraint, not the narrative that led to it. If it genuinely cannot be said in two lines, it belongs in a doc or a ticket, not above the code - Document limitations, known bugs, or edge cases where behavior may not align with expectations - Prefer sentence case in documentation and feature descriptions; capitalize official product/plugin names and exact UI labels, buttons, or titles, but keep generic feature terms lowercase to emphasize user actions diff --git a/apps/obsidian/src/components/DiscourseContextPopover.tsx b/apps/obsidian/src/components/DiscourseContextPopover.tsx index 2006d38c6..7a7d8e036 100644 --- a/apps/obsidian/src/components/DiscourseContextPopover.tsx +++ b/apps/obsidian/src/components/DiscourseContextPopover.tsx @@ -8,14 +8,9 @@ const POPOVER_CLASS = "dg-discourse-context-popover"; const VIEWPORT_MARGIN = 8; const EMPTY_MESSAGE = "No discourse relation found"; -/** - * Positions the popover under its badge, pulling it back inside the window when - * it would overflow. Measured after mount because the content height depends on - * how many relations the node has. - */ +/** Positions the popover under its badge, clamped inside the viewport. */ const positionPopover = (popover: HTMLElement, anchor: HTMLElement): void => { - // Geometry has to come from the window the anchor is in, not the main one, or - // a popover opened in a popout window gets clamped to the wrong viewport. + // The anchor's own window, or a popout gets clamped to the wrong viewport. const win = anchor.ownerDocument.defaultView ?? window; const anchorRect = anchor.getBoundingClientRect(); const { width, height } = popover.getBoundingClientRect(); @@ -44,14 +39,8 @@ type PopoverOptions = { }; /** - * The discourse context shown when a badge is selected. - * - * Reuses RelationshipSection, the same component the Discourse Context panel - * renders, so the two can never disagree about a node's relations. It needs - * only a TFile and PluginProvider — no workspace leaf — which is what makes it - * reusable here. - * - * Only one popover exists at a time; opening another closes the previous one. + * Discourse context shown when a badge is selected. Reuses RelationshipSection + * so it cannot disagree with the panel. Only one is open at a time. */ class DiscourseContextPopover { private containerEl: HTMLElement; @@ -82,9 +71,7 @@ class DiscourseContextPopover { "shadow-lg", ); - // CurrentRelationships renders nothing at all when a node has none, so - // without this the popover would open on an unexplained "Add a new - // relation" button. Created before the React host so it reads above it. + // CurrentRelationships renders nothing when empty, leaving a bare button. if (relationCount === 0) { this.containerEl.createDiv({ cls: "mb-2 text-sm text-[var(--text-muted)]", @@ -100,9 +87,7 @@ class DiscourseContextPopover { , ); - // A React 18 root does not commit synchronously, so measuring now would - // size an empty box and the flip-up-when-near-the-bottom check would never - // fire. Re-measured after paint, and again as the relation list fills in. + // A React 18 root commits async, so measure again after paint and on resize. positionPopover(this.containerEl, anchor); this.reposition = () => positionPopover(this.containerEl, anchor); this.win.requestAnimationFrame(this.reposition); @@ -123,23 +108,19 @@ class DiscourseContextPopover { event.preventDefault(); this.close(); }; - // Scrolling the note moves the badge out from under the popover, so the - // popover follows it away. Scrolling *within* the popover must not dismiss - // it — its own content scrolls, and reaching "Add a new relation" requires - // exactly that. + // Scrolling the note dismisses; scrolling the popover's own content must not. const closeOnScroll = (event: Event): void => { if (this.containerEl.contains(event.target as Node)) return; this.close(); }; - // Deferred so the click that opened the popover does not immediately - // dismiss it as an outside click. + // Deferred so the opening click is not read as an outside click. const attach = this.win.setTimeout(() => { doc.addEventListener("click", closeIfOutside, true); }, 0); doc.addEventListener("keydown", closeOnEscape); - // Capture phase, since scrolling happens inside panes rather than on window. + // Capture phase: scrolling happens inside panes, not on window. doc.addEventListener("scroll", closeOnScroll, true); this.cleanupListeners.push(() => { @@ -155,7 +136,7 @@ class DiscourseContextPopover { this.cleanupListeners = []; this.resizeObserver?.disconnect(); this.resizeObserver = null; - // Unmounting during React's own event handling warns, so defer it. + // Deferred: unmounting during React's event handling warns. const root = this.root; this.win.setTimeout(() => root.unmount(), 0); this.containerEl.remove(); diff --git a/apps/obsidian/src/components/discourseContextBadge.ts b/apps/obsidian/src/components/discourseContextBadge.ts index 4c54bd847..7a2c2f6fa 100644 --- a/apps/obsidian/src/components/discourseContextBadge.ts +++ b/apps/obsidian/src/components/discourseContextBadge.ts @@ -1,10 +1,7 @@ import { setIcon, setTooltip, TFile } from "obsidian"; import type { DiscourseNode } from "~/types"; -/** - * Marks a badge in the DOM. Both render paths check for this before adding one, - * since Obsidian re-runs post processors over already-rendered sections. - */ +/** Marks a badge so a re-run can find and replace it. */ export const DISCOURSE_CONTEXT_BADGE_CLASS = "dg-discourse-context-badge"; export type DiscourseContextBadgeProps = { @@ -23,12 +20,8 @@ const badgeTooltip = ({ }; /** - * The inline badge shown next to a link to a discourse node. - * - * Plain DOM rather than React so the CodeMirror widget and the Reading view - * post processor can share one implementation — neither has a React root, and - * mounting one per link would be far too heavy. Tailwind utilities work here - * because they compile to ordinary global classes. + * Inline badge next to a link to a discourse node. Plain DOM, not React, so both + * render paths share it without mounting a React root per link. */ export const createDiscourseContextBadge = ({ file, @@ -53,14 +46,13 @@ export const createDiscourseContextBadge = ({ badge.setAttribute("tabindex", "0"); const activate = (event: Event): void => { - // Stops Obsidian from following the link the badge sits next to. + // Do not follow the link the badge sits next to. event.preventDefault(); event.stopPropagation(); onActivate({ file, anchor: badge }); }; - // Without this the mousedown still lands in the editor and moves the caret, - // which in Live Preview expands the raw [[...]] markup under the popover. + // Otherwise the caret moves, expanding the raw [[...]] under the popover. badge.addEventListener("mousedown", (event: MouseEvent) => { event.preventDefault(); }); diff --git a/apps/obsidian/src/index.ts b/apps/obsidian/src/index.ts index 6df6af506..059878381 100644 --- a/apps/obsidian/src/index.ts +++ b/apps/obsidian/src/index.ts @@ -284,9 +284,7 @@ export default class DiscourseGraphPlugin extends Plugin { }), ); - // Dispatch a no-op CM6 transaction to every markdown editor so their - // ViewPlugin re-evaluates hasVisibleCanvasLeaf and shows/hides widgets. - // layout-change covers splits/moves, active-leaf-change covers tab switches. + // Re-evaluate ViewPlugins on splits/moves (layout-change) and tab switches. const refreshEditors = (): void => refreshMarkdownEditors(this.app); this.registerEvent(this.app.workspace.on("layout-change", refreshEditors)); this.registerEvent( @@ -497,9 +495,7 @@ export default class DiscourseGraphPlugin extends Plugin { this.fileChangeListener = null; } - // The popover lives on document.body with its own listeners, so it would - // otherwise outlive the plugin — including an Escape handler that would go - // on swallowing the key for the rest of the session. + // Lives on document.body with its own listeners; would outlive the plugin. closeDiscourseContextPopover(); this.relationsIndex.unload(); } diff --git a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts index 83eefc284..43986daac 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayExtension.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayExtension.ts @@ -25,10 +25,7 @@ class DiscourseContextBadgeWidget extends WidgetType { super(); } - /** - * Keyed on everything the badge displays, so it is rebuilt when its content - * changes and left alone on every other keystroke in the document. - */ + /** Keyed on what the badge displays, so keystrokes elsewhere do not rebuild it. */ eq(other: DiscourseContextBadgeWidget): boolean { return ( this.target.file.path === other.target.file.path && @@ -53,11 +50,7 @@ class DiscourseContextBadgeWidget extends WidgetType { }); } - /** - * Left at the CM6 default of true: the editor ignores events on the widget, - * so the badge's own click listener fires natively. Returning false hands the - * event to CM's input handling instead and the badge never reacts. - */ + /** True (the CM6 default) means the editor ignores the event, so our click handler runs. */ ignoreEvent(): boolean { return true; } @@ -68,7 +61,7 @@ const buildBadgeDecorations = ( plugin: DiscourseGraphPlugin, ): DecorationSet => { if (!plugin.settings.showDiscourseContextOverlay) return Decoration.none; - // Source mode shows raw markdown; a badge there would be noise. + // Source mode shows raw markdown; a badge there is noise. if (!view.state.field(editorLivePreviewField, false)) return Decoration.none; const sourcePath = view.state.field(editorInfoField, false)?.file?.path; @@ -108,14 +101,7 @@ const buildBadgeDecorations = ( return Decoration.set(widgets, true); }; -/** - * Renders the discourse context badge after each link to a discourse node in - * Live Preview. - * - * Rebuilds on document and viewport changes. Changes that originate outside the - * document — a relation added, a target's frontmatter finishing indexing — - * arrive as an empty transaction from registerDiscourseContextOverlayRefresh. - */ +/** Renders the badge after each discourse-node link in Live Preview. */ export const createDiscourseContextOverlayExtension = ( plugin: DiscourseGraphPlugin, ): ViewPlugin => @@ -132,11 +118,8 @@ export const createDiscourseContextOverlayExtension = ( } update(update: ViewUpdate): void { - // Everything that changes a badge from outside the document — the - // setting, and the relation counts themselves — arrives as an empty - // transaction, which changes neither the document nor the viewport. Both - // have to be compared explicitly, or the redraw silently does nothing - // and badges keep a count from before the last relation change. + // Setting and relation changes arrive as an empty transaction, which + // changes neither doc nor viewport, so both need comparing explicitly. const enabled = plugin.settings.showDiscourseContextOverlay; const indexVersion = plugin.relationsIndex.getVersion(); if ( diff --git a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts index 97a623e3a..271e54f39 100644 --- a/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts +++ b/apps/obsidian/src/utils/discourseContextOverlayPostProcessor.ts @@ -14,15 +14,6 @@ import { resolveDiscourseLinkTarget } from "./discourseLinkUtils"; import { getNodeTypeIdFromFrontmatter } from "./discourseLinkFrontmatter"; import { refreshMarkdownEditors } from "./markdownViewRefresh"; -/** - * Reading view's counterpart to the Live Preview extension. - * - * Obsidian runs post processors over rendered sections and reuses those - * sections, so this must be safe to run repeatedly over content that already - * has badges — hence the marker-class check per link rather than a one-shot - * pass. The same guard covers hover previews and exports, which render through - * this path too. - */ const REFRESH_DEBOUNCE_MS = 300; /** Only a discourse node's own frontmatter can change what a badge shows. */ @@ -35,11 +26,8 @@ const isDiscourseNodeFile = ( ); /** - * Adds, updates or removes the badge on every discourse-node link inside `el`. - * - * Safe to run repeatedly over the same content, which it has to be: Obsidian - * reuses rendered sections and re-runs post processors over them, and the - * refresh below re-applies this in place rather than re-rendering. + * Adds, updates or removes the badge on every discourse-node link in `el`. + * Idempotent: Obsidian reuses rendered sections and re-runs post processors. */ export const applyDiscourseContextBadges = ({ plugin, @@ -87,9 +75,7 @@ export const applyDiscourseContextBadges = ({ }), }); - // Replaced rather than skipped: Obsidian reuses rendered sections, so a - // badge left in place would keep showing a count from before the last - // relation change. + // Replaced, not skipped, or it keeps a count from before the last change. existing?.remove(); link.insertAdjacentElement("afterend", badge); } @@ -111,15 +97,8 @@ export const createDiscourseContextOverlayPostProcessor = }; /** - * Redraws both overlay surfaces when something they depend on changes outside - * the document they render — a relation added or removed, or a target's - * frontmatter finishing indexing. - * - * Reading view has no equivalent of CM6's update cycle, so nothing re-runs the - * post processor on its own. It is refreshed by re-applying badges over the - * already-rendered content rather than by calling previewMode.rerender(): - * rerender tears the preview down, and a pane that is not currently painting - * never rebuilds it, leaving Reading view permanently blank. + * Redraws both surfaces when relations or frontmatter change. Reading view is + * refreshed in place: rerender() blanks a pane that is not currently painting. */ export const refreshDiscourseContextOverlaySurfaces = ( plugin: DiscourseGraphPlugin, @@ -149,11 +128,7 @@ export const registerDiscourseContextOverlayRefresh = ( ); plugin.register(plugin.relationsIndex.onChange(refresh)); - // A link only resolves once its target's frontmatter is cached, so a note - // rendered before that lands needs a second pass. - // - // Scoped to "changed" rather than "resolved" on purpose: "resolved" also - // fires while a preview renders, which would make this re-entrant. + // "changed", not "resolved": resolved also fires while a preview renders. plugin.registerEvent( plugin.app.metadataCache.on("changed", (file) => { if (!isDiscourseNodeFile(plugin, file)) return; diff --git a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts index 444ab4d9f..168235760 100644 --- a/apps/obsidian/src/utils/discourseLinkFrontmatter.ts +++ b/apps/obsidian/src/utils/discourseLinkFrontmatter.ts @@ -1,11 +1,5 @@ import type { RelationInstance } from "~/types"; -/** - * Pure frontmatter/relation helpers behind discourseLinkUtils, kept free of - * Obsidian imports so the counting rules can be read and changed without - * untangling them from link resolution. - */ - const asString = (value: unknown): string | undefined => typeof value === "string" && value.length > 0 ? value : undefined; @@ -13,13 +7,7 @@ export const getNodeTypeIdFromFrontmatter = ( frontmatter: Record | undefined, ): string | undefined => asString(frontmatter?.nodeTypeId); -/** - * The ids a file's relations can be filed under. - * - * An imported node is referenced by its local nodeInstanceId and, in relations - * that arrived with the import, by its importedFromRid — so both must be - * queried for its relations to be found. - */ +/** An imported node is referenced by both its nodeInstanceId and its importedFromRid. */ export const getEndpointIdsFromFrontmatter = ( frontmatter: Record | undefined, ): string[] => { @@ -36,16 +24,8 @@ export const getEndpointIdsFromFrontmatter = ( }; /** - * Counts the relations the Discourse Context panel would actually list. - * - * Two kinds are excluded, and both have to be, or the badge advertises context - * the panel then refuses to show: - * - * - `tentative === false` marks an imported relation the user has not accepted - * yet, which the panel lists separately. Local relations leave it undefined. - * - A relation whose type is no longer configured is orphaned — deleting a - * relation type leaves its relations behind in relations.json — and the panel - * silently drops those. + * Counts what the panel would list. Excludes unaccepted imports and relations + * orphaned by a deleted relation type, both of which the panel hides. */ export const countDisplayableRelations = ({ relations, diff --git a/apps/obsidian/src/utils/discourseLinkUtils.ts b/apps/obsidian/src/utils/discourseLinkUtils.ts index 24d176ce7..28cf5e873 100644 --- a/apps/obsidian/src/utils/discourseLinkUtils.ts +++ b/apps/obsidian/src/utils/discourseLinkUtils.ts @@ -15,21 +15,8 @@ export type DiscourseLinkTarget = { }; /** - * Resolves a link to a discourse node and its relation count, synchronously. - * - * Every read here hits an already-in-memory cache — Obsidian's metadataCache - * for frontmatter, the plugin's settings for node types, and RelationsIndex for - * relations — because this runs per link on a render path, once per viewport - * update. - * - * Deliberately does not use getNodeTypeIdForFile/getNodeInstanceIdForFile: those - * poll for up to 500ms waiting on frontmatter for a just-created file, which is - * right for relation bookkeeping and wrong for rendering. If frontmatter is not - * cached yet this returns null and the caller redraws when the index or the - * metadata cache next reports a change. - * - * Returns null when the link does not resolve, the target is not a discourse - * node, or its node type is no longer configured. + * Resolves a link to a discourse node and its relation count from in-memory + * caches only; avoids getNodeTypeIdForFile, which polls 500ms for frontmatter. */ export const resolveDiscourseLinkTarget = ({ plugin, @@ -40,7 +27,7 @@ export const resolveDiscourseLinkTarget = ({ linktext: string; sourcePath: string; }): DiscourseLinkTarget | null => { - // Strips any #heading or #^block subpath, which is not part of the file path. + // Strips any #heading or #^block subpath. const { path } = parseLinktext(linktext); if (!path) return null; diff --git a/apps/obsidian/src/utils/internalLinkParsing.ts b/apps/obsidian/src/utils/internalLinkParsing.ts index 14b5877dc..d95edb4f3 100644 --- a/apps/obsidian/src/utils/internalLinkParsing.ts +++ b/apps/obsidian/src/utils/internalLinkParsing.ts @@ -1,23 +1,9 @@ -/** - * Pure parsing for internal links in raw markdown, kept free of Obsidian and - * CodeMirror imports so it can be reused by any caller that has raw markdown - * rather than an editor. - */ +// Shared by the CM6 extensions that scan raw markdown for internal links. -/** - * Wikilinks `[[...]]` and markdown links `[text](path.md)`. - * - * Embeds are not excluded here: the leading `!` sits outside the match, so the - * caller has to check the preceding character. - */ +/** Embeds are not matched: the leading `!` sits outside, so callers check it. */ export const INTERNAL_LINK_RE = /\[\[([^\]]+)\]\]|\[([^\]]+)\]\(([^)]+\.md)\)/g; -/** - * Extracts the link target from a wikilink or markdown link match. - * - * Any `#heading` subpath is left in place; resolving it is the caller's job, - * since Obsidian's own parseLinktext handles that. - */ +/** Target of a wikilink or markdown link; any `#subpath` is left for parseLinktext. */ export const extractLinktext = (match: string): string => { if (match.startsWith("[[")) { const inner = match.slice(2, -2); diff --git a/apps/obsidian/src/utils/markdownViewRefresh.ts b/apps/obsidian/src/utils/markdownViewRefresh.ts index 5f8652737..cc54f96bb 100644 --- a/apps/obsidian/src/utils/markdownViewRefresh.ts +++ b/apps/obsidian/src/utils/markdownViewRefresh.ts @@ -9,11 +9,8 @@ export const hasCodeMirrorView = (editor: unknown): editor is EditorWithCm => { }; /** - * Dispatches an empty CM6 transaction to every open markdown editor, which - * forces each ViewPlugin's update() to run and rebuild its decorations. - * - * Needed whenever something a ViewPlugin reads changes outside the editor — - * a setting, or which leaves are visible — since CM6 has no way to know. + * Empty CM6 transaction to every open editor, forcing ViewPlugin.update() to + * run when something it reads changes outside the editor. */ export const refreshMarkdownEditors = (app: App): void => { app.workspace.iterateAllLeaves((leaf) => { diff --git a/apps/obsidian/src/utils/relationsEndpointIndex.ts b/apps/obsidian/src/utils/relationsEndpointIndex.ts index 034dadc9e..f200a392e 100644 --- a/apps/obsidian/src/utils/relationsEndpointIndex.ts +++ b/apps/obsidian/src/utils/relationsEndpointIndex.ts @@ -1,18 +1,8 @@ import type { RelationInstance } from "~/types"; /** - * Pure indexing helpers behind RelationsIndex, kept free of Obsidian and - * relationsStore imports so the grouping and dedupe rules stay separate from - * snapshot loading and invalidation. - */ - -/** - * Groups relations by the node instance ids at either end, so a lookup by - * endpoint is a Map hit instead of a scan over every relation in the vault. - * - * A relation is filed under both its source and its destination. Self-relations - * (source === destination) are filed once so a single endpoint never yields the - * same relation twice. + * Groups relations by the ids at either end, so a lookup is a Map hit rather + * than a scan. Self-relations are filed once, not twice. */ export const buildEndpointIndex = ( relations: Record, @@ -39,13 +29,7 @@ export const buildEndpointIndex = ( return index; }; -/** - * Returns every relation touching any of `endpointIds`, deduplicated by id. - * - * A relation whose source and destination are both in `endpointIds` — which - * happens for an imported node matched by both its nodeInstanceId and its - * importedFromRid — must still be counted once. - */ +/** Relations touching any of `endpointIds`, deduped: an imported node matches on two ids. */ export const collectRelations = ({ index, endpointIds, diff --git a/apps/obsidian/src/utils/relationsIndex.ts b/apps/obsidian/src/utils/relationsIndex.ts index 8aa1e4713..64b154087 100644 --- a/apps/obsidian/src/utils/relationsIndex.ts +++ b/apps/obsidian/src/utils/relationsIndex.ts @@ -5,16 +5,8 @@ import { getRelationsFilePath, loadRelations } from "./relationsStore"; import { buildEndpointIndex, collectRelations } from "./relationsEndpointIndex"; /** - * In-memory view of relations.json. - * - * Reading relations straight from disk costs a full vault file read plus a JSON - * parse per call, which is fine for the Discourse Context panel but not for - * anything that renders per link. This keeps a parsed snapshot so callers on a - * render path can ask a synchronous question and get an answer. - * - * The snapshot is rebuilt from the vault's own modify/create/delete events, so - * writes made through saveRelations and edits arriving over sync are picked up - * the same way, without relationsStore needing to know this exists. + * Parsed snapshot of relations.json so a render path can ask synchronously, + * rebuilt from vault events (which covers our own writes and sync alike). */ export class RelationsIndex { private plugin: DiscourseGraphPlugin; @@ -22,19 +14,10 @@ export class RelationsIndex { private inFlight: Promise | null = null; private stale = false; private unloaded = false; - /** - * Incremented every time the snapshot is replaced. Lets a caller that cannot - * subscribe — a CodeMirror ViewPlugin, whose update() only sees transactions — - * detect that counts changed by comparing versions. - */ + /** Lets a ViewPlugin, which only sees transactions, detect a changed snapshot. */ private version = 0; private subscribers = new Set<() => void>(); - /** - * Bumped on every invalidation. A load that started before the bump is stale - * by the time it resolves, so it must not overwrite a newer snapshot — - * relations.json being modified mid-read is the normal case here, not an edge - * one, since saving a relation triggers exactly that. - */ + /** Guards against a load that started before an invalidation overwriting a newer one. */ private generation = 0; constructor(plugin: DiscourseGraphPlugin) { @@ -69,10 +52,7 @@ export class RelationsIndex { return this.version; } - /** - * Notifies when the snapshot changes, so a caller that rendered against a - * cold or stale index can render again. Returns an unsubscribe function. - */ + /** Fires when the snapshot changes. Returns an unsubscribe function. */ onChange(subscriber: () => void): () => void { this.subscribers.add(subscriber); return () => this.subscribers.delete(subscriber); @@ -87,20 +67,16 @@ export class RelationsIndex { this.inFlight = (async () => { try { const relationsFile = await loadRelations(this.plugin); - // A newer invalidation landed mid-read, so this result is already out - // of date; the reload it scheduled will supersede it. + // Superseded mid-read; the invalidation already scheduled a reload. if (generation !== this.generation || this.unloaded) return; this.index = buildEndpointIndex(relationsFile.relations ?? {}); this.stale = false; this.version += 1; } finally { - // Must clear on every path. Leaving it set would make ensureLoaded - // hand out a settled promise forever, so the snapshot would stay stale - // and every read would re-request a load that never runs. + // Every path, or ensureLoaded hands out a settled promise forever. this.inFlight = null; } - // An invalidation that arrived mid-read was skipped above; it still needs - // a load of its own. + // The skipped invalidation above still needs a load of its own. if (this.stale && !this.unloaded) { void this.ensureLoaded(); return; @@ -112,15 +88,8 @@ export class RelationsIndex { } /** - * Relations touching any of `endpointIds`. - * - * Returns an empty array while the snapshot is still cold; subscribers are - * notified once it lands. Callers on a render path should treat an empty - * result as "nothing to draw yet" rather than "no relations". - * - * Deliberately does not schedule a load — initialize() and invalidate() are - * the only things that do. Requesting one from a render path would make - * notify -> re-render -> read cycle forever. + * Empty while cold, so treat that as "not loaded yet", not "no relations". + * Never schedules a load: that would make notify -> re-render -> read loop. */ getRelationsForEndpointIds( endpointIds: Iterable, @@ -129,14 +98,7 @@ export class RelationsIndex { return collectRelations({ index: this.index, endpointIds }); } - /** - * Marks the snapshot for reload without discarding it. - * - * Dropping it outright would make every badge read 0 until the reload lands — - * and since saving a relation writes relations.json, that flash would happen - * on the very action the user just took. The previous counts are a better - * answer for those few milliseconds than a wrong one. - */ + /** Keeps the old snapshot while reloading, so badges do not flash to 0. */ private invalidate(): void { this.generation += 1; this.inFlight = null; diff --git a/apps/obsidian/src/utils/wikilinkDragHandler.ts b/apps/obsidian/src/utils/wikilinkDragHandler.ts index 12872979f..a1cde6245 100644 --- a/apps/obsidian/src/utils/wikilinkDragHandler.ts +++ b/apps/obsidian/src/utils/wikilinkDragHandler.ts @@ -10,6 +10,7 @@ import { import { TFile, WorkspaceLeaf } from "obsidian"; import { VIEW_TYPE_TLDRAW_DG_PREVIEW } from "~/constants"; import type DiscourseGraphPlugin from "~/index"; +import { extractLinktext, INTERNAL_LINK_RE } from "./internalLinkParsing"; const buildObsidianUrl = (vaultName: string, filePath: string): string => { return `obsidian://open?vault=${encodeURIComponent(vaultName)}&file=${encodeURIComponent(filePath)}`; @@ -42,29 +43,6 @@ const setDragData = ( // --- Live Preview --- -/** - * Extract the file path from a link match. - * Handles wikilinks (`[[path]]`, `[[path|alias]]`) and - * markdown links (`[text](path.md)`), decoding URL-encoded paths. - */ -const extractLinkPath = (match: string): string => { - // Wikilink: [[path]] or [[path|alias]] - if (match.startsWith("[[")) { - const inner = match.slice(2, -2); - const pipeIndex = inner.indexOf("|"); - return pipeIndex >= 0 ? inner.slice(0, pipeIndex) : inner; - } - - // Markdown link: [text](path) - const parenOpen = match.lastIndexOf("("); - const rawPath = match.slice(parenOpen + 1, -1); - try { - return decodeURIComponent(rawPath); - } catch (error) { - return rawPath; - } -}; - /** * Widget that renders a small drag handle next to an internal link. * CM6 widgets get `ignoreEvent() → true` by default, which means @@ -103,10 +81,6 @@ class WikilinkDragHandleWidget extends WidgetType { } } -// Matches wikilinks [[...]] and markdown links [text](path.md). -// Embed exclusion (![[...]] and ![text](...)) is handled in the loop. -const INTERNAL_LINK_RE = /\[\[([^\]]+)\]\]|\[([^\]]+)\]\(([^)]+\.md)\)/g; - const hasVisibleCanvasLeaf = (plugin: DiscourseGraphPlugin): boolean => plugin.app.workspace .getLeavesOfType(VIEW_TYPE_TLDRAW_DG_PREVIEW) @@ -133,7 +107,7 @@ const buildWidgetDecorations = ( view.state.doc.sliceString(checkPos, checkPos + 1) === "!"; if (isEmbed) continue; const matchEnd = from + match.index + match[0].length; - const linkPath = extractLinkPath(match[0]); + const linkPath = extractLinktext(match[0]); const widget = new WikilinkDragHandleWidget(linkPath, plugin); widgets.push(Decoration.widget({ widget, side: 1 }).range(matchEnd)); } From 050f5b8af08fd481327772aee9004b42d74e1ae2 Mon Sep 17 00:00:00 2001 From: Trang Doan Date: Mon, 7 Sep 2026 17:32:45 -0400 Subject: [PATCH 11/11] ENG-1249 Restore five files this branch only reformatted A repo-wide prettier run pulled these into the diff; they are unformatted on main, so staging them makes the pre-commit hook reformat them again. Committed with --no-verify so the revert sticks. Co-Authored-By: Claude Opus 5 --- .../canvas/utils/externalContentHandlers.ts | 15 ++++++--------- .../src/components/canvas/utils/toastUtils.ts | 2 +- apps/obsidian/src/utils/calcDiscourseNodeSize.ts | 1 + apps/obsidian/src/utils/colorUtils.ts | 1 + apps/obsidian/src/utils/loadImage.ts | 3 ++- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/obsidian/src/components/canvas/utils/externalContentHandlers.ts b/apps/obsidian/src/components/canvas/utils/externalContentHandlers.ts index 5ead3815e..643fb2de4 100644 --- a/apps/obsidian/src/components/canvas/utils/externalContentHandlers.ts +++ b/apps/obsidian/src/components/canvas/utils/externalContentHandlers.ts @@ -63,7 +63,9 @@ const resolveObsidianUrlToFile = ( let abstract = plugin.app.vault.getAbstractFileByPath(parsed.filePath); if (!(abstract instanceof TFile) && !parsed.filePath.endsWith(".md")) { - abstract = plugin.app.vault.getAbstractFileByPath(`${parsed.filePath}.md`); + abstract = plugin.app.vault.getAbstractFileByPath( + `${parsed.filePath}.md`, + ); } return abstract instanceof TFile ? abstract : null; }; @@ -78,8 +80,7 @@ const isDiscourseNodeFile = ( ): boolean => { if (!file.path.endsWith(".md")) return false; const frontmatter = getFrontmatterForFile(plugin.app, file); - const nodeTypeId = (frontmatter as { nodeTypeId?: string } | null) - ?.nodeTypeId; + const nodeTypeId = (frontmatter as { nodeTypeId?: string } | null)?.nodeTypeId; if (!nodeTypeId || typeof nodeTypeId !== "string") return false; return !!getNodeTypeById(plugin, nodeTypeId); }; @@ -115,9 +116,7 @@ export const handleExternalUrlContent = async ({ if (url.startsWith(OBSIDIAN_URL_PREFIX)) { const parsed = parseObsidianOpenUrl(url); if (!parsed) { - new Notice( - "Invalid Obsidian link. Only discourse nodes can be dropped on the canvas.", - ); + new Notice("Invalid Obsidian link. Only discourse nodes can be dropped on the canvas."); return; } @@ -183,9 +182,7 @@ const createDiscourseNodeShapeAtPoint = async ({ if (existing) { editor.setSelectedShapes([existing.id]); - editor.zoomToSelection({ - animation: { duration: editor.options.animationMediumMs }, - }); + editor.zoomToSelection({ animation: { duration: editor.options.animationMediumMs } }); return; } diff --git a/apps/obsidian/src/components/canvas/utils/toastUtils.ts b/apps/obsidian/src/components/canvas/utils/toastUtils.ts index e07a45b54..8b6c34cb1 100644 --- a/apps/obsidian/src/components/canvas/utils/toastUtils.ts +++ b/apps/obsidian/src/components/canvas/utils/toastUtils.ts @@ -20,4 +20,4 @@ export const showToast = ({ keepOpen: false, }; dispatchToastEvent(toast, targetCanvasId); -}; +}; \ No newline at end of file diff --git a/apps/obsidian/src/utils/calcDiscourseNodeSize.ts b/apps/obsidian/src/utils/calcDiscourseNodeSize.ts index 4cd105fdd..e17c7c5d3 100644 --- a/apps/obsidian/src/utils/calcDiscourseNodeSize.ts +++ b/apps/obsidian/src/utils/calcDiscourseNodeSize.ts @@ -72,3 +72,4 @@ export const calcDiscourseNodeSize = async ({ return { w, h: textHeight }; } }; + diff --git a/apps/obsidian/src/utils/colorUtils.ts b/apps/obsidian/src/utils/colorUtils.ts index a1ed9503c..091667fa9 100644 --- a/apps/obsidian/src/utils/colorUtils.ts +++ b/apps/obsidian/src/utils/colorUtils.ts @@ -55,6 +55,7 @@ export const getNodeTagColors = ( return { backgroundColor, textColor }; }; + export const getAllDiscourseNodeColors = ( nodeTypes: DiscourseNode[], ): Array<{ diff --git a/apps/obsidian/src/utils/loadImage.ts b/apps/obsidian/src/utils/loadImage.ts index 942b39bcc..08bc182a0 100644 --- a/apps/obsidian/src/utils/loadImage.ts +++ b/apps/obsidian/src/utils/loadImage.ts @@ -1,7 +1,7 @@ /** * Load an image and return its natural dimensions. * Supports both vault resource paths (app://...) and external URLs (https://...). - * + * * Note: This works with Obsidian's resource paths returned by app.vault.getResourcePath() * which are special app:// protocol URLs handled by Obsidian's Electron environment. */ @@ -38,3 +38,4 @@ export const loadImage = ( img.src = url; }); }; +