feat(slice-032): integrate logger interface across extractors and improve error handling

.github/workflows/_link-check.yml

@@ -32,5 +32,7 @@ jobs:
             --exclude 'https?://localhost'
             --exclude 'file://'
             --exclude 'mailto:'
+            --exclude-path './CHANGELOG.md'
+            --exclude 'github.com/dgtalbug/dextree/issues'
             './**/*.md'
           fail: true

packages/core/src/extractors/frameworks/detector.ts

@@ -22,10 +22,11 @@ export const detectFrameworks: DetectFrameworksFn = async (
     try {
       const hit = await runMatchersFor(def, params);
       if (hit !== null) results.push(hit);
-    } catch {
-      // FR-007: a single framework's matcher failing must not stop others.
-      // Failures are intentionally silent at the detector layer; the indexer
-      // wrapper logs the failure with the framework name for observability.
+    } catch (err) {
+      params.logger?.warn("Framework matcher failed", {
+        framework: def.name,
+        error: err instanceof Error ? err.message : String(err),
+      });
     }
   }
 

packages/core/src/extractors/frameworks/fsIO.ts

@@ -4,16 +4,18 @@ import picomatch from "picomatch";
 
 const ALWAYS_EXCLUDE = new Set(["node_modules", "dist", "build", "out", ".git"]);
 
+import type { Logger } from "../../types.js";
 import type { DetectFrameworksParams } from "./types.js";
 
 /**
  * Build a `DetectFrameworksParams`-compatible fsIO over `workspaceRoot`.
  * Skips heavy dirs (node_modules/dist/build/out/.git) and reads files
  * lazily. Errors during read are surfaced as `null` so matchers stay silent.
  */
-export function createNodeFsIO(workspaceRoot: string): DetectFrameworksParams {
+export function createNodeFsIO(workspaceRoot: string, logger?: Logger): DetectFrameworksParams {
   return {
     workspaceRoot,
+    ...(logger === undefined ? {} : { logger }),
     async readFile(rel: string) {
       try {
         return await readFile(join(workspaceRoot, rel), "utf8");

packages/core/src/extractors/frameworks/types.ts

@@ -7,7 +7,7 @@
  * source of truth for the spec, and this file is the runtime declaration.
  * Keep both in sync.
  */
-import type { FrameworkDetectionSource } from "../../types.js";
+import type { FrameworkDetectionSource, Logger } from "../../types.js";
 
 export interface ManifestKeyPathPresent {
   kind: "present";
@@ -62,6 +62,7 @@ export interface DetectFrameworksParams {
   workspaceRoot: string;
   readFile: (relativePath: string) => Promise<string | null>;
   listFiles: (glob: string) => Promise<readonly string[]>;
+  logger?: Logger;
 }
 
 export type DetectFrameworksFn = (

packages/core/src/extractors/index.ts

@@ -5,6 +5,7 @@ import { ImplementsExtractor } from "./ImplementsExtractor.js";
 import { NaiveCallExtractor } from "./NaiveCallExtractor.js";
 import { createExtractorRegistry } from "./registry.js";
 import type { ExtractorRegistry } from "./types.js";
+import type { Logger } from "../types.js";
 
 /**
  * Builds the default registry with all first-party extractors registered in the
@@ -18,8 +19,8 @@ import type { ExtractorRegistry } from "./types.js";
  * Tests that want isolation should call `createExtractorRegistry()` directly and
  * register only what they need.
  */
-export function createDefaultExtractorRegistry(): ExtractorRegistry {
-  const registry = createExtractorRegistry();
+export function createDefaultExtractorRegistry(logger?: Logger): ExtractorRegistry {
+  const registry = createExtractorRegistry(logger);
   registry.register(new BaselineTsJsExtractor());
   registry.register(new NaiveCallExtractor());
   registry.register(new ClassRelationExtractor());

packages/core/src/extractors/registry.test.ts

@@ -99,8 +99,9 @@ describe("ExtractorRegistry", () => {
   });
 
   it("isolates per-extractor failures and continues with surviving extractors", async () => {
-    const registry = createExtractorRegistry();
-    const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
+    const warn = vi.fn();
+    const logger = { debug: vi.fn(), info: vi.fn(), warn, error: vi.fn() };
+    const registry = createExtractorRegistry(logger);
 
     const broken: Extractor = {
       name: "broken",
@@ -125,10 +126,13 @@ describe("ExtractorRegistry", () => {
     const result = await registry.run(makeInput());
 
     expect(warn).toHaveBeenCalledTimes(1);
+    expect(warn).toHaveBeenCalledWith("Extractor failed", {
+      extractor: "broken",
+      file: "/workspace/src/x.ts",
+      error: "boom",
+    });
     expect(result.edges).toHaveLength(1);
     expect(result.edges[0]?.kind).toBe("OK");
-
-    warn.mockRestore();
   });
 
   it("throws when two extractors both populate `file`", async () => {

packages/core/src/extractors/registry.ts

@@ -1,4 +1,4 @@
-import type { ExtractedImportRef, StoredSymbol } from "../types.js";
+import type { ExtractedImportRef, Logger, StoredSymbol } from "../types.js";
 import type {
   EdgeRow,
   Extractor,
@@ -25,6 +25,11 @@ function toKnownSymbol(s: StoredSymbol): KnownSymbol {
 
 class InMemoryExtractorRegistry implements ExtractorRegistry {
   private readonly extractors: Extractor[] = [];
+  private logger: Logger | undefined;
+
+  constructor(logger?: Logger) {
+    this.logger = logger;
+  }
 
   register(extractor: Extractor): void {
     if (this.extractors.some((existing) => existing.name === extractor.name)) {
@@ -55,13 +60,12 @@ class InMemoryExtractorRegistry implements ExtractorRegistry {
       let result: ExtractionResult;
       try {
         result = await extractor.extract(enrichedInput);
-      } catch (error) {
+      } catch (err) {
         // Per FR-007 / contract: failure isolation. Log and continue.
-        console.warn({
+        this.logger?.warn("Extractor failed", {
           extractor: extractor.name,
-          version: extractor.version,
           file: input.absolutePath,
-          error,
+          error: err instanceof Error ? err.message : String(err),
         });
         continue;
       }
@@ -96,8 +100,8 @@ class InMemoryExtractorRegistry implements ExtractorRegistry {
   }
 }
 
-export function createExtractorRegistry(): ExtractorRegistry {
-  return new InMemoryExtractorRegistry();
+export function createExtractorRegistry(logger?: Logger): ExtractorRegistry {
+  return new InMemoryExtractorRegistry(logger);
 }
 
 export type { Extractor, ExtractorRegistry } from "./types.js";

packages/core/src/index.ts

@@ -36,6 +36,8 @@ import {
   type FrameworkInfo,
   type IndexResult,
   type Indexer,
+  type IndexerFactoryOptions,
+  type Logger,
   type SessionSummary,
   type StoredFile,
   type StoredSymbol,
@@ -60,8 +62,10 @@ export type {
   GraphEdgeKind,
   GraphNode,
   GraphNodeType,
+  IndexerFactoryOptions,
   IndexResult,
   Indexer,
+  Logger,
   SessionSummary,
   StoredFile,
   StoredSymbol,
@@ -122,30 +126,40 @@ const TS_LIKE_LANGUAGES = new Set([
 class DuckTreeIndexer implements Indexer {
   private databaseHandle: DatabaseHandle | null = null;
   private initializationPromise: Promise<void> | null = null;
-  private readonly registry: ExtractorRegistry = createDefaultExtractorRegistry();
+  private readonly registry: ExtractorRegistry;
   private readonly frameworkCache = new Map<string, readonly DetectedFramework[]>();
+  private readonly logger: Logger | undefined;
+  private readonly indexFileInFlight = new Map<string, Promise<IndexResult>>();
 
   constructor(
     private readonly dbPath: string,
     private readonly wasmDir: string,
-  ) {}
+    options?: IndexerFactoryOptions,
+  ) {
+    this.logger = options?.logger;
+    this.registry = createDefaultExtractorRegistry(this.logger);
+  }
 
   async initialize(): Promise<void> {
     if (this.initializationPromise !== null) {
       return this.initializationPromise;
     }
 
     this.initializationPromise = (async () => {
+      this.logger?.debug("Initializing DuckDB", { dbPath: this.dbPath });
+
       if (this.dbPath !== ":memory:") {
         await mkdir(dirname(this.dbPath), { recursive: true });
       }
 
       this.databaseHandle = await openDatabase(this.dbPath);
       await initializeSchema(this.databaseHandle.connection);
-      const migrationResult = await applyMigrations(this.databaseHandle.connection);
+      const migrationResult = await applyMigrations(this.databaseHandle.connection, this.logger);
       if (migrationResult.status === "failed") {
         throw new SchemaError(migrationResult.reason);
       }
+
+      this.logger?.debug("Initialized DuckDB", { dbPath: this.dbPath });
     })();
 
     await this.initializationPromise;
@@ -155,6 +169,25 @@ class DuckTreeIndexer implements Indexer {
     absolutePath: string,
     workspaceRoot: string,
     cacheIdentity?: WorkspaceCacheIdentity,
+  ): Promise<IndexResult> {
+    const existing = this.indexFileInFlight.get(absolutePath);
+    if (existing !== undefined) {
+      return existing;
+    }
+
+    const promise = this.doIndexFile(absolutePath, workspaceRoot, cacheIdentity);
+    this.indexFileInFlight.set(absolutePath, promise);
+    try {
+      return await promise;
+    } finally {
+      this.indexFileInFlight.delete(absolutePath);
+    }
+  }
+
+  private async doIndexFile(
+    absolutePath: string,
+    workspaceRoot: string,
+    cacheIdentity?: WorkspaceCacheIdentity,
   ): Promise<IndexResult> {
     const startedAt = Date.now();
     await this.initialize();
@@ -169,6 +202,10 @@ class DuckTreeIndexer implements Indexer {
       : null;
 
     try {
+      this.logger?.debug("indexFile start", {
+        relativePath: absolutePath.split("/").pop() ?? absolutePath,
+      });
+
       const result = await this.registry.run({
         absolutePath,
         workspaceRoot,
@@ -268,11 +305,18 @@ class DuckTreeIndexer implements Indexer {
 
       const symbols = await getSymbolsForFile(database.connection, extracted.file.relativePath);
 
+      const elapsedMs = Date.now() - startedAt;
+      this.logger?.debug("indexFile complete", {
+        relativePath: extracted.file.relativePath,
+        symbolCount: symbols.length,
+        elapsedMs,
+      });
+
       return {
         relativePath: extracted.file.relativePath,
         symbolCount: symbols.length,
         symbols,
-        elapsedMs: Date.now() - startedAt,
+        elapsedMs,
       };
     } finally {
       tree?.delete();
@@ -282,7 +326,7 @@ class DuckTreeIndexer implements Indexer {
   async detectWorkspaceFrameworks(workspaceRoot: string): Promise<readonly FrameworkInfo[]> {
     await this.initialize();
     const database = this.requireDatabaseHandle();
-    const detected = await detectFrameworks(createNodeFsIO(workspaceRoot));
+    const detected = await detectFrameworks(createNodeFsIO(workspaceRoot, this.logger));
     this.frameworkCache.set(workspaceRoot, detected);
     await replaceWorkspaceFrameworks(
       database.connection,
@@ -303,6 +347,7 @@ class DuckTreeIndexer implements Indexer {
     await this.initialize();
     const database = this.requireDatabaseHandle();
     await resolveWorkspaceCrossFileEdges(database.connection, workspaceRoot);
+    this.logger?.info("Finalized workspace cross-file edges", { workspaceRoot });
   }
 
   async validateWorkspaceCache(identity: WorkspaceCacheIdentity) {
@@ -378,6 +423,10 @@ class DuckTreeIndexer implements Indexer {
   }
 }
 
-export function createIndexer(dbPath: string, wasmDir: string): Indexer {
-  return new DuckTreeIndexer(dbPath, wasmDir);
+export function createIndexer(
+  dbPath: string,
+  wasmDir: string,
+  options?: IndexerFactoryOptions,
+): Indexer {
+  return new DuckTreeIndexer(dbPath, wasmDir, options);
 }

packages/core/src/storage/db.ts

@@ -1,11 +1,15 @@
 import type { DuckDBConnection, DuckDBInstance, DuckDBMaterializedResult } from "@duckdb/node-api";
 
+import type { Logger } from "../types.js";
+
 export interface DatabaseHandle {
   instance: DuckDBInstance;
   connection: DuckDBConnection;
   close(): void;
 }
 
+let inTransaction = false;
+
 export async function openDatabase(dbPath: string): Promise<DatabaseHandle> {
   const { DuckDBInstance } = await import("@duckdb/node-api");
   const instance = await DuckDBInstance.create(dbPath);
@@ -32,6 +36,9 @@ export async function openDatabase(dbPath: string): Promise<DatabaseHandle> {
  *   "no active transaction" (normal) or "aborted state cleared" (desired).
  * - On operation failure, ROLLBACK is attempted and any error it throws is
  *   swallowed so the original error always propagates to the caller.
+ * - A runtime guard throws `Error("Nested runInTransaction detected")` if
+ *   called while another `runInTransaction` is already in-flight. This
+ *   prevents silent rollback of the outer transaction.
  *
  * NOTE: `runInTransaction` must never be called from inside another
  * `runInTransaction` block. The defensive ROLLBACK would silently undo the
@@ -40,7 +47,13 @@ export async function openDatabase(dbPath: string): Promise<DatabaseHandle> {
 export async function runInTransaction<T>(
   connection: DuckDBConnection,
   operation: () => Promise<T>,
+  logger?: Logger,
 ): Promise<T> {
+  // Nesting guard — throws synchronously before any SQL is issued.
+  if (inTransaction) {
+    throw new Error("Nested runInTransaction detected");
+  }
+
   // Pre-flight: clear any lingering aborted transaction from a previous call.
   // "No active transaction" is expected on a clean connection and not an error.
   try {
@@ -49,20 +62,26 @@ export async function runInTransaction<T>(
     // Normal path — no active transaction to roll back.
   }
 
-  await connection.run("BEGIN TRANSACTION");
-
   try {
+    inTransaction = true;
+    logger?.debug("BEGIN TRANSACTION");
+    await connection.run("BEGIN TRANSACTION");
+
     const result = await operation();
+    logger?.debug("COMMIT");
     await connection.run("COMMIT");
     return result;
   } catch (error) {
+    logger?.error("ROLLBACK", error, {});
     // Wrap ROLLBACK so a failing rollback doesn't replace the original error.
     try {
       await connection.run("ROLLBACK");
     } catch {
       // Best-effort only — the pre-flight on the next call will clean this up.
     }
     throw error;
+  } finally {
+    inTransaction = false;
   }
 }