-
Notifications
You must be signed in to change notification settings - Fork 4
Surface thread title regeneration failures instead of silently completing #198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
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 |
|---|---|---|
|
|
@@ -92,6 +92,7 @@ const HANDLED_TURN_START_KEY_TTL = Duration.minutes(30); | |
| const DEFAULT_RUNTIME_MODE: RuntimeMode = "full-access"; | ||
| const DEFAULT_THREAD_TITLE = "New thread"; | ||
| const MAX_REGENERATION_ATTACHMENTS = 4; | ||
| const TITLE_REGENERATION_FALLBACK_ERROR = "Title generation failed unexpectedly."; | ||
| const MAX_THREAD_TITLE_CONTEXT_CHARS = 8_000; | ||
| const THREAD_TITLE_CONTEXT_TRUNCATION_MARKER = "[Earlier content truncated]\n\n"; | ||
|
|
||
|
|
@@ -890,13 +891,15 @@ const make = Effect.gen(function* () { | |
| readonly threadId: ThreadId; | ||
| readonly requestId: CommandId; | ||
| readonly title?: string; | ||
| readonly error?: string; | ||
| }) { | ||
| yield* orchestrationEngine.dispatch({ | ||
| type: "thread.title.regeneration.complete", | ||
| commandId: yield* serverCommandId("thread-title-regeneration-complete"), | ||
| threadId: input.threadId, | ||
| requestId: input.requestId, | ||
| ...(input.title !== undefined ? { title: input.title } : {}), | ||
| ...(input.error !== undefined ? { error: input.error } : {}), | ||
| }); | ||
| }); | ||
| const clearInterruptedThreadTitleRegenerations = Effect.fn( | ||
|
|
@@ -941,15 +944,32 @@ const make = Effect.gen(function* () { | |
| if (requestId === null) { | ||
| return; | ||
| } | ||
| // Generation failures are carried into the completion instead of being | ||
| // swallowed: without them the request completes "successfully" with no | ||
| // title and the user sees a spinner clear with nothing changed. Providers | ||
| // whose text generation is unimplemented fail this way every time. | ||
| const result = yield* regenerateThreadTitle(event, requestId).pipe( | ||
| Effect.catchTag("TextGenerationError", (error) => | ||
| Effect.logWarning("provider command reactor failed to regenerate thread title", { | ||
| threadId: event.payload.threadId, | ||
| detail: error.detail, | ||
| }).pipe( | ||
| Effect.as({ | ||
| _tag: "Failed", | ||
| // The completion payload requires a non-empty reason; a provider | ||
| // that fails without one still has to produce a visible error. | ||
| error: error.detail.trim() || TITLE_REGENERATION_FALLBACK_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 a provider exits with verbose output, this stores the entire AGENTS.md reference: AGENTS.md:L56-L62 Useful? React with 👍 / 👎. |
||
| } as const), | ||
| ), | ||
| ), | ||
| Effect.catchCause((cause) => { | ||
| if (Cause.hasInterruptsOnly(cause)) { | ||
| return Effect.failCause(cause); | ||
| } | ||
| return Effect.logWarning("provider command reactor failed to regenerate thread title", { | ||
| threadId: event.payload.threadId, | ||
| cause: Cause.pretty(cause), | ||
| }).pipe(Effect.as({ _tag: "Completed", title: undefined } as const)); | ||
| }).pipe(Effect.as({ _tag: "Failed", error: TITLE_REGENERATION_FALLBACK_ERROR } as const)); | ||
| }), | ||
| ); | ||
| if (result._tag === "Superseded") { | ||
|
|
@@ -959,7 +979,10 @@ const make = Effect.gen(function* () { | |
| const completion = { | ||
| threadId: event.payload.threadId, | ||
| requestId, | ||
| ...(result.title !== undefined ? { title: result.title } : {}), | ||
| ...(result._tag === "Completed" && result.title !== undefined | ||
| ? { title: result.title } | ||
| : {}), | ||
| ...(result._tag === "Failed" ? { error: result.error } : {}), | ||
| }; | ||
| yield* dispatchThreadTitleRegenerationCompletion(completion).pipe( | ||
| Effect.catchCause((cause) => { | ||
|
|
||
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
ProjectionSnapshotQuery.test.tsruns, bothgetSnapshot()andgetShellSnapshot()now returntitleRegenerationFailure: nullbecause this mapping always materializes the field, but the exactassert.deepEqualexpectations beginning at lines 283 and 398 omit it. Those existing backend tests therefore fail even for threads without a regeneration failure; update the expected fixtures and cover the non-null projection path.AGENTS.md reference: AGENTS.md:L29-L33
Useful? React with 👍 / 👎.