From 7627a05af454b8eb20cd8e04892b64d460a0ba7c Mon Sep 17 00:00:00 2001 From: Lars Date: Fri, 6 Feb 2026 13:02:33 +0100 Subject: [PATCH] Implement inverse edge existence check and enhance edge creation logic - Added `checkInverseEdgeExists` function to verify if an inverse edge already exists between two nodes, preventing duplicate edges from being created. - Updated `createInverseEdge` to utilize the new check, ensuring that inverse edges are only created when necessary. - Enhanced logging for better traceability during edge creation and existence checks, improving overall debugging capabilities. - Introduced a new function `getInverseEdgeType` to retrieve the inverse edge type from the vocabulary, supporting the new functionality. --- src/analysis/templateMatching.ts | 104 ++++++++++++++++++++++- src/ui/ChainWorkbenchModal.ts | 106 +++++++++++++++++------ src/workbench/writerActions.ts | 140 +++++++++++++++++++++++++++++++ 3 files changed, 323 insertions(+), 27 deletions(-) diff --git a/src/analysis/templateMatching.ts b/src/analysis/templateMatching.ts index bc02ef8..b1454ed 100644 --- a/src/analysis/templateMatching.ts +++ b/src/analysis/templateMatching.ts @@ -427,6 +427,20 @@ function normalizePathForComparison(path: string): string { * Uses resolved paths and headingsMatch for heading comparison (block-id variants). * Paths are normalized so different slash styles (e.g. Windows) still match. */ +/** + * Get inverse edge type for a canonical type from edge vocabulary. + */ +function getInverseEdgeType( + canonical: string | undefined, + edgeVocabulary: EdgeVocabulary | null +): string | null { + if (!canonical || !edgeVocabulary) { + return null; + } + const entry = edgeVocabulary.byCanonical.get(canonical); + return entry?.inverse ?? null; +} + function findEdgeBetween( fromKey: string, toKey: string, @@ -446,6 +460,7 @@ function findEdgeBetween( // This prevents early return when a different edge type matches first const hasAllowedRoles = allowedEdgeRoles && allowedEdgeRoles.length > 0; + // First, try to find edge in forward direction (from→to) for (const edge of allEdges) { const sourceFile = "sectionHeading" in edge.source ? edge.source.file : edge.source.file; const resolvedSourceFile = sourceFile.includes("/") || sourceFile.endsWith(".md") @@ -481,9 +496,6 @@ function findEdgeBetween( const canonical = canonicalEdgeType(edge.rawEdgeType); const edgeRole = getEdgeRole(edge.rawEdgeType, canonical, chainRoles); - // Debug logging for guides edges (only via logger, controlled by log level) - // Removed console.log - use getTemplateMatchingLogger().debug() if needed - // CRITICAL FIX: If allowedEdgeRoles is specified, only return if this edge's role matches // This prevents early return when a different edge type matches first. // This fixes the issue where edges later in the list (e.g., guides at index 12) were @@ -505,6 +517,92 @@ function findEdgeBetween( } } + // PROBLEM 2 FIX: If no edge found in forward direction, check reverse direction (to→from) + // This handles cases where only a backlink exists (inverse edge) + // A chain should be considered complete if the semantic relationship exists in either direction + for (const edge of allEdges) { + const sourceFile = "sectionHeading" in edge.source ? edge.source.file : edge.source.file; + const resolvedSourceFile = sourceFile.includes("/") || sourceFile.endsWith(".md") + ? sourceFile + : edgeTargetResolutionMap.get(sourceFile) || sourceFile; + let resolvedTargetFile = edgeTargetResolutionMap.get(edge.target.file); + if (!resolvedTargetFile) { + if (!edge.target.file.includes("/") && !edge.target.file.endsWith(".md")) { + const sourceBasename = resolvedSourceFile.split("/").pop()?.replace(/\.md$/, "") || ""; + if (sourceBasename === edge.target.file || resolvedSourceFile.endsWith(edge.target.file)) { + resolvedTargetFile = resolvedSourceFile; + } + } + if (!resolvedTargetFile) { + resolvedTargetFile = edge.target.file; + } + } + const resolvedSourceNorm = normalizePathForComparison(resolvedSourceFile); + const resolvedTargetNorm = normalizePathForComparison(resolvedTargetFile); + + const sourceHeading = "sectionHeading" in edge.source ? edge.source.sectionHeading : null; + const targetHeading = edge.target.heading; + + // Check reverse direction: edge goes from to→from (inverse of what we're looking for) + if ( + resolvedSourceNorm === toFileNorm && + headingsMatch(sourceHeading, toHeading) && + resolvedTargetNorm === fromFileNorm && + headingsMatch(targetHeading, fromHeading) + ) { + // This is a reverse edge - check if it's the inverse of an allowed edge type + const canonical = canonicalEdgeType(edge.rawEdgeType); + + // If we have allowedEdgeRoles, we need to check if this inverse edge's role matches + // For inverse edges, we need to find the forward canonical type and check its role + if (hasAllowedRoles && allowedEdgeRoles.length > 0) { + // For each allowed role, check if this inverse edge's canonical type is the inverse + // of a canonical type that has that role + let matchedRole: string | null = null; + + if (chainRoles && canonical) { + // Check if this canonical type (which is the inverse) corresponds to a forward type + // that has one of the allowed roles + for (const [roleName, role] of Object.entries(chainRoles.roles)) { + if (allowedEdgeRoles.includes(roleName)) { + // Check if any edge type in this role has an inverse that matches our edge + for (const forwardType of role.edge_types) { + const forwardCanonical = canonicalEdgeType(forwardType); + if (forwardCanonical) { + const inverseOfForward = getInverseEdgeType(forwardCanonical, edgeVocabulary); + if (inverseOfForward === canonical) { + // This inverse edge corresponds to a forward edge with the allowed role! + matchedRole = roleName; + break; + } + } + } + if (matchedRole) break; + } + } + } + + if (matchedRole) { + // Return the inverse edge, but indicate it's in reverse direction + // The role is the forward role (e.g., "guides" for a "guided_by" edge) + return { edgeRole: matchedRole, rawEdgeType: edge.rawEdgeType }; + } + // Continue searching + continue; + } + + // No allowed roles restriction: if we found a reverse edge, it means the relationship exists + // Return it with the inferred role (if possible) + const edgeRole = canonical && chainRoles ? getEdgeRole(edge.rawEdgeType, canonical, chainRoles) : null; + // Try to infer role from the inverse edge type + const inferredRole = edgeRole || (chainRoles ? inferRoleFromRawType(edge.rawEdgeType, chainRoles) : null); + + // For inverse edges, we might need to map the role back to the forward role + // But for now, return the edge as-is - the relationship exists in reverse direction + return { edgeRole: inferredRole, rawEdgeType: edge.rawEdgeType }; + } + } + return null; } diff --git a/src/ui/ChainWorkbenchModal.ts b/src/ui/ChainWorkbenchModal.ts index 2f4a977..ac12191 100644 --- a/src/ui/ChainWorkbenchModal.ts +++ b/src/ui/ChainWorkbenchModal.ts @@ -23,6 +23,8 @@ export class ChainWorkbenchModal extends Modal { private selectedMatch: WorkbenchMatch | null = null; private filterStatus: string | null = null; private searchQuery: string = ""; + private treeContainer: HTMLElement | null = null; + private clickHandlerBound: ((e: MouseEvent) => void) | null = null; constructor( app: App, @@ -86,13 +88,17 @@ export class ChainWorkbenchModal extends Modal { const mainContainer = contentEl.createDiv({ cls: "workbench-main" }); // Left: Tree View - const treeContainer = mainContainer.createDiv({ cls: "workbench-tree" }); - treeContainer.createEl("h3", { text: "Templates & Chains" }); + this.treeContainer = mainContainer.createDiv({ cls: "workbench-tree" }); + this.treeContainer.createEl("h3", { text: "Templates & Chains" }); // Right: Details View const detailsContainer = mainContainer.createDiv({ cls: "workbench-details" }); detailsContainer.createEl("h3", { text: "Chain Details" }); + // Register click handler ONCE (event delegation) + this.clickHandlerBound = this.handleTreeClick.bind(this); + this.treeContainer.addEventListener("click", this.clickHandlerBound); + this.render(); } @@ -140,15 +146,15 @@ export class ChainWorkbenchModal extends Modal { } private renderTreeView(matches: WorkbenchMatch[]): void { - const treeContainer = this.contentEl.querySelector(".workbench-tree"); - if (!treeContainer) return; + if (!this.treeContainer) return; - treeContainer.empty(); - treeContainer.createEl("h3", { text: "Templates & Chains" }); + // Clear content but keep the container and event handler + this.treeContainer.empty(); + this.treeContainer.createEl("h3", { text: "Templates & Chains" }); // Show message if no matches if (matches.length === 0) { - const emptyMessage = treeContainer.createDiv({ cls: "workbench-empty-message" }); + const emptyMessage = this.treeContainer.createDiv({ cls: "workbench-empty-message" }); emptyMessage.createEl("p", { text: "Keine Chains gefunden." }); if (this.filterStatus || this.searchQuery) { emptyMessage.createEl("p", { @@ -169,11 +175,16 @@ export class ChainWorkbenchModal extends Modal { } // Create template tree - const templateTree = treeContainer.createDiv({ cls: "template-tree" }); + const templateTree = this.treeContainer.createDiv({ cls: "template-tree" }); + + // Store match references for event delegation + const matchMap = new Map(); + const templateMatchCounts = new Map(); for (const [templateName, templateMatches] of matchesByTemplate.entries()) { const templateItem = templateTree.createDiv({ cls: "template-tree-item" }); + // Templates start collapsed by default const templateHeader = templateItem.createDiv({ cls: "template-tree-header" }); templateHeader.createSpan({ cls: "template-tree-toggle", text: "▶" }); templateHeader.createSpan({ cls: "template-tree-name", text: templateName }); @@ -181,6 +192,10 @@ export class ChainWorkbenchModal extends Modal { const chainsContainer = templateItem.createDiv({ cls: "template-tree-chains" }); + // Add data attribute to identify template header and store match count + templateHeader.setAttribute("data-template-name", templateName); + templateMatchCounts.set(templateHeader, templateMatches.length); + // Add chains for this template for (const match of templateMatches) { const chainItem = chainsContainer.createDiv({ cls: "chain-item" }); @@ -188,6 +203,9 @@ export class ChainWorkbenchModal extends Modal { chainItem.addClass("selected"); } + // Store match reference for event delegation + matchMap.set(chainItem, match); + const chainHeader = chainItem.createDiv({ cls: "chain-item-header" }); chainHeader.createSpan({ cls: "chain-status-icon", @@ -215,22 +233,52 @@ export class ChainWorkbenchModal extends Modal { text: `Notes: ${Array.from(notes).slice(0, 3).join(", ")}${notes.size > 3 ? "..." : ""}` }); } - - chainItem.addEventListener("click", (e) => { - e.stopPropagation(); - this.selectedMatch = match; - this.render(); - }); - - templateHeader.addEventListener("click", () => { - templateItem.classList.toggle("expanded"); - const toggle = templateHeader.querySelector(".template-tree-toggle"); - if (toggle) { - toggle.textContent = templateItem.classList.contains("expanded") ? "▼" : "▶"; - } - }); } } + + // Store matchMap and counts on the container for event handler access + (this.treeContainer as any).__matchMap = matchMap; + (this.treeContainer as any).__templateMatchCounts = templateMatchCounts; + } + + private handleTreeClick(e: MouseEvent): void { + if (!this.treeContainer) return; + + const target = e.target as HTMLElement; + + // Check if click is on template header or its children + const templateHeader = target.closest(".template-tree-header") as HTMLElement | null; + if (templateHeader) { + e.stopPropagation(); + const templateItem = templateHeader.closest(".template-tree-item") as HTMLElement | null; + if (templateItem) { + const wasExpanded = templateItem.classList.contains("expanded"); + templateItem.classList.toggle("expanded"); + const isExpanded = templateItem.classList.contains("expanded"); + const toggle = templateHeader.querySelector(".template-tree-toggle"); + if (toggle) { + toggle.textContent = isExpanded ? "▼" : "▶"; + } + const matchCounts = (this.treeContainer as any).__templateMatchCounts as Map | undefined; + const matchCount = matchCounts?.get(templateHeader) ?? 0; + console.log(`[Chain Workbench] Template ${templateHeader.getAttribute("data-template-name")} toggled, expanded: ${isExpanded} (was: ${wasExpanded}), matches: ${matchCount}`); + } + return; + } + + // Check if click is on chain item + const chainItem = target.closest(".chain-item") as HTMLElement | null; + if (chainItem) { + e.stopPropagation(); + const matchMap = (this.treeContainer as any).__matchMap as Map | undefined; + const match = matchMap?.get(chainItem); + if (match) { + this.selectedMatch = match; + this.render(); + console.log(`[Chain Workbench] Chain item selected: ${match.templateName}`); + } + return; + } } private renderGlobalTodos(): void { @@ -1231,13 +1279,17 @@ export class ChainWorkbenchModal extends Modal { const mainContainer = contentEl.createDiv({ cls: "workbench-main" }); // Left: Tree View - const treeContainer = mainContainer.createDiv({ cls: "workbench-tree" }); - treeContainer.createEl("h3", { text: "Templates & Chains" }); + this.treeContainer = mainContainer.createDiv({ cls: "workbench-tree" }); + this.treeContainer.createEl("h3", { text: "Templates & Chains" }); // Right: Details View const detailsContainer = mainContainer.createDiv({ cls: "workbench-details" }); detailsContainer.createEl("h3", { text: "Chain Details" }); + // Register click handler ONCE (event delegation) + this.clickHandlerBound = this.handleTreeClick.bind(this); + this.treeContainer.addEventListener("click", this.clickHandlerBound); + // Now render the content console.log("[Chain Workbench] About to call render() - model.matches.length:", this.model.matches.length); this.render(); @@ -1249,6 +1301,12 @@ export class ChainWorkbenchModal extends Modal { } onClose(): void { + // Clean up event handler + if (this.treeContainer && this.clickHandlerBound) { + this.treeContainer.removeEventListener("click", this.clickHandlerBound); + this.clickHandlerBound = null; + } + this.treeContainer = null; const { contentEl } = this; contentEl.empty(); } diff --git a/src/workbench/writerActions.ts b/src/workbench/writerActions.ts index 1decec0..3c6d186 100644 --- a/src/workbench/writerActions.ts +++ b/src/workbench/writerActions.ts @@ -275,6 +275,129 @@ export async function insertEdgeForward( } } +/** + * Check if an inverse edge already exists between two nodes. + * Returns true if an edge exists in reverse direction (to→from) with the inverse edge type. + * PROBLEM 1 FIX: Prevents duplicate inverse edges from being created. + * + * IMPORTANT: This function normalizes edge types (aliases → canonical) before comparison, + * so it works correctly even if the existing edge uses an alias. + */ +async function checkInverseEdgeExists( + app: App, + fromNodeRef: { file: string; heading: string | null }, + toNodeRef: { file: string; heading: string | null }, + inverseEdgeType: string, + vocabulary: Vocabulary +): Promise { + try { + // Build index for target file (where inverse edge would be) + const { buildNoteIndex } = await import("../analysis/graphIndex"); + + // Try to find target file + let targetFile: TFile | null = null; + const possiblePaths = [ + toNodeRef.file, + toNodeRef.file + ".md", + toNodeRef.file.replace(/\.md$/, ""), + toNodeRef.file.replace(/\.md$/, "") + ".md", + ]; + + for (const path of possiblePaths) { + const found = app.vault.getAbstractFileByPath(path); + if (found && found instanceof TFile) { + targetFile = found; + break; + } + } + + // Also try resolving as wikilink + if (!targetFile) { + const basename = toNodeRef.file.replace(/\.md$/, "").split("/").pop() || toNodeRef.file; + const resolved = app.metadataCache.getFirstLinkpathDest(basename, fromNodeRef.file); + if (resolved) { + targetFile = resolved; + } + } + + if (!targetFile) { + return false; // Target file doesn't exist, can't have inverse edge + } + + const { edges } = await buildNoteIndex(app, targetFile); + + // Normalize the expected inverse edge type to canonical (handles aliases) + const expectedInverseCanonical = vocabulary.getCanonical(inverseEdgeType); + if (!expectedInverseCanonical) { + // If inverse type can't be normalized, fall back to direct comparison + console.warn("[checkInverseEdgeExists] Could not normalize inverse edge type:", inverseEdgeType); + } + + // Build source basename for matching + const sourceBasename = fromNodeRef.file.replace(/\.md$/, "").split("/").pop() || fromNodeRef.file; + + // Check if any edge in target file points back to source with inverse type + for (const edge of edges) { + // Normalize the edge's raw type to canonical (handles aliases) + const edgeCanonical = vocabulary.getCanonical(edge.rawEdgeType); + + // Compare canonical types if both can be normalized, otherwise fall back to direct comparison + const typeMatches = expectedInverseCanonical && edgeCanonical + ? edgeCanonical === expectedInverseCanonical + : edge.rawEdgeType.toLowerCase() === inverseEdgeType.toLowerCase(); + + if (!typeMatches) { + continue; + } + + // Check if edge target matches source node + const targetBasename = edge.target.file.replace(/\.md$/, "").split("/").pop() || edge.target.file; + + // Match: edge source is in target file, edge target matches source node + const sourceFile = "sectionHeading" in edge.source ? edge.source.file : edge.source.file; + const sourceFileBasename = sourceFile.replace(/\.md$/, "").split("/").pop() || sourceFile; + + // Check file match + if (sourceFileBasename !== sourceBasename) { + continue; + } + + // Check heading match (if applicable) + const sourceHeading = "sectionHeading" in edge.source ? edge.source.sectionHeading : null; + if (fromNodeRef.heading) { + // Source has heading - edge target must match + if (edge.target.heading !== fromNodeRef.heading) { + continue; + } + } else { + // Source has no heading - edge target should also have no heading (or be note-level) + if (edge.target.heading !== null) { + continue; + } + } + + // Also check source heading if edge is section-level + if ("sectionHeading" in edge.source && sourceHeading) { + // Edge is from a section in target file - this is correct for inverse edges + // The source heading doesn't need to match fromNodeRef.heading for inverse edges + } + + console.log("[checkInverseEdgeExists] Found existing inverse edge:", { + edgeType: edge.rawEdgeType, + sourceFile: sourceFileBasename, + targetFile: targetBasename, + targetHeading: edge.target.heading + }); + return true; // Inverse edge already exists! + } + + return false; + } catch (error) { + console.error("[checkInverseEdgeExists] Error checking for inverse edge:", error); + return false; // On error, assume no inverse edge exists + } +} + /** * Automatically create inverse edge in target note/section. */ @@ -311,6 +434,23 @@ async function createInverseEdge( const inverseEdgeType = inverseCanonical; console.log("[createInverseEdge] Forward:", forwardEdgeType, "-> Canonical:", canonical, "-> Inverse canonical:", inverseCanonical, "-> Using:", inverseEdgeType); + + // PROBLEM 1 FIX: Check if inverse edge already exists before creating + // IMPORTANT: Pass vocabulary to normalize aliases to canonical types + const inverseExists = await checkInverseEdgeExists( + app, + todo.fromNodeRef, + todo.toNodeRef, + inverseEdgeType, + vocabulary + ); + + if (inverseExists) { + console.log("[createInverseEdge] Inverse edge already exists, skipping creation"); + return; // Don't create duplicate + } + + console.log("[createInverseEdge] No inverse edge found, proceeding with creation"); // Find target file const targetFileRef = todo.toNodeRef.file;