diff --git a/NON_DESTRUCTIVE_EDITS_MODEL_TODO.md b/NON_DESTRUCTIVE_EDITS_MODEL_TODO.md new file mode 100644 index 000000000..3b4ba5c36 --- /dev/null +++ b/NON_DESTRUCTIVE_EDITS_MODEL_TODO.md @@ -0,0 +1,218 @@ +# Non-Destructive Edits: Model Layer To-Do + +## Scope (Model Layer Only) +- Build a durable schema for non-destructive edits that preserves originals and edit history. +- Include only database/model work in `server/src/schema/*` (+ migration files). +- Do not include controller, service, job orchestration, or UI changes in this pass. + +## Execution Classification (Required) +- `[TDD]` means red/green automated verification is expected (migration/integration/schema tests). +- `[HUMAN]` means this cannot be fully confirmed functionally via automated tests and requires manual human completion/sign-off. +- Policy: every `[HUMAN]` checkbox must be completed by a human before this work is considered done. + +## Current Baseline (Already in Repo) +- `asset_edit` stores per-asset edit operations (`action`, `parameters`, `sequence`). +- `asset.isEdited` is maintained by DB triggers (`asset_edit_insert`, `asset_edit_delete`). +- `asset_file` stores separate edited renditions using `isEdited` and unique (`assetId`, `type`, `isEdited`). + +## Why Model Work Is Still Needed +- Edit history is currently destructive: replacing edits deletes prior `asset_edit` rows. +- There is no first-class revision entity for versioning, rollback lineage, or concurrency control. +- Edited files are not explicitly tied to a specific edit revision. +- Some important invariants are only enforced in application code (not at DB level). + +## Target Model (End State) +- Add `asset_edit_revision` table as the source of truth for edit versions per asset. +- Keep `asset_edit` as operation rows, but attach each row to a revision (`revisionId`). +- Add `asset.currentEditRevisionId` so active edits are explicit and fast to query. +- Add `asset_file.editRevisionId` to link derived files to the exact revision they were generated from. +- Keep compatibility columns (`asset.isEdited`, `asset_file.isEdited`) during transition; drive them from triggers. + +## Required Data Invariants +- Original file path/checksum in `asset` remains immutable by edit operations. +- At most one active revision per asset at a time. +- `asset.currentEditRevisionId` must reference a revision belonging to the same `asset.id`. +- Revision operation order must be deterministic and gapless from `sequence = 0`. +- Edited files (`asset_file`) must reference a valid revision for the same asset. +- `asset.isEdited = true` iff `asset.currentEditRevisionId IS NOT NULL` (during compatibility phase). + +## Implementation To-Do + +### 1) Schema Contracts and Naming Freeze +- [ ] [HUMAN] Finalize canonical names: + - [ ] [HUMAN] `asset_edit_revision` (new table) + - [ ] [HUMAN] `asset.currentEditRevisionId` (new nullable FK) + - [ ] [HUMAN] `asset_file.editRevisionId` (new nullable FK) + - [ ] [HUMAN] `asset_edit.revisionId` (new nullable -> then required FK) +- [ ] [HUMAN] Decide lifecycle states for revisions (recommended: `pending`, `active`, `archived`, `failed`). +- [ ] [HUMAN] Write a short ADR note in this file before coding migrations (1-2 paragraphs). + +### 2) Add New Schema Objects +- [ ] [TDD] Update `server/src/schema/enums.ts`: + - [ ] [TDD] Add revision-state enum registration. +- [ ] [TDD] Add `server/src/schema/tables/asset-edit-revision.table.ts`: + - [ ] [TDD] `id` (uuid PK) + - [ ] [TDD] `assetId` (FK -> `asset.id`, cascade on delete/update) + - [ ] [TDD] `createdAt` (timestamp) + - [ ] [TDD] `createdById` (nullable FK -> `user.id`, set null on delete) + - [ ] [TDD] `state` (enum, default `pending`) + - [ ] [TDD] `activatedAt` (nullable timestamp) + - [ ] [TDD] `supersededAt` (nullable timestamp) + - [ ] [TDD] `sourceWidth` / `sourceHeight` (nullable int, optional but recommended) + - [ ] [TDD] `outputWidth` / `outputHeight` (nullable int, optional but recommended) + - [ ] [TDD] `operationCount` (int default `0`) + - [ ] [TDD] `hash` (nullable text for dedupe/idempotency, optional) +- [ ] [TDD] Update `server/src/schema/tables/asset-edit.table.ts`: + - [ ] [TDD] Add `revisionId` FK -> `asset_edit_revision.id` (nullable during transition). + - [ ] [TDD] Keep `assetId` during transition for easier backfill; remove later only after service refactor. + - [ ] [TDD] Keep unique (`assetId`, `sequence`) for compatibility phase. + - [ ] [TDD] Add unique (`revisionId`, `sequence`) as new canonical order key. +- [ ] [TDD] Update `server/src/schema/tables/asset.table.ts`: + - [ ] [TDD] Add nullable FK `currentEditRevisionId` -> `asset_edit_revision.id` (`SET NULL` on delete). + - [ ] [TDD] Keep `isEdited` column for backward compatibility. +- [ ] [TDD] Update `server/src/schema/tables/asset-file.table.ts`: + - [ ] [TDD] Add nullable FK `editRevisionId` -> `asset_edit_revision.id` (`CASCADE` update, `SET NULL` delete). + - [ ] [TDD] Keep `isEdited` for compatibility. +- [ ] [TDD] Update `server/src/schema/index.ts`: + - [ ] [TDD] Register new table in `tables`. + - [ ] [TDD] Add `asset_edit_revision` to `DB` interface. + +### 3) DB-Level Constraints and Indexes +- [ ] [TDD] Add constraints to `asset_edit_revision`: + - [ ] [TDD] Check `operationCount >= 0`. + - [ ] [TDD] Check timestamps are consistent (`activatedAt <= supersededAt` when both are present). +- [ ] [TDD] Add partial unique index for one active revision per asset: + - [ ] [TDD] unique (`assetId`) where `state = 'active'`. +- [ ] [TDD] Add constraints to `asset_edit`: + - [ ] [TDD] Check `sequence >= 0`. + - [ ] [TDD] Check `jsonb_typeof(parameters) = 'object'`. + - [ ] [TDD] Add deferred constraint trigger that enforces per-revision gapless sequence (`0..N-1`) at commit time. +- [ ] [HUMAN] Decide whether action-specific parameter checks are too rigid for long-term evolution. +- [ ] [TDD] If approved, add action-specific parameter shape checks (or trigger validation) for critical actions only. +- [ ] [TDD] Add constraints to `asset_file`: + - [ ] [TDD] Check `isEdited = (editRevisionId IS NOT NULL)` during compatibility phase. + - [ ] [TDD] Keep existing uniqueness on (`assetId`, `type`, `isEdited`) until downstream code is updated. +- [ ] [TDD] Add helpful indexes: + - [ ] [TDD] `asset_edit_revision (assetId, state, createdAt desc)` + - [ ] [TDD] `asset_edit (revisionId, sequence)` + - [ ] [TDD] `asset_file (assetId, editRevisionId, type)` + - [ ] [TDD] `asset (currentEditRevisionId)` (if not auto-indexed by FK tooling) + +### 4) Trigger/Function Updates (Model-Side Logic) +- [ ] [TDD] Update `server/src/schema/functions.ts` with revision-aware trigger functions: + - [ ] [TDD] On revision activation, auto-supersede prior active revision for the same asset. + - [ ] [TDD] Keep `asset.isEdited` synchronized from `asset.currentEditRevisionId`. + - [ ] [TDD] Maintain `asset_edit_revision.operationCount` on `asset_edit` insert/delete. + - [ ] [TDD] Validate `asset.currentEditRevisionId` belongs to the same asset (`asset.id`). + - [ ] [TDD] Validate `asset_file.editRevisionId` belongs to the same asset (`asset_file.assetId`). +- [ ] [TDD] Add write-compatibility trigger for legacy inserts: + - [ ] [TDD] Before insert on `asset_edit`, if `revisionId` is null, assign from `asset.currentEditRevisionId`. + - [ ] [TDD] If `asset.currentEditRevisionId` is null, create and activate a revision, then assign it. +- [ ] [TDD] Preserve existing behavior for compatibility: + - [ ] [TDD] `asset_edit_insert`/`asset_edit_delete` must continue to produce correct `asset.isEdited` while service layer catches up. +- [ ] [TDD] Add migration override entries when function SQL is intentionally managed as overrides. + +### 5) Migration Plan (Order Matters) +- [ ] [TDD] Create migration `A`: add enum + `asset_edit_revision` + nullable new FK columns + indexes (no hard checks yet). +- [ ] [TDD] Create migration `B`: backfill revisions from existing data: + - [ ] [TDD] One `active` revision per asset that currently has rows in `asset_edit`. + - [ ] [TDD] Populate `asset.currentEditRevisionId`. + - [ ] [TDD] Populate `asset_edit.revisionId` by asset mapping. + - [ ] [TDD] Populate `asset_file.editRevisionId` for rows where `asset_file.isEdited = true`. + - [ ] [TDD] Set `operationCount`. +- [ ] [TDD] Create migration `C`: add triggers/functions (including legacy `revisionId` assignment and cross-asset guards) and add constraints as `NOT VALID` where possible. +- [ ] [TDD] Create migration `D`: validate constraints; make `asset_edit.revisionId` `NOT NULL` only after compatibility-trigger tests are green. +- [ ] [TDD] Keep rollback safety: + - [ ] [TDD] Each migration `down` should reverse schema and trigger changes without data loss for original asset files. +- [ ] [HUMAN] Run rollout sequence on staging/prod in change window and sign off on operational safety. + +### 6) Data Verification Queries (Run After Backfill) +- [ ] [TDD] Add this SQL block to migration notes and assert all counts are zero in migration/integration tests. +- [ ] [HUMAN] Run the same queries against staging/prod after migration and record results in rollout notes. + +```sql +-- 1) Edited flag consistency +select count(*) as mismatches +from asset +where ("isEdited" = true and "currentEditRevisionId" is null) + or ("isEdited" = false and "currentEditRevisionId" is not null); + +-- 2) Current revision belongs to same asset +select count(*) as bad_current_links +from asset a +join asset_edit_revision r on r.id = a."currentEditRevisionId" +where r."assetId" <> a.id; + +-- 3) Edited files are linked to a revision +select count(*) as missing_revision_links +from asset_file f +where f."isEdited" = true and f."editRevisionId" is null; + +-- 4) Edited files reference revision for same asset +select count(*) as cross_asset_file_links +from asset_file f +join asset_edit_revision r on r.id = f."editRevisionId" +where r."assetId" <> f."assetId"; + +-- 5) More than one active revision per asset +select count(*) as multi_active_assets +from ( + select "assetId" + from asset_edit_revision + where "state" = 'active' + group by "assetId" + having count(*) > 1 +) t; + +-- 6) Operation ordering gaps (per revision) +with ordered as ( + select "revisionId", "sequence", + row_number() over (partition by "revisionId" order by "sequence") - 1 as expected + from asset_edit +) +select count(*) as sequence_gaps +from ordered +where "sequence" <> expected; + +-- 7) operationCount mismatch +select count(*) as operation_count_mismatch +from asset_edit_revision r +left join ( + select "revisionId", count(*)::int as cnt + from asset_edit + group by "revisionId" +) e on e."revisionId" = r.id +where coalesce(e.cnt, 0) <> r."operationCount"; +``` + +### 7) Model-Focused Test Coverage +- [ ] [TDD] Add/extend migration tests for: + - [ ] [TDD] clean DB -> up migrations. + - [ ] [TDD] existing edit data -> backfill correctness. + - [ ] [TDD] down migration sanity. +- [ ] [TDD] Add schema drift guard: + - [ ] [TDD] run `schema-check` and ensure no unexpected drift. +- [ ] [TDD] Add trigger behavior tests (integration-level): + - [ ] [TDD] activate revision updates asset flags. + - [ ] [TDD] deleting last edit operation does not orphan active state incorrectly. + - [ ] [TDD] edited file links reject cross-asset revision references. + - [ ] [TDD] legacy insert path without `revisionId` remains functional. + - [ ] [TDD] sequence gap creation is rejected at transaction commit. + - [ ] [TDD] concurrent activation attempts cannot produce two active revisions for one asset. + +### 8) Deferred Cleanup (After Service Layer Is Updated) +- [ ] [HUMAN] Confirm production traffic is fully revision-based before removing compatibility paths. +- [ ] [TDD] Remove transitional compatibility constraints/triggers that only support old code paths. +- [ ] [TDD] Remove legacy auto-assignment trigger for `asset_edit.revisionId` after service rollout. +- [ ] [TDD] Consider dropping `asset_edit.assetId` once all reads/writes are revision-based. +- [ ] [TDD] Consider replacing `asset_file.isEdited` with computed semantics from `editRevisionId`. +- [ ] [HUMAN] Decide whether `asset.isEdited` remains as denormalized cache or is dropped. + +## Definition of Done (Model Layer) +- [ ] [TDD] Schema supports multiple preserved edit revisions per asset without deleting old revisions. +- [ ] [TDD] Active revision is explicit, constrained, and query-efficient. +- [ ] [TDD] Edited file rows are revision-linked and integrity-checked (including same-asset validation). +- [ ] [TDD] Backfill is complete and verification queries return zero mismatches in automated tests. +- [ ] [TDD] Migrations are reversible and `schema-check` shows no drift. +- [ ] [HUMAN] Staging/prod verification queries are run and signed off by a human. +- [ ] [HUMAN] All `[HUMAN]` tasks in this file are checked off by a human owner.