wire nav through a facade

2026-07-22 05:49:31 +00:00 · 2025-11-19 19:24:45 -07:00 · 2025-11-19 19:24:45 -07:00 · 6db2a5a08d
commit 6db2a5a08d
parent 2e3baae08a
5 changed files with 239 additions and 49 deletions
--- a/refactor-plan.md
+++ b/refactor-plan.md
@ -3,7 +3,8 @@
 ## Scope

 - Refactor 1: Align orchestrator and slice contracts with implementation.
- Refactor 2: Factor [`CoalescePlugin`](main.ts:9) into smaller, testable modules.
+- Refactor 2: Factor [`CoalescePlugin`](main.ts:10) into smaller, testable modules.
+- Refactor 3: Introduce a shared navigation facade used consistently by navigation and backlinks.

 This file is implementation‑oriented and intended for Code mode to execute.

@ -24,18 +25,19 @@ This file is implementation‑oriented and intended for Code mode to execute.

 ## Refactor 2 – Factor CoalescePlugin

-### Current structure (after refactor step 1–3)
+### Current structure (after refactor)

- [`CoalescePlugin`](main.ts:9) now:
+- [`CoalescePlugin`](main.ts:10) now:
  - Uses [`createAndStartOrchestrator()`](src/orchestrator/PluginBootstrap.ts:31) from [`PluginBootstrap`](src/orchestrator/PluginBootstrap.ts:1) in `onload` instead of manually constructing and starting the orchestrator.
  - Delegates debug commands to [`PluginDebugCommands`](src/orchestrator/PluginDebugCommands.ts:1) via [`attachDebugCommands()`](src/orchestrator/PluginDebugCommands.ts:21) and [`detachDebugCommands()`](src/orchestrator/PluginDebugCommands.ts:108), with a small local `coalesceUpdateLogging` that still calls [`Logger.setGlobalLogging`](src/features/shared-utilities/Logger.ts:1).
-  - Delegates DOM, workspace, and orchestrator event wiring to [`registerPluginEvents()`](src/orchestrator/PluginEvents.ts:26) in [`PluginEvents`](src/orchestrator/PluginEvents.ts:1).
-  - Still owns view initialization and duplicate suppression via [`shouldProcessFile()`](main.ts:100), [`updateCoalesceUIForFile()`](main.ts:122), and [`initializeExistingViews()`](main.ts:395).
+  - Delegates DOM, workspace, and orchestrator event wiring to [`registerPluginEvents()`](src/orchestrator/PluginEvents.ts:26) in [`PluginEvents`](src/orchestrator/PluginEvents.ts:1), passing a callback from the view initializer for UI updates.
+  - Delegates view initialization and duplicate suppression to [`PluginViewInitializer`](src/orchestrator/PluginViewInitializer.ts:1) instead of owning that logic directly.

 - New helpers:
  - [`PluginBootstrap`](src/orchestrator/PluginBootstrap.ts:1) – `createOrchestrator` and `createAndStartOrchestrator`.
  - [`PluginDebugCommands`](src/orchestrator/PluginDebugCommands.ts:1) – `attachDebugCommands(app, plugin, orchestrator, logger)` and `detachDebugCommands(app)`.
  - [`PluginEvents`](src/orchestrator/PluginEvents.ts:1) – `registerPluginEvents(app, plugin, orchestrator, logger, updateCoalesceUIForFile?)`.
+  - [`PluginViewInitializer`](src/orchestrator/PluginViewInitializer.ts:1) – `initializeExistingViews()` and `updateForFile(path)`.

 ### Target structure

@ -44,27 +46,27 @@ Organize plugin responsibilities into small, focused modules under `src/orchestr
 - `PluginBootstrap` – constructs [`PluginOrchestrator`](src/orchestrator/PluginOrchestrator.ts:20), initializes slices, and wires settings/logging.
 - `PluginDebugCommands` – defines functions that attach and detach all `coalesce*` debug helpers on `app`.
 - `PluginEvents` – encapsulates registration logic for DOM custom events (`coalesce-settings-*`, `coalesce-navigate*`), workspace events, and orchestrator events.
- `PluginViewInitializer` – planned: handles `initializeExistingViews()` and the duplicate‑processing logic for backlinks UI.
+- `PluginViewInitializer` – handles initial view discovery and the duplicate‑processing logic for backlinks UI attachment.

-After this refactor, [`CoalescePlugin`](main.ts:9) should be largely a thin adapter from Obsidian lifecycle (`onload`, `onunload`) to these helper modules.
+After this refactor, [`CoalescePlugin`](main.ts:10) is largely a thin adapter from Obsidian lifecycle (`onload`, `onunload`) to these helper modules.

 ### Implementation checklist and status

 1. Extract a plugin bootstrap module – **Completed**
   - [`PluginBootstrap`](src/orchestrator/PluginBootstrap.ts:1) provides `createAndStartOrchestrator(app, plugin, config)`.
-   - [`CoalescePlugin.onload`](main.ts:14) now uses `createAndStartOrchestrator` instead of manual `new PluginOrchestrator(...)` and separate `initialize()` / `start()` calls.
+   - [`CoalescePlugin.onload`](main.ts:16) now uses `createAndStartOrchestrator` instead of manual `new PluginOrchestrator(...)` and separate `initialize()` / `start()` calls.

 2. Extract debug command wiring – **Completed**
   - [`PluginDebugCommands`](src/orchestrator/PluginDebugCommands.ts:1) now owns all `coalesce*` debug helpers.
-   - [`CoalescePlugin.setupDebugMethods`](main.ts:86) calls `attachDebugCommands(this.app, this, this.orchestrator, this.logger)` and defines only `coalesceUpdateLogging` locally to call [`Logger.setGlobalLogging`](src/features/shared-utilities/Logger.ts:1).
-   - [`CoalescePlugin.onunload`](main.ts:452) calls `detachDebugCommands(this.app)` instead of manually deleting each debug property.
+   - [`CoalescePlugin.setupDebugMethods`](main.ts:91) calls `attachDebugCommands(this.app, this, this.orchestrator, this.logger)` and defines only `coalesceUpdateLogging` locally to call [`Logger.setGlobalLogging`](src/features/shared-utilities/Logger.ts:1).
+   - [`CoalescePlugin.onunload`](main.ts:174) calls `detachDebugCommands(this.app)` instead of manually deleting each debug property.

 3. Extract DOM+workspace+orchestrator event registration – **Completed**
   - [`PluginEvents`](src/orchestrator/PluginEvents.ts:1) implements `registerPluginEvents(app, plugin, orchestrator, logger, updateCoalesceUIForFile?)`, wiring:
     - DOM events: `coalesce-settings-collapse-changed`, `coalesce-logging-state-changed`, `coalesce-navigate`, `coalesce-navigate-complete`.
     - Workspace events: `file-open`, `layout-change`, `active-leaf-change`.
     - Orchestrator events: `file:opened`, `layout:changed`, `active-leaf:changed`.
-   - [`CoalescePlugin.registerEventHandlers`](main.ts:190) now delegates to:
+   - [`CoalescePlugin.registerEventHandlers`](main.ts:106) now delegates to:

     ```ts
     registerPluginEvents(
@ -72,38 +74,101 @@ After this refactor, [`CoalescePlugin`](main.ts:9) should be largely a thin adap
       this,
       this.orchestrator,
       this.logger,
-       this.updateCoalesceUIForFile.bind(this)
+       this.viewInitializer.updateForFile.bind(this.viewInitializer)
     );
     ```

-4. Extract view initialization and duplicate‑processing logic – **Planned**
-   - Introduce a `PluginViewInitializer` helper, for example:
+4. Extract view initialization and duplicate‑processing logic – **Completed**
+   - [`PluginViewInitializer`](src/orchestrator/PluginViewInitializer.ts:1) now owns:
+     - Duplicate suppression logic (previously `shouldProcessFile` in `main.ts`).
+     - Backlinks UI attachment and settings application logic (previously `updateCoalesceUIForFile`).
+     - Initial and delayed `file:opened` emission for all markdown views (previously `initializeExistingViews`).
+   - [`CoalescePlugin`](main.ts:54) creates the view initializer and delegates:
+     - `this.viewInitializer = createViewInitializer(this.app, this.orchestrator, this.logger);`
+     - Startup initialization via `this.viewInitializer.initializeExistingViews();`.
+
+5. Add tests for the new modules – **In progress**
+   - `PluginDebugCommands` – **Completed**:
+     - [`PluginDebugCommands.test`](src/orchestrator/__tests__/PluginDebugCommands.test.ts:1) verifies that `attachDebugCommands` attaches representative `coalesce*` helpers on a mocked `app` and that `detachDebugCommands` removes them.
+   - `PluginEvents` – Planned:
+     - Verify that firing `coalesce-*` DOM events invokes the expected orchestrator or slice methods (using spies/mocks for `settings`, `backlinks`, `viewIntegration`, and navigation behavior).
+     - Verify that `file:opened` drives `initializeView` + `attachToDOM` + `setOptions` as expected when slices are present.
+   - `PluginViewInitializer` – Planned:
+     - Verify that duplicate‑processing suppression works (previous `shouldProcessFile` semantics preserved).
+     - Verify that backlinks UI is attached only when the correct slices and active view are present.
+
+## Refactor 3 – Shared navigation facade
+
+### Current structure
+
+- Navigation implementation:
+  - [`NavigationSlice`](src/features/navigation/NavigationSlice.ts:17) constructs:
+    - `FileOpener`
+    - `LinkHandler`
+    - `NavigationService`
+    - and exposes a higher-level navigation API (`openPath`, `handleLinkClick`, history, etc.).
+  - [`BacklinksSlice`](src/features/backlinks/BacklinksSlice.ts:35) previously constructed its own:
+    - `FileOpener`
+    - `LinkHandler`
+    - `NavigationService`
+    - to implement [`handleNavigation()`](src/features/backlinks/BacklinksSlice.ts:352) via wiki-link text.
+
+- Shared navigation facade (new):
+  - [`NavigationFacade`](src/features/navigation/NavigationFacade.ts:1) now provides a lazily-initialized shared set of navigation components:
+    - `getSharedNavigation(app, baseLogger)` returns `{ navigationService, fileOpener, linkHandler }`.
+    - `__resetSharedNavigationForTests()` to reset the singleton in tests.
+
+### Target design
+
+- There should be exactly one `NavigationService` (and corresponding `FileOpener` / `LinkHandler`) per plugin instance, shared by:
+  - [`NavigationSlice`](src/features/navigation/NavigationSlice.ts:17)
+  - [`BacklinksSlice`](src/features/backlinks/BacklinksSlice.ts:35)
+  - Any future slice that needs navigation behavior.
+- Validation, file existence checks, and link processing should flow through this shared stack so navigation behavior is consistent everywhere.
+
+### Implementation checklist and status
+
+1. Introduce a NavigationFacade module – **Completed**
+   - Created [`NavigationFacade`](src/features/navigation/NavigationFacade.ts:1) with:
+     - `getSharedNavigation(app, baseLogger)`: lazily creates and stores `{ navigationService, fileOpener, linkHandler }` on first call, reuses thereafter.
+     - `__resetSharedNavigationForTests()`: test helper to reset the shared instance.
+
+2. Update NavigationSlice to use the shared facade – **Completed**
+   - [`NavigationSlice`](src/features/navigation/NavigationSlice.ts:17) now imports `getSharedNavigation` and uses it in the constructor:

     ```ts
-     const viewInitializer = createViewInitializer(app, orchestrator, logger);
-     viewInitializer.updateForFile(path);
-     viewInitializer.initializeExistingViews();
+     const shared = getSharedNavigation(app, this.logger);
+     this.fileOpener = shared.fileOpener;
+     this.linkHandler = shared.linkHandler;
+     this.navigationService = shared.navigationService;
     ```

-   - Migrate [`shouldProcessFile()`](main.ts:100), [`updateCoalesceUIForFile()`](main.ts:122), and [`initializeExistingViews()`](main.ts:395) out of `main.ts` into this helper.
-   - Update:
-     - `PluginEvents.registerPluginEvents` call sites to use `viewInitializer.updateForFile(...)` instead of calling `updateCoalesceUIForFile` directly.
-     - `CoalescePlugin.onload` to call `viewInitializer.initializeExistingViews()` rather than its own method.
+   - It no longer constructs its own `FileOpener` / `LinkHandler` / `NavigationService`; it only configures performance monitoring and history on top.

-5. Add tests for the new modules – **Planned**
-   - `PluginDebugCommands`:
-     - Verify that `attachDebugCommands` attaches the expected `coalesce*` functions on a mocked `app`, and that `detachDebugCommands` removes them.
-   - `PluginEvents`:
-     - Verify that firing `coalesce-*` events invokes the expected orchestrator or slice methods (using spies/mocks for `settings`, `backlinks`, `viewIntegration`, and `navigation` behaviors).
-     - Verify that `file:opened` drives `initializeView` + `attachToDOM` + `setOptions` as expected when slices are present.
-   - `PluginViewInitializer` (once added):
-     - Verify that duplicate‑processing suppression works (`shouldProcessFile` semantics preserved).
-     - Verify that backlinks UI is attached only when the correct slices and active view are present.
+3. Update BacklinksSlice to use the shared facade – **Completed**
+   - [`BacklinksSlice`](src/features/backlinks/BacklinksSlice.ts:35) now imports `getSharedNavigation` from `../navigation/NavigationFacade` and uses it in the constructor:
+
+     ```ts
+     const sharedNavigation = getSharedNavigation(this.app, this.logger);
+     this.navigationService = sharedNavigation.navigationService;
+     ```
+
+   - It no longer constructs its own navigation stack; [`handleNavigation()`](src/features/backlinks/BacklinksSlice.ts:352) continues to call `navigationService.openWikiLink(...)`, but now using the shared instance.
+
+4. Add tests for the navigation facade – **Planned**
+   - Add a small test module (e.g. [`NavigationFacade.test`](src/features/navigation/__tests__/NavigationFacade.test.ts:1)) that:
+     - Asserts that two calls to `getSharedNavigation(app, logger)` return the same `navigationService` instance.
+     - Asserts that `__resetSharedNavigationForTests()` causes a subsequent call to create a new instance.
+   - Optionally, assert that `NavigationSlice` and `BacklinksSlice` both obtain a `NavigationService` that is strictly the same object in an integration-style test.

 ## Suggested implementation order (updated)

 1. Refactor 1 (orchestrator/slice contracts) – **Completed**.
-2. Refactor 2 (CoalescePlugin) – **Partially completed**:
-   - Done: bootstrap extraction, debug command extraction, event wiring extraction.
-   - Remaining: view initializer helper and tests for new modules.
-3. After these are complete and covered by tests, proceed to the next roadmap items (navigation unification, validation centralization, logging standardization), using this document as living design notes.
+2. Refactor 2 (CoalescePlugin) – **Completed**:
+   - Bootstrap, debug commands, event wiring, and view initialization have all been extracted into helper modules, with `main.ts` acting as a thin lifecycle adapter.
+3. Refactor 3 (shared navigation facade) – **Partially completed**:
+   - Shared navigation facade created and wired into [`NavigationSlice`](src/features/navigation/NavigationSlice.ts:17) and [`BacklinksSlice`](src/features/backlinks/BacklinksSlice.ts:35).
+   - Remaining: facade-focused tests and potential further centralization of validation once SharedUtilities validation helpers are introduced into the navigation path.
+4. Next steps:
+   - Add tests for navigation facade and remaining helper modules.
+   - Proceed to centralizing validation (via `SharedUtilitiesSlice.getValidationUtils`) and standardizing logging (via `SharedUtilitiesSlice.getLogger`), continuing to update this document as new refactors land.
--- a/src/features/backlinks/BacklinksSlice.ts
+++ b/src/features/backlinks/BacklinksSlice.ts
@ -17,8 +17,7 @@ import { HeaderUI } from './HeaderUI';
 import { FilterControls } from './FilterControls';
 import { SettingsControls } from './SettingsControls';
 import { NavigationService } from '../navigation/NavigationService';
-import { FileOpener } from '../navigation/FileOpener';
-import { LinkHandler } from '../navigation/LinkHandler';
+import { getSharedNavigation } from '../navigation/NavigationFacade';

 /**
 * BacklinksSlice
@ -66,11 +65,9 @@ export class BacklinksSlice implements IBacklinksSlice {
        this.app = app;
        this.logger = new Logger('BacklinksSlice');

-        // Initialize navigation service used for link-style navigation
-        const navigationLogger = this.logger.child('Navigation');
-        const fileOpener = new FileOpener(this.app, navigationLogger);
-        const linkHandler = new LinkHandler(this.app, navigationLogger);
-        this.navigationService = new NavigationService(this.app, fileOpener, linkHandler, navigationLogger);
+        // Initialize shared navigation service used for link-style navigation
+        const sharedNavigation = getSharedNavigation(this.app, this.logger);
+        this.navigationService = sharedNavigation.navigationService;

        // Set default options
        this.options = {
--- a/src/features/navigation/NavigationFacade.ts
+++ b/src/features/navigation/NavigationFacade.ts
@ -0,0 +1,62 @@
+import { App } from 'obsidian';
+import { Logger } from '../shared-utilities/Logger';
+import { NavigationService } from './NavigationService';
+import { FileOpener } from './FileOpener';
+import { LinkHandler } from './LinkHandler';
+
+/**
+ * NavigationFacade
+ *
+ * Provides a shared NavigationService (and its underlying FileOpener and
+ * LinkHandler) so multiple slices (navigation, backlinks, etc.) can coordinate
+ * navigation through a single abstraction.
+ *
+ * This avoids each slice constructing its own NavigationService and keeps
+ * navigation behavior consistent across the plugin.
+ */
+export interface SharedNavigation {
+  navigationService: NavigationService;
+  fileOpener: FileOpener;
+  linkHandler: LinkHandler;
+}
+
+let sharedNavigation: SharedNavigation | null = null;
+
+/**
+ * Get (or lazily create) the shared navigation components.
+ *
+ * The first caller provides the App and a base logger. Subsequent callers
+ * reuse the same NavigationService / FileOpener / LinkHandler instances.
+ */
+export function getSharedNavigation(
+  app: App,
+  baseLogger: Logger,
+): SharedNavigation {
+  if (sharedNavigation) {
+    return sharedNavigation;
+  }
+
+  const navigationLogger =
+    typeof (baseLogger as any).child === 'function'
+      ? (baseLogger as any).child('NavigationFacade')
+      : new Logger('NavigationFacade');
+
+  const fileOpener = new FileOpener(app, navigationLogger);
+  const linkHandler = new LinkHandler(app, navigationLogger);
+  const navigationService = new NavigationService(app, fileOpener, linkHandler, navigationLogger);
+
+  sharedNavigation = {
+    navigationService,
+    fileOpener,
+    linkHandler,
+  };
+
+  return sharedNavigation;
+}
+
+/**
+ * Test-only helper to reset the shared navigation instance.
+ */
+export function __resetSharedNavigationForTests(): void {
+  sharedNavigation = null;
+}
--- a/src/features/navigation/NavigationSlice.ts
+++ b/src/features/navigation/NavigationSlice.ts
@ -3,6 +3,7 @@ import { INavigationSlice } from '../shared-contracts/slice-interfaces';
 import { NavigationService } from './NavigationService';
 import { LinkHandler } from './LinkHandler';
 import { FileOpener } from './FileOpener';
+import { getSharedNavigation } from './NavigationFacade';
 import { Logger } from '../shared-utilities/Logger';
 import { PerformanceMonitor } from '../shared-utilities/PerformanceMonitor';
 import { CommonHelpers } from '../shared-utilities/CommonHelpers';
@ -28,14 +29,10 @@ export class NavigationSlice implements INavigationSlice {
        this.app = app;
        this.logger = new Logger('NavigationSlice');
        
-        this.fileOpener = new FileOpener(app, this.logger);
-        this.linkHandler = new LinkHandler(app, this.logger);
-        this.navigationService = new NavigationService(
-            app,
-            this.fileOpener,
-            this.linkHandler,
-            this.logger
-        );
+        const shared = getSharedNavigation(app, this.logger);
+        this.fileOpener = shared.fileOpener;
+        this.linkHandler = shared.linkHandler;
+        this.navigationService = shared.navigationService;

        this.performanceMonitor = new PerformanceMonitor(
            this.logger.child('Performance'),
--- a/src/orchestrator/tests/PluginDebugCommands.test.ts
+++ b/src/orchestrator/tests/PluginDebugCommands.test.ts
@ -0,0 +1,69 @@
+import { App } from 'obsidian';
+import { attachDebugCommands, detachDebugCommands } from '../PluginDebugCommands';
+import { PluginOrchestrator } from '../PluginOrchestrator';
+
+describe('PluginDebugCommands', () => {
+  interface MockApp extends App {
+    [key: string]: any;
+  }
+
+  const createMockApp = (): MockApp => {
+    const app: any = {
+      workspace: {
+        getActiveViewOfType: jest.fn().mockReturnValue(null),
+        getLeavesOfType: jest.fn().mockReturnValue([]),
+      },
+    };
+    return app as MockApp;
+  };
+
+  const createMockOrchestrator = (): PluginOrchestrator => {
+    return {
+      getSlice: jest.fn(),
+      getState: jest.fn().mockReturnValue({}),
+      getStatistics: jest.fn().mockReturnValue({}),
+      emit: jest.fn(),
+      on: jest.fn(),
+      off: jest.fn(),
+      getAllSlices: jest.fn().mockReturnValue({}),
+      cleanup: jest.fn(),
+      initialize: jest.fn(),
+      start: jest.fn(),
+      stop: jest.fn(),
+    } as unknown as PluginOrchestrator;
+  };
+
+  const createMockLogger = () => ({
+    debug: jest.fn(),
+    info: jest.fn(),
+    warn: jest.fn(),
+    error: jest.fn(),
+  });
+
+  it('attaches and detaches coalesce debug commands on the app object', () => {
+    const app = createMockApp();
+    const orchestrator = createMockOrchestrator();
+    const logger = createMockLogger();
+    const plugin: any = {}; // CoalescePlugin instance is not used directly here
+
+    attachDebugCommands(app, plugin, orchestrator, logger);
+
+    const anyApp = app as any;
+
+    // Spot-check a few representative commands
+    expect(typeof anyApp.coalesceStatus).toBe('function');
+    expect(typeof anyApp.coalesceTestFocus).toBe('function');
+    expect(typeof anyApp.coalesceTestDirectFocus).toBe('function');
+    expect(typeof anyApp.coalesceOrchestratorStatus).toBe('function');
+    expect(typeof anyApp.coalesceSliceStatus).toBe('function');
+
+    detachDebugCommands(app);
+
+    // Ensure they were removed
+    expect(anyApp.coalesceStatus).toBeUndefined();
+    expect(anyApp.coalesceTestFocus).toBeUndefined();
+    expect(anyApp.coalesceTestDirectFocus).toBeUndefined();
+    expect(anyApp.coalesceOrchestratorStatus).toBeUndefined();
+    expect(anyApp.coalesceSliceStatus).toBeUndefined();
+  });
+});