From d4386763d5feb2d615ed38ce555a6fd35ea3c67b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 23 Sep 2026 17:39:26 -0700 Subject: [PATCH] fix(opencode-usage): merge a migrated session's two rows per column (#22550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session that lived through OpenCode 2's V1 import has a row in both `session` and `session_v2`, and neither is complete. #22391 resolved the pair by ranking whole rows on one number — total token count, ties to `session_v2` — which let that number decide everything else on the row. Three consequences, each reproduced against that PR's own fixtures: - A recorded cost could be zeroed. `session_v2` wins on tokens while carrying `cost = 0`, and row parsing maps a zero cost to `null`, so a legacy row's $12.50 disappeared. Cost is re-derived by the same lossy reduce as the tokens, but only the tokens were guarded. - The token comparison decided metadata. A legacy row with more tokens supplied a stale pre-migration directory, and a legacy row with a NULL model erased the model `session_v2` had — 23 of 234 shared ids on a real migrated database have a model only on the v2 side. - Winner-takes-all is per row, so a legacy row holding the input tokens and a v2 row holding the cache reads reported one of them as zero. Metadata now comes from the generation OpenCode still writes, with older generations filling only its NULLs; usage columns take a per-column MAX. Both rows aggregate the same assistant messages, and the import can only drop messages, never invent them, so each column's MAX is a tighter lower bound on the truth than either row and can never exceed it. The relation stays exactly one row per id — the highest-priority generation holding it — every column stays `columnExists`-guarded with a SQL fallback, and a database with a single session table builds the same SQL it did before. Cache schema version 4 -> 5 so existing caches rescan. --- .../opencode-usage/opencode-usage-provider.ts | 4 +- .../opencode-usage-row-queries.ts | 134 ++++++++++---- .../opencode-usage-sqlite-fixture.ts | 7 +- .../opencode2-session-usage.test.ts | 175 ++++++++++++++++++ 4 files changed, 279 insertions(+), 41 deletions(-) diff --git a/src/main/opencode-usage/opencode-usage-provider.ts b/src/main/opencode-usage/opencode-usage-provider.ts index 0fbf9ef7814..9d91c876028 100644 --- a/src/main/opencode-usage/opencode-usage-provider.ts +++ b/src/main/opencode-usage/opencode-usage-provider.ts @@ -7,7 +7,9 @@ import type { } from './types' // Why: v4 reads OpenCode 2's `session_v2` table; v3 caches miss every v2 session. -export const OPENCODE_USAGE_SCHEMA_VERSION = 4 +// v5 merges a migrated session's two rows per column instead of picking one, so +// v4 caches hold zeroed costs and pre-migration metadata. +export const OPENCODE_USAGE_SCHEMA_VERSION = 5 export const openCodeUsageProvider = { id: 'opencode', diff --git a/src/main/opencode-usage/opencode-usage-row-queries.ts b/src/main/opencode-usage/opencode-usage-row-queries.ts index 8d44a20d363..9ed82360cac 100644 --- a/src/main/opencode-usage/opencode-usage-row-queries.ts +++ b/src/main/opencode-usage/opencode-usage-row-queries.ts @@ -33,25 +33,20 @@ type OpenCodeSessionUsageRow = { // Why: OpenCode 2 copies every v1 `session` row into `session_v2` and then only // writes there, so a migrated opencode.db holds both tables and the same session // id in each. Reading `session` alone loses every OpenCode 2 session (#15841); -// reading both unfiltered would double-count the migrated ones. Newest first, -// which only breaks ties — the fuller row wins, see `buildSessionTableSelect`. +// reading both unfiltered would double-count the migrated ones. First entry is +// the generation OpenCode still writes to. const SESSION_TABLES_BY_PRIORITY = ['session_v2', 'session'] as const -// Columns the usage scan reads off a session row, with the SQL literal to -// substitute when a schema generation lacks the column. -const SESSION_SOURCE_COLUMNS: Record = { +// What the session *is*, with the SQL literal to substitute when no generation +// carries the column. The live generation answers these; an older twin only +// fills in what the live one left NULL. +const SESSION_METADATA_COLUMNS: Record = { project_id: 'NULL', directory: 'NULL', title: 'NULL', model: 'NULL', time_created: '0', - time_updated: 'NULL', - cost: '0', - tokens_input: '0', - tokens_output: '0', - tokens_reasoning: '0', - tokens_cache_read: '0', - tokens_cache_write: '0' + time_updated: 'NULL' } const SESSION_TOKEN_COLUMNS = [ @@ -62,13 +57,69 @@ const SESSION_TOKEN_COLUMNS = [ 'tokens_cache_write' ] as const +// What the session *spent*. Merged per column, never row-at-a-time. +const SESSION_USAGE_COLUMNS = ['cost', ...SESSION_TOKEN_COLUMNS] as const + const SESSION_TOKEN_TOTAL = SESSION_TOKEN_COLUMNS.map((name) => `s.${name}`).join(' + ') -/** The same total against one raw session table, which may be missing columns. */ -function sessionTableTokenTotal(db: Database.Database, table: string, alias: string): string { - return SESSION_TOKEN_COLUMNS.map((name) => - columnExists(db, table, name) ? `${alias}.${name}` : '0' - ).join(' + ') +/** One generation's row for a session id, merged into the select that owns it. */ +type SessionContributor = { table: string; alias: string } + +function columnRef( + db: Database.Database, + contributor: SessionContributor, + name: string +): string | null { + return columnExists(db, contributor.table, name) ? `${contributor.alias}.${name}` : null +} + +// Why the live generation rather than whichever row has the bigger numbers: +// after the v1 import, `session` is frozen while `session_v2` keeps being +// written, so a pre-migration directory, title or timestamp survives in the +// legacy twin indefinitely. The import also derives `session_v2.model` from the +// last user message when the v1 row had none (`transformSession` in upstream +// `v1-migration.bun.ts`), so the legacy row is the one that can be NULL here — +// 23 of 234 shared ids on a real migrated database. Older generations only fill +// NULLs. +function buildMetadataExpression( + db: Database.Database, + contributors: readonly SessionContributor[], + name: string +): string { + const fallback = SESSION_METADATA_COLUMNS[name] ?? 'NULL' + const refs = contributors + .map((contributor) => columnRef(db, contributor, name)) + .filter((ref) => ref !== null) + if (refs.length === 0 || contributors.length === 1) { + return refs[0] ?? fallback + } + const tail = fallback === 'NULL' ? [] : [fallback] + return `COALESCE(${[...refs, ...tail].join(', ')})` +} + +// Why per column rather than picking a winning row: both rows aggregate the same +// assistant messages of the same session. The import re-derives every v2 total +// from decoded messages and drops the ones that fail to decode, so each v2 +// column starts at or below its frozen legacy twin and then grows as the session +// keeps running. Neither side can invent usage, so each column's MAX is a +// strictly tighter lower bound on the truth than either row alone and can never +// exceed it. Choosing a row instead lets a token comparison zero a recorded +// cost, or a cost comparison zero recorded tokens. +function buildUsageExpression( + db: Database.Database, + contributors: readonly SessionContributor[], + name: string +): string { + const refs = contributors + .map((contributor) => columnRef(db, contributor, name)) + .filter((ref) => ref !== null) + if (refs.length === 0 || contributors.length === 1) { + return refs[0] ?? '0' + } + // An outer-joined generation is NULL for ids it never held, and SQLite's + // scalar MAX() returns NULL if any argument is. + const guarded = refs.map((ref) => `COALESCE(${ref}, 0)`) + return guarded.length === 1 ? (guarded[0] ?? '0') : `MAX(${guarded.join(', ')})` } function listSessionTables(db: Database.Database): string[] { @@ -83,27 +134,34 @@ function buildSessionTableSelect( index: number ): string { const table = tables[index] ?? '' - const columns = Object.entries(SESSION_SOURCE_COLUMNS).map( - ([name, fallback]) => `${columnExists(db, table, name) ? `t.${name}` : fallback} AS ${name}` - ) - // Why the fuller row rather than the newer one: `session_v2` is not reliably a - // superset. Upstream's importer recomputes v2 totals from decoded messages, so - // a session whose messages fail to decode lands below its frozen legacy row; a - // v2 table without the token columns at all scores 0 and would otherwise erase - // the legacy row's usage entirely. Ties go to the higher-priority table, so a - // faithful copy still resolves to `session_v2`. - const total = sessionTableTokenTotal(db, table, 't') + // Only lower-priority generations join in: a higher-priority one holding this + // id would have excluded the row outright, so it has nothing to contribute. + const contributors: SessionContributor[] = [ + { table, alias: 't' }, + ...tables + .slice(index + 1) + .map((other, offset) => ({ table: other, alias: `o${index + offset + 1}` })) + ] + const columns = [ + ...Object.keys(SESSION_METADATA_COLUMNS).map( + (name) => `${buildMetadataExpression(db, contributors, name)} AS ${name}` + ), + ...SESSION_USAGE_COLUMNS.map( + (name) => `${buildUsageExpression(db, contributors, name)} AS ${name}` + ) + ] + const joins = contributors + .slice(1) + .map((other) => `LEFT JOIN ${other.table} ${other.alias} ON ${other.alias}.id = t.id`) + .join(' ') + // Exactly one select claims each id: the highest-priority generation holding + // it. Exclusive because every lower select rejects an id a higher one has, + // exhaustive because the highest one holding it never rejects it. const exclusions = tables - .map((other, otherIndex) => { - if (otherIndex === index) { - return null - } - const beats = otherIndex < index ? '>=' : '>' - return `NOT EXISTS (SELECT 1 FROM ${other} o WHERE o.id = t.id AND ${sessionTableTokenTotal(db, other, 'o')} ${beats} ${total})` - }) - .filter((clause) => clause !== null) + .slice(0, index) + .map((other) => `NOT EXISTS (SELECT 1 FROM ${other} o WHERE o.id = t.id)`) .join(' AND ') - return `SELECT t.id, ${columns.join(', ')} FROM ${table} t${exclusions ? ` WHERE ${exclusions}` : ''}` + return `SELECT t.id, ${columns.join(', ')} FROM ${table} t${joins ? ` ${joins}` : ''}${exclusions ? ` WHERE ${exclusions}` : ''}` } /** A single deduplicated session relation spanning every session table generation. */ @@ -132,8 +190,8 @@ function getAssistantSessionMessageCount(db: Database.Database): number { return row?.count ?? 0 } -// `some`, not `every`: a table missing the token columns scores 0 in the source's -// tie-break, so it can never outrank — or erase — a sibling that carries them. +// `some`, not `every`: the merged row takes each usage column from whichever +// generation carries it, so one table missing them costs nothing. function hasSessionUsageColumns(db: Database.Database, tables: readonly string[]): boolean { return tables.some((table) => ['cost', 'tokens_input', 'tokens_output', 'tokens_reasoning', 'tokens_cache_read'].every( diff --git a/src/main/opencode-usage/opencode-usage-sqlite-fixture.ts b/src/main/opencode-usage/opencode-usage-sqlite-fixture.ts index 3263ad964bf..c79b9af72a7 100644 --- a/src/main/opencode-usage/opencode-usage-sqlite-fixture.ts +++ b/src/main/opencode-usage/opencode-usage-sqlite-fixture.ts @@ -82,7 +82,8 @@ export type OpenCodeUsageFixtureSession = { id: string directory: string title?: string - model?: string + /** `null` writes a NULL `model`, the shape a v1 row has before the import derives one. */ + model?: string | null cost?: number tokensInput?: number tokensOutput?: number @@ -134,7 +135,9 @@ function insertSession( session.title ?? 'OpenCode session', created, session.timeUpdated ?? created + 60_000, - session.model ?? '{"providerID":"anthropic","modelID":"claude-sonnet-4-5"}' + session.model === undefined + ? '{"providerID":"anthropic","modelID":"claude-sonnet-4-5"}' + : session.model ] const usage = withUsageColumns ? [ diff --git a/src/main/opencode-usage/opencode2-session-usage.test.ts b/src/main/opencode-usage/opencode2-session-usage.test.ts index d2ccaf8b48a..46e6909274a 100644 --- a/src/main/opencode-usage/opencode2-session-usage.test.ts +++ b/src/main/opencode-usage/opencode2-session-usage.test.ts @@ -268,6 +268,21 @@ describe('OpenCode 2 session_v2 usage', () => { }) }) + it('keeps a session that only session_v2 has when a legacy twin is absent', () => { + const path = createFixture({ + generation: 'migrated', + legacySessions: [{ id: 'ses_shared', directory: WORKTREE, tokensInput: 5 }], + v2Sessions: [ + { id: 'ses_shared', directory: WORKTREE, tokensInput: 5 }, + { id: 'ses_v2_only', directory: WORKTREE, cost: 3.25, tokensInput: 70 } + ] + }) + + const events = readEvents(path) + const v2Only = events.find((event) => event.sessionId === 'ses_v2_only') + expect(v2Only).toMatchObject({ inputTokens: 70, estimatedCostUsd: 3.25, cwd: WORKTREE }) + }) + it('falls back to the project worktree when the session has no directory', async () => { const path = createFixture({ generation: 'v2-only', @@ -282,3 +297,163 @@ describe('OpenCode 2 session_v2 usage', () => { }) }) }) + +// Why: a migrated session has two rows for one session, and neither is complete. +// `session_v2` is the row OpenCode still writes, so it says what the session is; +// each usage column is a lossy re-derivation, so each column takes the larger of +// the two. Ranking whole rows by one number let that number decide cost, +// directory and model too. +describe('OpenCode 2 migrated session column merge', () => { + const SHARED = 'ses_shared' + + it('keeps a recorded cost the recomputed session_v2 row lost', () => { + const path = createFixture({ + generation: 'migrated', + legacySessions: [{ id: SHARED, directory: WORKTREE, tokensInput: 100, cost: 12.5 }], + v2Sessions: [{ id: SHARED, directory: WORKTREE, tokensInput: 200, cost: 0 }] + }) + + const events = readEvents(path) + expect(events).toHaveLength(1) + expect(events[0]?.inputTokens).toBe(200) + expect(events[0]?.estimatedCostUsd).toBe(12.5) + }) + + it('attributes usage to the directory session_v2 records now', () => { + const path = createFixture({ + generation: 'migrated', + legacySessions: [ + { id: SHARED, directory: '/old/pre-migration-path', title: 'Old title', tokensInput: 900 } + ], + v2Sessions: [ + { id: SHARED, directory: '/new/current-path', title: 'New title', tokensInput: 120 } + ] + }) + + const events = readEvents(path) + expect(events).toHaveLength(1) + // The legacy row still wins the token column; it must not drag metadata with it. + expect(events[0]?.inputTokens).toBe(900) + expect(events[0]?.cwd).toBe('/new/current-path') + }) + + it('keeps the model session_v2 derived when the legacy row has none', () => { + const path = createFixture({ + generation: 'migrated', + // The import fills session_v2.model from the last user message when the v1 + // row had none: 23 of 234 shared ids on a real migrated database. + legacySessions: [{ id: SHARED, directory: WORKTREE, model: null, tokensInput: 900 }], + v2Sessions: [ + { + id: SHARED, + directory: WORKTREE, + model: '{"providerID":"anthropic","modelID":"claude-opus-4-1"}', + tokensInput: 120 + } + ] + }) + + const events = readEvents(path) + expect(events).toHaveLength(1) + expect(events[0]?.inputTokens).toBe(900) + expect(events[0]?.model).toBe('anthropic/claude-opus-4-1') + }) + + it('takes each usage column from whichever generation recorded more', () => { + const path = createFixture({ + generation: 'migrated', + legacySessions: [ + { id: SHARED, directory: WORKTREE, tokensInput: 1000, tokensCacheRead: 0, cost: 1 } + ], + v2Sessions: [ + { id: SHARED, directory: WORKTREE, tokensInput: 0, tokensCacheRead: 1200, cost: 2 } + ] + }) + + const events = readEvents(path) + expect(events).toHaveLength(1) + expect(events[0]).toMatchObject({ + inputTokens: 1000, + cachedInputTokens: 1200, + estimatedCostUsd: 2, + totalTokens: 2200 + }) + }) + + it('resolves a faithful migrated copy exactly as the v2 row alone', () => { + const session = { + id: SHARED, + directory: WORKTREE, + cost: 0.75, + tokensInput: 100, + tokensOutput: 20, + tokensReasoning: 5, + tokensCacheRead: 900, + tokensCacheWrite: 300 + } + const migrated = readEvents( + createFixture({ + generation: 'migrated', + legacySessions: [session], + v2Sessions: [session] + }) + ) + + expect(migrated).toEqual( + readEvents(createFixture({ generation: 'v2-only', v2Sessions: [session] })) + ) + }) + + it('leaves an OpenCode 1-only database untouched by the merge', () => { + const path = createFixture({ + generation: 'v1', + legacySessions: [ + { + id: 'ses_v1', + directory: WORKTREE, + cost: 1.25, + tokensInput: 11, + tokensOutput: 3, + tokensCacheRead: 7, + tokensCacheWrite: 2 + } + ] + }) + + expect(readEvents(path)).toEqual([ + expect.objectContaining({ + sessionId: 'ses_v1', + cwd: WORKTREE, + model: 'anthropic/claude-sonnet-4-5', + estimatedCostUsd: 1.25, + inputTokens: 11, + outputTokens: 3, + cachedInputTokens: 7, + totalTokens: 23 + }) + ]) + }) + + it('emits exactly one row per session id across both generations', () => { + const path = createFixture({ + generation: 'migrated', + legacySessions: [ + { id: SHARED, directory: WORKTREE, tokensInput: 900 }, + { id: 'ses_legacy_only', directory: WORKTREE, tokensInput: 42 } + ], + v2Sessions: [ + { id: SHARED, directory: WORKTREE, tokensInput: 120 }, + { id: 'ses_v2_only', directory: WORKTREE, tokensInput: 7 } + ] + }) + + const db = new Database(path, { readonly: true, fileMustExist: true }) + try { + const ids = selectUsageRows(db).map((row) => row.id) + expect(ids).toHaveLength(3) + expect(new Set(ids).size).toBe(3) + } finally { + db.close() + } + }) +})