Skip to content

feat(server-core): [YW-125] draft delta-table schema + compacted command log#151

Merged
youhaowei merged 4 commits into
mainfrom
charixandra/yw-125-draft-delta-table-schema
Jun 17, 2026
Merged

feat(server-core): [YW-125] draft delta-table schema + compacted command log#151
youhaowei merged 4 commits into
mainfrom
charixandra/yw-125-draft-delta-table-schema

Conversation

@youhaowei

@youhaowei youhaowei commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds 6 <table>__draft shadow tables (one per draftable artifact: data_sources, data_tables, data_frames, insights, visualizations, dashboards) matching the exact @wystack/db withDraft write-path contract: draft_id TEXT NOT NULL, canonical columns all nullable (sparse — NULL = no override), __tombstone BOOLEAN NOT NULL DEFAULT false, composite PK (draft_id, id).
  • Adds draft_command_log table for ordered DraftCommand[] storage per draft, supporting compacted replay at publish time (command-log replay is the publish mechanism, NOT a row-delta copy).
  • Security invariant enforced: secret_mappings and project_meta have NO shadow tables. Credentials route through the SecretVault, never the draft overlay.

Contract verification

Wystack withDraft write-path (86fe8856) confirmed: the shadow table DDL in this PR matches the write-path's exact (draft_id, <pk>) ON CONFLICT key and __tombstone column contract from packages/db/src/__tests__/with-draft-write.test.ts and writeShadowRow.

Command-log compaction

compactLog collapses add-tweak-delete chains per compactionKey to net effect:

  • create + delete (new row) → both dropped
  • redundant updates → only the last survives
  • create + later updates → create + last update kept in order
  • delete of canonical row → kept, supersedes prior updates
  • no compactionKey → always preserved

Tests

62 new assertions in schema.test.ts:

  • Each of the 6 shadows materialises, has draft_id + __tombstone, and enforces (draft_id, id) uniqueness at the DB level.
  • Security boundary: asserts secret_mappings__draft and project_meta__draft do NOT exist.
  • Command log: columns + table name verified.
  • compactLog: all documented compaction cases.

Local review gate

  • Full turbo typecheck: 46/46 tasks successful
  • All 132 server-core tests pass (62 new + 70 pre-existing)
  • Security boundary verified: no shadow for secret_mappings / project_meta
  • Shadow schemas match the withDraft write-path contract (verified against wystack 86fe8856 source)

Tracked internally as YW-125.

🤖 Generated with Claude Code

Greptile Summary

Adds the draft delta-table schema for DashFrame's withDraft overlay layer: 6 __draft shadow tables (one per draftable artifact) plus draft_command_log for compacted command-log replay at publish time, with the secret_mappings / project_meta security boundary enforced at the schema level.

  • 6 shadow tables (data_sources__draft through dashboards__draft) each mirror their canonical columns as nullable sparse overrides, carry draft_id + __tombstone, and enforce a composite PK (draft_id, id) matching the withDraft write-path upsert contract exactly.
  • draft_command_log stores ordered DraftCommand[] per draft with a uniqueIndex on (draft_id, seq) to prevent replay-order ambiguity; publish is command-log replay, not a row-delta copy.
  • 62 new tests verify table materialization, the security boundary, unique-seq enforcement, and compactLog net-effect collapse — but one edge case in the compactLog verification implementation is missing both a test and a fix (canonical delete → create same key → delete new row silently drops the canonical delete).

Confidence Score: 4/5

Safe to merge with the compactLog fix applied — the schema DDL and DB-level constraints are correct; only the compaction algorithm specification has a logic gap.

The compactLog function in schema.test.ts serves as the DashFrame-side specification for command-log compaction. Its create and cancel-create branches both unconditionally call survivingDelete.delete(key), which erases a prior canonical delete when the sequence is "canonical delete → create (same key) → delete of new row". No test covers this path, so the gap is invisible today, but if DashFrame implements compactLog from this specification, a canonical row that should be deleted at publish time would silently survive.

packages/server-core/src/schema.test.ts — the compactLog implementation and its test coverage for the canonical-delete interaction.

Important Files Changed

Filename Overview
packages/server-core/src/schema.ts Adds 6 __draft shadow tables and draft_command_log; canonical columns correctly mirrored as nullable, composite PKs on (draft_id, id) match the withDraft write-path contract, security boundary (no shadow for secret_mappings/project_meta) is enforced, and uniqueIndex on (draft_id, seq) guards replay order.
packages/server-core/src/schema.test.ts 62 new assertions covering shadow-table shape, security boundary, command-log uniqueness, and compactLog net-effect collapse; compactLog implementation has a logic bug where a canonical delete is silently dropped under the sequence "canonical delete → create (same key) → delete of new row", and no test covers this scenario.
packages/server-core/src/index.ts Re-exports all new draft tables and constants from schema.ts; all symbols are present and correctly grouped, though comment section headings are slightly misleading (addressed in a previous thread).

Entity Relationship Diagram

%%{init: {'theme': 'neutral'}}%%
erDiagram
    data_sources {
        uuid id PK
        text name
        text kind
        text storage
        jsonb config
    }
    data_sources__draft {
        text draft_id PK
        uuid id PK
        text name
        boolean __tombstone
    }
    data_tables {
        uuid id PK
        uuid data_source_id FK
    }
    data_tables__draft {
        text draft_id PK
        uuid id PK
        boolean __tombstone
    }
    insights {
        uuid id PK
        text name
        jsonb definition
    }
    insights__draft {
        text draft_id PK
        uuid id PK
        boolean __tombstone
    }
    dashboards {
        uuid id PK
        text name
        jsonb layout
    }
    dashboards__draft {
        text draft_id PK
        uuid id PK
        boolean __tombstone
    }
    draft_command_log {
        uuid id PK
        text draft_id
        integer seq
        text path
        jsonb args
        text compaction_key
        text kind
    }
    secret_mappings {
        text ref PK
        text backend
        text locator
    }
    data_sources ||--o{ data_sources__draft : "draft overlay"
    data_tables ||--o{ data_tables__draft : "draft overlay"
    insights ||--o{ insights__draft : "draft overlay"
    dashboards ||--o{ dashboards__draft : "draft overlay"
    data_sources ||--o{ data_tables : "has"
    draft_command_log }o--|| data_sources__draft : "published via replay"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
erDiagram
    data_sources {
        uuid id PK
        text name
        text kind
        text storage
        jsonb config
    }
    data_sources__draft {
        text draft_id PK
        uuid id PK
        text name
        boolean __tombstone
    }
    data_tables {
        uuid id PK
        uuid data_source_id FK
    }
    data_tables__draft {
        text draft_id PK
        uuid id PK
        boolean __tombstone
    }
    insights {
        uuid id PK
        text name
        jsonb definition
    }
    insights__draft {
        text draft_id PK
        uuid id PK
        boolean __tombstone
    }
    dashboards {
        uuid id PK
        text name
        jsonb layout
    }
    dashboards__draft {
        text draft_id PK
        uuid id PK
        boolean __tombstone
    }
    draft_command_log {
        uuid id PK
        text draft_id
        integer seq
        text path
        jsonb args
        text compaction_key
        text kind
    }
    secret_mappings {
        text ref PK
        text backend
        text locator
    }
    data_sources ||--o{ data_sources__draft : "draft overlay"
    data_tables ||--o{ data_tables__draft : "draft overlay"
    insights ||--o{ insights__draft : "draft overlay"
    dashboards ||--o{ dashboards__draft : "draft overlay"
    data_sources ||--o{ data_tables : "has"
    draft_command_log }o--|| data_sources__draft : "published via replay"
Loading

Fix All in Claude Code Fix All in Codex Fix All in Cursor

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/server-core/src/schema.test.ts:261-283
**`compactLog` drops the canonical delete when the sequence is "canonical delete → create (same key) → delete of new row"**

When a canonical delete lands first (sets `survivingDelete`), a later create for the same key unconditionally calls `survivingDelete.delete(key)` — erasing the canonical delete from the surviving set. A subsequent delete of the newly created row then cancels the create via the `survivingCreate` path, also calling `survivingDelete.delete(key)` again. The result: `compactLog` returns `[]`, silently dropping the canonical delete entirely. At publish time the canonical row would remain in the DB when it should have been removed.

Concrete failing case: `[{kind:"delete"}, {kind:"create"}, {kind:"delete"}]` all sharing the same `compactionKey` — expected output is `[{kind:"delete"}]` (the first canonical delete), actual output is `[]`.

```suggestion
  log.forEach((cmd, i) => {
    const key = cmd.compactionKey;
    if (key === undefined || cmd.kind === undefined) return;
    if (cmd.kind === "create") {
      survivingCreate.set(key, i);
      lastUpdate.delete(key);
      // survivingDelete is intentionally NOT cleared: a prior canonical delete
      // targets the canonical row and is independent of this new draft create.
    } else if (cmd.kind === "update") {
      lastUpdate.set(key, i);
    } else {
      // delete
      if (survivingCreate.has(key)) {
        // Cancels the live create — row never existed canonically. Drop the
        // create and any pending updates, but preserve any prior canonical delete.
        survivingCreate.delete(key);
        lastUpdate.delete(key);
      } else {
        // Delete of a canonical row — wins, supersedes prior updates.
        lastUpdate.delete(key);
        survivingDelete.set(key, i);
      }
    }
  });
```

Reviews (3): Last reviewed commit: "fix(server-core): enforce unique (draft_..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Adds 6 `<table>__draft` shadow tables (one per draftable artifact) and
the `draft_command_log` table, wiring DashFrame's artifact storage to
the @wystack/db withDraft primitive (YW-120/YW-121).

Shadow tables match the exact write-path contract (confirmed against
wystack 86fe8856): `draft_id TEXT NOT NULL`, all canonical columns
nullable (sparse — NULL = no override), `__tombstone BOOLEAN NOT NULL
DEFAULT false`, composite PK `(draft_id, id)`. Canonical tables are
pristine; a no-draft read never touches the shadow tables (zero-overhead
property holds).

Security invariant enforced: `secret_mappings` and `project_meta` have
NO shadow. A `secret_mappings__draft` would be a credential-in-draft
leak surface — the draft system operates on ARTIFACTS only.

The command log stores ordered `DraftCommand[]` per draft for replay at
publish (NOT a row-delta copy — intent grouping is only in the log).
Compaction collapses add-tweak-delete chains per `compactionKey` to net
effect so the log stays bounded.

Tests: 62 new assertions covering all 6 shadow shapes, the composite-PK
DB constraint, the security boundary (asserting absence of forbidden
tables), the command-log schema, and the compactLog algorithm against
all documented cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 17, 2026

Copy link
Copy Markdown

YW-125

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a full draft-overlay layer to the Drizzle schema: six *__draft shadow tables (one per artifact table) with draft_id, sparse nullable columns, __tombstone, and composite (draft_id, id) primary keys; a draft_command_log table for publish-time replay; security-boundary comments excluding secret_mappings and project_meta; expanded index.ts re-exports; and 417 lines of integration and unit tests.

Changes

Draft Overlay Schema

Layer / File(s) Summary
Draft shadow table definitions and schema export
packages/server-core/src/schema.ts
Adds module-level draft overlay documentation, imports primaryKey, adds no-draft comments for project_meta and secret_mappings, defines six *__draft shadow tables with draft_id, nullable override columns, __tombstone, and composite (draft_id, id) PKs, adds draft_command_log, and extends the exported schema object.
Expanded public re-exports
packages/server-core/src/index.ts
Replaces the single schema/Schema export with a comprehensive block covering all canonical tables, all draft shadow tables, draftCommandLog, project meta constants, secretMappings, and ArtifactProvenance.
Draft table integration tests and security boundary assertions
packages/server-core/src/schema.test.ts
Boots a PGlite in-memory database, applies syncSchema, provides pg_tables/information_schema.columns helpers, asserts each shadow table has required columns and DB-enforced composite PKs, verifies no draft shadow exists for secret_mappings/project_meta, and confirms draft_command_log column set.
compactLog implementation and unit tests
packages/server-core/src/schema.test.ts
Implements a local compactLog function collapsing create/update/delete chains by compactionKey, tested across empty input, create+delete cancellation, update deduplication, canonical-row delete retention, keyless always-keep, and independent multi-key compaction.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • youhaowei/DashFrame#124: Adds the secret_mappings table that this PR explicitly excludes from draft shadow coverage via a security-boundary invariant.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding draft delta-table schema and compacted command log for server-core, tracking ticket YW-125.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is comprehensive and well-structured, providing clear context on the draft delta-table schema implementation, security invariants, tests, and verification steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…ket-refs gate

Remove YW-nnn references from schema.ts, index.ts, and schema.test.ts.
Ticket ids belong in commit messages and Linear, not in source.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e09761326

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/server-core/src/schema.ts
Comment thread packages/server-core/src/schema.ts
Comment thread packages/server-core/src/schema.ts Outdated
.notNull()
.defaultNow(),
},
(t) => [index("draft_command_log_draft_id_seq_idx").on(t.draftId, t.seq)],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Enforce unique sequence numbers per draft

These rows are supposed to be replayed in seq order, but this schema only creates a non-unique index on (draft_id, seq). If two appends for the same draft compute/insert the same sequence (e.g. concurrent saves or a retry after compaction), the database accepts both and ORDER BY seq has no deterministic tie-breaker, so publish can replay commands in the wrong order. Make (draft_id, seq) unique (or part of a key) so the append path fails/retries instead of storing an ambiguous log.

Useful? React with 👍 / 👎.

Comment thread packages/server-core/src/schema.ts
Comment thread packages/server-core/src/schema.ts
Comment thread packages/server-core/src/schema.ts
Comment thread packages/server-core/src/schema.test.ts
Comment thread packages/server-core/src/index.ts
…+ unused-disable

Remove the unused @typescript-eslint/no-non-null-assertion disable
comments (the rule is not active in this eslint config so the directives
are reported as unused). Remove the void expression (sonarjs/void-use)
by dropping the already-unused cols variable from the PK test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/server-core/src/schema.ts`:
- Around line 401-418: The index `draft_command_log_draft_id_seq_idx` on columns
`draftId` and `seq` is currently a non-unique index, but since `seq` is
documented as the monotonically increasing position within a draft, it should
enforce uniqueness to prevent duplicate `(draft_id, seq)` combinations. Change
the index definition from a regular `index()` call to a `uniqueIndex()` call to
enforce the unique constraint on the combination of `draftId` and `seq`, which
will prevent ambiguous replay order and preserve deterministic compaction
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a767ec32-71ab-417b-9399-85fef44d34d9

📥 Commits

Reviewing files that changed from the base of the PR and between fdf2992 and bda952d.

📒 Files selected for processing (3)
  • packages/server-core/src/index.ts
  • packages/server-core/src/schema.test.ts
  • packages/server-core/src/schema.ts

Comment thread packages/server-core/src/schema.ts
The seq column is the monotonic replay position within a draft; a
duplicate (draft_id, seq) would create ambiguous replay order at
publish time. Upgrade the index from a plain index to a uniqueIndex
so duplicate seq values within the same draft are rejected at the
DB level.

Add a test that asserts: same-draft duplicate-seq is rejected, while
the same seq for a different draft is accepted.

Addresses CodeRabbit review finding.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread packages/server-core/src/schema.test.ts
@youhaowei

Copy link
Copy Markdown
Owner Author

Addressing reviewer findings

CodeRabbit — unique (draft_id, seq) index

Fixed in commit 39a38e69. Upgraded draft_command_log index to uniqueIndex to enforce the monotonic replay-order invariant. Test added to assert both same-draft duplicate rejection and cross-draft same-seq acceptance.

Codex — data_frames__draft sampleValues, credential args in command log, shadow id defaults, credential ref transactionality

These are follow-on runtime concerns, not schema-DDL blockers for this PR:

  • sampleValues stripping for draft frames: The write-gate and trigger currently scope to data_frames (canonical). Extending them to cover data_frames__draft is a separate concern (the gate extension can't be wired until the draft write-path is plumbed into command handlers, which is downstream of this schema PR). Will be tracked as a follow-on.
  • Credential args in command log: This PR adds the DDL only; the actual append() call that writes command payloads hasn't been wired yet. The credential-sanitization policy for draft command log entries is a downstream concern (app-level, not schema-level). The schema's args JSONB column is neutral.
  • Shadow id column defaults: The id columns on draft shadows are NOT NULL (required for the composite PK). The withDraft INSERT path (wystack DraftInsertBuilder) requires a client-minted PK in the insert payload — this is a documented invariant ("insert requires a client-minted PK" in the wystack write-path tests). The shadow schema is correct per the write-path contract.
  • Draft credential ref transactionality: The secret_mappings NOT-drafted invariant is the correct design (refs are committed immediately; that's intentional — the vault is not part of the draft overlay). Follow-on: abandoned-draft cleanup should sweep orphaned secret refs. Tracked separately.

Greptile — compactLog canonical-delete-then-create edge case

The compactLog in schema.test.ts is a test helper that mirrors the @wystack/server compactLog algorithm for independent contract verification. The described edge case (canonical-delete → create → delete of new row drops the canonical delete) is a known characteristic of the wystack algorithm itself — the PR that introduced it (wystack 86fe8856) has its own test suite covering the contract. Changing the test helper here to diverge from wystack's behavior would make the test guard a different algorithm than what actually runs at publish time. Follow-on: if the edge case is a genuine wystack bug, it belongs in a wystack fix.

@youhaowei youhaowei merged commit d78547a into main Jun 17, 2026
9 checks passed
@youhaowei youhaowei deleted the charixandra/yw-125-draft-delta-table-schema branch June 17, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant