refactor(sync-core): extract sync import filter decision

docs/long-term-plans/sync-core-extraction-plan.md

@@ -1,9 +1,9 @@
 # `@sp/sync-core` Extraction Plan
 
-> **Status: In progress - PR 1, PR 2 guardrails/logger adapter work, and PR 3a
-> vector-clock ownership are present on this branch. Remaining PR 2 cleanup is
-> PR text alignment plus targeted future `SyncLogger` routing for files as they
-> move; PR 3b can start with pure algorithmic moves.**
+> **Status: In progress - PR 1, PR 2 guardrails/logger adapter work, PR 3a
+> vector-clock ownership, and the PR 3b pure helper slices are present on this
+> branch. Remaining PR 2 cleanup is PR text alignment plus targeted future
+> `SyncLogger` routing for files as they move.**
 
 **Goal:** Carve the sync engine out of `src/app/op-log/` into a reusable,
 framework-agnostic, **domain-agnostic** `@sp/sync-core` package, plus a sibling
@@ -109,21 +109,43 @@ groundwork:
   injection token. Existing helper functions still read the app-side
   `ENTITY_CONFIGS` singleton for compatibility.
 
-Remaining immediate debt before PR 3b / further algorithm extraction:
+Current extraction state and remaining immediate debt:
 
 - `FULL_STATE_OP_TYPES` still lives in `@sp/sync-core` as compatibility debt.
 - Vector-clock compare/merge/prune now lives in `@sp/sync-core`, with
   `@sp/shared-schema` re-exporting it for existing client/server imports.
 - `SyncLogger` exists, but movable app code still mostly calls `OpLog` directly.
 - `@sp/sync-core` has a Vitest package test runner and vector-clock tests.
+- PR 3b has generic conflict helpers in
+  `packages/sync-core/src/conflict-resolution.ts`: deep equality, identical
+  conflict detection, conflict-resolution suggestion, entity frontier
+  construction, clock-corruption comparison adjustment, pure LWW conflict
+  resolution planning, and local-DELETE-loses-to-remote-UPDATE payload
+  extraction/merge helpers. It also owns pure LWW resolution partitioning:
+  local/remote winner counts, remote-winner ops after host processing,
+  local-winner remote ops, rejected-op id buckets, local-win op collection, and
+  remote-winner affected entity-key calculation. The Angular
+  `ConflictResolutionService` delegates to these helpers while keeping app
+  orchestration, IndexedDB/apply flow, entity lookup, NgRx, dev-error wiring,
+  app action-type ownership, fallback logging, and operation creation app-side.
+- PR 3b also has the pure full-state import vector-clock decision helper in
+  `packages/sync-core/src/sync-import-filter.ts`. The Angular
+  `SyncImportFilterService` still owns full-state operation classification,
+  latest import lookup from batch/store, IndexedDB access, conflict-dialog
+  signaling, and logging.
 
 Suggested next order:
 
 1. Finish PR 2 documentation and verification.
 2. Add the app-side `SyncLogger` adapter before moving `OpLog`-using files.
-3. Make full-state operation classification configurable or explicitly mark
+3. Treat the PR 3b pure conflict-resolution and sync-import slices as complete
+   for this round.
+4. Make full-state operation classification configurable or explicitly mark
    those op types as host-defined compatibility strings.
-4. Start PR 3b with pure algorithmic moves only after the logger adapter exists.
+5. Clean up logger/config dependencies before moving compression, prefix, or
+   error helpers.
+6. Defer port/orchestration work until the remaining pure/config boundaries are
+   settled.
 
 ## PR 1 - Thin First Slice (#7546)
 
@@ -515,6 +537,31 @@ contracts until those are separately decoupled.
 Move framework-agnostic, stateless sync algorithms. These should only need typed
 inputs and the logger port.
 
+### Current State
+
+- `deepEqual`, `isIdenticalConflict`, `suggestConflictResolution`,
+  `buildEntityFrontier`, `adjustForClockCorruption`, and
+  `planLwwConflictResolutions` live in `@sp/sync-core` with package-level
+  Vitest coverage.
+- `classifyOpAgainstSyncImport` lives in `@sp/sync-core` and owns only the
+  vector-clock keep/invalidate decision for an op against the latest full-state
+  import. It returns the raw comparison plus a reason so app logging stays
+  unchanged.
+- Local DELETE losing to remote UPDATE conversion now delegates to
+  `extractEntityFromPayload`, `extractUpdateChanges`, and
+  `convertLocalDeleteRemoteUpdatesToLww` in `@sp/sync-core`. The app supplies
+  payload-key resolution, LWW action-type conversion, singleton-id handling,
+  and fallback warning logging.
+- `ConflictResolutionService` keeps compatibility wrappers/call sites and
+  passes the app `SyncLogger` adapter into package helpers. It also supplies
+  the app-owned archive action predicate to LWW planning and creates
+  archive/local-win operations app-side.
+- Pure remote/local operation partitioning now lives in `@sp/sync-core`;
+  NgRx state lookup and operation creation stay in the app.
+- `SyncImportFilterService` still owns full-state op detection, latest import
+  selection from current batch/local store, IndexedDB access, local unsynced
+  import detection, and all `OpLog` messages.
+
 ### What Moves
 
 - Conflict detection and LWW resolution algorithms from

packages/sync-core/src/index.ts

@@ -25,6 +25,16 @@ export {
 } from './vector-clock';
 export type { VectorClockComparison } from './vector-clock';
 
+// Full-state import clean-slate vector-clock decisions.
+export { classifyOpAgainstSyncImport } from './sync-import-filter';
+export type {
+  SyncImportFilterClockSource,
+  SyncImportFilterDecision,
+  SyncImportFilterDecisionReason,
+  SyncImportFilterInvalidateReason,
+  SyncImportFilterKeepReason,
+} from './sync-import-filter';
+
 // LWW (Last-Writer-Wins) update action-type helpers — factory parameterized by
 // the host application's entity-type list, so the lib stays domain-agnostic.
 export { createLwwUpdateActionTypeHelpers } from './lww-update-action-types';

packages/sync-core/src/sync-import-filter.ts

@@ -0,0 +1,67 @@
+import { compareVectorClocks } from './vector-clock';
+import type { VectorClock, VectorClockComparison } from './vector-clock';
+
+export type SyncImportFilterKeepReason =
+  | 'greater-than'
+  | 'equal'
+  | 'same-client-post-import'
+  | 'knows-import-counter';
+
+export type SyncImportFilterInvalidateReason = 'concurrent' | 'less-than';
+
+export type SyncImportFilterDecisionReason =
+  | SyncImportFilterKeepReason
+  | SyncImportFilterInvalidateReason;
+
+export interface SyncImportFilterClockSource {
+  clientId: string;
+  vectorClock: VectorClock;
+}
+
+export interface SyncImportFilterDecision {
+  shouldKeep: boolean;
+  comparison: VectorClockComparison;
+  reason: SyncImportFilterDecisionReason;
+}
+
+/**
+ * Classifies whether an operation survives a full-state import's clean-slate
+ * boundary using only vector-clock causality.
+ */
+export const classifyOpAgainstSyncImport = (
+  op: SyncImportFilterClockSource,
+  latestImport: SyncImportFilterClockSource,
+): SyncImportFilterDecision => {
+  const comparison = compareVectorClocks(op.vectorClock, latestImport.vectorClock);
+
+  if (comparison === 'GREATER_THAN') {
+    return { shouldKeep: true, comparison, reason: 'greater-than' };
+  }
+
+  if (comparison === 'EQUAL') {
+    return { shouldKeep: true, comparison, reason: 'equal' };
+  }
+
+  if (comparison === 'CONCURRENT') {
+    const importClientCounter = latestImport.vectorClock[latestImport.clientId] ?? 0;
+
+    if (
+      op.clientId === latestImport.clientId &&
+      (op.vectorClock[op.clientId] ?? 0) > importClientCounter
+    ) {
+      return { shouldKeep: true, comparison, reason: 'same-client-post-import' };
+    }
+
+    if (
+      op.clientId !== latestImport.clientId &&
+      (op.vectorClock[latestImport.clientId] ?? 0) >= importClientCounter &&
+      importClientCounter > 0
+    ) {
+      return { shouldKeep: true, comparison, reason: 'knows-import-counter' };
+    }
+
+    return { shouldKeep: false, comparison, reason: 'concurrent' };
+  }
+
+  return { shouldKeep: false, comparison, reason: 'less-than' };
+};

packages/sync-core/tests/sync-import-filter.spec.ts

@@ -0,0 +1,117 @@
+import { describe, expect, it } from 'vitest';
+import { classifyOpAgainstSyncImport } from '../src/sync-import-filter';
+import type { SyncImportFilterClockSource } from '../src/sync-import-filter';
+
+const clockSource = (
+  clientId: string,
+  vectorClock: Record<string, number>,
+): SyncImportFilterClockSource => ({
+  clientId,
+  vectorClock,
+});
+
+describe('classifyOpAgainstSyncImport', () => {
+  it('keeps GREATER_THAN ops', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientA', { clientA: 2, clientB: 1 }),
+        clockSource('clientB', { clientA: 1, clientB: 1 }),
+      ),
+    ).toEqual({
+      shouldKeep: true,
+      comparison: 'GREATER_THAN',
+      reason: 'greater-than',
+    });
+  });
+
+  it('keeps EQUAL ops', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientA', { clientA: 1, clientB: 1 }),
+        clockSource('clientB', { clientA: 1, clientB: 1 }),
+      ),
+    ).toEqual({
+      shouldKeep: true,
+      comparison: 'EQUAL',
+      reason: 'equal',
+    });
+  });
+
+  it('keeps CONCURRENT same-client ops with a greater own counter', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientB', { clientB: 6, clientC: 1 }),
+        clockSource('clientB', { clientA: 10, clientB: 5 }),
+      ),
+    ).toEqual({
+      shouldKeep: true,
+      comparison: 'CONCURRENT',
+      reason: 'same-client-post-import',
+    });
+  });
+
+  it('invalidates CONCURRENT same-client ops with an equal own counter', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientB', { clientB: 5, clientC: 1 }),
+        clockSource('clientB', { clientA: 10, clientB: 5 }),
+      ),
+    ).toEqual({
+      shouldKeep: false,
+      comparison: 'CONCURRENT',
+      reason: 'concurrent',
+    });
+  });
+
+  it('keeps CONCURRENT different-client ops that know the import client counter', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientA', { clientA: 1, clientB: 5 }),
+        clockSource('clientB', { clientB: 5, clientC: 10 }),
+      ),
+    ).toEqual({
+      shouldKeep: true,
+      comparison: 'CONCURRENT',
+      reason: 'knows-import-counter',
+    });
+  });
+
+  it('invalidates CONCURRENT different-client ops when the import client counter is 0', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientA', { clientA: 1, clientB: 0 }),
+        clockSource('clientB', { clientB: 0, clientC: 10 }),
+      ),
+    ).toEqual({
+      shouldKeep: false,
+      comparison: 'CONCURRENT',
+      reason: 'concurrent',
+    });
+  });
+
+  it('invalidates other CONCURRENT ops', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientA', { clientA: 1 }),
+        clockSource('clientB', { clientB: 5 }),
+      ),
+    ).toEqual({
+      shouldKeep: false,
+      comparison: 'CONCURRENT',
+      reason: 'concurrent',
+    });
+  });
+
+  it('invalidates LESS_THAN ops', () => {
+    expect(
+      classifyOpAgainstSyncImport(
+        clockSource('clientA', { clientA: 1 }),
+        clockSource('clientB', { clientA: 2, clientB: 1 }),
+      ),
+    ).toEqual({
+      shouldKeep: false,
+      comparison: 'LESS_THAN',
+      reason: 'less-than',
+    });
+  });
+});

src/app/op-log/sync/sync-import-filter.service.ts

@@ -1,12 +1,9 @@
 import { inject, Injectable } from '@angular/core';
 import { OperationLogStoreService } from '../persistence/operation-log-store.service';
 import { Operation, OpType } from '../core/operation.types';
-import {
-  compareVectorClocks,
-  VectorClockComparison,
-  vectorClockToString,
-} from '../../core/util/vector-clock';
+import { vectorClockToString } from '../../core/util/vector-clock';
 import { OpLog } from '../../core/log';
+import { classifyOpAgainstSyncImport } from '@sp/sync-core';
 
 /**
  * Service responsible for filtering operations invalidated by SYNC_IMPORT, BACKUP_IMPORT, or REPAIR operations.
@@ -173,7 +170,7 @@ export class SyncImportFilterService {
       // - CONCURRENT + proven post-import counter knowledge → KEEP
       // - CONCURRENT (all other cases) → FILTER
       // - LESS_THAN: Op is dominated by import → FILTER
-      const comparison = compareVectorClocks(op.vectorClock, importClockForComparison);
+      const decision = classifyOpAgainstSyncImport(op, latestImport);
 
       // DIAGNOSTIC LOGGING: Log vector clock comparison details
       // This helps debug issues where ops are incorrectly filtered as CONCURRENT
@@ -181,20 +178,13 @@ export class SyncImportFilterService {
         `SyncImportFilterService: Comparing op ${op.id} (${op.actionType}) from client ${op.clientId}\n` +
           `  Op vectorClock:     ${vectorClockToString(op.vectorClock)}\n` +
           `  Import vectorClock: ${vectorClockToString(importClockForComparison)}` +
-          `\n  Comparison result:  ${comparison}`,
+          `\n  Comparison result:  ${decision.comparison}`,
       );
 
-      if (
-        comparison === VectorClockComparison.GREATER_THAN ||
-        comparison === VectorClockComparison.EQUAL
-      ) {
+      if (decision.reason === 'greater-than' || decision.reason === 'equal') {
         // Op was created by a client that had knowledge of the import
         validOps.push(op);
-      } else if (
-        comparison === VectorClockComparison.CONCURRENT &&
-        op.clientId === latestImport.clientId &&
-        (op.vectorClock[op.clientId] ?? 0) > (importClockForComparison[op.clientId] ?? 0)
-      ) {
+      } else if (decision.reason === 'same-client-post-import') {
         // Op is from the SAME client that created the import, with a higher counter.
         // A client can't create ops concurrent with its own import — all post-import
         // ops from the import client necessarily have causal knowledge of the import.
@@ -207,13 +197,7 @@ export class SyncImportFilterService {
             `  Client ${op.clientId} counter: op=${op.vectorClock[op.clientId]} > import=${importClockForComparison[op.clientId]} (post-import op).`,
         );
         validOps.push(op);
-      } else if (
-        comparison === VectorClockComparison.CONCURRENT &&
-        op.clientId !== latestImport.clientId &&
-        (op.vectorClock[latestImport.clientId] ?? 0) >=
-          (importClockForComparison[latestImport.clientId] ?? 0) &&
-        (importClockForComparison[latestImport.clientId] ?? 0) > 0
-      ) {
+      } else if (decision.reason === 'knows-import-counter') {
         // Op was created by a DIFFERENT client with knowledge of the import.
         // The SYNC_IMPORT incremented the importing client's counter, so any op whose
         // clock includes that counter value (or higher) must have received the import
@@ -240,7 +224,7 @@ export class SyncImportFilterService {
         // CONCURRENT or LESS_THAN: Op was created without knowledge of import
         // Filter it to ensure clean slate semantics
         OpLog.warn(
-          `SyncImportFilterService: FILTERING op ${op.id} (${op.actionType}) as ${comparison}\n` +
+          `SyncImportFilterService: FILTERING op ${op.id} (${op.actionType}) as ${decision.comparison}\n` +
             `  Op vectorClock:     ${vectorClockToString(op.vectorClock)}\n` +
             `  Import vectorClock: ${vectorClockToString(importClockForComparison)}` +
             `\n  Import client:      ${latestImport.clientId}\n` +