# Epic 8: Gap Analysis Against V10 Design Document

**Date:** 2026-03-12
**Scope:** Comparison of Epic 8 files (`design-spec.md`, `tech-spec.md`, `jira-ticket.md`) against `docs/designs/data-sync/V10.md`.
**Note:** This replaces the previous partial gap analysis which correctly identified the missing Stage 5 Conflict Resolution UI. That finding is retained below as Gap 1, along with 11 additional gaps.

---

## Overall Assessment

Epic 8 delivers solid frontend and backend scaffolding: the `field_metadata` model, the DLQ dashboard with inspect/retry/dismiss workflow, and the dependency-inverted mock-first approach are well-aligned with the project's incremental rollout strategy. The Blade + React architecture, `BaseApiController` usage, and `ApiClient` singleton integration all follow SuiteX conventions correctly.

However, there are significant gaps: the entire Stage 5 Conflict Resolution UI is missing, the DLQ table diverges from V10's prescribed `sync_error_queue` schema, a conflict policy option is missing from the field mapper, and the field mapper UI omits two critical `field_metadata` columns. The DLQ mock payloads also carry forward envelope field naming issues identified in previous epic analyses.

---

## Gap 1: Missing Stage 5 — Conflict Resolution Dashboard (Critical)

*Previously identified in the existing gap analysis. Retained here for completeness.*

**V10 Reference:** Stage 5 (line 160-168):

> **Conflict queue UI** showing base snapshot, local change, remote current, suggested merge, timestamps, audit trail.
> APIs to resolve conflict (accept SuiteX, accept NetSuite, accept suggested merge, or edit then apply).

V10 Human Resolution Process (lines 529-534):

> 1. Conflict queued with full context (Base, our changes, remote current, suggested merged payload, links to audit).
> 2. Human visits Conflict UI, reviews differences, chooses action.
> 3. When human applies resolution: orchestrator applies resolved change to target(s) and clears conflict.
> 4. While waiting for resolution, new events are accepted but blocked from automated apply (record-level lock).

**Current State:** Epic 8 builds only the DLQ Dashboard (structural JSON validation failures). It does not address the `conflicts` table or any three-way merge UI. The `conflicts` table (V10 line 258) stores records where the merge service encountered a field-level collision that couldn't be auto-resolved by policy (e.g., `manual` conflict policy). Without a resolution UI, these records remain permanently unresolved and the affected records stay locked.

**Impact:** Any record with a `manual` conflict policy will be permanently blocked from sync once a concurrent edit occurs. This is a P1 gap for production readiness.

**Required Change:** Add a third UI component and API surface:

1. **Backend:**
   - `GET /api/v1/sync/conflicts` — paginated list of unresolved conflicts, scoped by tenant
   - `GET /api/v1/sync/conflicts/{id}` — detail view returning Base, Local, Remote snapshots
   - `POST /api/v1/sync/conflicts/{id}/resolve` — accepts the human-approved field values

2. **Frontend:**
   - `ConflictDashboard.jsx` — data table of unresolved conflicts with record type, record ID, conflicting fields, and timestamps
   - `ConflictResolutionModal.jsx` — three-column (or unified diff) view showing Base, SuiteX (Local), NetSuite (Remote) values per conflicted field, with action buttons: Accept SuiteX, Accept NetSuite, Accept Suggested Merge, Edit & Apply

3. **Backend behavior on resolution:**
   - Package the resolved state into a new canonical event and publish to `events.merged`, bypassing the merge service
   - Clear the conflict record status
   - Release the `record_lock` for the affected record

**Affected Files:** `design-spec.md`, `tech-spec.md`, `jira-ticket.md` (scope expansion).

---

## Gap 2: `dead_letter_events` Table Should Be `sync_error_queue` (High)

**V10 Reference:** Appendix D ERD (lines 946-954) defines `sync_error_queue` with columns: `id`, `record_type`, `record_id`, `reason`, `details` (JSON), `created_at`, `status`. V10 line 422: "`sync_error_queue`: stores errors requiring human intervention, linked to `record_lock` entries and conflict records."

**Current State:** Epic 8 invents a new table called `dead_letter_events` with a different column structure: `tenant_id`, `record_type`, `record_id`, `payload`, `error_reason`, `status`. This table is not referenced anywhere in V10.

**Impact:** Creates a parallel, non-standard error storage mechanism. When the Orchestrator (Epic 7) is built, it will use `sync_error_queue` per V10, requiring the DLQ dashboard to be rewritten to query a different table. Additionally, V10's `sync_error_queue` is linked to `record_lock` entries; the custom table has no such linkage.

**Required Change:** Rename `dead_letter_events` to `sync_error_queue` and align the schema:

```sql
CREATE TABLE sync_error_queue (
    id BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    tenant_id BIGINT UNSIGNED NOT NULL,
    record_type VARCHAR(100) NOT NULL,
    record_id VARCHAR(255) NOT NULL,
    error_class VARCHAR(50) NOT NULL DEFAULT 'validation_error',
    reason TEXT NOT NULL,
    details JSON NOT NULL COMMENT 'Diagnostic context: original payload, stack trace, metadata',
    status ENUM('pending', 'retried', 'dismissed', 'resolved') NOT NULL DEFAULT 'pending',
    record_lock_id BIGINT UNSIGNED NULL COMMENT 'FK to record_lock if record is locked',
    created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
    INDEX idx_tenant_status (tenant_id, status),
    INDEX idx_record (record_type, record_id)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;
```

Key alignment changes:
- `error_reason` → `reason` (matches V10 column name)
- `payload` → `details` (matches V10; the full canonical payload is stored inside `details` as a nested JSON key)
- Add `error_class` column (validation_error, auth_error, transient, conflict — per V10 error taxonomy)
- Add `record_lock_id` nullable FK for future `record_lock` integration

Update the Eloquent model accordingly:

```php
class SyncError extends Model
{
    protected $connection = 'mysql';
    protected $table = 'sync_error_queue';

    protected $fillable = [
        'tenant_id',
        'record_type',
        'record_id',
        'error_class',
        'reason',
        'details',
        'status',
        'record_lock_id',
    ];

    protected $casts = [
        'details' => 'array',
    ];
}
```

**Affected Files:** `tech-spec.md` (model, controller, mock responses), `jira-ticket.md`.

---

## Gap 3: Missing `last-write-wins` Conflict Policy Option (High)

**V10 Reference:** Lines 391-394 define four conflict policies:

> - `netsuite-wins`
> - `suitex-wins`
> - `last-write-wins` (timestamp-based, use sparingly)
> - `manual` (always produce a conflict record)

**Current State:** The `SyncMetadataController` validation rule and the Field Mapper UI dropdown only offer three options: `netsuite-wins`, `suitex-wins`, `manual`. The `last-write-wins` policy is missing.

**Impact:** Tenants cannot configure timestamp-based resolution for fields where it is appropriate (e.g., notes, description fields where the latest edit is always correct). The merge service would have no way to receive this configuration.

**Required Change:**

1. Update the validation rule:
```php
'conflict_policy' => 'required|in:netsuite-wins,suitex-wins,last-write-wins,manual',
```

2. Update the Field Mapper UI dropdown to include `last-write-wins` with a visual warning indicator noting V10's caution: "use sparingly — risk of silent data loss."

**Affected Files:** `tech-spec.md` (controller validation, UI dropdown).

---

## Gap 4: Field Mapper UI Missing `is_synced` and `is_readonly` Controls (High)

**V10 Reference:** Appendix P2.2 (lines 2451-2461) — the `field_metadata` DDL includes `is_synced boolean` and `is_readonly boolean`. V10 line 399: "`field_metadata.is_readonly = true`" marks NetSuite-owned/computed fields that SuiteX must never overwrite.

**Current State:** The Field Mapper form only exposes three controls: Field ID, Data Type, Conflict Policy. The `is_synced` and `is_readonly` columns are on the Eloquent model's `$fillable` array but have no UI controls for setting them. They default to whatever the database default is (likely null/false).

**Impact:**
- Without `is_synced`: Admins cannot mark a custom field as "not synced," meaning every field in `field_metadata` would be included in sync events. V10 says SuiteX "maintains a per-record configuration describing which fields are part of the synchronized surface area" — this is driven by `is_synced`.
- Without `is_readonly`: Admins cannot protect NetSuite computed/formula fields from being overwritten. V10 says "NetSuite-owned, computed, or read-only fields are never overwritten by SuiteX" — this requires `is_readonly = true`.

**Required Change:** Add two toggle/checkbox controls to the Field Mapper form:

1. **Sync Enabled** (`is_synced`) — boolean toggle, default `true`. When `false`, the field is stored in metadata but excluded from sync events.
2. **Read-Only** (`is_readonly`) — boolean toggle, default `false`. When `true`, the field is never included in outbound writes to NetSuite (SuiteX treats NetSuite as authoritative).

Update the validation rules accordingly:
```php
'is_synced'   => 'boolean',
'is_readonly' => 'boolean',
```

**Affected Files:** `tech-spec.md` (controller validation, UI form), `design-spec.md` (requirements table).

---

## Gap 5: DLQ Mock Payload Uses `eventType` Instead of `operation` (Medium)

**V10 Reference:** Per the Epic 1 gap analysis, the canonical envelope field should be `operation`, not `eventType`.

**Current State:** The mock DLQ response payloads in `tech-spec.md` (lines 305, 333) use:
```json
"eventType": "update"
```
and:
```json
"eventType": "create"
```

**Impact:** Frontend developers building against these mock contracts will use `eventType` in their display logic. When the real Orchestrator produces payloads with `operation`, the UI will show `undefined` for the event type field.

**Required Change:** Replace `eventType` with `operation` in all mock DLQ response payloads:
```json
"operation": "update"
```

**Affected Files:** `tech-spec.md` (mock response JSON).

---

## Gap 6: DLQ Mock Payload Missing Required Envelope Fields (Medium)

**V10 Reference:** The canonical envelope (per V10 and Epic 1 gap analysis) requires: `orderingKey`, `baseVersion`, `actorId`, `transactionGroupId`, `fullSnapshotRef`.

**Current State:** The mock DLQ payloads in `tech-spec.md` include `schemaVersion`, `eventId`, `accountId`, `recordType`, `recordId`, `eventType`, `source`, `sourceSystem`, `timestamp`, `writeId`, `changes` — but are missing `orderingKey`, `baseVersion`, `actorId`, `transactionGroupId`, and `fullSnapshotRef`.

**Impact:** The DlqInspectorModal will render these payloads faithfully. When real payloads arrive with the additional fields, the modal will display them correctly (since it's a raw JSON viewer). However, mock payloads that don't match the real envelope shape make it harder for developers to identify issues during development and may lead to the DlqDashboard table columns not showing useful attribution data (e.g., `actorId` for "who caused this error").

**Required Change:** Update mock DLQ payloads to include all canonical envelope fields:
```json
{
  "schemaVersion": "v1",
  "eventId": "018e5c2a-1234-7abc-8def-000000000001",
  "accountId": 4515181,
  "recordType": "project",
  "recordId": "12345",
  "operation": "update",
  "source": "suitex",
  "sourceSystem": "suitex",
  "timestamp": "2026-03-09T14:29:58.000000Z",
  "orderingKey": "project:12345",
  "baseVersion": null,
  "writeId": "018e5c2a-5678-7abc-8def-000000000002",
  "actorId": "8832",
  "transactionGroupId": null,
  "fullSnapshotRef": null,
  "changes": { ... }
}
```

**Affected Files:** `tech-spec.md` (mock response JSON).

---

## Gap 7: DLQ "Retry" Action Does Not Re-Queue the Event (Medium)

**V10 Reference:** V10 error handling (lines 2386-2397) defines a retry policy table where events move through queues. Line 516: "No silent failures: every error lands in an appropriate queue." The retry action should re-publish the event for processing, not merely flip a status flag.

**Current State:** The `SyncDlqController::retry()` method only updates the database status to `retried`:
```php
$event->update(['status' => 'retried']);
```

It does not re-publish the payload to any Pub/Sub topic or queue for actual reprocessing.

**Impact:** "Retry" becomes a cosmetic label change with no operational effect. The event remains dead.

**Required Change:** For the mocked MVP phase, the status-only update is acceptable since the Orchestrator isn't built. However, document the intended production behavior:

1. Read the full payload from the `details` column
2. Re-publish it to `events.raw` via `SyncEventPublisherInterface`
3. Only update the status to `retried` after successful re-publication
4. If re-publication fails, leave status as `pending` and surface the error

Add a code comment or doc block on the `retry()` method noting this future requirement. When Epic 7 (Orchestrator) is complete, the retry method must be updated.

**Affected Files:** `tech-spec.md` (controller retry method).

---

## Gap 8: Missing `error_class` in `DeadLetterEvent` Model (Medium)

**V10 Reference:** V10 error taxonomy (lines 507-511) defines four error classes: transient, permanent (validation), conflict, operational. The retry policy table (lines 2386-2392) shows different handling per class. The mock DLQ response in the tech-spec includes `error_class: "validation_error"`.

**Current State:** The `DeadLetterEvent` model's `$fillable` array does not include `error_class`:
```php
protected $fillable = [
    'tenant_id', 'record_type', 'record_id',
    'payload', 'error_reason', 'status',
];
```

Yet the mock API response includes `error_class` — meaning it would come from nowhere if queried from the database.

**Impact:** The DLQ dashboard has no way to filter or display errors by class. Operators cannot distinguish between validation errors (fix the data), auth errors (fix credentials), and transient errors (just retry).

**Required Change:** Add `error_class` to the model's `$fillable` and the migration. The DLQ dashboard should also expose a filter for `error_class` alongside the existing `status` and `record_type` filters.

**Affected Files:** `tech-spec.md` (model, controller index filters, UI filter controls).

---

## Gap 9: No Delete Endpoint for Field Metadata (Medium)

**V10 Reference:** The `field_metadata` table uses a composite PK `(account_id, record_type, field_id)` (line 2460). Records should be manageable — add, update, and remove.

**Current State:** The `SyncMetadataController` only defines `index` (GET) and `store` (POST with `updateOrCreate`). There is no `destroy` (DELETE) endpoint.

**Impact:** If a tenant admin incorrectly maps a field or wants to stop syncing a custom field, they have no way to remove the mapping through the UI. They would need direct database access.

**Required Change:** Add a DELETE endpoint:

```php
// Route
Route::delete('metadata/{recordType}/{fieldId}', [SyncMetadataController::class, 'destroy'])
    ->name('api.v1.sync.metadata.destroy');

// Controller method
public function destroy(string $recordType, string $fieldId): JsonResponse
{
    try {
        $tenantId = $this->getCurrentTenantId();

        $deleted = FieldMetadata::where('tenant_id', $tenantId)
            ->where('record_type', $recordType)
            ->where('field_id', $fieldId)
            ->delete();

        if (!$deleted) {
            return $this->notFoundResponse();
        }

        return $this->successResponse(null, 204);
    } catch (\Exception $e) {
        $this->logError('Failed to delete field metadata', $e);
        return $this->errorResponse('Failed to delete field metadata', 500);
    }
}
```

Add a delete button/icon to each row in the SyncFieldMapper table UI.

**Affected Files:** `tech-spec.md` (routes, controller, UI table rows).

---

## Gap 10: `field_metadata` PK Definition and Migration (Low)

**V10 Reference:** Line 2460: `PRIMARY KEY (account_id, record_type, field_id)`.

**Current State:** The `FieldMetadata` model sets `$incrementing = false`, correctly indicating a non-auto-increment PK. However, no migration is shown and no PK column is defined on the model. The composite PK must be explicitly set in the migration since Eloquent doesn't natively support composite PKs well.

**Impact:** Without explicit composite PK definition, the migration may default to an `id` auto-increment column, contradicting V10's design and the model's `$incrementing = false` setting.

**Required Change:** Ensure the migration defines the composite PK:

```php
Schema::connection('mysql')->create('field_metadata', function (Blueprint $table) {
    $table->unsignedBigInteger('tenant_id');
    $table->string('record_type', 100);
    $table->string('field_id', 255);
    $table->string('field_type', 50);
    $table->boolean('is_synced')->default(true);
    $table->boolean('is_readonly')->default(false);
    $table->json('normalization_rule')->nullable();
    $table->string('conflict_policy', 50)->default('manual');
    $table->timestamps();

    $table->primary(['tenant_id', 'record_type', 'field_id']);
    $table->index('tenant_id');
});
```

Also set `protected $primaryKey` on the model or override `getKeyName()` to return the composite key array (or use a package like `spatie/laravel-composite-primary-key` if the codebase supports it). Alternatively, add a surrogate `id` auto-increment PK and enforce uniqueness via a unique composite index — this is often simpler with Eloquent's `findOrFail()`.

**Affected Files:** `tech-spec.md` (model, migration).

---

## Gap 11: No `record_lock` Integration on Error Events (Low — Future)

**V10 Reference:** Line 422: "`sync_error_queue` stores errors requiring human intervention, linked to `record_lock` entries and conflict records." Lines 486-488: "SuiteX creates a conflict record and marks the record as locked. Automated writers for that record are paused while still accepting and queuing new events."

**Current State:** Neither the `dead_letter_events` model nor the DLQ controller reference `record_lock`. When an error occurs, the affected record is not locked, meaning the Orchestrator could continue processing events for a record that has a known error.

**Impact:** For the mocked MVP, this is acceptable since no Orchestrator is running. However, when the pipeline is live, failing to lock records on error could lead to cascading failures or data corruption.

**Required Change:** Document as a future requirement. When `sync_error_queue` is integrated with the real Orchestrator:
1. Creating a `sync_error_queue` entry must also create a `record_lock` entry for `(record_type, record_id)`
2. Dismissing or resolving the error must release the lock
3. The DLQ dashboard should display lock status per record

**Affected Files:** `design-spec.md` (add as documented future requirement).

---

## Gap 12: No Distinction Between `events.error` and `events.dlq` Topics (Low)

**V10 Reference:** Appendix E (lines 971-972) defines two separate topics:
- `events.error` — retry-able errors
- `events.dlq` — dead-letter (exhausted retries)

V10 retry policy (lines 2386-2397) shows that events flow through different queues based on error class and retry count. Only after max retries do events move to dead-letter.

**Current State:** Epic 8 treats all failed events as a single category in `dead_letter_events`. There's no differentiation between events still being retried (on `events.error`) and events that have exhausted retries (true dead letters on `events.dlq`).

**Impact:** The dashboard would show events that are actively being retried alongside truly dead events, confusing operators. Operators might manually retry an event that the system is already retrying automatically.

**Required Change:** For the mocked MVP, a single table with an `error_class` filter (Gap 8) is sufficient. Document that in production, the dashboard should distinguish between:
- **Active errors** (retry in progress, sourced from `events.error` subscriptions) — show retry count and next retry time
- **Dead letters** (exhausted retries, sourced from `events.dlq`) — show "Retry" action button

This distinction could be modeled as `status` values: `retrying` (auto), `dead` (exhausted), `pending_review` (manual required), `dismissed`, `resolved`.

**Affected Files:** `design-spec.md` (add as documented future behavior).

---

## Summary of Required Changes

| Gap | Severity | Files Affected | Change Type |
|-----|----------|----------------|-------------|
| 1. Missing Conflict Resolution UI (Stage 5) | **Critical** | design-spec, tech-spec, jira-ticket | Add ConflictDashboard + resolution API |
| 2. Table should be `sync_error_queue` not `dead_letter_events` | **High** | tech-spec, jira-ticket | Rename table, align schema to V10 |
| 3. Missing `last-write-wins` conflict policy | **High** | tech-spec | Add to validation + UI dropdown |
| 4. Missing `is_synced` / `is_readonly` UI controls | **High** | tech-spec, design-spec | Add toggles to Field Mapper form |
| 5. Mock payload uses `eventType` not `operation` | **Medium** | tech-spec | Rename in mock JSON |
| 6. Mock payload missing envelope fields | **Medium** | tech-spec | Add orderingKey, actorId, baseVersion, etc. |
| 7. Retry action doesn't re-queue | **Medium** | tech-spec | Document intended production behavior |
| 8. Missing `error_class` in model | **Medium** | tech-spec | Add to model, migration, and UI filters |
| 9. No DELETE endpoint for field metadata | **Medium** | tech-spec | Add destroy route + controller method |
| 10. `field_metadata` PK definition | **Low** | tech-spec | Define composite PK in migration |
| 11. No `record_lock` integration | **Low** | design-spec | Document as future requirement |
| 12. No `events.error` vs `events.dlq` distinction | **Low** | design-spec | Document as future behavior |

---

## Corrected `sync_error_queue` Migration (MySQL 8.x)

Incorporating gaps 2 and 8:

```sql
CREATE TABLE sync_error_queue (
    id BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY,
    tenant_id BIGINT UNSIGNED NOT NULL,
    record_type VARCHAR(100) NOT NULL,
    record_id VARCHAR(255) NOT NULL,
    error_class VARCHAR(50) NOT NULL DEFAULT 'validation_error'
        COMMENT 'V10 error taxonomy: validation_error, auth_error, transient, conflict, operational',
    reason TEXT NOT NULL
        COMMENT 'Human-readable error description',
    details JSON NOT NULL
        COMMENT 'Full diagnostic context including original canonical payload',
    status ENUM('pending', 'retrying', 'retried', 'dismissed', 'resolved') NOT NULL DEFAULT 'pending',
    record_lock_id BIGINT UNSIGNED NULL
        COMMENT 'FK to record_lock when record is locked during review',
    created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,

    INDEX idx_tenant_status (tenant_id, status),
    INDEX idx_record (record_type, record_id),
    INDEX idx_error_class (error_class)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4
  COMMENT='V10 sync_error_queue — stores events requiring human intervention';
```

## Corrected `field_metadata` Migration (MySQL 8.x)

Incorporating gap 10:

```sql
CREATE TABLE field_metadata (
    tenant_id BIGINT UNSIGNED NOT NULL,
    record_type VARCHAR(100) NOT NULL,
    field_id VARCHAR(255) NOT NULL,
    field_type VARCHAR(50) NOT NULL
        COMMENT 'NetSuite field type: string, integer, boolean, date, list',
    is_synced BOOLEAN NOT NULL DEFAULT TRUE
        COMMENT 'Whether this field is included in sync events',
    is_readonly BOOLEAN NOT NULL DEFAULT FALSE
        COMMENT 'NetSuite-owned fields that SuiteX must never overwrite',
    normalization_rule JSON NULL
        COMMENT 'JSON config for value normalization before comparison',
    conflict_policy VARCHAR(50) NOT NULL DEFAULT 'manual'
        COMMENT 'netsuite-wins, suitex-wins, last-write-wins, manual',
    created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,

    PRIMARY KEY (tenant_id, record_type, field_id),
    INDEX idx_tenant (tenant_id)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4
  COMMENT='V10 Appendix P2.2 — per-field sync configuration';
```