Repository navigation
Surface thread title regeneration failures instead of silently completing #198
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -683,8 +683,16 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" | |
| command, | ||
| threadId: command.threadId, | ||
| }); | ||
| const requestIsCurrent = thread.titleRegeneration?.requestId === command.requestId; | ||
| const pending = thread.titleRegeneration; | ||
| const requestIsCurrent = pending?.requestId === command.requestId; | ||
| const occurredAt = yield* nowIso; | ||
| // A failed completion keeps the record and stamps the reason instead of | ||
| // clearing it, so the sidebar can say why the title did not change. A | ||
| // successful (or no-op) completion clears it back to "not regenerating". | ||
| const nextTitleRegeneration = | ||
| command.error !== undefined && pending != null | ||
| ? { requestId: pending.requestId, startedAt: pending.startedAt, error: command.error } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an upgraded server reports a generation failure to an older web or desktop client, retaining a non-null Useful? React with 👍 / 👎. |
||
| : null; | ||
| return { | ||
| ...(yield* withEventBase({ | ||
| aggregateKind: "thread", | ||
|
|
@@ -696,7 +704,7 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand" | |
| payload: { | ||
| threadId: command.threadId, | ||
| ...(requestIsCurrent && command.title !== undefined ? { title: command.title } : {}), | ||
| ...(requestIsCurrent ? { titleRegeneration: null } : {}), | ||
| ...(requestIsCurrent ? { titleRegeneration: nextTitleRegeneration } : {}), | ||
| updatedAt: requestIsCurrent ? occurredAt : thread.updatedAt, | ||
| }, | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { assert, it } from "@effect/vitest"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as Layer from "effect/Layer"; | ||
| import * as SqlClient from "effect/unstable/sql/SqlClient"; | ||
|
|
||
| import { runMigrations } from "../Migrations.ts"; | ||
| import * as NodeSqliteClient from "../NodeSqliteClient.ts"; | ||
|
|
||
| const layer = it.layer(Layer.mergeAll(NodeSqliteClient.layerMemory())); | ||
|
|
||
| layer("036_ProjectionThreadTitleRegenerationError", (it) => { | ||
| it.effect("adds the title regeneration error column", () => | ||
| Effect.gen(function* () { | ||
| const sql = yield* SqlClient.SqlClient; | ||
|
|
||
| yield* runMigrations({ toMigrationInclusive: 38 }); | ||
| yield* runMigrations({ toMigrationInclusive: 39 }); | ||
|
|
||
| const columns = yield* sql<{ readonly name: string }>` | ||
| PRAGMA table_info(projection_threads) | ||
| `; | ||
| const names = new Set(columns.map((column) => column.name)); | ||
| assert.ok(names.has("title_regeneration_error")); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("is idempotent when the column already exists", () => | ||
| Effect.gen(function* () { | ||
| yield* runMigrations({ toMigrationInclusive: 39 }); | ||
| yield* runMigrations({ toMigrationInclusive: 39 }); | ||
|
|
||
| const sql = yield* SqlClient.SqlClient; | ||
| const columns = yield* sql<{ readonly name: string }>` | ||
| PRAGMA table_info(projection_threads) | ||
| `; | ||
| assert.strictEqual( | ||
| columns.filter((column) => column.name === "title_regeneration_error").length, | ||
| 1, | ||
| ); | ||
| }), | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import * as Effect from "effect/Effect"; | ||
| import * as SqlClient from "effect/unstable/sql/SqlClient"; | ||
|
|
||
| export default Effect.gen(function* () { | ||
| const sql = yield* SqlClient.SqlClient; | ||
| const columns = yield* sql<{ readonly name: string }>` | ||
| PRAGMA table_info(projection_threads) | ||
| `; | ||
|
|
||
| if (!columns.some((column) => column.name === "title_regeneration_error")) { | ||
| yield* sql` | ||
| ALTER TABLE projection_threads | ||
| ADD COLUMN title_regeneration_error TEXT | ||
| `; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a provider exits with verbose output, this stores the entire
TextGenerationError.detailwithout a size limit. For example,CodexTextGeneration.ts:239-248builds the detail from the complete stderr/stdout stream; the value is then persisted in the event and projection and included in every shell snapshot until the failure is cleared, so one noisy failure can permanently inflate SQLite rows and WebSocket payloads. Normalize or truncate the user-facing reason before dispatching the completion.AGENTS.md reference: AGENTS.md:L56-L62
Useful? React with 👍 / 👎.