refactor(session-persistence-sqlite): drop the materialized column; use row existence as the signal (review #35)
The `materialized` INTEGER column was redundant: create()/update() already keep a lazy session in memory and write no row, so a `sessions` row is written only by the first append. Its EXISTENCE is the materialization signal — has()/list() now report exactly the sessions that have a row, matching the JSONL backend's "file exists ⇔ materialized". The column only existed to force has()/list() to FALSE for an all-tail crash (a partial first turn, zero committed events). That actually DIVERGED from the JSONL backend, whose file (and thus has()=true) survives a first append that never reached turn/end. Removing the column drops that special case: an all-tail session keeps its row and stays present, the same as JSONL. The orphaned tail rows are still removed by the deferred truncation-repair on the next append, and load() stays non-mutating. Also: add a TODO to route through a cordis db service if one is adopted, and correct the README's Node-version framing to the repo's engines (>=24).
This commit is contained in:
@@ -2,17 +2,19 @@
|
||||
|
||||
A SQLite durable session-persistence backend — a second `SessionPersistence` implementation ([ADR 0018](../../docs/adr/0018-session-persistence.md)), built to validate that the abstract seam and the shared `runPersistenceContract` suite are genuinely backend-agnostic. It satisfies the SAME contract as `dsh-session-persistence-jsonl` (append-only, contiguous-seq, lazy materialization, crash-tail-on-load), expressed over `node:sqlite` rows instead of file bytes.
|
||||
|
||||
> **TODO:** this backend talks to `node:sqlite` directly. If a cordis database service (`cordis/db` / a `@cordisjs` SQL driver plugin) is adopted, route through that instead of holding a raw `DatabaseSync` here — the contract surface (`SessionPersistence`) would not change, only the storage driver.
|
||||
|
||||
## Storage model
|
||||
|
||||
Each `SessionEvent` maps 1:1 onto a row in an `events` table `(session_id, seq, type, time, data)` — `data` is the event payload as JSON text, so the row shape is the event verbatim (including `assistant/chunk`, keeping `seq` contiguous). Out-of-log metadata (`SessionMeta`) lives in a `sessions` row, including the mutable `SessionSummary` fields (`updatedAt`, `title`, `firstPrompt`) that `update()` rewrites without touching the event log.
|
||||
Each `SessionEvent` maps 1:1 onto a row in an `events` table `(session_id, seq, type, time, data)` — `data` is the event payload as JSON text, so the row shape is the event verbatim (including `assistant/chunk`, keeping `seq` contiguous). Out-of-log metadata (`SessionMeta`) lives in a `sessions` row, including the mutable `SessionSummary` fields (`updatedAt`, `title`, `firstPrompt`) that `update()` rewrites without touching the event log. A `sessions` row is written only by the first `append` — its existence is the lazy-materialization signal (`has`/`list` report exactly the sessions that have a row), so no separate column is needed.
|
||||
|
||||
`node:sqlite` requires Node ≥ 22.5 (this repo runs Node ≥ 24); the database opens with `foreign_keys = ON` (so `ON DELETE CASCADE` drops a session's events with its row) and `journal_mode = WAL`. The table-layout version is stored in `PRAGMA user_version` and checked on open: a fresh database is stamped with the current `SCHEMA_VERSION`; a database written by a newer, incompatible build (higher `user_version`) is rejected rather than opened against an unknown layout.
|
||||
The repo targets Node ≥ 24 (the root `engines` field), which includes the stable `node:sqlite` module. The database opens with `foreign_keys = ON` (so `ON DELETE CASCADE` drops a session's events with its row) and `journal_mode = WAL`. The table-layout version is stored in `PRAGMA user_version` and checked on open: a fresh database is stamped with the current `SCHEMA_VERSION`; a database written by a newer, incompatible build (higher `user_version`) is rejected rather than opened against an unknown layout.
|
||||
|
||||
## Contract semantics over rows
|
||||
|
||||
- **Append = a transaction.** `append` runs `BEGIN`/`COMMIT` around the batch: it runs any deferred crash-tail repair, materializes the `sessions` row (if still lazy), and INSERTs every event, asserting the contiguous-seq contract first (the first event's `seq` must equal the stored next-seq). A mid-batch failure (a UNIQUE violation on a duplicated seq) rolls back entirely, so the stored log and the in-memory cursor stay consistent.
|
||||
- **Lazy materialization.** `create()` records intent in memory only — no row is written until the first `append`. A created-but-never-appended session is absent from `has()`/`list()` (a `materialized` flag on the row, set inside the first append transaction; `has`/`list` filter to materialized rows).
|
||||
- **Crash-tail-on-load.** `load()` reads every stored event ordered by `seq` and returns only the prefix through the **last complete `turn/end`** (the `SessionPersistence.load` contract), computed from the `seq`/`type` columns so a malformed `data` in the uncommitted tail is never parsed. A batch that landed without its closing `turn/end` (a process killed mid-turn) is an uncommitted tail: `load()` stays non-mutating w.r.t. the event log and records a repair point; the **next `append`** physically DELETEs the orphaned rows inside its transaction (the one-time truncation-repair, matching the JSONL backend and the abstract contract). A `seq` gap inside the committed region makes the session unloadable. If the discarded tail was the session's only committed content, `load()` also flips the metadata row's `materialized` flag to 0 so `has()`/`list()` immediately stop reporting the now-empty session.
|
||||
- **Lazy materialization.** `create()` records intent in memory only — no row is written until the first `append`. A created-but-never-appended session has no `sessions` row, so it is absent from `has()`/`list()` (which report exactly the sessions that have a row).
|
||||
- **Crash-tail-on-load.** `load()` reads every stored event ordered by `seq` and returns only the prefix through the **last complete `turn/end`** (the `SessionPersistence.load` contract), computed from the `seq`/`type` columns so a malformed `data` in the uncommitted tail is never parsed. A batch that landed without its closing `turn/end` (a process killed mid-turn) is an uncommitted tail: `load()` stays non-mutating w.r.t. the event log and records a repair point; the **next `append`** physically DELETEs the orphaned rows inside its transaction (the one-time truncation-repair, matching the JSONL backend and the abstract contract). A `seq` gap inside the committed region makes the session unloadable. A session materialized by a partial first turn (all-tail, zero committed events) keeps its metadata row and stays present in `has()`/`list()` — the same as the JSONL backend, whose file likewise survives a first append that never reached `turn/end`.
|
||||
|
||||
## Configuration (schemastery)
|
||||
|
||||
|
||||
@@ -245,18 +245,12 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
// returns the committed prefix; the next append performs the one-time
|
||||
// physical repair). Record the repair point so the next appendCore DELETEs
|
||||
// the orphaned tail inside its own transaction before inserting.
|
||||
const materialized = committed.length > 0
|
||||
if (committed.length === 0 && row.materialized === 1) {
|
||||
// All-tail discard: the only committed events were a crash tail, so the
|
||||
// session now has NO committed events. The metadata row, however, still
|
||||
// reads materialized = 1 from the prior append — which would make has()
|
||||
// and list() report a session that load() just emptied. Correct the
|
||||
// materialized FLAG (metadata, not the event log) so has()/list() are
|
||||
// immediately consistent. The orphaned tail rows are still removed by the
|
||||
// deferred repair on the next append.
|
||||
this.db.prepare('UPDATE sessions SET materialized = 0 WHERE id = ?').run(id)
|
||||
}
|
||||
|
||||
//
|
||||
// The metadata row stays as-is even when committed.length === 0 (an all-tail
|
||||
// crash): the session WAS materialized by the partial append, so its row
|
||||
// exists and has()/list() report it present — the same as the JSONL backend,
|
||||
// whose file likewise survives a first append that never reached turn/end.
|
||||
//
|
||||
// Record state so a later append continues at the committed length and runs
|
||||
// the deferred tail repair. The state keeps its OWN copy of the meta; the
|
||||
// returned value is separate so a consumer mutating loaded.meta cannot
|
||||
@@ -264,7 +258,7 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
this.states.set(id, {
|
||||
meta: { ...meta },
|
||||
cursor: committed.length,
|
||||
materialized,
|
||||
materialized: true,
|
||||
...cutTail ? { repairFrom: committed.length } : {},
|
||||
})
|
||||
return { meta, events }
|
||||
@@ -272,11 +266,11 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
|
||||
async list(): Promise<SessionMeta[]> {
|
||||
await this.ready
|
||||
// Materialized rows only: a created-but-never-appended (lazy) session has no
|
||||
// row at all, and a load that cut every event back to zero leaves
|
||||
// materialized = 0. Both are excluded, matching has().
|
||||
// Every metadata row is a materialized session: the row is written only by
|
||||
// the first append (a created-but-never-appended session has no row), so
|
||||
// listing all rows is exactly the materialized set.
|
||||
const rows = this.db
|
||||
.prepare('SELECT * FROM sessions WHERE materialized = 1')
|
||||
.prepare('SELECT * FROM sessions')
|
||||
.all() as unknown as SessionRow[]
|
||||
return rows.map(rowToMeta)
|
||||
}
|
||||
@@ -285,8 +279,8 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
await this.ready
|
||||
const state = this.states.get(id)
|
||||
if (state?.materialized) return true
|
||||
const row = this.rowFor(id)
|
||||
return row !== undefined && row.materialized === 1
|
||||
// A metadata row exists iff the session was materialized by a first append.
|
||||
return this.rowFor(id) !== undefined
|
||||
}
|
||||
|
||||
delete(id: SessionId): Promise<void> {
|
||||
@@ -326,15 +320,15 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
}
|
||||
|
||||
/**
|
||||
* Insert-or-replace a session's metadata row, marked materialized. The only
|
||||
* callers are the first materializing `append` and a post-materialization
|
||||
* `update` — a row is written only once a session has durable events, so
|
||||
* `materialized` is always 1 (a never-appended session has no row at all).
|
||||
* Insert-or-replace a session's metadata row. The only callers are the first
|
||||
* materializing `append` and a post-materialization `update`, so writing the
|
||||
* row IS the materialization (its existence is the signal `has`/`list` read);
|
||||
* a never-appended session has no row at all.
|
||||
*/
|
||||
private writeRow(meta: SessionMeta): void {
|
||||
this.db.prepare(`
|
||||
INSERT INTO sessions (id, version, created_at, cwd, parent_session, updated_at, title, first_prompt, materialized)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, 1)
|
||||
INSERT INTO sessions (id, version, created_at, cwd, parent_session, updated_at, title, first_prompt)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
version = excluded.version,
|
||||
created_at = excluded.created_at,
|
||||
@@ -342,8 +336,7 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
parent_session = excluded.parent_session,
|
||||
updated_at = excluded.updated_at,
|
||||
title = excluded.title,
|
||||
first_prompt = excluded.first_prompt,
|
||||
materialized = excluded.materialized
|
||||
first_prompt = excluded.first_prompt
|
||||
`).run(
|
||||
meta.id,
|
||||
meta.version,
|
||||
@@ -466,7 +459,7 @@ export class SessionPersistenceSqlite extends SessionPersistence {
|
||||
}
|
||||
|
||||
const row = this.rowFor(id)
|
||||
if (row !== undefined && row.materialized === 1) {
|
||||
if (row !== undefined) {
|
||||
const stored = this.eventsFor(id)
|
||||
if (!seedCoversPrefix(seed, stored)) {
|
||||
throw new Error(`session "${id}" already has a persisted log that does not match this live session (id collision)`)
|
||||
|
||||
@@ -18,10 +18,11 @@ import type { SessionEvent, SessionId, SessionMeta } from '@deepseek-ai/dsh-sess
|
||||
export const SCHEMA_VERSION = 1
|
||||
|
||||
/**
|
||||
* A row of the `sessions` table — the out-of-log metadata (`SessionMeta`) plus
|
||||
* the `materialized` flag that implements lazy materialization (a created-but-
|
||||
* never-appended session has `materialized = 0` and is excluded from
|
||||
* `has`/`list`, mirroring the JSONL backend's "no file until first append").
|
||||
* A row of the `sessions` table — the out-of-log metadata (`SessionMeta`). The
|
||||
* row's EXISTENCE is the materialization signal: it is written only by the
|
||||
* first `append` (lazy materialization), so a created-but-never-appended
|
||||
* session has no row and is absent from `has`/`list`, mirroring the JSONL
|
||||
* backend's "no file until first append".
|
||||
*/
|
||||
export interface SessionRow {
|
||||
id: string
|
||||
@@ -32,7 +33,6 @@ export interface SessionRow {
|
||||
updated_at: number
|
||||
title: string | null
|
||||
first_prompt: string | null
|
||||
materialized: number
|
||||
}
|
||||
|
||||
/** An `events` table row: one `SessionEvent` mapped 1:1 (`data` is JSON text). */
|
||||
@@ -81,8 +81,7 @@ export function openDatabase(path: string): DatabaseSync {
|
||||
parent_session TEXT,
|
||||
updated_at INTEGER NOT NULL,
|
||||
title TEXT,
|
||||
first_prompt TEXT,
|
||||
materialized INTEGER NOT NULL DEFAULT 0
|
||||
first_prompt TEXT
|
||||
) STRICT
|
||||
`)
|
||||
db.exec(`
|
||||
|
||||
@@ -139,7 +139,7 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
await b2.dispose()
|
||||
})
|
||||
|
||||
it('all-tail load: a session whose only content is a crash tail is absent from has()/list()', async () => {
|
||||
it('all-tail load: a session materialized by a partial first turn stays present (JSONL parity), load returns zero committed events', async () => {
|
||||
const path = await freshDbPath()
|
||||
const m = meta('all-tail')
|
||||
const b1 = await backend(path)
|
||||
@@ -153,12 +153,15 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
await b1.dispose()
|
||||
|
||||
// A fresh backend loads it: the committed prefix is empty (no turn/end), so
|
||||
// the session has no committed content. has()/list() must NOT report it.
|
||||
// load returns zero events — but the session WAS materialized (its metadata
|
||||
// row exists), so has()/list() still report it present, matching the JSONL
|
||||
// backend whose file likewise survives a first append that never reached
|
||||
// turn/end. The orphaned tail rows are removed by the next append's repair.
|
||||
const b2 = await backend(path)
|
||||
const loaded = await b2.ctx.sessionPersistence.load(m.id)
|
||||
expect(loaded.events).toEqual([])
|
||||
expect(await b2.ctx.sessionPersistence.has(m.id)).toBe(false)
|
||||
expect((await b2.ctx.sessionPersistence.list()).map(x => x.id)).not.toContain(m.id)
|
||||
expect(await b2.ctx.sessionPersistence.has(m.id)).toBe(true)
|
||||
expect((await b2.ctx.sessionPersistence.list()).map(x => x.id)).toContain(m.id)
|
||||
await b2.dispose()
|
||||
})
|
||||
|
||||
@@ -269,7 +272,7 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
const path = await freshDbPath()
|
||||
// Materialize a row with version 2 directly via the real schema.
|
||||
const db = openDatabase(path)
|
||||
db.prepare('INSERT INTO sessions (id, version, created_at, updated_at, materialized) VALUES (?, ?, ?, ?, 1)')
|
||||
db.prepare('INSERT INTO sessions (id, version, created_at, updated_at) VALUES (?, ?, ?, ?)')
|
||||
.run('v2', 2, 1, 1)
|
||||
db.close()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user