From 211e9036208d1e966507fd85615d55ee68e6ef71 Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Sat, 23 May 2026 13:03:51 +0000 Subject: [PATCH] v1.13.20-drop-legacy-cols: final phase of v1.13.0 strangler-fig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the dual-write into messages.tool_calls / messages.tool_results JSON columns and drops the columns. message_parts is now the only source of truth for tool calls and tool results. 10 dual-write sites stripped (5 in tool-phase.ts, 2 in routes/skills.ts, 2 in routes/messages.ts, 1 in routes/chats.ts fork-clone). The recon-driven grep caught 2 sites beyond the original v1.13.2 roadmap inventory and an extra fixture file (tool_cost_stats.test.ts) with a direct legacy-column INSERT. messages_with_parts view rewritten to parts-only subselects (COALESCE fallbacks gone). View runs via CREATE OR REPLACE so it lands before the column DROPs in startup DDL — Postgres rejects column-drop on view-referenced cols. v1.12.1 cleanup DO block (DROP CONSTRAINT messages_status_check / messages_role_check) removed; those one-shots have done their work. Adversarial review caught a runtime bug the green test suite missed: the discard_stale endpoint (chats.ts) had a RETURNING ... tool_calls, tool_results clause that would have crashed on every 60s-no-token-activity recovery in production. Fixed by switching to two-step UPDATE returning id, then SELECT from messages_with_parts so parts-synthesized fields keep flowing on the wire. Message API type retains tool_calls? / tool_results? — the view synthesizes those keys from parts so the wire shape is unchanged; frontend reads need no update. Override on the original v1.13.2 plan, captured in the openspec proposal. 339/339 server tests passing (including 7 DB-integration tests that applied the schema migration to a live DB and ran the parts-only view end-to-end). tsc + web build clean. Pairs with v1.13.0-ai-sdk-v6 (introduced the dual-write) and v1.13.1-B (moved the read path to messages_with_parts). Umbrella v1.13 tag ships on this same commit, marking the strangler-fig closed. CLAUDE.md picks up Sam's pre-existing edits documenting tag-naming and CHANGELOG conventions — both already in use by v1.13.19 / v1.13.20. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 4 + CLAUDE.md | 6 +- apps/server/src/routes/chats.ts | 18 ++- apps/server/src/routes/messages.ts | 24 ++-- apps/server/src/routes/skills.ts | 14 +- apps/server/src/schema.sql | 70 ++++------ .../__tests__/tool_cost_stats.test.ts | 26 ++-- .../src/services/inference/tool-phase.ts | 45 ++----- boocode_roadmap.md | 2 + .../v1.13.20-drop-legacy-cols/proposal.md | 126 ++++++++++++++++++ .../v1.13.20-drop-legacy-cols/tasks.md | 104 +++++++++++++++ 11 files changed, 317 insertions(+), 122 deletions(-) create mode 100644 openspec/changes/v1.13.20-drop-legacy-cols/proposal.md create mode 100644 openspec/changes/v1.13.20-drop-legacy-cols/tasks.md diff --git a/CHANGELOG.md b/CHANGELOG.md index fea9e4b..ed5a3b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes per release tag. Most recent on top, ordered by tag creation date (which matches the git history). Tag names follow `vMAJOR.MINOR.PATCH-slug` — the slug describes what shipped, so the tag name alone is enough to recall the batch. +## v1.13.20-drop-legacy-cols — 2026-05-23 + +Final phase of the v1.13.0 strangler-fig migration. Removes the dual-write into `messages.tool_calls` / `messages.tool_results` JSON columns and drops the columns themselves; `message_parts` is now the only source of truth for tool-call and tool-result data. 10 dual-write sites stripped (5 in `tool-phase.ts`, 2 in `routes/skills.ts`, 2 in `routes/messages.ts`, 1 in `routes/chats.ts` fork-clone) — recon's grep-driven inventory caught 2 sites beyond the original v1.13.2 roadmap count. `messages_with_parts` view simplified to parts-only subselects (COALESCE fallbacks gone) and rewritten via `CREATE OR REPLACE VIEW` BEFORE the column DROP since Postgres rejects column-drop on view-referenced cols. Adversarial review caught a runtime bug the green test suite missed: `chats.ts:/api/chats/:id/discard_stale` had a `RETURNING ... tool_calls, tool_results, ...` clause referencing the dropped columns; would have crashed on every 60s-no-token-activity recovery in production. Fixed by switching to two-step UPDATE-then-SELECT-from-view so the response keeps the parts-synthesized fields. `Message` API type retains `tool_calls?` / `tool_results?` fields (override on the original v1.13.2 plan) — the view continues to populate them from parts, so the wire shape is unchanged and the frontend needs no updates. v1.12.1 cleanup block (`DROP CONSTRAINT messages_status_check`/`messages_role_check`) removed — those one-shots have done their work. `tool_cost_stats.test.ts` had a direct `INSERT INTO messages` touching the legacy columns that wasn't in the roadmap's inventory; rewritten to parts-table inserts and confirmed semantically faithful. 339/339 server tests passing including the 7 DB-integration tests (live-DB applied the schema migration and ran the parts-only view end-to-end). Pairs with `v1.13.0-ai-sdk-v6` (which introduced the dual-write) and `v1.13.1-B` (which moved the read path to `messages_with_parts`); umbrella `v1.13` tag ships on the same commit. + ## v1.13.19-html-artifact-panes — 2026-05-23 Pane-based artifact viewer with on-request HTML support. Every assistant message gets an "Open in pane" icon button (`PanelRightOpen`, mobile 44px tap-target) in `MessageBubble`'s ActionRow; click opens the message in the workspace splitter as either a Markdown pane (Copy raw source + Download `.md`) or an HTML pane (Download `.html` only, no Copy). The HTML path triggers when the model emits a self-contained `` or fenced ` ```html` artifact (opt-in only — `BOOCHAT.md` rule says Markdown is default at every length; HTML only on explicit user request like "render this as HTML"). Backend detection in `finalizeCompletion` (`error-handler.ts`) writes a new `message_parts.kind='html_artifact'` row with payload `{html_content, char_count, title}` (`` → first `<h1>` → first 80 chars of inner text). Schema CHECK extended via the v1.13.13 drop-and-re-add pattern. 1MB cap is graceful — over-cap artifacts skip the part write and plain content lands; decision factored into a pure `decideHtmlArtifactWrite` helper so the warn-and-skip branch is unit-testable without mocking the full InferenceContext. Pane state is reference-only (`{chat_id, message_id, title}`) — content is fetched on mount, keeping `sessions.workspace_panes` jsonb small and avoiding 1MB blobs riding the `session_workspace_updated` WS frame. New `services/artifacts.ts` ships slug derivation (Markdown: first `#` heading → first 6 words; HTML: `<title>` → `<h1>` → inner text) and write helpers that realpath the artifacts directory after `mkdir` to close a symlink-escape gap (`assertArtifactsDirSafe`). `routes/artifacts.ts` exposes POST `/api/chats/:id/messages/:msg_id/artifacts/download?fmt=md|html` (writes to `<projectRoot>/.boocode/artifacts/<slug>-<ts>.<ext>`) plus GET `/api/projects/:project_id/artifacts/:filename` with `Content-Disposition: attachment`, `X-Content-Type-Options: nosniff`, and `Content-Security-Policy: sandbox` defense-in-depth on LLM-served HTML. iframe sandbox locks to `allow-scripts allow-clipboard-write allow-downloads` with no `allow-same-origin` and uses `srcDoc` (not `src`) for opaque-origin isolation. Frontend extracts `MarkdownRenderer.tsx` from `MessageBubble`'s inline `MarkdownBody` for reuse; `MarkdownArtifactPane.tsx` / `HtmlArtifactPane.tsx` render with loading + error states. 404-vs-real-error discrimination in `openInPane`: a real network/500 failure toasts and bails instead of silently masquerading as a Markdown pane. 31 new server unit tests (slug derivation, detection positive/negative, write helpers, symlink-escape, 1MB cap, real-symlink filesystem test); 332/332 server tests passing; `tsc -p apps/web/tsconfig.app.json --noEmit` clean; `pnpm -C apps/web build` green. Smoke deferred to first deploy. diff --git a/CLAUDE.md b/CLAUDE.md index a9fa740..7b5cbc0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,7 +46,7 @@ Tests: `pnpm -C apps/server test` runs the vitest suite. No test harness on `app - **Zod** for request validation and config parsing. Key services: -- **`services/inference/`** — Public surface re-exported via `inference/index.ts`; callers import from `./services/inference/index.js` explicitly (NodeNext doesn't honor directory-index resolution). Layout: `turn.ts` (runAssistantTurn / runInference / createInferenceRunner; exports `InferenceFrame`, `InferenceContext`, `TurnArgs`, `StreamResult`), `stream-phase.ts` (streamCompletion as a v1.13.1-A AI SDK adapter + executeStreamPhase), `provider.ts` (`upstreamModel(baseURL, modelId)` wrapping `createOpenAICompatible` against llama-swap), `tool-phase.ts` (executeToolPhase; value back-edges into turn.ts for the runAssistantTurn recursion — cycle safe because deref at call time, not module top-level), `sentinel-summaries.ts` (runCapHitSummary + runDoomLoopSummary + their sentinel inserters), `error-handler.ts` (handleAbortOrError, finalizeCompletion), `payload.ts` (buildMessagesPayload, loadContext, maybeFlagForCompaction, `OpenAiMessage`), `sentinels.ts` (`detectDoomLoop`, `DOOM_LOOP_THRESHOLD`, sentinel predicates), `budget.ts` (resolveToolBudget), `xml-parser.ts` (qwen3.6 XML tool-call fallback — KEEP, AI SDK doesn't handle inline-XML tool calls), `parts.ts` (v1.13.0 dual-write helpers: `partsFromAssistantMessage`, `partsFromToolMessage`, `insertParts`), `prune.ts` (v1.13.4 two-tier compaction; `selectPruneTargets` is the pure decision helper), `types.ts` (`StreamPhaseState`, `DB_FLUSH_INTERVAL_MS`). **`TurnArgs`** is the per-turn state envelope threaded through the `executeToolPhase → runAssistantTurn` recursion; reset in `runInference` at user-message boundary. Add new per-turn state to `TurnArgs`, not module-level closures. +- **`services/inference/`** — Public surface re-exported via `inference/index.ts`; callers import from `./services/inference/index.js` explicitly (NodeNext doesn't honor directory-index resolution). Layout: `turn.ts` (runAssistantTurn / runInference / createInferenceRunner; exports `InferenceFrame`, `InferenceContext`, `TurnArgs`, `StreamResult`), `stream-phase.ts` (streamCompletion as a v1.13.1-A AI SDK adapter + executeStreamPhase), `provider.ts` (`upstreamModel(baseURL, modelId)` wrapping `createOpenAICompatible` against llama-swap), `tool-phase.ts` (executeToolPhase; value back-edges into turn.ts for the runAssistantTurn recursion — cycle safe because deref at call time, not module top-level), `sentinel-summaries.ts` (runCapHitSummary + runDoomLoopSummary + their sentinel inserters), `error-handler.ts` (handleAbortOrError, finalizeCompletion), `payload.ts` (buildMessagesPayload, loadContext, maybeFlagForCompaction, `OpenAiMessage`), `sentinels.ts` (`detectDoomLoop`, `DOOM_LOOP_THRESHOLD`, sentinel predicates), `budget.ts` (resolveToolBudget), `xml-parser.ts` (qwen3.6 XML tool-call fallback — KEEP, AI SDK doesn't handle inline-XML tool calls), `parts.ts` (parts-table write helpers: `partsFromAssistantMessage`, `partsFromToolMessage`, `insertParts` — v1.13.20 made parts the sole source of truth), `prune.ts` (v1.13.4 two-tier compaction; `selectPruneTargets` is the pure decision helper), `types.ts` (`StreamPhaseState`, `DB_FLUSH_INTERVAL_MS`). **`TurnArgs`** is the per-turn state envelope threaded through the `executeToolPhase → runAssistantTurn` recursion; reset in `runInference` at user-message boundary. Add new per-turn state to `TurnArgs`, not module-level closures. - **AI SDK v6 streamCompletion adapter** (v1.13.1-A; `services/inference/stream-phase.ts`). `streamText` is the underlying call; the BooCode layer above (executeStreamPhase, finalize, dual-write) is shape-preserved via an adapter. Five gotchas the LSP/test suite won't catch: - **Abort signals are swallowed.** `streamText`'s `fullStream` iterator exits cleanly when `abortSignal` fires — no throw. Post-iteration `if (signal?.aborted) throw <AbortError>` is required; without it the row finalizes as `complete` instead of `cancelled`. Comment in stream-phase.ts pins this; don't refactor it away. - **Usage lands only at stream end** via `await result.usage` (`inputTokens` / `outputTokens` v6 names → mapped to `promptTokens` / `completionTokens` for the existing onUsage callback). Mid-stream live tok/s is gone vs v1.12.2; ChatThroughput shows a single value at stream end. @@ -63,7 +63,7 @@ Key services: - **`services/compaction.ts`** + **`services/model-context.ts`** — v1.11.0 anchored rolling summary (single `summary=true` assistant row per chat, supersedes itself on each compaction). Triggered when `chats.needs_compaction` is set after an inference turn exceeds `usable(ctx_max) = floor(0.85 × ctx_max)` (v1.13.9 opencode-pattern early trigger; was `ctx_max - 20k` pre-v1.13.9, which gave only 7.6% headroom at 262k and 0 budget for ≤20k contexts). **`ctx_max` comes from `model-context.getModelContext()` which fetches `${LLAMA_SWAP_URL}/upstream/<model>/props`** — NOT from `parsed.timings.n_ctx` (the stream completion's `timings` doesn't carry n_ctx; that read was dead code until v1.11.3 ripped it out). First inferences after a boocode boot may have `ctx_max=NULL` if llama-swap hasn't loaded the model yet; negative cache TTL is 60s, recovers on next turn. v1.13.6: `buildHeadPayload` embeds `reasoning_parts` as a `<reasoning>...</reasoning>` prose prefix on the assistant `content` (OpenAI wire shape has no structured reasoning field; the summarizer reads text). Standalone tag when content is empty (tool-call-only turn). `buildHeadPayload` + `OpenAiMessage` exported for test access — keep them exported. - **`services/system-prompt.ts`** — `buildSystemPrompt` is the string-returning shim; `buildSystemPromptWithFingerprint` is the canonical impl returning `{prompt, fingerprint, drift}`. v1.13.8 instrumentation: SHA-256 of the assembled prefix is logged per `buildMessagesPayload` call (msg `prefix-fingerprint`, level=info); a `Map<sessionId, lastHash>` observer fires `prefix-drift` (level=warn) on hash change with a field-level `changed_inputs` diff. Smoke proved the prefix is byte-stable across turns in steady-state — the originally-planned `system_prompt_cache` DB table was dropped as redundant against the v1.12.0 input-layer mtime caches (BOOCHAT.md here + AGENTS.md global+per-project in `agents.ts:safeStat`). - **`services/inference/budget.ts`** — tool-call budgets: `BUDGET_READ_ONLY = 30`, `BUDGET_NON_READ_ONLY = 10` (forward-looking; no write tools yet), `BUDGET_NO_AGENT = 30` (v1.13.7; was 15 — every tool in `ALL_TOOLS` is read-only today, so no-agent mode shares the read-only-agent cap). Per-agent `max_tool_calls` from AGENTS.md frontmatter overrides. -- **`messages_with_parts` view** (v1.13.1-B; `schema.sql`). Read sites that need `tool_calls` / `tool_results` / `reasoning_parts` SELECT from this view, NOT `messages` directly. `COALESCE`s parts-table rows over the legacy JSON columns, so pre-v1.13.0 history still resolves. Writes still target `messages`; the v1.13.0 dual-write into `message_parts` keeps both halves in sync. New payload-assembly code must use the view — calling `messages.tool_calls` directly will miss anything written post-v1.13.1-B if the JSON column ever drifts (and dual-write makes that easy to miss). Shapes: `tool_calls jsonb[]`, `tool_results jsonb` single object, `reasoning_parts jsonb[]` of `{text}`. +- **`messages_with_parts` view** (v1.13.1-B; `schema.sql`). Read sites that need `tool_calls` / `tool_results` / `reasoning_parts` SELECT from this view, NOT `messages` directly. v1.13.20 dropped the legacy `messages.tool_calls` / `messages.tool_results` JSON columns; the view now reads parts-only subselects. Writes target `message_parts` exclusively via `insertParts` (or via the helpers `partsFromAssistantMessage` / `partsFromToolMessage`). The `Message` wire type still carries `tool_calls?` / `tool_results?` because the view synthesizes them from parts — frontend reads are unchanged. Shapes: `tool_calls jsonb[]`, `tool_results jsonb` single object, `reasoning_parts jsonb[]` of `{text}`. If you ever need to UPDATE a message and return its full Message shape, do a two-step UPDATE returning `id` followed by SELECT from the view — RETURNING off the bare `messages` table no longer carries the tool fields. - **`services/file_ops.ts`** — Shared file operation implementations used by both inference tools and HTTP routes. - **`services/auto_name.ts`** — Non-streaming LLM call to generate 4-word session titles after first assistant reply. @@ -118,6 +118,8 @@ Required: `DATABASE_URL`, `LLAMA_SWAP_URL`. Optional: `PORT` (3000), `HOST` (0.0 - Sam reviews all diffs and commits manually. Do not commit unless explicitly asked. - Per-batch docs live under `openspec/changes/<slug>/{proposal,tasks,design}.md`. Already-shipped batches are snapshots in `openspec/changes/archived/`. New batches follow the proposal+tasks shape; see `openspec/README.md` for the convention. +- Tag naming: `vMAJOR.MINOR.PATCH-slug` (e.g. `v1.13.13-ws-publish`). Monotonic per minor — the slug describes the batch's content so the tag name alone is enough to recall what shipped. No letter suffixes (`-a`/`-b`), no pseudo-ranges (`v1.11.x`), no slug-only sub-versions sharing a number (`v1.13.15-tools` + `-openspec` + `-agentlint` — split into sequential patches instead). +- `CHANGELOG.md` is the per-tag release log, most-recent on top. When a new tag is created, add a `## <tag> — <YYYY-MM-DD>` section with a 3–6 sentence paragraph summarizing what shipped, drawn from the commit body. Cross-reference other tags by name when the batch builds on, fixes, or pairs with prior work (e.g. "pairs with `v1.13.12-ws-schemas`", "fixed in `v1.13.5-stability-bundle`"). No nested bullets — one paragraph. - Deploy: `cd /opt/boocode && docker compose up --build -d` (or `docker compose build --no-cache boocode && docker compose up -d` if you suspect a layer-cache issue). - Git push to Gitea: `GIT_SSH_COMMAND="ssh -i /opt/boocode/secrets/boocode_gitea -o IdentitiesOnly=yes" git push origin <branch>`. The default agent identity is rejected; the in-repo deploy key (`secrets/`, gitignored) is the working one. Transient `Connection reset by peer` retries cleanly after `sleep 5`. - Don't accumulate `.bak-*` files. Clean them up in the same batch or immediately after merge. diff --git a/apps/server/src/routes/chats.ts b/apps/server/src/routes/chats.ts index 037a183..ad6bc5c 100644 --- a/apps/server/src/routes/chats.ts +++ b/apps/server/src/routes/chats.ts @@ -296,13 +296,13 @@ export function registerChatRoutes( `; await tx` INSERT INTO messages ( - session_id, chat_id, role, content, kind, tool_calls, tool_results, + session_id, chat_id, role, content, kind, status, tokens_used, ctx_used, ctx_max, started_at, finished_at, created_at, metadata ) SELECT ${source.session_id}, ${chat!.id}, role, content, kind, - tool_calls, tool_results, status, + status, tokens_used, ctx_used, ctx_max, started_at, finished_at, clock_timestamp() + ( ROW_NUMBER() OVER (ORDER BY created_at ASC, id ASC) * INTERVAL '1 microsecond' @@ -385,21 +385,25 @@ export function registerChatRoutes( reply.code(409); return { error: 'message is not stale yet', age_seconds: msg.age_seconds }; } - const updated = await sql<Message[]>` + const updated = await sql<{ id: string }[]>` UPDATE messages SET status = 'failed', content = COALESCE(content, ''), finished_at = clock_timestamp() WHERE id = ${msg.id} AND status = 'streaming' - RETURNING id, session_id, chat_id, role, content, kind, tool_calls, tool_results, - status, last_seq, tokens_used, ctx_used, ctx_max, started_at, finished_at, - created_at, metadata, summary, tail_start_id, compacted_at + RETURNING id `; if (updated.length === 0) { // Race: the row flipped out of 'streaming' between our SELECT and UPDATE. reply.code(409); return { error: 'message status changed mid-request' }; } + // v1.13.20: re-fetch via messages_with_parts so the returned shape + // carries parts-synthesized tool_calls / tool_results. The dropped + // legacy columns can no longer be selected directly. + const refreshed = await sql<Message[]>` + SELECT * FROM messages_with_parts WHERE id = ${msg.id} + `; broker.publishUserFrame('default', { type: 'chat_status', chat_id: msg.chat_id, @@ -411,7 +415,7 @@ export function registerChatRoutes( message_id: msg.id, chat_id: msg.chat_id, }); - return updated[0]; + return refreshed[0]; } ); diff --git a/apps/server/src/routes/messages.ts b/apps/server/src/routes/messages.ts index 86cca2a..690a3df 100644 --- a/apps/server/src/routes/messages.ts +++ b/apps/server/src/routes/messages.ts @@ -605,15 +605,11 @@ export function registerMessageRoutes( const toolMessageId = toolRow.message_id; const result = await sql.begin(async (tx) => { - await tx` - UPDATE messages - SET tool_results = ${tx.json(newToolResults as never)} - WHERE id = ${toolMessageId} - `; - // v1.13.0: replace the pending tool_result part inserted at message - // creation (tool-phase.ts) with the answered one. Delete-then-insert - // is simpler than UPDATE because parts are append-style elsewhere; - // the UNIQUE (message_id, sequence) constraint blocks plain insert. + // v1.13.20: parts-only. Replace the pending tool_result part inserted + // at message creation (tool-phase.ts) with the answered one. Delete- + // then-insert is simpler than UPDATE because parts are append-style + // elsewhere; the UNIQUE (message_id, sequence) constraint blocks + // plain insert. await tx`DELETE FROM message_parts WHERE message_id = ${toolMessageId} AND kind = 'tool_result'`; await tx` INSERT INTO message_parts (message_id, sequence, kind, payload) @@ -796,13 +792,9 @@ export function registerMessageRoutes( }; const toolMessageId = toolRow.message_id; const dbResult = await sql.begin(async (tx) => { - await tx` - UPDATE messages - SET tool_results = ${tx.json(newToolResults as never)} - WHERE id = ${toolMessageId} - `; - // Same delete+insert dance as /answer — UNIQUE (message_id, sequence) - // blocks plain UPDATE on append-style parts. + // v1.13.20: parts-only. Same delete+insert dance as /answer — + // UNIQUE (message_id, sequence) blocks plain UPDATE on append-style + // parts. await tx`DELETE FROM message_parts WHERE message_id = ${toolMessageId} AND kind = 'tool_result'`; await tx` INSERT INTO message_parts (message_id, sequence, kind, payload) diff --git a/apps/server/src/routes/skills.ts b/apps/server/src/routes/skills.ts index 6f4802f..f8965c6 100644 --- a/apps/server/src/routes/skills.ts +++ b/apps/server/src/routes/skills.ts @@ -86,12 +86,12 @@ export function registerSkillsRoutes( const result = await sql.begin(async (tx) => { const [synthAssistant] = await tx<{ id: string }[]>` - INSERT INTO messages (session_id, chat_id, role, content, tool_calls, status, created_at) - VALUES (${sessionId}, ${chat.id}, 'assistant', '', ${sql.json(toolCalls as never)}, 'complete', clock_timestamp()) + INSERT INTO messages (session_id, chat_id, role, content, status, created_at) + VALUES (${sessionId}, ${chat.id}, 'assistant', '', 'complete', clock_timestamp()) RETURNING id `; - // v1.13.0: dual-write the synthetic assistant message's tool_call. - // Single skill_use tool_call, no text content, so one part at seq 0. + // v1.13.20: parts-only write. Single skill_use tool_call, no text + // content, so one part at seq 0. await tx` INSERT INTO message_parts (message_id, sequence, kind, payload) VALUES (${synthAssistant!.id}, 0, 'tool_call', ${tx.json({ @@ -101,11 +101,11 @@ export function registerSkillsRoutes( } as never)}) `; const [toolMsg] = await tx<{ id: string }[]>` - INSERT INTO messages (session_id, chat_id, role, content, tool_results, status, created_at) - VALUES (${sessionId}, ${chat.id}, 'tool', '', ${sql.json(toolResults as never)}, 'complete', clock_timestamp()) + INSERT INTO messages (session_id, chat_id, role, content, status, created_at) + VALUES (${sessionId}, ${chat.id}, 'tool', '', 'complete', clock_timestamp()) RETURNING id `; - // v1.13.0: dual-write the synthetic tool result (the skill body). + // v1.13.20: parts-only write of the synthetic tool result (skill body). await tx` INSERT INTO message_parts (message_id, sequence, kind, payload) VALUES (${toolMsg!.id}, 0, 'tool_result', ${tx.json(toolResults as never)}) diff --git a/apps/server/src/schema.sql b/apps/server/src/schema.sql index f490e2f..8a9070d 100644 --- a/apps/server/src/schema.sql +++ b/apps/server/src/schema.sql @@ -97,49 +97,42 @@ END $$; -- v1.13.1-B: read-path view. Read sites SELECT FROM messages_with_parts -- instead of messages so tool_calls / tool_results / reasoning_parts come --- from the granular message_parts table. The COALESCE means pre-v1.13.0 --- history (no parts rows) still resolves via the legacy JSON columns; the --- dual-write from v1.13.0 keeps both in sync for all rows written since. --- Writes continue to target `messages` directly — the view is read-only. --- Shapes match the in-memory ToolCall / ToolResult types: tool_calls is a --- jsonb array of {id, name, args}, tool_results is a single jsonb object --- {tool_call_id, output, truncated, error?}. reasoning_parts is new — only --- consumed by the inference history fetch (payload.ts) so v1.13.1-C can --- wire reasoning into the model payload. Not surfaced in external APIs yet. +-- from the granular message_parts table. +-- v1.13.20: post column-drop. The legacy COALESCE fallback over +-- messages.tool_calls / messages.tool_results was removed because those +-- columns no longer exist on the table (see the ALTER TABLE DROP COLUMN +-- statements below). Writes continue to target `messages` directly — the +-- view is read-only. Shapes match the in-memory ToolCall / ToolResult +-- types: tool_calls is a jsonb array of {id, name, args}, tool_results is +-- a single jsonb object {tool_call_id, output, truncated, error?}. +-- reasoning_parts is consumed by the inference history fetch (payload.ts) +-- for v1.13.1-C reasoning round-tripping. Not surfaced in external APIs. CREATE OR REPLACE VIEW messages_with_parts AS SELECT m.id, m.session_id, m.chat_id, m.role, m.content, m.kind, m.status, m.last_seq, m.tokens_used, m.ctx_used, m.ctx_max, m.started_at, m.finished_at, m.created_at, m.metadata, m.summary, m.tail_start_id, m.compacted_at, - -- v1.13.4: prune semantics need to distinguish "no parts row exists" - -- (pre-v1.13.0 fallback to legacy column) from "all parts hidden" - -- (prune intended — return null/empty so the row drops from the model - -- payload). A naive COALESCE would fall back to the legacy column when - -- every part is hidden, undoing the prune. CASE on EXISTS(any kind) - -- splits the two cases. - CASE - WHEN EXISTS (SELECT 1 FROM message_parts pp - WHERE pp.message_id = m.id AND pp.kind = 'tool_call') - THEN (SELECT jsonb_agg(p.payload ORDER BY p.sequence) - FROM message_parts p - WHERE p.message_id = m.id AND p.kind = 'tool_call' AND p.hidden_at IS NULL) - ELSE m.tool_calls - END AS tool_calls, - CASE - WHEN EXISTS (SELECT 1 FROM message_parts pp - WHERE pp.message_id = m.id AND pp.kind = 'tool_result') - THEN (SELECT p.payload - FROM message_parts p - WHERE p.message_id = m.id AND p.kind = 'tool_result' AND p.hidden_at IS NULL - ORDER BY p.sequence LIMIT 1) - ELSE m.tool_results - END AS tool_results, + (SELECT jsonb_agg(p.payload ORDER BY p.sequence) + FROM message_parts p + WHERE p.message_id = m.id AND p.kind = 'tool_call' AND p.hidden_at IS NULL) AS tool_calls, + (SELECT p.payload + FROM message_parts p + WHERE p.message_id = m.id AND p.kind = 'tool_result' AND p.hidden_at IS NULL + ORDER BY p.sequence LIMIT 1) AS tool_results, (SELECT jsonb_agg(p.payload ORDER BY p.sequence) FROM message_parts p WHERE p.message_id = m.id AND p.kind = 'reasoning' AND p.hidden_at IS NULL) AS reasoning_parts FROM messages m; +-- v1.13.20: drop legacy tool_calls/tool_results columns. Reads have routed +-- through messages_with_parts since v1.13.1-B; dual-writes removed in this +-- batch. The view above was simplified to remove COALESCE fallbacks before +-- this drop (Postgres rejects column-drop on view-referenced columns). +-- Idempotent via IF EXISTS. +ALTER TABLE messages DROP COLUMN IF EXISTS tool_calls; +ALTER TABLE messages DROP COLUMN IF EXISTS tool_results; + -- v1.13.10: per-tool token cost rolling window. Derives from -- messages_with_parts (the v1.13.1-B view that COALESCEs message_parts over -- the legacy JSON column) so this works whether the chat predates v1.13.0 @@ -290,19 +283,6 @@ BEGIN END IF; END $$; --- v1.12.1: drop stale inline CHECK constraints that were superseded by the --- named *_chk variants above. messages_status_check missed 'cancelled' and --- messages_role_check missed 'system' — both narrower than what's in use. -DO $$ -BEGIN - IF EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'messages_status_check') THEN - ALTER TABLE messages DROP CONSTRAINT messages_status_check; - END IF; - IF EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'messages_role_check') THEN - ALTER TABLE messages DROP CONSTRAINT messages_role_check; - END IF; -END $$; - -- v1.2-project-ux: projects.status + projects.gitea_remote -- KEEP IN SYNC: apps/server/src/types/api.ts PROJECT_STATUSES ALTER TABLE projects ADD COLUMN IF NOT EXISTS status TEXT NOT NULL DEFAULT 'open'; diff --git a/apps/server/src/services/__tests__/tool_cost_stats.test.ts b/apps/server/src/services/__tests__/tool_cost_stats.test.ts index 87ea7ab..8e34eea 100644 --- a/apps/server/src/services/__tests__/tool_cost_stats.test.ts +++ b/apps/server/src/services/__tests__/tool_cost_stats.test.ts @@ -78,16 +78,18 @@ describeFn('tool_cost_stats view (v1.13.10)', () => { args: {}, })); const created = opts.createdAt ?? new Date(); + // v1.13.20: parts-only. messages.tool_calls column was dropped; the + // tool_cost_stats view reads through messages_with_parts which derives + // tool_calls from message_parts rows. const rows = await sql<{ id: string }[]>` INSERT INTO messages ( session_id, chat_id, role, content, kind, status, - tool_calls, tokens_used, ctx_used, + tokens_used, ctx_used, metadata, created_at ) VALUES ( ${sessionId}, ${chatId}, 'assistant', '', 'message', ${opts.status ?? 'complete'}, - ${sql.json(toolCalls as never)}, ${opts.tokensUsed}, ${opts.ctxUsed}, ${opts.metadata ? sql.json(opts.metadata as never) : null}, @@ -95,7 +97,14 @@ describeFn('tool_cost_stats view (v1.13.10)', () => { ) RETURNING id `; - return rows[0]!.id; + const messageId = rows[0]!.id; + for (let i = 0; i < toolCalls.length; i++) { + await sql` + INSERT INTO message_parts (message_id, sequence, kind, payload) + VALUES (${messageId}, ${i}, 'tool_call', ${sql.json(toolCalls[i] as never)}) + `; + } + return messageId; } it('returns empty when no tool calls exist for a tool name', async () => { @@ -197,18 +206,17 @@ describeFn('tool_cost_stats view (v1.13.10)', () => { it('reads tool_calls via messages_with_parts (parts-authoritative)', async () => { const t = tname('parts'); - // Insert an assistant row with messages.tool_calls=NULL but a - // message_parts row carrying the tool_call. The view reads via - // messages_with_parts, which COALESCEs the parts table over the legacy - // column — so this row should still aggregate. + // v1.13.20: post-column-drop the only source for tool_calls is + // message_parts. This test asserts the same path the view always took + // (parts-derived), now that the legacy column COALESCE fallback is gone. const rows = await sql<{ id: string }[]>` INSERT INTO messages ( session_id, chat_id, role, content, kind, status, - tool_calls, tokens_used, ctx_used + tokens_used, ctx_used ) VALUES ( ${sessionId}, ${chatId}, 'assistant', '', 'message', 'complete', - NULL, 200, 5000 + 200, 5000 ) RETURNING id `; diff --git a/apps/server/src/services/inference/tool-phase.ts b/apps/server/src/services/inference/tool-phase.ts index de2ef19..6cd430c 100644 --- a/apps/server/src/services/inference/tool-phase.ts +++ b/apps/server/src/services/inference/tool-phase.ts @@ -110,7 +110,6 @@ export async function executeToolPhase( UPDATE messages SET content = ${content}, status = 'complete', - tool_calls = ${ctx.sql.json(toolCalls as never)}, tokens_used = ${completionTokens}, ctx_used = ${promptTokens}, ctx_max = ${nCtx}, @@ -118,15 +117,11 @@ export async function executeToolPhase( WHERE id = ${assistantMessageId} RETURNING tokens_used, ctx_used, ctx_max, finished_at `; - // v1.13.0: dual-write to message_parts. v1.13.1-B made parts authoritative - // for reads via the messages_with_parts view; the JSON column write above - // remains for v1.13.1 fallback compatibility (dropped in v1.13.2). + // v1.13.20: message_parts is the sole source of truth for tool_calls. + // Legacy messages.tool_calls column was dropped; reads route through the + // messages_with_parts view. // v1.13.1-C: include result.reasoning so models with separate reasoning // channels (qwen3.6) get a kind='reasoning' part at sequence 0. - // TODO(v1.13.1): wrap the UPDATE above and this insertParts in a single - // sql.begin before flipping read authority to message_parts. Without the - // transaction, a crash between the two leaves an orphan message that - // becomes invisible in the parts-authoritative read path. await insertParts( ctx.sql, partsFromAssistantMessage({ @@ -192,16 +187,9 @@ export async function executeToolPhase( if (tc.name === 'ask_user_input') { pausingForUserInput = true; const sentinel = { tool_call_id: tc.id, output: null, truncated: false }; - await ctx.sql` - UPDATE messages - SET tool_results = ${ctx.sql.json(sentinel as never)} - WHERE id = ${toolMessageId} - `; - // v1.13.0: mirror the pending sentinel into message_parts. The - // answer-endpoint UPDATE later (messages.ts:576) will delete and - // re-insert this part when the user submits their answer. - // TODO(v1.13.1): wrap the INSERT + UPDATE + insertParts triple in - // a per-iteration sql.begin before flipping read authority. + // v1.13.20: parts-only. The answer-endpoint UPDATE later + // (messages.ts) will delete and re-insert this part when the user + // submits their answer. await insertParts( ctx.sql, partsFromToolMessage({ tool_results: sentinel }).map((p) => ({ @@ -234,11 +222,7 @@ export async function executeToolPhase( output: `denied: ${resolution.reason}`, truncated: false, }; - await ctx.sql` - UPDATE messages - SET tool_results = ${ctx.sql.json(stored as never)} - WHERE id = ${toolMessageId} - `; + // v1.13.20: parts-only write. await insertParts( ctx.sql, partsFromToolMessage({ tool_results: stored }).map((p) => ({ @@ -261,11 +245,7 @@ export async function executeToolPhase( // (state may have changed in the meantime) so we don't stash it here. pausingForUserInput = true; const sentinel = { tool_call_id: tc.id, output: null, truncated: false }; - await ctx.sql` - UPDATE messages - SET tool_results = ${ctx.sql.json(sentinel as never)} - WHERE id = ${toolMessageId} - `; + // v1.13.20: parts-only write. await insertParts( ctx.sql, partsFromToolMessage({ tool_results: sentinel }).map((p) => ({ @@ -285,14 +265,7 @@ export async function executeToolPhase( truncated: tres.truncated, ...(tres.error ? { error: tres.error } : {}), }; - await ctx.sql` - UPDATE messages - SET tool_results = ${ctx.sql.json(stored as never)} - WHERE id = ${toolMessageId} - `; - // v1.13.0: dual-write the tool_result part. - // TODO(v1.13.1): wrap the INSERT + UPDATE + insertParts triple in a - // per-iteration sql.begin before flipping read authority. + // v1.13.20: parts-only write. Reads route through messages_with_parts. await insertParts( ctx.sql, partsFromToolMessage({ tool_results: stored }).map((p) => ({ diff --git a/boocode_roadmap.md b/boocode_roadmap.md index 8b8533f..ca3d24c 100644 --- a/boocode_roadmap.md +++ b/boocode_roadmap.md @@ -201,6 +201,8 @@ Both columns must read 0. After v1.13.2 ships, tag the umbrella `v1.13` on the same commit (or on -C — Sam's call). +**Shipped as `v1.13.20-drop-legacy-cols` on 2026-05-23 with umbrella `v1.13` tagged on the same commit.** Slug renamed at ship time (the "v1.13.2" planning name predated the patch-monotonic-per-minor convention). Calendar wait dropped — single-user self-hosted, no production rollback constraint. Recon caught 2 additional dual-write sites beyond the roadmap's 8 (chats.ts fork-clone + extras in tool-phase.ts) and an additional fixture file (`tool_cost_stats.test.ts`) with a direct legacy-column INSERT. Adversarial review caught a `RETURNING tool_calls, tool_results` clause in the `discard_stale` endpoint that the green test suite missed — fixed by two-step UPDATE-then-SELECT-from-view so the parts-synthesized fields keep flowing on the response. Type-pruning step on `Message.tool_calls` / `Message.tool_results` skipped (the view still populates them from parts; preserving the API contract was simpler than ripping it). + ----- ## v1.14 — Phase C: outer agent loop diff --git a/openspec/changes/v1.13.20-drop-legacy-cols/proposal.md b/openspec/changes/v1.13.20-drop-legacy-cols/proposal.md new file mode 100644 index 0000000..dce38cf --- /dev/null +++ b/openspec/changes/v1.13.20-drop-legacy-cols/proposal.md @@ -0,0 +1,126 @@ +# v1.13.20-drop-legacy-cols — drop messages.tool_calls + messages.tool_results + +Final phase of the v1.13.0 strangler-fig migration. Removes the dual-write into `messages.tool_calls` / `messages.tool_results` JSON columns and drops the columns themselves. After this batch, `message_parts` is the only source of truth for tool-call and tool-result data. + +Tag `v1.13` (umbrella) ships on the same commit per the original roadmap entry. + +## Why + +v1.13.0 (AI SDK v6 migration) introduced `message_parts` as the new canonical store for tool calls, tool results, reasoning, text, synthesis, and now html_artifact. To stay safe during the migration, every write site also dual-wrote to the legacy `messages.tool_calls` / `messages.tool_results` JSON columns, and `messages_with_parts` view COALESCEs over both. Reads have been migrated; dual-writes are pure overhead at this point. + +Verification query (per the original v1.13.2 plan) returns `0 / 0` orphan rows. Today's DB is also empty (0 messages on the live instance), so the COUNT query alone is weakly informative — the safety check shifts to a code-level audit: every dual-write site listed in the v1.13.2 roadmap entry must be located and its parts-write half kept, JSON-column half removed. + +## Scope + +### S1. Remove dual-write from every site + +Per the v1.13.2 roadmap entry, dual-writes live at: + +- `services/inference/tool-phase.ts` — 3 sites +- `services/inference/error-handler.ts` — `finalizeCompletion` +- `routes/skills.ts` — 2 sites +- `routes/messages.ts` — answer flow +- `routes/chats.ts` — fork flow + +Implementer must grep for every UPDATE / INSERT that touches `tool_calls` or `tool_results` columns and verify it has a paired `insertParts(...)` call. Keep the parts write, remove the column write. If a site only writes to the JSON column with no parts pair — STOP and escalate (would indicate a bug in the v1.13.0 dual-write rollout we haven't caught). + +### S2. Simplify `messages_with_parts` view + +Current view COALESCEs parts-table rows over legacy JSON columns to support pre-v1.13.0 history. After this batch, the JSON columns no longer exist — drop the COALESCE fallbacks. The view should read only from `message_parts` joined to `messages`. + +### S3. Drop the columns + +```sql +ALTER TABLE messages DROP COLUMN tool_calls; +ALTER TABLE messages DROP COLUMN tool_results; +``` + +Idempotent via `IF EXISTS`. Apply unconditionally on startup (matches the rest of `schema.sql`'s shape). + +### S4. Remove from API types + +`Message` interface in `apps/server/src/types/api.ts` AND `apps/web/src/api/types.ts` — drop `tool_calls?` and `tool_results?` fields. The API boundary is unchanged because every consumer already reads parts-derived values through `messages_with_parts`. Mirror byte-for-byte. + +### S5. Drop the stale `messages_status_check` cleanup DO block from v1.12.1 if still present + +Per the v1.13.2 roadmap entry, there's a v1.12.1 `DO $$ DROP CONSTRAINT messages_status_check` block that was meant to clean up the old anonymous constraint. If still present in `schema.sql`, remove — it's been one-shot effective. + +### S6. Update test fixtures + +`inference.test.ts` and `compaction.test.ts` (and any other test file the grep finds) construct Message-shaped fixtures with `tool_calls: null, tool_results: null` literals. Rewrite ~30 fixtures to construct via `message_parts` rows where the test actually exercises tool calls. For tests that don't exercise tool calls at all, just drop the now-absent fields. + +`partsFromAssistantMessage` and `partsFromToolMessage` helpers in `parts.ts` currently take `tool_calls` and `tool_results` as args (because that's what the legacy Message shape carried). Keep their input shapes — they're useful constructors. The change is at the call sites, not the helpers. + +## Non-goals + +- **No changes to `message_parts` schema.** It's correct as-is. +- **No changes to the `messages_with_parts` view name or interface.** Just the implementation simplifies. +- **No removal of `partsFromAssistantMessage` / `partsFromToolMessage`.** They're useful as constructors; their job becomes producing parts from raw ToolCall/ToolResult objects, not from a legacy Message row. +- **No frontend changes beyond the type mirror.** Web reads parts via `messages_with_parts` already. +- **No reads from the legacy columns in any code path.** Verify with grep. + +## Hard rules + +- No git commits during dispatch. Sam commits manually (handled by controller after all dispatches done). +- Backups: every modified file → `.bak-v1.13.20-20260523`. +- TS strict, no `any`. +- No new deps. +- Schema migration: additive-or-destructive but idempotent (`IF EXISTS` on the column drops). +- Run the full server test suite after — must be green. +- Frontend: `tsc -p apps/web/tsconfig.app.json --noEmit` + `pnpm -C apps/web build` clean. + +## Stop checkpoints + +1. **After recon** (grep-driven inventory of dual-write call sites + read sites still touching the legacy columns): stop, hand back inventory. The roadmap listed 7+ sites; verify nothing's been missed. +2. **After code edits, before schema migration**: stop, hand back diff + test results. Confirm the parts write at every former dual-write site still happens. +3. **After schema migration applies in dev**: stop, run tests, run a fresh `applySchema()` cycle (boot twice), confirm idempotent. + +## Smoke plan + +1. **Fresh boot.** Restart the boocode container, confirm `applySchema()` completes without error. +2. **Idempotent boot.** Restart again, confirm no error on the second pass (column DROP IF EXISTS is a no-op). +3. **Send a chat that triggers a tool call.** Confirm: + - Assistant message lands with content + reasoning + tool_call parts (all in `message_parts`). + - Tool result lands as a `tool_result` part. + - `messages_with_parts` returns the same shape the frontend expects (verify by reading the live chat in the UI). +4. **DB inspection.** `\d messages` — confirm `tool_calls` and `tool_results` columns are gone. +5. **Compaction roundtrip.** Trigger a compaction-eligible turn (long context); confirm the rolling summary still anchors correctly and uses parts as input. + +## Done when + +- All dual-write sites converted to parts-only writes. +- View simplified, columns dropped, types updated. +- Test suite green. +- Frontend typecheck + build clean. +- Smoke green. +- Tagged `v1.13.20-drop-legacy-cols` AND the umbrella `v1.13` on the same commit. +- CHANGELOG.md entry + roadmap retrospective bullet. + +## Files expected to touch + +**Backend:** +- `apps/server/src/schema.sql` — DROP columns + simplify view + remove v1.12.1 cleanup block +- `apps/server/src/services/inference/tool-phase.ts` — remove 3 dual-write sites +- `apps/server/src/services/inference/error-handler.ts` — remove dual-write in `finalizeCompletion` +- `apps/server/src/routes/skills.ts` — remove 2 dual-write sites +- `apps/server/src/routes/messages.ts` — remove dual-write in answer flow +- `apps/server/src/routes/chats.ts` — remove dual-write in fork +- `apps/server/src/types/api.ts` — drop `tool_calls?` / `tool_results?` from Message +- `apps/server/src/services/__tests__/inference.test.ts` — fixture rewrites +- `apps/server/src/services/__tests__/compaction.test.ts` — fixture rewrites +- `apps/server/src/services/__tests__/parts.test.ts` — likely some fixture updates +- `apps/server/src/services/__tests__/tool_cost_stats.test.ts` — likely some fixture updates +- `apps/server/src/services/__tests__/system-prompt.test.ts` — likely some fixture updates + +**Frontend:** +- `apps/web/src/api/types.ts` — mirror Message change + +**Docs:** +- `BOOCHAT.md` — no change expected (rules don't mention the legacy columns) +- `boocode_roadmap.md` — retrospective bullet +- `CHANGELOG.md` — new section +- `CLAUDE.md` — drop the v1.13.0 dual-write notes that no longer apply (audit the surrounding paragraphs) + +## Estimate + +~150 LoC net (mostly deletions). Mechanical work — same per-batch shape as v1.13.18. diff --git a/openspec/changes/v1.13.20-drop-legacy-cols/tasks.md b/openspec/changes/v1.13.20-drop-legacy-cols/tasks.md new file mode 100644 index 0000000..93712cc --- /dev/null +++ b/openspec/changes/v1.13.20-drop-legacy-cols/tasks.md @@ -0,0 +1,104 @@ +# v1.13.20-drop-legacy-cols tasks + +## B1 — Recon (STOP after this step) + +- [ ] Grep `apps/server/src/**/*.ts` for every `tool_calls` and `tool_results` mention. Categorize each hit as: + - **dual-write** (an UPDATE / INSERT that writes the JSON column) + - **read** (a SELECT that reads the JSON column, or code that destructures it from a row) + - **type-only** (interface / type field reference) + - **test fixture** (literal in a test file) + - **comment / docs** +- [ ] Confirm the v1.13.2 roadmap inventory is complete: + - tool-phase.ts: 3 sites + - error-handler.ts (`finalizeCompletion`): 1 site + - routes/skills.ts: 2 sites + - routes/messages.ts (answer flow): 1 site + - routes/chats.ts (fork): 1 site + - Any extras the grep finds: list them +- [ ] Confirm no READ sites still touching the legacy columns (everything should go through `messages_with_parts`). If reads remain, flag them — they need to migrate to the view BEFORE dropping the columns. +- [ ] Hand back inventory as a per-file table: file, line, kind (dual-write / read / type / fixture), action (delete / migrate-to-view / type-prune). + +## B2 — Backups + +- [ ] `cp <file> <file>.bak-v1.13.20-20260523` for every file in B1's action list before editing. + +## B3 — Remove dual-writes + +- [ ] Remove the JSON-column UPDATE / INSERT at every site identified in B1 as a dual-write. Keep the paired `insertParts(...)` call. +- [ ] If a site only writes the JSON column with no parts pair (would indicate a bug from v1.13.0) — STOP, report as BLOCKED. +- [ ] Verify by grep: zero remaining writes to `tool_calls` or `tool_results` outside of `schema.sql` and test fixtures. + +## B4 — Simplify `messages_with_parts` view + +- [ ] Open `schema.sql`. Find the view definition. +- [ ] Drop the COALESCE fallbacks that read `m.tool_calls` / `m.tool_results` from `messages`. +- [ ] View now reads only from `message_parts` joined to `messages`. +- [ ] Confirm view's output column shapes are unchanged: `tool_calls jsonb[]`, `tool_results jsonb` single object, `reasoning_parts jsonb[]`. + +## B5 — Drop columns + +- [ ] `ALTER TABLE messages DROP COLUMN IF EXISTS tool_calls;` +- [ ] `ALTER TABLE messages DROP COLUMN IF EXISTS tool_results;` +- [ ] Idempotent on re-run. +- [ ] Apply order in `schema.sql`: AFTER the view is updated (view depends on the columns; can't drop a column referenced by a view). +- [ ] Actually verify the order — if the view references the columns, you must drop the view first OR change it before the ALTER. + +## B6 — Remove v1.12.1 cleanup block + +- [ ] Find the `DO $$ DROP CONSTRAINT messages_status_check` block in `schema.sql` (likely near the messages CHECK constraints). +- [ ] Confirm it's safe to remove (the constraint should have been dropped long ago). +- [ ] Delete the block. + +## B7 — Type pruning + +- [ ] `apps/server/src/types/api.ts` — remove `tool_calls?` and `tool_results?` from the `Message` interface. +- [ ] `apps/web/src/api/types.ts` — mirror byte-for-byte. +- [ ] Search for any other type references — `ToolCallsField`, `ToolResultsField`, etc. + +## B8 — Test fixture updates + +- [ ] Run `pnpm -C apps/server test` to see what breaks. +- [ ] For each failing test that constructs a `Message` literal with `tool_calls: null` / `tool_results: null` — remove those fields. +- [ ] For tests that exercised tool-call behavior via the legacy columns, rewrite to construct via `message_parts` rows. +- [ ] Confirm: `pnpm -C apps/server test` — all green. + +## B9 — Type / build verification + +- [ ] `npx tsc --noEmit -p apps/server` — 0 errors. +- [ ] `npx tsc -p apps/web/tsconfig.app.json --noEmit` — 0 errors. +- [ ] `pnpm -C apps/web build` — green. + +## B10 — STOP checkpoint, hand back diff + +- [ ] Hand controller the diff for backend changes + test results. + +## B11 — Schema deploy + +- [ ] `docker compose up --build -d` rebuilds with new schema. +- [ ] Boot twice in sequence — confirm idempotent (column DROP IF EXISTS is a no-op on the second boot). +- [ ] `docker exec boocode_db psql -U boocode -d boocode -c "\d messages"` — confirm columns absent. +- [ ] `docker logs boocode 2>&1 | tail -50` — confirm no schema errors. + +## B12 — Smoke + +- [ ] Live-smoke: send a chat that triggers at least one tool call. Confirm: + - [ ] Assistant message renders with content + tool_call ActionRow. + - [ ] Tool result renders. + - [ ] No console errors in browser or `docker logs boocode`. +- [ ] Trigger a compaction-eligible turn (long context). Confirm rolling summary anchors correctly. + +## B13 — Docs + +- [ ] `CHANGELOG.md` entry for v1.13.20-drop-legacy-cols. +- [ ] `boocode_roadmap.md` retrospective bullet on the v1.13.2 section (note the slug rename and ship date). +- [ ] `CLAUDE.md` — drop the v1.13.0 dual-write notes that no longer apply. Audit the surrounding paragraphs. + +## B14 — Tag + push + rebuild + +- [ ] `git add` only the v1.13.20 batch files (per CLAUDE.md convention). +- [ ] `git commit` with HEREDOC commit message. +- [ ] `git tag v1.13.20-drop-legacy-cols` AND `git tag v1.13` (umbrella, per original v1.13.2 plan). +- [ ] Push: `GIT_SSH_COMMAND="ssh -i /opt/boocode/secrets/boocode_gitea -o IdentitiesOnly=yes" git push origin main` +- [ ] Push both tags. +- [ ] `docker compose up --build -d`. +- [ ] Curl health check.