# Epic 4: Gap Analysis Against V10 Design Document

**Date:** 2026-03-12
**Scope:** Comparison of Epic 4 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 two critical issues (observer timing and marker injection) but missed several envelope-level and architectural gaps.

---

## Overall Assessment

Epic 4 correctly identifies the Strangler Fig migration pattern, feature flag gating, dependency-inverted publisher interface, and the need to run shadow events alongside the legacy sync. The existing partial gap analysis (`design-spec-gap-analysis.md`) already caught the two most critical issues: (1) observer fires before legacy NetSuite success is confirmed (ghost events), and (2) missing marker field injection. Both of those remain valid and are incorporated below as Gaps 1 and 2.

However, the epic has additional gaps: the canonical envelope is missing several required fields from V10, changes are emitted as full-state snapshots rather than field-level deltas, delete events use a non-canonical tombstone format, and V10's preferred transactional outbox pattern is not addressed.

---

## Gap 1: Shadow Event Fires Before Legacy NetSuite Success (Critical)

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

**V10 Reference:** Legacy API Coexistence (lines 295-298):

> **Shadow Event Emission:** Immediately after a successful legacy API write (upon receiving the NetSuite `internalId`), the legacy SuiteX codebase must publish a "Shadow" `ChangeEvent` to the `events.raw` Pub/Sub topic.

**Current State:** The `ProjectObserver` and `ProjectTaskObserver` dispatch `FormatAndPublishSyncEvent` on Eloquent `created`/`updated`/`deleted` lifecycle events. These fire after the SuiteX database commit (`$afterCommit = true`) but BEFORE the legacy NetSuite API call completes. If the legacy call fails (503, 429, validation error), a shadow event is already queued for an operation that never reached NetSuite.

**Impact:** Ghost events in the event store. The `events.raw` topic and the `events` table would contain records of changes that NetSuite never received, corrupting the `current_state` projection and potentially triggering the merge service to send duplicate writes.

**Required Change:** Move `FormatAndPublishSyncEvent::dispatch()` from the Eloquent Observer into the legacy NetSuite sync service, AFTER receiving a successful HTTP response. The `writeId` must be generated before the API call so it can be injected as a marker field (Gap 2) and then used in the shadow event.

**Affected Files:** `tech-spec.md` (observers and job flow), `design-spec.md` (architecture description).

---

## Gap 2: Missing Marker Field Injection into Legacy NetSuite Payload (Critical)

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

**V10 Reference:** Circular Update Prevention appendix, Section 3 (lines 2788-2803):

> **ALL** writes to NetSuite, including those from the legacy synchronous REST API during the migration phase, MUST include these markers. Failure to inject these markers in legacy API calls will bypass suppression and trigger infinite circular update loops.

V10 V10 changelog (lines 3677):

> Explicitly updated the Circular Update Prevention appendix to strictly require marker field injection on all legacy API payloads.

**Current State:** Neither the observers nor the `FormatAndPublishSyncEvent` job inject `custbody_suitex_write_id` or `custbody_suitex_write_source` into the legacy outbound NetSuite payload.

**Impact:** Without marker fields, the NetSuite UE script will treat SuiteX-originated writes as normal NetSuite changes, emit a new event, and that event will loop back to SuiteX, creating an infinite update cycle.

**Required Change:** Before sending the legacy HTTP request to NetSuite:
1. Generate a `$writeId` UUID
2. Inject `custbody_suitex_write_id = $writeId` and `custbody_suitex_write_source = 'suitex'` into the outbound payload
3. After successful response, pass the same `$writeId` to the shadow event

**Affected Files:** `tech-spec.md` (observer + mapper flow), legacy NetSuite sync service code.

---

## Gap 3: Missing `orderingKey` in Canonical Envelope (High)

**V10 Reference:** Appendix C1 (line 878) requires `orderingKey` as a required field. V10 line 36: "a `ChangeEvent` is produced and published to the durable backbone with an ordering key derived from `(account, recordType, recordId)`."

**Current State:** The `CanonicalEventMapper.buildEnvelope()` method does not produce an `orderingKey` field. Without it, Pub/Sub cannot enforce per-record message ordering.

**Required Change:** Add `orderingKey` to the envelope:

```php
'orderingKey' => $recordType . ':' . (string) $model->id,
```

**Affected Files:** `tech-spec.md` (`CanonicalEventMapper`).

---

## Gap 4: Missing `actorId` in Canonical Envelope (High)

**V10 Reference:** The canonical envelope (line 203) includes `actorId: "8832"`. Per the Epic 1 gap analysis, `actorId` should be a required field in the v1 schema.

**Current State:** The `CanonicalEventMapper.buildEnvelope()` does not include `actorId`. V10 requires it so the event store can serve as an audit log answering "who made this change."

**Required Change:** Add `actorId` to the envelope, resolving the authenticated user:

```php
'actorId' => (string) (auth()->id() ?? 'system'),
```

For queued jobs, ensure the authenticated user context is captured at dispatch time (in the observer) and passed to the job, since `auth()->id()` is not available inside a queued job by default.

**Affected Files:** `tech-spec.md` (`CanonicalEventMapper`, `FormatAndPublishSyncEvent` constructor).

---

## Gap 5: Missing `baseVersion` in Canonical Envelope (High)

**V10 Reference:** Line 288: "Create events for create/update/delete with `baseVersion` = local projection version." The events table DDL (line 247) includes `base_version bigint`.

**Current State:** The `CanonicalEventMapper.buildEnvelope()` does not include `baseVersion`. This field tells the three-way merge algorithm which version of the record the change was derived from.

**Impact:** Without `baseVersion`, the merge service cannot determine the `Base` snapshot for three-way merge. It would have to fall back to reconstructing base from the last known projection, which is more expensive and less reliable.

**Required Change:** For the initial shadow event phase (before `current_state` projections exist), `baseVersion` can be null. Add it explicitly:

```php
'baseVersion' => null, // Will carry projection version once current_state is implemented
```

When `current_state` projections are implemented (later epics), this should be populated with the current projection version for the record.

**Affected Files:** `tech-spec.md` (`CanonicalEventMapper`).

---

## Gap 6: `eventType` Should Be `operation` (Medium)

**V10 Reference:** Per the Epic 1 gap analysis, V10's Appendix C1 schema, idempotency key format, and UE scripts all use `operation`, not `eventType`. The canonical envelope should use `operation` for consistency.

**Current State:** The `CanonicalEventMapper` produces `'eventType' => $eventType` and the observer passes `'create'`, `'update'`, `'delete'` as the `$eventType` parameter. The variable and field names throughout the codebase use `eventType`.

**Required Change:** Rename the field to `operation` in the envelope. Rename the `$eventType` parameter to `$operation` throughout:

- `FormatAndPublishSyncEvent::dispatch($model, 'project', 'create', $writeId)` → uses `$operation` parameter
- `CanonicalEventMapper::buildEnvelope()` → produces `'operation' => $operation`

**Affected Files:** `tech-spec.md` (observers, mapper, job constructor).

---

## Gap 7: Changes Emitted as Full State, Not Field-Level Deltas (Medium)

**V10 Reference:** Line 22: "All changes are emitted as immutable events with `changes` (field-level deltas)." Line 210: "`changes` should be minimal (field-level) when possible."

**Current State:** The `mapChanges()` method emits the current values of ALL mapped fields on every event, regardless of whether they changed:

```php
return [
    'title'      => $model->title,
    'isInactive' => (bool) ($model->isinactive ?? false),
    'startDate'  => $model->startdate ? Carbon::parse($model->startdate)->toDateString() : null,
];
```

For an update where only `title` changed, the event still includes `isInactive` and `startDate`, making them appear as intentional changes. This creates false-positive deltas that the merge service would attempt to apply to NetSuite.

**Impact:** The three-way merge service would see unchanged fields as "SuiteX wants to write these values" and would apply them against NetSuite's current state. If NetSuite had changed `isInactive` since the last sync, the merge would flag a false conflict.

**Required Change:** Use Eloquent's `getDirty()` method to emit only changed fields:

```php
private function mapChanges(Model $model, string $recordType): array
{
    $dirty = $model->getDirty();
    $fieldMap = $this->getFieldMap($recordType);
    $changes = [];

    foreach ($fieldMap as $dbColumn => $canonicalName) {
        if (array_key_exists($dbColumn, $dirty)) {
            $changes[$canonicalName] = $this->normalizeValue($dirty[$dbColumn], $canonicalName);
        }
    }

    return $changes;
}
```

For `create` events, all fields should be included (since everything is "new"). For `delete` events, the `changes` object should be empty or minimal (see Gap 8).

Note: `getDirty()` returns changed attributes only during `updated` events. For `created` events, all attributes are considered dirty, which is the correct behavior.

**Affected Files:** `tech-spec.md` (`CanonicalEventMapper`).

---

## Gap 8: Delete Tombstone Uses Non-Canonical Format (Medium)

**V10 Reference:** The canonical envelope applies to ALL event types including deletes. V10's ChangeEvent schema (Appendix C1, line 883) includes `"operation": {"enum": ["create", "update", "delete"]}` -- deletes use the same envelope.

**Current State:** The `jira-ticket.md` specifies a custom tombstone format for deletes:

```json
{
  "writeId": "<uuid>",
  "sourceSystem": "suitex",
  "action": "deleted",
  "id": 1234
}
```

This uses `action` instead of `operation`/`eventType`, omits `schemaVersion`, `eventId`, `accountId`, `recordType`, `source`, `timestamp`, `orderingKey`, and has `id` instead of `recordId`. This format would fail Stage 1 schema validation at the merge service.

**Required Change:** Delete events must use the full canonical envelope with `operation: "delete"` and an empty (or minimal) `changes` object:

```json
{
  "schemaVersion": "v1",
  "eventId": "uuid",
  "accountId": "tenant-id",
  "recordType": "project",
  "recordId": "1234",
  "operation": "delete",
  "source": "suitex",
  "timestamp": "2026-03-12T10:00:00Z",
  "orderingKey": "project:1234",
  "sourceSystem": "suitex",
  "writeId": "uuid",
  "actorId": "8832",
  "changes": {}
}
```

**Affected Files:** `jira-ticket.md` (tombstone section), `tech-spec.md` (`CanonicalEventMapper` for delete events).

---

## Gap 9: No Transactional Outbox Pattern (Medium — Architectural)

**V10 Reference:** Lines 289-292:

> SuiteX outbound events use a transactional outbox pattern wherever possible:
> - Domain-layer code writes both the business change and an `outbox_events` row in the same database transaction.
> - A dedicated SuiteX background worker reads `outbox_events`, publishes `ChangeEvent` messages to the backbone, and marks outbox rows as dispatched.
> - If publishing fails, the outbox row remains and is retried without duplicating business effects.

**Current State:** Epic 4 uses Eloquent Observers with `$afterCommit = true` that dispatch a Laravel queued job to Redis. This is the "explicit retry with idempotency keys" fallback mentioned in V10 (line 37), not the preferred outbox pattern.

**Impact:** If Redis is unavailable at dispatch time, the event is silently lost. With the transactional outbox pattern, the outbox row is written in the same transaction as the business change, guaranteeing the event is never lost (the background worker will eventually pick it up). The observer + Redis approach has a gap between "database committed" and "job queued to Redis" where failures cause event loss.

**Required Change:** This is an architectural decision. For the MVP shadow phase (where events are non-critical duplicates of the legacy sync), the observer approach is acceptable with the understanding that some events may be lost. However, the design spec should:

1. Acknowledge that the observer pattern is a temporary simplification for shadow mode
2. Document the intent to migrate to the transactional outbox pattern when shadow mode is replaced by the primary event pipeline
3. Consider adding an `outbox_events` table now (even if the outbox worker isn't built yet) so the migration path is clear

**Affected Files:** `design-spec.md` (architectural boundaries section).

---

## Gap 10: Missing `transactionGroupId` in Envelope (Low)

**V10 Reference:** The canonical envelope (line 204) includes `transactionGroupId`. Even when null, it should be present in the envelope structure.

**Current State:** The `CanonicalEventMapper.buildEnvelope()` does not include `transactionGroupId`.

**Required Change:** Add it as null:

```php
'transactionGroupId' => null,
```

**Affected Files:** `tech-spec.md` (`CanonicalEventMapper`).

---

## Gap 11: Hardcoded Field Mappings Instead of `field_metadata` Lookup (Low)

**V10 Reference:** Line 41: "SuiteX maintains a per-record configuration describing which fields are part of the synchronized surface area." Appendix P2.2 defines the `field_metadata` table with `is_synced` flags that drive which fields are included.

**Current State:** The `CanonicalEventMapper.mapChanges()` method hardcodes the field list:

```php
if ($recordType === 'project') {
    return [
        'title'      => $model->title,
        'isInactive' => (bool) ($model->isinactive ?? false),
        'startDate'  => $model->startdate ? ... : null,
    ];
}
```

**Impact:** Adding new synced fields requires a code change and deployment. V10 envisions this as a configuration-driven process via `field_metadata`.

**Required Change:** For the MVP shadow phase, hardcoded mappings are acceptable. However, document the intent to refactor to `field_metadata`-driven mapping when the table is populated (Epic 1 deliverable). The mapper should be structured to make this refactor straightforward -- e.g., extracting the field map into a method that can later query `field_metadata`.

**Affected Files:** `tech-spec.md` (`CanonicalEventMapper`, add a note about future `field_metadata` integration).

---

## Gap 12: Jira-Ticket ProjectPayload startDate Format Inconsistency (Low)

**V10 Reference:** Epic 1 normalization standards: "Dates must be strictly normalized to ISO8601 UTC strings." Appendix K4: Date fields → `YYYY-MM-DD`; Datetime fields → full ISO8601 UTC.

**Current State:** The `jira-ticket.md` ProjectPayload schema defines `startDate` as `"format": "date-time"` (full datetime), but the `tech-spec.md` mapper uses `->toDateString()` which produces `YYYY-MM-DD` (date-only). These are inconsistent.

**Impact:** The schema validator would reject the mapper's output because `"2025-11-15"` does not match the `date-time` format pattern.

**Required Change:** Align the schema and mapper. Since `startDate` is a date-only field in NetSuite (no time component per Appendix K4), the schema should use `"format": "date"` and the mapper should continue producing `YYYY-MM-DD`.

**Affected Files:** `jira-ticket.md` (ProjectPayload schema).

---

## Summary of Required Changes

| Gap | Severity | Files Affected | Change Type |
|-----|----------|----------------|-------------|
| 1. Observer fires before NetSuite success | **Critical** | tech-spec, design-spec | Move dispatch to post-API-success |
| 2. Missing marker field injection | **Critical** | tech-spec, legacy sync code | Inject custbody_suitex_write_* |
| 3. Missing `orderingKey` | **High** | tech-spec | Add to envelope |
| 4. Missing `actorId` | **High** | tech-spec | Add to envelope + capture auth context |
| 5. Missing `baseVersion` | **High** | tech-spec | Add as null (for now) |
| 6. `eventType` → `operation` | **Medium** | tech-spec | Rename field and parameter |
| 7. Full state instead of field-level deltas | **Medium** | tech-spec | Use `getDirty()` for updates |
| 8. Non-canonical delete tombstone | **Medium** | jira-ticket, tech-spec | Use full canonical envelope |
| 9. No transactional outbox pattern | **Medium** | design-spec | Document as temporary; plan migration |
| 10. Missing `transactionGroupId` | **Low** | tech-spec | Add as null |
| 11. Hardcoded field mappings | **Low** | tech-spec | Document intent to use field_metadata |
| 12. startDate format mismatch | **Low** | jira-ticket | Fix schema to `format: "date"` |

---

## Corrected CanonicalEventMapper

Incorporating gaps 3-8, 10:

```php
class CanonicalEventMapper
{
    public function __construct(private TenantService $tenantService) {}

    public function buildEnvelope(
        Model $model,
        string $recordType,
        string $operation,
        string $writeId,
        ?string $actorId = null
    ): array {
        return [
            'schemaVersion'      => 'v1',
            'eventId'            => (string) Str::uuid(),
            'accountId'          => $this->tenantService->getCurrentTenantId(),
            'recordType'         => $recordType,
            'recordId'           => (string) $model->id,
            'operation'          => $operation,
            'source'             => 'suitex',
            'timestamp'          => now()->timezone('UTC')->toIso8601String(),
            'orderingKey'        => $recordType . ':' . (string) $model->id,
            'baseVersion'        => null,
            'changes'            => $this->mapChanges($model, $recordType, $operation),
            'fullSnapshotRef'    => null,
            'sourceSystem'       => 'suitex',
            'writeId'            => $writeId,
            'actorId'            => $actorId ?? 'system',
            'transactionGroupId' => null,
        ];
    }

    private function mapChanges(Model $model, string $recordType, string $operation): array
    {
        if ($operation === 'delete') {
            return [];
        }

        if ($operation === 'create') {
            return $this->mapAllFields($model, $recordType);
        }

        return $this->mapDirtyFields($model, $recordType);
    }

    private function mapDirtyFields(Model $model, string $recordType): array
    {
        $dirty = $model->getDirty();
        $fieldMap = $this->getFieldMap($recordType);
        $changes = [];

        foreach ($fieldMap as $dbColumn => $config) {
            if (array_key_exists($dbColumn, $dirty)) {
                $changes[$config['canonical']] = $this->normalizeValue(
                    $dirty[$dbColumn],
                    $config['type']
                );
            }
        }

        return $changes;
    }

    private function mapAllFields(Model $model, string $recordType): array
    {
        $fieldMap = $this->getFieldMap($recordType);
        $changes = [];

        foreach ($fieldMap as $dbColumn => $config) {
            $changes[$config['canonical']] = $this->normalizeValue(
                $model->getAttribute($dbColumn),
                $config['type']
            );
        }

        return $changes;
    }

    private function getFieldMap(string $recordType): array
    {
        // Hardcoded for MVP; migrate to field_metadata lookup in future epic
        return match ($recordType) {
            'project' => [
                'title'      => ['canonical' => 'title', 'type' => 'string'],
                'isinactive' => ['canonical' => 'isInactive', 'type' => 'boolean'],
                'startdate'  => ['canonical' => 'startDate', 'type' => 'date'],
            ],
            'projecttask' => [
                'title'     => ['canonical' => 'title', 'type' => 'string'],
                'status'    => ['canonical' => 'status', 'type' => 'string'],
                'startdate' => ['canonical' => 'startDate', 'type' => 'date'],
                'enddate'   => ['canonical' => 'endDate', 'type' => 'date'],
            ],
            default => [],
        };
    }

    private function normalizeValue(mixed $value, string $type): mixed
    {
        if ($value === null) {
            return null;
        }

        return match ($type) {
            'boolean' => (bool) $value,
            'date'    => Carbon::parse($value)->toDateString(),
            'string'  => (string) $value,
            default   => $value,
        };
    }
}
```