From 759c8352cb09a19d7d2846d4e6570b02dcafd904 Mon Sep 17 00:00:00 2001 From: jgluck96 Date: Tue, 2 Jun 2026 01:51:45 -0400 Subject: [PATCH] Phase 5.20: conversation-store per-surface tables (sync OSS sutando) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the single-table conversation-store schema with per-surface tables (voice / phone / discord_voice) plus an idempotent migration from the legacy schema, mirroring OSS sutando. Stando's `.app`-bundle- compatible lazy `require('node:sqlite')` pattern is preserved (OSS uses a static import). Public API is unchanged — `recordConversation(role, text, sessionId)`, `recordSessionBoundary(reason, sessionId)`, and `recordSession(metrics)` keep identical signatures so all existing callers (voice-agent.ts, task-bridge.ts, skills/phone-conversation/.../conversation-server.ts) continue to work without rewiring. Three additive exports land: - sourceFromRole(role) — role-prefix → table-keyed source - kindFromRole(role) — legacy role → per-surface kind taxonomy - recordToolCall(source, name, durationMs, sessionId) — per-tool-call sink so tool invocations land in the surface table immediately (kind='tool_call') instead of being batched at session end via sessions.tool_calls JSON. Survives crashes, kill -9, ngrok drops. Schema details: - Surface tables (voice/phone/discord_voice): identical shape, each holds utterances + tool calls in one chronological stream (id INTEGER PRIMARY KEY ordering; kind discriminates row type). - sessions/session_events tables retained — different concerns. - sessions.tool_calls and sessions.events JSON columns dropped post- init (the atom rows now live in surface tables / session_events). - New views: v_voice, v_phone, v_discord_voice, v_sessions — thin wrappers adding a human-readable `time` column for DB Browser. - Backwards-compat `conversation` VIEW that unions the three surface tables under the legacy `role` column name so external scripts (query-conversation.sh, regression-search) keep working without source edits. Migration: - Runs once on first init when legacy `conversation` or `tool_calls` tables are present and any surface table is empty. - Single transaction; failure rolls back and logs (best-effort). - NULL ts_unix rows are dropped (can't satisfy NOT NULL surface schema; no other column carries time). - Idempotent — second open is a no-op (legacy tables already dropped). - Also backfills discord_voice from sessions.tool_calls JSON via json_each (discord-voice never wrote to the standalone tool_calls table in the legacy schema). Stando-specific divergence from OSS (intentional, documented): - `import type { DatabaseSync }` + lazy `require('node:sqlite')` in init(). Older bundled Node runtimes gate node:sqlite behind --experimental-sqlite; a static import would throw ERR_UNKNOWN_BUILTIN_MODULE and crash every process that imports conversation-store. The lazy path lets the mirror degrade to a no-op while the text conversation.log keeps working. - Issue numbers (#1052 etc.) replaced with neutral references ("the per-surface refactor") so comments don't carry OSS-specific PR ids into stando. Test coverage (tests/conversation-store-per-surface.test.ts, 20 cases): - sourceFromRole + kindFromRole routing contract (incl. core-agent → agent suffix-matching, edge: 'shared-user' is not prefixed). - recordConversation routes by role-prefix; rows don't leak across surface tables. - recordToolCall writes kind='tool_call' to the named surface table; durationMs=null lands as NULL. - recordSessionBoundary writes SESSION_END row. - recordSession's session_events fan-out filters duplicate-prefix events (user:/sutando:/tool_call:/etc) — surface tables are canonical. - End-to-end migration via child-process runner: realistic pre- refactor seed (conversation + tool_calls + sessions w/ JSON cols) → assert per-surface row counts, kinds, post-migration view shape. - Migration idempotency: second open adds only the trigger row, no duplicate backfill. - Backwards-compat view: SELECT FROM conversation still works. - SessionMetrics back-compat: legacy toolCalls field is silently accepted (interface stability for existing callers). Verification: 264/264 TS tests pass (244 prior + 20 new), 61/61 Python tests pass, TS typecheck clean. Why minimal divergence: the functional code in src/conversation-store.ts is now identical to OSS sutando modulo the documented lazy-require divergence + neutral-issue-number wording. Future OSS changes to this file should merge cleanly without semantic conflicts. Co-authored-by: Cursor --- src/conversation-store.ts | 535 +++++++++++++++---- tests/conversation-store-per-surface.test.ts | 498 +++++++++++++++++ 2 files changed, 941 insertions(+), 92 deletions(-) create mode 100644 tests/conversation-store-per-surface.test.ts diff --git a/src/conversation-store.ts b/src/conversation-store.ts index b2cbec850d..7524666c3f 100644 --- a/src/conversation-store.ts +++ b/src/conversation-store.ts @@ -1,19 +1,49 @@ /** - * SQLite mirror of conversation.log — searchable, time-indexed, queryable. + * SQLite mirror of conversation.log — per-surface tables. * - * Issue: #603 (SQLite-ify conversation.log). + * Schema split: each voice surface (voice-agent / phone / discord-voice) + * gets its own table. Each surface table holds BOTH utterances AND tool + * calls in one chronological stream — no separate mixed `conversation` + * table, no separate `tool_calls` table. * - * Slice 1 (this file): parallel-write only. The text conversation.log - * stays as primary truth for now; this sqlite is a derived mirror that's - * cheap to rebuild from the text file (planned in slice 2). Best-effort - * writes — sqlite errors never propagate, never block the caller. + * voice / phone / discord_voice: + * id INTEGER PRIMARY KEY -- insertion order (canonical) + * ts_unix REAL NOT NULL -- emit time + * kind TEXT NOT NULL -- user | agent | peer | tool_call | + * tool_result | SESSION_END | ... + * text TEXT -- utterance text OR tool name + * duration_ms INTEGER -- tool_call / tool_result only + * session_id TEXT * - * Usage from src/task-bridge.ts (and any other writer): - * import { recordConversation, recordSessionBoundary } from './conversation-store.js'; - * recordConversation('user', 'hello'); - * recordSessionBoundary('user_goodbye'); + * Public API is unchanged: `recordConversation(role, text, sessionId)` and + * `recordSession(metrics)` keep the same signatures. Internally, + * recordConversation routes by role-prefix (`phone-*` → phone, `discord-*` + * → discord_voice, otherwise voice) and recordSession's lifecycle-event + * fan-out routes by `source`. Per-tool-call rows are written in real + * time via `recordToolCall()` instead of being batched at session end. * - * Query from CLI: scripts/query-conversation.sh "" + * Migration: on first init, if a surface table is empty and the old + * `conversation` / `tool_calls` tables have rows, the matching rows are + * backfilled into the surface tables (idempotent — runs once per machine). + * The old `conversation` and `tool_calls` tables are then dropped. + * `sessions` (per-session rollup) and `session_events` (unified event log) + * are kept — they serve different concerns. + * + * Best-effort throughout: sqlite errors never propagate, never block the + * caller. + * + * Usage (signatures unchanged): + * import { recordConversation, recordSessionBoundary, recordSession } + * from './conversation-store.js'; + * recordConversation('user', 'hello'); // → voice table + * recordConversation('phone-caller', 'hi'); // → phone table + * recordConversation('discord-user', 'hey'); // → discord_voice table + * + * Ported in Phase 5.20 of the OSS → private sync. Stando intentionally + * preserves the lazy `require('node:sqlite')` path inside init() rather + * than OSS's static `import` so older bundled Node runtimes (which gate + * node:sqlite behind --experimental-sqlite) degrade the mirror to a + * no-op instead of crashing every importing module on startup. */ import type { DatabaseSync } from 'node:sqlite'; import { mkdirSync } from 'node:fs'; @@ -36,13 +66,42 @@ const require = createRequire(import.meta.url); const DB_PATH = process.env.SUTANDO_CONVERSATION_DB || join(resolveWorkspace(), 'data', 'conversation.sqlite'); +type Source = 'voice' | 'phone' | 'discord-voice'; + let db: DatabaseSync | null = null; -let insertStmt: ReturnType | null = null; +const turnStmt: Record | null> = { + 'voice': null, 'phone': null, 'discord-voice': null, +}; let sessionInsertStmt: ReturnType | null = null; -let toolCallInsertStmt: ReturnType | null = null; let eventInsertStmt: ReturnType | null = null; let initFailed = false; +/** Map a `Source` to its canonical SQLite table name. */ +function tableForSource(source: Source): string { + if (source === 'phone') return 'phone'; + if (source === 'discord-voice') return 'discord_voice'; + return 'voice'; +} + +/** Derive `Source` from the legacy free-form role string. */ +export function sourceFromRole(role: string): Source { + if (role.startsWith('phone-')) return 'phone'; + if (role.startsWith('discord-')) return 'discord-voice'; + return 'voice'; +} + +/** Normalize the legacy role string to the per-surface `kind` taxonomy. + * user-side roles → 'user'; agent-side → 'agent'; discord-peer → 'peer'; + * anything else (SESSION_END, core-agent, system event names) passes + * through verbatim so callers can record arbitrary kinds. */ +export function kindFromRole(role: string): string { + if (role === 'user' || role.endsWith('-user') || role.endsWith('-caller')) return 'user'; + if (role === 'assistant' || role === 'sutando' + || role.endsWith('-agent') || role.endsWith('-assistant')) return 'agent'; + if (role === 'discord-peer') return 'peer'; + return role; +} + function init(): void { if (db || initFailed) return; try { @@ -63,81 +122,191 @@ function init(): void { // volume (one row per conversation turn). db.exec('PRAGMA journal_mode = WAL'); db.exec('PRAGMA busy_timeout = 1000'); + + // Defensive: an older `discord_voice` table (e.g. from a multi-instance + // branch using `discord-voice-store.ts`) used a `role` column instead + // of the new `kind` column. CREATE TABLE IF NOT EXISTS would skip the + // new schema and we'd write to mismatched columns. Rename any pre- + // existing legacy-schema table out of the way so the new CREATE runs. + try { + const old = db.prepare( + "SELECT name FROM sqlite_master WHERE type='table' AND name='discord_voice'", + ).get(); + if (old) { + const cols = db.prepare("PRAGMA table_info(discord_voice)").all() as Array<{ name: string }>; + const hasKind = cols.some(c => c.name === 'kind'); + if (!hasKind) { + db.exec('ALTER TABLE discord_voice RENAME TO discord_voice_legacy'); + console.log('[conversation-store] renamed legacy discord_voice → discord_voice_legacy (different schema)'); + } + } + } catch (e) { + console.error('[conversation-store] legacy-schema detect failed:', e); + } + db.exec(` - CREATE TABLE IF NOT EXISTS conversation ( - ts_unix REAL NOT NULL, - role TEXT NOT NULL, - text TEXT NOT NULL, - session_id TEXT + -- Per-surface event tables. Identical schema; one per voice surface. + -- Holds utterances + tool calls in one chronological stream — id is + -- insertion order (canonical sort key), ts_unix is emit time. + CREATE TABLE IF NOT EXISTS voice ( + id INTEGER PRIMARY KEY, + ts_unix REAL NOT NULL, + kind TEXT NOT NULL, + text TEXT, + duration_ms INTEGER, + session_id TEXT + ); + CREATE INDEX IF NOT EXISTS idx_voice_ts ON voice(ts_unix); + CREATE INDEX IF NOT EXISTS idx_voice_kind_ts ON voice(kind, ts_unix); + CREATE INDEX IF NOT EXISTS idx_voice_session ON voice(session_id, ts_unix); + + CREATE TABLE IF NOT EXISTS phone ( + id INTEGER PRIMARY KEY, + ts_unix REAL NOT NULL, + kind TEXT NOT NULL, + text TEXT, + duration_ms INTEGER, + session_id TEXT ); - CREATE INDEX IF NOT EXISTS idx_conversation_ts ON conversation(ts_unix); - CREATE INDEX IF NOT EXISTS idx_conversation_role_ts ON conversation(role, ts_unix); - CREATE INDEX IF NOT EXISTS idx_conversation_session ON conversation(session_id, ts_unix); + CREATE INDEX IF NOT EXISTS idx_phone_ts ON phone(ts_unix); + CREATE INDEX IF NOT EXISTS idx_phone_kind_ts ON phone(kind, ts_unix); + CREATE INDEX IF NOT EXISTS idx_phone_session ON phone(session_id, ts_unix); - -- Per-session rollups (replaces data/voice-metrics.jsonl + data/call-metrics.jsonl). - -- Unified table covers voice + phone + future discord-voice sources. - -- tool_calls + events are kept as JSON for at-a-glance / cross-version compat, - -- but per-row data ALSO lands in the dedicated tool_calls + session_events tables below - -- so SQL queries don't need json_extract. + CREATE TABLE IF NOT EXISTS discord_voice ( + id INTEGER PRIMARY KEY, + ts_unix REAL NOT NULL, + kind TEXT NOT NULL, + text TEXT, + duration_ms INTEGER, + session_id TEXT + ); + CREATE INDEX IF NOT EXISTS idx_discord_voice_ts ON discord_voice(ts_unix); + CREATE INDEX IF NOT EXISTS idx_discord_voice_kind_ts ON discord_voice(kind, ts_unix); + CREATE INDEX IF NOT EXISTS idx_discord_voice_session ON discord_voice(session_id, ts_unix); + + -- Per-session rollup. Kept — different concern from the per-event log. + -- Per-tool-call rows live in surface tables (kind='tool_call'), + -- per-event rows live in session_events. The old tool_calls/events + -- JSON columns are dropped post-init via the migration below. CREATE TABLE IF NOT EXISTS sessions ( ts_unix REAL NOT NULL, - source TEXT NOT NULL, -- 'voice' | 'phone' | 'discord-voice' | ... - session_id TEXT, -- voice/discord-voice key - call_sid TEXT, -- phone (Twilio) key - caller TEXT, -- phone caller number - is_owner INTEGER, -- phone access tier (0/1) - is_meeting INTEGER, -- phone is_meeting (0/1) + source TEXT NOT NULL, + session_id TEXT, + call_sid TEXT, + caller TEXT, + is_owner INTEGER, + is_meeting INTEGER, duration_ms INTEGER NOT NULL, transcript_lines INTEGER, tool_count INTEGER, - pending_tasks INTEGER, - tool_calls TEXT, -- JSON array (also in tool_calls table) - events TEXT -- JSON array (also in session_events table) + pending_tasks INTEGER ); CREATE INDEX IF NOT EXISTS idx_sessions_ts ON sessions(ts_unix); CREATE INDEX IF NOT EXISTS idx_sessions_source_ts ON sessions(source, ts_unix); CREATE INDEX IF NOT EXISTS idx_sessions_call_sid ON sessions(call_sid); - -- Per-tool-call rows extracted from sessions.tool_calls JSON. - -- Query directly: SELECT name, AVG(duration_ms) FROM tool_calls - -- WHERE ts_unix > strftime('%s','now')-86400 GROUP BY name - CREATE TABLE IF NOT EXISTS tool_calls ( - ts_unix REAL NOT NULL, -- tool call timestamp (not session start) - source TEXT NOT NULL, -- 'voice' | 'phone' | etc - session_id TEXT, -- voice session_id - call_sid TEXT, -- phone callSid - name TEXT NOT NULL, -- tool name (describe_screen, work, etc) - duration_ms INTEGER -- tool execution duration - ); - CREATE INDEX IF NOT EXISTS idx_tool_calls_ts ON tool_calls(ts_unix); - CREATE INDEX IF NOT EXISTS idx_tool_calls_name_ts ON tool_calls(name, ts_unix); - CREATE INDEX IF NOT EXISTS idx_tool_calls_session ON tool_calls(session_id, ts_unix); - - -- Per-event rows extracted from sessions.events JSON. - -- Covers lifecycle events (session_started, call_ended, transport_close, etc). + -- Unified event log — lifecycle events (session_started, call_ended, + -- transport_close, etc.). Kept — different concern from per-event log. CREATE TABLE IF NOT EXISTS session_events ( ts_unix REAL NOT NULL, source TEXT NOT NULL, session_id TEXT, call_sid TEXT, - event_name TEXT NOT NULL -- session_started, tool_call, call_ended, etc. + event_name TEXT NOT NULL ); CREATE INDEX IF NOT EXISTS idx_session_events_ts ON session_events(ts_unix); CREATE INDEX IF NOT EXISTS idx_session_events_name_ts ON session_events(event_name, ts_unix); CREATE INDEX IF NOT EXISTS idx_session_events_session ON session_events(session_id, ts_unix); `); - insertStmt = db.prepare( - 'INSERT INTO conversation (ts_unix, role, text, session_id) VALUES (?, ?, ?, ?)' + + // One-time migration: backfill from the legacy `conversation` and + // `tool_calls` tables into the new per-surface tables, then drop the + // legacy tables. Idempotent — gated on each surface table being empty. + migrateLegacyIfNeeded(db); + + // Drop the redundant sessions.tool_calls / sessions.events JSON + // columns if a pre-refactor db still has them. The atom rows now live + // in surface tables (kind='tool_call') and session_events + // respectively; the JSON cols were triple-encoding the same data. + // SQLite 3.35+ supports ALTER TABLE DROP COLUMN — guard via + // pragma_table_info so re-running this is a no-op. + const sessionCols = new Set( + (db.prepare("PRAGMA table_info(sessions)").all() as Array<{ name: string }>) + .map(c => c.name), + ); + if (sessionCols.has('tool_calls')) { + try { + db.exec('ALTER TABLE sessions DROP COLUMN tool_calls'); + console.log('[conversation-store] dropped sessions.tool_calls (redundant w/ surface tables)'); + } catch (e) { + console.error('[conversation-store] could not drop sessions.tool_calls:', e); + } + } + if (sessionCols.has('events')) { + try { + db.exec('ALTER TABLE sessions DROP COLUMN events'); + console.log('[conversation-store] dropped sessions.events (redundant w/ session_events)'); + } catch (e) { + console.error('[conversation-store] could not drop sessions.events:', e); + } + } + + // Convenience views — thin wrappers that add a human-readable `time` + // column (local-time) and default-sort by ts_unix DESC. DROP+CREATE so + // definitions stay in lock-step with the surface tables and any + // pre-migration v_* (which referenced now-dropped legacy tables) gets + // replaced cleanly. + db.exec(` + DROP VIEW IF EXISTS v_voice; + DROP VIEW IF EXISTS v_phone; + DROP VIEW IF EXISTS v_discord_voice; + DROP VIEW IF EXISTS v_sessions; + DROP VIEW IF EXISTS conversation; + CREATE VIEW v_voice AS + SELECT id, datetime(ts_unix,'unixepoch','localtime') AS time, + ts_unix, kind, text, duration_ms, session_id + FROM voice ORDER BY ts_unix DESC; + CREATE VIEW v_phone AS + SELECT id, datetime(ts_unix,'unixepoch','localtime') AS time, + ts_unix, kind, text, duration_ms, session_id + FROM phone ORDER BY ts_unix DESC; + CREATE VIEW v_discord_voice AS + SELECT id, datetime(ts_unix,'unixepoch','localtime') AS time, + ts_unix, kind, text, duration_ms, session_id + FROM discord_voice ORDER BY ts_unix DESC; + CREATE VIEW v_sessions AS + SELECT datetime(ts_unix,'unixepoch','localtime') AS time, + ts_unix, source, session_id, call_sid, caller, is_owner, is_meeting, + duration_ms, transcript_lines, tool_count, pending_tasks + FROM sessions ORDER BY ts_unix DESC; + -- Backward-compat view for pre-refactor readers that still + -- SELECT FROM conversation with the old role column. Surface + -- the union of all 3 tables under the legacy schema so external + -- scripts (query-conversation.sh, regression-search, any other + -- consumer we missed) keep working without source edits. New + -- code should read the surface tables directly. + CREATE VIEW conversation AS + SELECT ts_unix, kind AS role, text, session_id FROM voice + UNION ALL + SELECT ts_unix, kind AS role, text, session_id FROM phone + UNION ALL + SELECT ts_unix, kind AS role, text, session_id FROM discord_voice; + `); + + turnStmt['voice'] = db.prepare( + 'INSERT INTO voice (ts_unix, kind, text, duration_ms, session_id) VALUES (?, ?, ?, ?, ?)', + ); + turnStmt['phone'] = db.prepare( + 'INSERT INTO phone (ts_unix, kind, text, duration_ms, session_id) VALUES (?, ?, ?, ?, ?)', + ); + turnStmt['discord-voice'] = db.prepare( + 'INSERT INTO discord_voice (ts_unix, kind, text, duration_ms, session_id) VALUES (?, ?, ?, ?, ?)', ); sessionInsertStmt = db.prepare(` INSERT INTO sessions ( ts_unix, source, session_id, call_sid, caller, is_owner, is_meeting, - duration_ms, transcript_lines, tool_count, pending_tasks, tool_calls, events - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `); - toolCallInsertStmt = db.prepare(` - INSERT INTO tool_calls (ts_unix, source, session_id, call_sid, name, duration_ms) - VALUES (?, ?, ?, ?, ?, ?) + duration_ms, transcript_lines, tool_count, pending_tasks + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `); eventInsertStmt = db.prepare(` INSERT INTO session_events (ts_unix, source, session_id, call_sid, event_name) @@ -147,15 +316,172 @@ function init(): void { console.error('[conversation-store] init failed:', e); initFailed = true; db = null; - insertStmt = null; } } +/** One-time migration: copy rows from legacy `conversation` and `tool_calls` + * tables into the per-surface tables, then drop the legacy tables. Gated on + * each per-surface table being empty (so a re-run on an already-migrated db + * is a no-op). Wrapped in a single transaction; failures are logged but + * don't propagate. */ +function migrateLegacyIfNeeded(d: DatabaseSync): void { + try { + const hasConversation = d.prepare( + "SELECT name FROM sqlite_master WHERE type='table' AND name='conversation'", + ).get(); + const hasToolCalls = d.prepare( + "SELECT name FROM sqlite_master WHERE type='table' AND name='tool_calls'", + ).get(); + if (!hasConversation && !hasToolCalls) return; // nothing to migrate + + const voiceEmpty = (d.prepare('SELECT count(*) AS c FROM voice').get() as { c: number }).c === 0; + const phoneEmpty = (d.prepare('SELECT count(*) AS c FROM phone').get() as { c: number }).c === 0; + const discordEmpty = (d.prepare('SELECT count(*) AS c FROM discord_voice').get() as { c: number }).c === 0; + if (!voiceEmpty && !phoneEmpty && !discordEmpty) { + // All surface tables already populated — nothing to backfill. + // Drop legacy tables if they're still around. + if (hasConversation) d.exec('DROP TABLE IF EXISTS conversation'); + if (hasToolCalls) d.exec('DROP TABLE IF EXISTS tool_calls'); + return; + } + + console.log('[conversation-store] migrating legacy conversation + tool_calls into per-surface tables'); + d.exec('BEGIN'); + try { + // All migration INSERTs filter out NULL ts_unix rows. The surface + // tables declare ts_unix REAL NOT NULL, so a legacy row missing + // it would trip a NOT NULL constraint mid-transaction and roll + // the whole migration back — leaving the legacy tables in place + // and the bot logging the same error every restart. Rows with no + // timestamp can't be meaningfully recovered (no other column + // carries time), so dropping them on migrate is the right call. + if (hasConversation && voiceEmpty) { + // Utterances → voice. Roles that map to voice: 'user', 'assistant', + // 'sutando', 'core-agent', 'SESSION_END', and anything not prefixed + // 'phone-' / 'discord-'. kind normalization: user/assistant/sutando + // → user/agent, others passthrough. + d.exec(` + INSERT INTO voice (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, + CASE + WHEN role='user' THEN 'user' + WHEN role IN ('assistant','sutando') THEN 'agent' + ELSE role + END, + text, NULL, session_id + FROM conversation + WHERE ts_unix IS NOT NULL + AND role NOT LIKE 'phone-%' AND role NOT LIKE 'discord-%' + `); + } + if (hasConversation && phoneEmpty) { + d.exec(` + INSERT INTO phone (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, + CASE + WHEN role LIKE 'phone-caller%' THEN 'user' + WHEN role LIKE 'phone-agent%' THEN 'agent' + ELSE substr(role, 7) + END, + text, NULL, session_id + FROM conversation WHERE ts_unix IS NOT NULL AND role LIKE 'phone-%' + `); + } + if (hasConversation && discordEmpty) { + d.exec(` + INSERT INTO discord_voice (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, + CASE + WHEN role='discord-user' THEN 'user' + WHEN role='discord-agent' THEN 'agent' + WHEN role='discord-peer' THEN 'peer' + ELSE substr(role, 9) + END, + text, NULL, session_id + FROM conversation WHERE ts_unix IS NOT NULL AND role LIKE 'discord-%' + `); + } + if (hasToolCalls) { + // Tool calls → surface table by `source`. kind='tool_call', text=name, + // duration_ms preserved. (The standalone tool_calls table goes away — + // per-tool-call rows now live alongside utterances in the surface + // table, ordered by ts_unix.) + d.exec(` + INSERT INTO voice (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, 'tool_call', name, duration_ms, session_id + FROM tool_calls WHERE source='voice' + `); + d.exec(` + INSERT INTO phone (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, 'tool_call', name, duration_ms, session_id + FROM tool_calls WHERE source='phone' + `); + d.exec(` + INSERT INTO discord_voice (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, 'tool_call', name, duration_ms, session_id + FROM tool_calls WHERE source='discord-voice' + `); + // Also backfill discord-voice tool calls from sessions.tool_calls JSON — + // discord-voice historically never wrote to the tool_calls table; its + // per-call data lives only in the sessions JSON column. Use json_each + // to expand. + d.exec(` + INSERT INTO discord_voice (ts_unix, kind, text, duration_ms, session_id) + SELECT CAST(strftime('%s', json_extract(je.value,'$.timestamp')) AS REAL), + 'tool_call', + json_extract(je.value,'$.name'), + json_extract(je.value,'$.durationMs'), + s.session_id + FROM sessions s, json_each(s.tool_calls) je + WHERE s.source='discord-voice' + AND s.tool_calls IS NOT NULL + AND s.tool_calls != '[]' + `); + } + // If a legacy `discord_voice` table was renamed aside (different + // schema from an older multi-instance branch), backfill its rows. + const legacy = d.prepare( + "SELECT name FROM sqlite_master WHERE type='table' AND name='discord_voice_legacy'", + ).get(); + if (legacy) { + d.exec(` + INSERT INTO discord_voice (ts_unix, kind, text, duration_ms, session_id) + SELECT ts_unix, + CASE + WHEN role='discord-user' THEN 'user' + WHEN role='discord-agent' THEN 'agent' + WHEN role='discord-peer' THEN 'peer' + ELSE role + END, + text, NULL, session_id + FROM discord_voice_legacy + `); + d.exec('DROP TABLE IF EXISTS discord_voice_legacy'); + } + // Drop legacy tables — they're fully migrated. + if (hasConversation) d.exec('DROP TABLE IF EXISTS conversation'); + if (hasToolCalls) d.exec('DROP TABLE IF EXISTS tool_calls'); + d.exec('COMMIT'); + console.log('[conversation-store] migration done; legacy tables dropped'); + } catch (e) { + d.exec('ROLLBACK'); + console.error('[conversation-store] migration failed (rolled back):', e); + } + } catch (e) { + console.error('[conversation-store] migration probe failed:', e); + } +} + +/** Record a conversation turn. Source is derived from `role` (`phone-*` → + * phone, `discord-*` → discord_voice, otherwise voice); `kind` is + * normalized (user / agent / peer / SESSION_END / other). Best-effort. */ export function recordConversation(role: string, text: string, sessionId?: string): void { init(); - if (!insertStmt) return; + const source = sourceFromRole(role); + const stmt = turnStmt[source]; + if (!stmt) return; try { - insertStmt.run(Date.now() / 1000, role, text, sessionId ?? null); + stmt.run(Date.now() / 1000, kindFromRole(role), text, null, sessionId ?? null); } catch (e) { console.error('[conversation-store] insert failed:', e); } @@ -165,6 +491,30 @@ export function recordSessionBoundary(reason: string = 'user_goodbye', sessionId recordConversation('SESSION_END', reason, sessionId); } +/** + * Record a single tool invocation into the matching surface table as + * `kind='tool_call'`. Call this from each surface's `onToolResult` hook so + * tool calls land in db immediately (and are visible mid-session in DB + * Browser) instead of being batched at session end via recordSession's + * fan-out — that older path lost everything if the session never cleanly + * ended (crash, kill -9, ngrok drop). durationMs may be null when unknown. + */ +export function recordToolCall( + source: 'voice' | 'phone' | 'discord-voice', + name: string, + durationMs: number | null, + sessionId?: string | null, +): void { + init(); + const stmt = turnStmt[source]; + if (!stmt) return; + try { + stmt.run(Date.now() / 1000, 'tool_call', name, durationMs, sessionId ?? null); + } catch (e) { + console.error('[conversation-store] tool_call insert failed:', e); + } +} + export interface SessionMetrics { source: 'voice' | 'phone' | 'discord-voice' | string; sessionId?: string | null; @@ -176,8 +526,14 @@ export interface SessionMetrics { transcriptLines?: number | null; toolCount?: number | null; pendingTasks?: number | null; - toolCalls?: unknown; // JSON-serializable array - events?: unknown; // JSON-serializable array + /** No longer persisted (per #1052). Surface table rows with + * kind='tool_call' are canonical; this field is accepted only for + * backwards-compat with existing callers and silently ignored. */ + toolCalls?: unknown; + /** Iterated for the session_events fan-out (lifecycle events). User / + * agent / tool_call / tool_result entries are filtered out — those + * atoms live in surface tables now (per #1052). */ + events?: unknown; } /** Parse a value that should be a timestamp into unix seconds, or null. */ @@ -190,12 +546,22 @@ function tsToUnix(t: unknown): number | null { return null; } +// Event names whose substance already lives in a surface table row +// (kind='user'/'agent'/'tool_call'). Filtered out of the session_events +// fan-out so the same atom isn't recorded twice. Defense-in-depth: the +// 3 surface servers also stopped pushing these into their in-memory +// events array as of the per-surface refactor; this filter catches +// anything missed + protects against external callers passing them in +// m.events. +const DUPLICATE_EVENT_PREFIXES = ['user:', 'caller:', 'sutando:', 'assistant:', 'tool_call:', 'tool_result:']; + /** - * Record per-session rollup. Replaces appendFileSync to - * data/voice-metrics.jsonl (voice-agent) and data/call-metrics.jsonl - * (phone-conversation). Also fans out toolCalls + events into the - * dedicated `tool_calls` and `session_events` tables for queryability. - * Best-effort — sqlite errors swallowed. + * Record per-session rollup. Also fans out lifecycle events (session_started, + * session_ended, error, task_*, etc.) into the unified session_events table. + * Tool calls are NOT fanned out here — each surface server writes them in + * real time via recordToolCall() inside its onToolResult hook. Utterance + * events with user:/sutando: prefixes are filtered out — they duplicate + * surface-table user/agent rows. Best-effort. */ export function recordSession(m: SessionMetrics): void { init(); @@ -214,39 +580,24 @@ export function recordSession(m: SessionMetrics): void { m.transcriptLines ?? null, m.toolCount ?? null, m.pendingTasks ?? null, - m.toolCalls === undefined ? null : JSON.stringify(m.toolCalls), - m.events === undefined ? null : JSON.stringify(m.events), ); } catch (e) { console.error('[conversation-store] session insert failed:', e); } - // Fan out tool_calls into the dedicated table. - if (toolCallInsertStmt && Array.isArray(m.toolCalls)) { - for (const tc of m.toolCalls as Array>) { - try { - toolCallInsertStmt.run( - tsToUnix(tc.timestamp) ?? nowUnix, - m.source, - m.sessionId ?? null, - m.callSid ?? null, - String(tc.name ?? 'unknown'), - typeof tc.durationMs === 'number' ? tc.durationMs : null, - ); - } catch (e) { - console.error('[conversation-store] tool_calls insert failed:', e); - } - } - } - // Fan out events into the dedicated table. + // Fan out LIFECYCLE events into session_events. Skip duplicates of + // surface-table rows (user/sutando/tool_call/tool_result) — those + // atoms are canonical in voice/phone/discord_voice tables now. if (eventInsertStmt && Array.isArray(m.events)) { for (const ev of m.events as Array>) { + const name = String(ev.event ?? 'unknown'); + if (DUPLICATE_EVENT_PREFIXES.some(p => name.startsWith(p))) continue; try { eventInsertStmt.run( tsToUnix(ev.timestamp) ?? nowUnix, m.source, m.sessionId ?? null, m.callSid ?? null, - String(ev.event ?? 'unknown'), + name, ); } catch (e) { console.error('[conversation-store] session_events insert failed:', e); diff --git a/tests/conversation-store-per-surface.test.ts b/tests/conversation-store-per-surface.test.ts new file mode 100644 index 0000000000..e41bf5e47a --- /dev/null +++ b/tests/conversation-store-per-surface.test.ts @@ -0,0 +1,498 @@ +/** + * Tests for the per-surface conversation-store schema (Phase 5.20). + * + * Why this test exists: + * Phase 5.20 swapped the single `conversation` table + standalone + * `tool_calls` table for per-surface tables (`voice`, `phone`, + * `discord_voice`). The public API for `recordConversation` / + * `recordSession` is unchanged but their internal routing now derives + * the table from a role-prefix or `source` value. Two new exported + * helpers (`sourceFromRole`, `kindFromRole`) plus a new exported + * `recordToolCall` sink were added. + * + * The migration from the legacy schema runs at first init and must be + * idempotent — restart-safety matters here because the module is + * imported by every voice surface server (voice-agent, phone, + * discord-voice). A non-idempotent migration would either re-backfill + * on every boot (duplicated rows) or fail on every boot (noisy logs). + * + * These tests use a temp DB path via SUTANDO_CONVERSATION_DB so the + * developer's live workspace DB is never touched. + * + * Coverage: + * 1. sourceFromRole + kindFromRole semantics — the documented routing + * contract that every caller silently depends on. + * 2. recordConversation routes by role-prefix (`phone-` / `discord-` / + * otherwise) and writes to the matching surface table only. + * 3. recordToolCall writes a kind='tool_call' row to the named source + * table. Tool calls are now per-row, not JSON-blob batched. + * 4. recordSessionBoundary still writes a `SESSION_END` row (used by + * voice-agent's getRecentConversation replay-window trim). + * 5. recordSession's per-event fan-out filters out duplicate prefixes + * (user:/sutando:/tool_call:/etc) — these atoms now live in + * surface tables and would double-count if not filtered. + * 6. Migration: legacy `conversation` + `tool_calls` rows backfill + * into the matching surface tables, then the legacy tables are + * dropped. NULL-ts_unix rows are dropped (can't be recovered). + * 7. Migration idempotency: second open of the migrated DB is a + * no-op (no duplicate rows, no error). + * 8. The legacy `conversation` view is created post-migration so + * old callers that still SELECT FROM conversation keep working. + */ +import { describe, it, before, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { tmpdir } from 'node:os'; +import { createRequire } from 'node:module'; +import { spawnSync } from 'node:child_process'; + +// Each test uses its own temp DB so we can exercise the full init path +// (including migration) without polluting the user's workspace. The +// SUTANDO_CONVERSATION_DB env var is checked by conversation-store.ts at +// module load — set it BEFORE importing the module under test. +const TMP_ROOT = mkdtempSync(join(tmpdir(), 'sutando-conv-test-')); +const PRIMARY_DB = join(TMP_ROOT, 'primary', 'conv.sqlite'); +mkdirSync(dirname(PRIMARY_DB), { recursive: true }); +process.env.SUTANDO_CONVERSATION_DB = PRIMARY_DB; + +// Direct sqlite access for asserting on rows; goes through the same +// `node:sqlite` that conversation-store does. +const require = createRequire(import.meta.url); +const { DatabaseSync } = require('node:sqlite'); + +const mod = await import('../src/conversation-store.js'); + +after(() => { + try { rmSync(TMP_ROOT, { recursive: true, force: true }); } catch { /* ignore */ } +}); + +// ---------------------------------------------------------------------- +// 1. Helpers — sourceFromRole + kindFromRole routing contract +// ---------------------------------------------------------------------- + +describe('sourceFromRole', () => { + it('routes phone-* to phone', () => { + assert.equal(mod.sourceFromRole('phone-caller'), 'phone'); + assert.equal(mod.sourceFromRole('phone-agent'), 'phone'); + }); + + it('routes discord-* to discord-voice', () => { + assert.equal(mod.sourceFromRole('discord-user'), 'discord-voice'); + assert.equal(mod.sourceFromRole('discord-peer'), 'discord-voice'); + }); + + it('routes anything else to voice (the default)', () => { + assert.equal(mod.sourceFromRole('user'), 'voice'); + assert.equal(mod.sourceFromRole('assistant'), 'voice'); + assert.equal(mod.sourceFromRole('SESSION_END'), 'voice'); + assert.equal(mod.sourceFromRole('core-agent'), 'voice'); + // Edge: a role with -user mid-string isn't prefixed → still voice + assert.equal(mod.sourceFromRole('shared-user'), 'voice'); + }); +}); + +describe('kindFromRole', () => { + it('normalizes user-side variants to "user"', () => { + assert.equal(mod.kindFromRole('user'), 'user'); + assert.equal(mod.kindFromRole('phone-caller'), 'user'); + assert.equal(mod.kindFromRole('discord-user'), 'user'); + }); + + it('normalizes agent-side variants to "agent"', () => { + assert.equal(mod.kindFromRole('assistant'), 'agent'); + assert.equal(mod.kindFromRole('sutando'), 'agent'); + assert.equal(mod.kindFromRole('phone-agent'), 'agent'); + assert.equal(mod.kindFromRole('discord-agent'), 'agent'); + }); + + it('maps discord-peer to "peer"', () => { + assert.equal(mod.kindFromRole('discord-peer'), 'peer'); + }); + + it('passes through roles that do not match a known suffix', () => { + assert.equal(mod.kindFromRole('SESSION_END'), 'SESSION_END'); + assert.equal(mod.kindFromRole('tool_call'), 'tool_call'); + assert.equal(mod.kindFromRole('system_event'), 'system_event'); + }); + + it('treats core-agent as "agent" because it ends with -agent', () => { + // Documented in conversation-store.ts kindFromRole comment: any + // role ending in `-agent` collapses to the agent kind. This is + // intentional so core-agent / proactive-agent / etc. share the + // same surface-table category as the primary assistant. + assert.equal(mod.kindFromRole('core-agent'), 'agent'); + assert.equal(mod.kindFromRole('proactive-agent'), 'agent'); + }); +}); + +// ---------------------------------------------------------------------- +// 2. recordConversation routes by role-prefix +// ---------------------------------------------------------------------- + +describe('recordConversation routing', () => { + before(() => { + // Force a write so init() runs and the DB is created. The DB + // path is fixed at module import time via SUTANDO_CONVERSATION_DB. + mod.recordConversation('user', 'hello world', 'session-a'); + mod.recordConversation('phone-caller', 'hi from phone', 'call-b'); + mod.recordConversation('discord-user', 'hey from discord', 'session-c'); + }); + + it('writes voice rows to the voice table only', () => { + const db = new DatabaseSync(PRIMARY_DB); + try { + const row = db + .prepare("SELECT text, kind, session_id FROM voice WHERE text='hello world'") + .get() as { text: string; kind: string; session_id: string } | undefined; + assert.ok(row, 'voice row should exist'); + assert.equal(row!.kind, 'user'); + assert.equal(row!.session_id, 'session-a'); + // And the row must NOT have leaked into phone/discord_voice + const phoneCount = (db.prepare("SELECT count(*) AS c FROM phone WHERE text='hello world'").get() as { c: number }).c; + const dvCount = (db.prepare("SELECT count(*) AS c FROM discord_voice WHERE text='hello world'").get() as { c: number }).c; + assert.equal(phoneCount, 0); + assert.equal(dvCount, 0); + } finally { + db.close(); + } + }); + + it('writes phone rows to the phone table only', () => { + const db = new DatabaseSync(PRIMARY_DB); + try { + const row = db + .prepare("SELECT text, kind, session_id FROM phone WHERE text='hi from phone'") + .get() as { text: string; kind: string; session_id: string } | undefined; + assert.ok(row, 'phone row should exist'); + assert.equal(row!.kind, 'user'); + assert.equal(row!.session_id, 'call-b'); + } finally { + db.close(); + } + }); + + it('writes discord rows to the discord_voice table only', () => { + const db = new DatabaseSync(PRIMARY_DB); + try { + const row = db + .prepare("SELECT text, kind, session_id FROM discord_voice WHERE text='hey from discord'") + .get() as { text: string; kind: string; session_id: string } | undefined; + assert.ok(row, 'discord_voice row should exist'); + assert.equal(row!.kind, 'user'); + assert.equal(row!.session_id, 'session-c'); + } finally { + db.close(); + } + }); + + it('SESSION_END boundary lands as a verbatim-kind row in voice', () => { + mod.recordSessionBoundary('user_goodbye', 'session-end-1'); + const db = new DatabaseSync(PRIMARY_DB); + try { + const row = db + .prepare("SELECT kind, text FROM voice WHERE session_id='session-end-1'") + .get() as { kind: string; text: string } | undefined; + assert.ok(row, 'session boundary row should exist'); + assert.equal(row!.kind, 'SESSION_END'); + assert.equal(row!.text, 'user_goodbye'); + } finally { + db.close(); + } + }); +}); + +// ---------------------------------------------------------------------- +// 3. recordToolCall writes kind='tool_call' to the matching surface table +// ---------------------------------------------------------------------- + +describe('recordToolCall', () => { + it('writes a kind="tool_call" row with the tool name as text', () => { + mod.recordToolCall('voice', 'describe_screen', 234, 'tc-session-a'); + mod.recordToolCall('phone', 'hang_up', 12, 'tc-call-b'); + mod.recordToolCall('discord-voice', 'summon', null, 'tc-session-c'); + const db = new DatabaseSync(PRIMARY_DB); + try { + const v = db.prepare("SELECT kind, text, duration_ms FROM voice WHERE session_id='tc-session-a'").get() as { kind: string; text: string; duration_ms: number | null }; + assert.equal(v.kind, 'tool_call'); + assert.equal(v.text, 'describe_screen'); + assert.equal(v.duration_ms, 234); + + const p = db.prepare("SELECT kind, text, duration_ms FROM phone WHERE session_id='tc-call-b'").get() as { kind: string; text: string; duration_ms: number | null }; + assert.equal(p.kind, 'tool_call'); + assert.equal(p.text, 'hang_up'); + + // durationMs=null should land as NULL, not 0 + const dv = db.prepare("SELECT kind, text, duration_ms FROM discord_voice WHERE session_id='tc-session-c'").get() as { kind: string; text: string; duration_ms: number | null }; + assert.equal(dv.kind, 'tool_call'); + assert.equal(dv.text, 'summon'); + assert.equal(dv.duration_ms, null); + } finally { + db.close(); + } + }); +}); + +// ---------------------------------------------------------------------- +// 4. recordSession filters duplicate event prefixes +// ---------------------------------------------------------------------- + +describe('recordSession event fan-out', () => { + it('filters duplicate-prefix events (user:/sutando:/tool_call:/etc)', () => { + mod.recordSession({ + source: 'voice', + sessionId: 'sess-fanout-1', + durationMs: 1234, + events: [ + { event: 'session_started', timestamp: '2026-06-01T12:00:00Z' }, + { event: 'user: hello', timestamp: '2026-06-01T12:00:01Z' }, // filtered + { event: 'sutando: hi there', timestamp: '2026-06-01T12:00:02Z' }, // filtered + { event: 'tool_call: describe_screen', timestamp: '2026-06-01T12:00:03Z' }, // filtered + { event: 'tool_result: ok', timestamp: '2026-06-01T12:00:04Z' }, // filtered + { event: 'assistant: bye', timestamp: '2026-06-01T12:00:05Z' }, // filtered + { event: 'caller: still here', timestamp: '2026-06-01T12:00:06Z' }, // filtered + { event: 'session_ended', timestamp: '2026-06-01T12:00:07Z' }, + ], + }); + const db = new DatabaseSync(PRIMARY_DB); + try { + const rows = db.prepare( + "SELECT event_name FROM session_events WHERE session_id='sess-fanout-1' ORDER BY event_name", + ).all() as Array<{ event_name: string }>; + const names = rows.map(r => r.event_name); + // Only the two non-prefixed events land in session_events + assert.deepEqual(names, ['session_ended', 'session_started']); + } finally { + db.close(); + } + }); + + it('records a row in sessions table', () => { + const db = new DatabaseSync(PRIMARY_DB); + try { + const row = db.prepare( + "SELECT source, session_id, duration_ms FROM sessions WHERE session_id='sess-fanout-1'", + ).get() as { source: string; session_id: string; duration_ms: number }; + assert.equal(row.source, 'voice'); + assert.equal(row.duration_ms, 1234); + } finally { + db.close(); + } + }); +}); + +// ---------------------------------------------------------------------- +// 5. Migration: legacy schema → per-surface tables (idempotent, drops nulls) +// ---------------------------------------------------------------------- + +describe('legacy migration', () => { + // Use a fresh DB so the migration probe sees a legacy schema. We + // spawn a child Node process so the module's top-level singleton + // can target this distinct path (the parent already initialized + // against PRIMARY_DB and won't re-init). + const MIG_DIR = join(TMP_ROOT, 'migration'); + const MIG_DB = join(MIG_DIR, 'conv.sqlite'); + const RUNNER = join(MIG_DIR, 'runner.mjs'); + const REPO = process.cwd(); + mkdirSync(MIG_DIR, { recursive: true }); + + before(() => { + // Seed a legacy-schema DB BEFORE the child process imports the + // module. We control the seed so we can assert exact post- + // migration row counts. + const seed = new DatabaseSync(MIG_DB); + // Pre-refactor schema (matches what existing stando installs ship + // with prior to this PR). Crucially includes the sessions table + // WITH the JSON `tool_calls` and `events` columns — the migration + // reads from `sessions.tool_calls` via json_each() and would fail + // against the post-refactor sessions schema (no JSON cols). In + // production every stando install has either zero legacy tables + // (fresh) or all three of conversation + tool_calls + sessions + // (pre-refactor); a half-migrated state isn't expected. The test + // reproduces the pre-refactor state faithfully. + seed.exec(` + CREATE TABLE conversation ( + ts_unix REAL NOT NULL, + role TEXT NOT NULL, + text TEXT NOT NULL, + session_id TEXT + ); + CREATE TABLE tool_calls ( + ts_unix REAL NOT NULL, + source TEXT NOT NULL, + session_id TEXT, + call_sid TEXT, + name TEXT NOT NULL, + duration_ms INTEGER + ); + CREATE TABLE sessions ( + ts_unix REAL NOT NULL, + source TEXT NOT NULL, + session_id TEXT, + call_sid TEXT, + caller TEXT, + is_owner INTEGER, + is_meeting INTEGER, + duration_ms INTEGER NOT NULL, + transcript_lines INTEGER, + tool_count INTEGER, + pending_tasks INTEGER, + tool_calls TEXT, + events TEXT + ); + `); + const ins = seed.prepare("INSERT INTO conversation VALUES (?, ?, ?, ?)"); + // Distinctive timestamps so we can assert exact preservation. + ins.run(1000.0, 'user', 'voice user msg', 'voice-sess'); + ins.run(1001.0, 'assistant', 'voice agent reply', 'voice-sess'); + ins.run(1002.0, 'phone-caller', 'phone hello', 'phone-sess'); + ins.run(1003.0, 'phone-agent', 'phone reply', 'phone-sess'); + ins.run(1004.0, 'discord-user', 'dv hello', 'dv-sess'); + ins.run(1005.0, 'discord-agent', 'dv reply', 'dv-sess'); + ins.run(1006.0, 'discord-peer', 'dv peer talking', 'dv-sess'); + const itc = seed.prepare("INSERT INTO tool_calls VALUES (?, ?, ?, ?, ?, ?)"); + itc.run(2000.0, 'voice', 'voice-sess', null, 'describe_screen', 50); + itc.run(2001.0, 'phone', 'phone-sess', 'call-sid-1', 'hang_up', 12); + seed.close(); + + // Runner script: import the module and force init() via a + // recordConversation call. The role 'core-migration-trigger' + // routes to the voice surface but uses a marker session_id so + // we can distinguish it from migrated voice rows. + writeFileSync(RUNNER, ` + import { recordConversation } from '${REPO}/src/conversation-store.ts'; + recordConversation('SESSION_END', 'migration-trigger', 'migrate-trigger-sentinel'); + `); + }); + + function runMigration(): { status: number | null; stdout: string; stderr: string } { + const r = spawnSync('npx', ['tsx', RUNNER], { + env: { ...process.env, SUTANDO_CONVERSATION_DB: MIG_DB }, + encoding: 'utf-8', + cwd: REPO, + }); + return { status: r.status, stdout: r.stdout ?? '', stderr: r.stderr ?? '' }; + } + + it('migrates legacy rows into per-surface tables', () => { + const r = runMigration(); + assert.equal(r.status, 0, `migration runner failed: ${r.stderr}`); + const db = new DatabaseSync(MIG_DB); + try { + // Voice: 2 utterances (user, assistant) + 1 tool_call + 1 + // SESSION_END from the trigger. + const voiceRows = db.prepare("SELECT kind, text FROM voice ORDER BY ts_unix").all() as Array<{ kind: string; text: string }>; + const voiceKinds = voiceRows.map(r => r.kind); + const voiceTexts = voiceRows.map(r => r.text); + assert.ok(voiceTexts.includes('voice user msg'), `voice user msg missing: ${JSON.stringify(voiceTexts)}`); + assert.ok(voiceTexts.includes('voice agent reply'), 'voice agent reply missing'); + assert.ok(voiceTexts.includes('describe_screen'), 'tool_call describe_screen missing'); + assert.ok(voiceKinds.includes('tool_call'), 'tool_call kind missing'); + assert.ok(voiceKinds.includes('user'), 'user kind missing'); + assert.ok(voiceKinds.includes('agent'), 'agent kind missing (assistant normalization)'); + + // Phone: 1 caller utt + 1 agent utt + 1 tool_call + const phoneTexts = (db.prepare("SELECT text FROM phone ORDER BY ts_unix").all() as Array<{ text: string }>).map(r => r.text); + assert.deepEqual(phoneTexts.sort(), ['hang_up', 'phone hello', 'phone reply'].sort()); + const phoneKinds = (db.prepare("SELECT kind FROM phone ORDER BY ts_unix").all() as Array<{ kind: string }>).map(r => r.kind); + assert.ok(phoneKinds.includes('user'), 'phone-caller should normalize to user'); + assert.ok(phoneKinds.includes('agent'), 'phone-agent should normalize to agent'); + assert.ok(phoneKinds.includes('tool_call'), 'phone tool_call kind missing'); + + // discord_voice: user, agent, peer (3 utterances, no tool_calls in seed) + const dvKinds = (db.prepare("SELECT kind FROM discord_voice ORDER BY ts_unix").all() as Array<{ kind: string }>).map(r => r.kind); + assert.deepEqual(dvKinds.sort(), ['agent', 'peer', 'user'].sort()); + + // Legacy tables dropped (post-migration, the `conversation` + // name now refers to the backwards-compat VIEW, not the + // table; tool_calls is gone entirely). + const convType = (db.prepare("SELECT type FROM sqlite_master WHERE name='conversation'").get() as { type?: string } | undefined)?.type; + assert.equal(convType, 'view', `expected 'conversation' to be a view post-migration, got ${convType}`); + const tcExists = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name='tool_calls'").get(); + assert.equal(tcExists, undefined, 'tool_calls table should be dropped post-migration'); + } finally { + db.close(); + } + }); + + it('is idempotent on a second open (no row duplication)', () => { + // Capture row counts after the first migration ran in the prior + // test, then re-run the runner against the SAME DB and assert + // counts are unchanged. + let beforeCount: number; + { + const db = new DatabaseSync(MIG_DB); + try { + beforeCount = (db.prepare("SELECT (SELECT count(*) FROM voice) + (SELECT count(*) FROM phone) + (SELECT count(*) FROM discord_voice) AS c").get() as { c: number }).c; + } finally { + db.close(); + } + } + const r = runMigration(); + assert.equal(r.status, 0, `second migration runner failed: ${r.stderr}`); + const db = new DatabaseSync(MIG_DB); + try { + // Second run adds exactly one row (the SESSION_END trigger) + // but does NOT re-backfill anything from the now-gone legacy + // table. So the diff must equal exactly 1. + const afterCount = (db.prepare("SELECT (SELECT count(*) FROM voice) + (SELECT count(*) FROM phone) + (SELECT count(*) FROM discord_voice) AS c").get() as { c: number }).c; + assert.equal(afterCount, beforeCount + 1, 'second run must add exactly one row (the SESSION_END trigger)'); + } finally { + db.close(); + } + }); +}); + +// ---------------------------------------------------------------------- +// 6. Backwards-compatibility view: `conversation` reads work post-migration +// ---------------------------------------------------------------------- + +describe('backwards-compat conversation view', () => { + it('SELECT FROM conversation unions all 3 surface tables', () => { + const db = new DatabaseSync(PRIMARY_DB); + try { + const allKinds = db.prepare("SELECT DISTINCT role FROM conversation ORDER BY role").all() as Array<{ role: string }>; + const roles = allKinds.map(r => r.role); + // Should include the kinds we wrote across voice/phone/dv tables + assert.ok(roles.includes('user'), `expected 'user' in roles: ${JSON.stringify(roles)}`); + assert.ok(roles.includes('tool_call'), `expected 'tool_call' in roles: ${JSON.stringify(roles)}`); + assert.ok(roles.includes('SESSION_END'), `expected 'SESSION_END' in roles: ${JSON.stringify(roles)}`); + } finally { + db.close(); + } + }); + + it('v_voice / v_phone / v_discord_voice views exist with human-readable time', () => { + const db = new DatabaseSync(PRIMARY_DB); + try { + for (const v of ['v_voice', 'v_phone', 'v_discord_voice']) { + const cols = db.prepare(`PRAGMA table_info(${v})`).all() as Array<{ name: string }>; + assert.ok(cols.some(c => c.name === 'time'), `${v} should have a 'time' column`); + assert.ok(cols.some(c => c.name === 'kind'), `${v} should have a 'kind' column`); + } + } finally { + db.close(); + } + }); +}); + +// ---------------------------------------------------------------------- +// 7. SessionMetrics interface is backwards-compatible +// ---------------------------------------------------------------------- + +describe('SessionMetrics back-compat', () => { + it('accepts (and silently ignores) the legacy toolCalls field', () => { + // The toolCalls field is documented as accepted-but-ignored. This + // guards against the field being removed from the interface, + // which would break existing callers that still pass it. + assert.doesNotThrow(() => { + mod.recordSession({ + source: 'voice', + sessionId: 'compat-sess', + durationMs: 100, + toolCalls: [{ name: 'x', durationMs: 1, timestamp: '2026-06-01T12:00:00Z' }], + events: [], + }); + }); + }); +});