Repository navigation
fix(session): retry a failed session title on later turns and report why it failed #1688
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
b2d6a80
58b4320
966518b
ec8f74a
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 |
|---|---|---|
|
|
@@ -4863,6 +4863,42 @@ NOTE: At any point in time through this workflow you should feel free to ask the | |
| return result | ||
| } | ||
|
|
||
| // altimate_change start — how many user messages of a session may each trigger a title request | ||
| // while the session still carries its placeholder title. | ||
| const TITLE_MAX_ATTEMPTS = 5 | ||
|
Collaborator
Author
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. Minor: the PR's cost statement undercounts. "At most four extra small-model requests" counts Fix: correct the "Cost side effects" section of the PR description. Does not block: the cost is still small, and it is bounded per compaction cycle. |
||
| /** Sessions with a title request in flight: one at a time, so a slow request and the next | ||
| * turn's retry cannot both set the title. */ | ||
| const titleInFlight = new Set<string>() | ||
| /** Name, message and HTTP status of a title failure. Covers errors that are not Error instances | ||
| * (openai-compatible in-band errors are strings, Anthropic's are plain objects) and the SDK's | ||
| * RetryError, which carries the status on its last attempt rather than on itself. */ | ||
| function describeTitleError(err: unknown): { name: string; message: string; status?: number } { | ||
| const statusOf = (value: unknown) => { | ||
| const o = value as { statusCode?: unknown; lastError?: { statusCode?: unknown } } | undefined | ||
| const code = o?.statusCode ?? o?.lastError?.statusCode | ||
| return typeof code === "number" ? code : undefined | ||
| } | ||
| // The name becomes a telemetry dimension: an identifier-shaped token or nothing. | ||
| const safeName = (value: unknown, fallback: string) => | ||
| typeof value === "string" && /^[A-Za-z0-9_.:-]{1,64}$/.test(value) ? value : fallback | ||
| if (err instanceof Error) return { name: safeName(err.name, "Error"), message: err.message, status: statusOf(err) } | ||
| if (err && typeof err === "object") { | ||
| const o = err as Record<string, unknown> | ||
| const name = safeName(o.name, safeName(o.type, "object")) | ||
| let message = typeof o.message === "string" ? o.message : "" | ||
| if (!message) { | ||
| try { | ||
| message = JSON.stringify(err).slice(0, 500) | ||
| } catch { | ||
| message = "[object]" | ||
| } | ||
| } | ||
| return { name, message, status: statusOf(err) } | ||
| } | ||
| return { name: typeof err, message: String(err) } | ||
| } | ||
| // altimate_change end | ||
|
|
||
| async function ensureTitle(input: { | ||
| session: Session.Info | ||
| history: MessageV2.WithParts[] | ||
|
|
@@ -4878,10 +4914,16 @@ NOTE: At any point in time through this workflow you should feel free to ask the | |
| ) | ||
| if (firstRealUserIdx === -1) return | ||
|
|
||
| const isFirst = | ||
| input.history.filter((m) => m.info.role === "user" && !m.parts.every((p) => "synthetic" in p && p.synthetic)) | ||
| .length === 1 | ||
| if (!isFirst) return | ||
| // altimate_change start — retry while the placeholder stands. Upstream requested a title only | ||
| // on the first user message, so one failed small-model call left `New session - <time>` for the | ||
| // rest of the session (field report, altimate-code 0.12.8). Later turns retry with the same | ||
| // first-message context, bounded so a permanently broken small model does not cost a failed | ||
| // request on every turn. | ||
| const attempt = input.history.filter( | ||
|
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. P2: The five-attempt bound and first-message context are not stable across compaction because Prompt for AI agents
Collaborator
Author
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. Minor:
Fix: record title attempts on their own (for example per session), rather than deriving them from visible history, and exclude compaction parts when finding the first message. Does not block: edge input. It needs the placeholder to survive until compaction or heavy queueing, and the worst outcome is a late or odd title, or the placeholder that base would have kept anyway. |
||
| (m) => m.info.role === "user" && !m.parts.every((p) => "synthetic" in p && p.synthetic), | ||
| ).length | ||
| if (attempt > TITLE_MAX_ATTEMPTS) return | ||
|
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. P2: Queued later turns still do not retry title generation because the caller invokes Prompt for AI agents
Comment on lines
+4922
to
+4925
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.
|
||
| // altimate_change end | ||
|
|
||
| // Gather all messages up to and including the first real user message for context | ||
| // This includes any shell/subtask executions that preceded the user's first prompt | ||
|
|
@@ -4895,56 +4937,103 @@ NOTE: At any point in time through this workflow you should feel free to ask the | |
|
|
||
| const agent = await Agent.get("title") | ||
| if (!agent) return | ||
| const model = await iife(async () => { | ||
| // altimate_change start — agent.model carries core ProviderV2.ID/ModelV2.ID brands; re-brand to fork ProviderID/ModelID (identity at runtime) | ||
| if (agent.model) | ||
| return await Provider.getModel(ProviderID.make(agent.model.providerID), ModelID.make(agent.model.modelID)) | ||
| // altimate_change end | ||
| return ( | ||
| (await Provider.getSmallModel(input.providerID)) ?? (await Provider.getModel(input.providerID, input.modelID)) | ||
| ) | ||
| }) | ||
| const result = await LLM.stream({ | ||
| agent, | ||
| user: firstRealUser.info as MessageV2.User, | ||
| system: [], | ||
| small: true, | ||
| tools: {}, | ||
| // altimate_change start — title generation is toolless, but without an explicit "none" the | ||
| // historical-tool-stub injection in LLM.stream repopulates `tools` from any tool parts in | ||
| // the context, which both re-declares tools this request cannot use and suppresses the | ||
| // toolless message flattening. compaction.ts passes the same flag for the same reason. | ||
| toolChoice: "none" as const, | ||
| // altimate_change end | ||
| model, | ||
| abort: new AbortController().signal, | ||
| sessionID: input.session.id, | ||
| retries: 2, | ||
| messages: [ | ||
| { | ||
| role: "user", | ||
| content: "Generate a title for this conversation:\n", | ||
| }, | ||
| ...(hasOnlySubtaskParts | ||
| ? [{ role: "user" as const, content: subtaskParts.map((p) => p.prompt).join("\n") }] | ||
| : await MessageV2.toModelMessages(contextMessages, model)), | ||
| ], | ||
| }) | ||
| const text = await Promise.resolve(result.text).catch((err: unknown) => { | ||
| log.error("failed to generate title", { error: err }) | ||
| return undefined | ||
| }) | ||
| if (text) { | ||
| const cleaned = text | ||
| .replace(/<think>[\s\S]*?<\/think>\s*/g, "") | ||
| .split("\n") | ||
| .map((line: string) => line.trim()) | ||
| .find((line: string) => line.length > 0) | ||
| if (!cleaned) return | ||
|
|
||
| const title = cleaned.length > 100 ? cleaned.substring(0, 97) + "..." : cleaned | ||
| return Session.setTitle({ sessionID: input.session.id, title }) | ||
| // altimate_change start — read the stream itself. `result.text` rejects with the SDK's generic | ||
| // "No output generated. Check the stream for errors." and the provider's own error reaches only | ||
| // the stream's error part, so the log never said why a title failed. The model lookup sits in | ||
| // the same try: a `small_model` the provider does not list used to throw out of this | ||
| // fire-and-forget call with no log line and no event. Record the real error and report it, so | ||
| // the next field report is answerable from App Insights. | ||
| if (titleInFlight.has(input.session.id)) return | ||
| titleInFlight.add(input.session.id) | ||
| let text = "" | ||
| let failure: unknown | ||
| let model: Provider.Model | undefined | ||
| try { | ||
|
Collaborator
Author
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. Major: failures before the stream skip the new reporting. This
Base has the same gap. It blocks here because the PR says the cause will be read from the next report's telemetry, and the new paragraph in Fix: move the lookup and |
||
| model = await iife(async () => { | ||
| // agent.model carries core ProviderV2.ID/ModelV2.ID brands; re-brand to fork ProviderID/ModelID (identity at runtime) | ||
| if (agent.model) | ||
| return await Provider.getModel(ProviderID.make(agent.model.providerID), ModelID.make(agent.model.modelID)) | ||
| return ( | ||
| (await Provider.getSmallModel(input.providerID)) ?? | ||
| (await Provider.getModel(input.providerID, input.modelID)) | ||
| ) | ||
| }) | ||
| const result = await LLM.stream({ | ||
| agent, | ||
| user: firstRealUser.info as MessageV2.User, | ||
| system: [], | ||
| small: true, | ||
| tools: {}, | ||
| // title generation is toolless, but without an explicit "none" the historical-tool-stub | ||
| // injection in LLM.stream repopulates `tools` from any tool parts in the context, which both | ||
| // re-declares tools this request cannot use and suppresses the toolless message flattening. | ||
| // compaction.ts passes the same flag for the same reason. | ||
| toolChoice: "none" as const, | ||
| model, | ||
| abort: new AbortController().signal, | ||
| sessionID: input.session.id, | ||
| retries: 2, | ||
| messages: [ | ||
| { | ||
| role: "user", | ||
| content: "Generate a title for this conversation:\n", | ||
| }, | ||
| ...(hasOnlySubtaskParts | ||
| ? [{ role: "user" as const, content: subtaskParts.map((p) => p.prompt).join("\n") }] | ||
| : await MessageV2.toModelMessages(contextMessages, model)), | ||
| ], | ||
| }) | ||
| for await (const part of result.fullStream) { | ||
| if (part.type === "text-delta") text += part.text | ||
| else if (part.type === "error") failure = part.error | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| } catch (err) { | ||
| failure = err | ||
| } finally { | ||
| titleInFlight.delete(input.session.id) | ||
| } | ||
| const cleaned = text | ||
| .replace(/<think>[\s\S]*?<\/think>\s*/g, "") | ||
| .split("\n") | ||
| .map((line: string) => line.trim()) | ||
| .find((line: string) => line.length > 0) | ||
| // An answer with no usable line (empty, or thinking only) is a failed attempt too, so it is | ||
| // reported rather than silently using up one of the five. | ||
| if (failure !== undefined || !cleaned) { | ||
| const err = failure ?? new Error(text.trim() ? "the model returned no title line" : "the model returned no text") | ||
| const described = describeTitleError(err) | ||
| const providerID = model?.providerID ?? input.providerID | ||
| const modelID = model?.id ?? input.modelID | ||
| // Name, message and status only: a provider error object can carry the request body. | ||
| log.error("failed to generate title", { | ||
| errorType: described.name, | ||
| message: described.message.slice(0, 500), | ||
|
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. P2: This still logs unsanitized provider error text: for a plain error object without Prompt for AI agents |
||
| status: described.status, | ||
| attempt, | ||
| providerID, | ||
| modelID, | ||
| }) | ||
| Telemetry.track({ | ||
| type: "title_generation_failed", | ||
| timestamp: Date.now(), | ||
| session_id: input.session.id, | ||
| provider_id: providerID, | ||
| model_id: modelID, | ||
| attempt, | ||
| error_type: described.name, | ||
| error_class: Telemetry.classifyError(described.message), | ||
| ...(described.status !== undefined ? { http_status: described.status } : {}), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| const title = cleaned.length > 100 ? cleaned.substring(0, 97) + "..." : cleaned | ||
| // The user may have named the session while the request was in flight: a generated title only | ||
| // ever replaces the placeholder, and when that cannot be checked the title is left for a retry. | ||
| const current = await Session.get(input.session.id).catch(() => undefined) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| if (!current || !Session.isDefaultTitle(current.title)) return | ||
| // altimate_change end | ||
| return Session.setTitle({ sessionID: input.session.id, title }) | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
|
||
| } | ||
|
|
||
| // altimate_change start — Effect Context.Service facade over the existing Promise namespace. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -616,6 +616,8 @@ namespace TestLLMServer { | |
| readonly url: string | ||
| readonly push: (...input: (Item | Reply)[]) => Effect.Effect<void> | ||
| readonly pushMatch: (match: Match, ...input: (Item | Reply)[]) => Effect.Effect<void> | ||
| /** Answers for the next title requests, in order; the automatic "E2E Title" resumes after them. */ | ||
| readonly title: (...input: (Item | Reply)[]) => Effect.Effect<void> | ||
| readonly textMatch: (match: Match, value: string, opts?: { usage?: Usage }) => Effect.Effect<void> | ||
| readonly toolMatch: (match: Match, name: string, input: unknown) => Effect.Effect<void> | ||
| readonly text: (value: string, opts?: { usage?: Usage }) => Effect.Effect<void> | ||
|
|
@@ -648,6 +650,7 @@ export class TestLLMServer extends Context.Service<TestLLMServer, TestLLMServer. | |
|
|
||
| let hits: Hit[] = [] | ||
| let list: Queue[] = [] | ||
| let titles: Item[] = [] // altimate_change — answers for title requests, see handle() | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
Collaborator
Author
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. Minor: Fix: add Does not block: test-only, and no current test combines |
||
| let waits: Wait[] = [] | ||
| let misses: Hit[] = [] | ||
|
|
||
|
|
@@ -679,8 +682,22 @@ export class TestLLMServer extends Context.Service<TestLLMServer, TestLLMServer. | |
| const body = yield* req.json.pipe(Effect.orElseSucceed(() => ({}))) | ||
| const current = hit(req.originalUrl, body, { ...req.headers }) | ||
| if (isTitleRequest(body)) { | ||
| // altimate_change start — a queued title item answers the request instead of the automatic | ||
| // "E2E Title", so a test can make title generation fail. Kept apart from the main queue: | ||
| // a title request carries the first user message, so body matchers would catch it too. | ||
| const queued = titles.shift() | ||
| hits = [...hits, current] | ||
| yield* notify() | ||
| if (queued) { | ||
| if (queued.type !== "sse") return fail(queued) | ||
| if (mode === "responses") return send(responses(queued, modelFrom(body))) | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
|
||
| if (queued.reset) { | ||
| yield* reset(queued) | ||
| return HttpServerResponse.empty() | ||
| } | ||
| return send(queued) | ||
| } | ||
| // altimate_change end | ||
| const auto: Sse = { type: "sse", head: [role()], tail: [textLine("E2E Title"), finishLine("stop")] } | ||
| if (mode === "responses") return send(responses(auto, modelFrom(body))) | ||
| return send(auto) | ||
|
|
@@ -720,6 +737,12 @@ export class TestLLMServer extends Context.Service<TestLLMServer, TestLLMServer. | |
| pushMatch: Effect.fn("TestLLMServer.pushMatch")(function* (match: Match, ...input: (Item | Reply)[]) { | ||
| queueMatch(match, ...input) | ||
| }), | ||
| // altimate_change start — answers for the next title requests, in order; the automatic | ||
| // "E2E Title" resumes once they are used up | ||
| title: Effect.fn("TestLLMServer.title")(function* (...input: (Item | Reply)[]) { | ||
| titles = [...titles, ...input.map((value) => item(value))] | ||
| }), | ||
| // altimate_change end | ||
| textMatch: Effect.fn("TestLLMServer.textMatch")(function* ( | ||
| match: Match, | ||
| value: string, | ||
|
|
@@ -767,6 +790,7 @@ export class TestLLMServer extends Context.Service<TestLLMServer, TestLLMServer. | |
| reset: Effect.sync(() => { | ||
| hits = [] | ||
| list = [] | ||
| titles = [] // altimate_change | ||
| waits = [] | ||
| misses = [] | ||
| }), | ||
|
|
||
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.
Script executed:
Repository: AltimateAI/altimate-code
Length of output: 12947
Script executed:
Repository: AltimateAI/altimate-code
Length of output: 26237
Script executed:
Repository: AltimateAI/altimate-code
Length of output: 30574
Script executed:
Repository: AltimateAI/altimate-code
Length of output: 14153
Script executed:
Repository: AltimateAI/altimate-code
Length of output: 27127
Script executed:
Repository: AltimateAI/altimate-code
Length of output: 7720
Restrict
error_typeto safe values.Anthropic in-band errors can contain any string in
type. If that string contains message text, telemetry can send it despite the documented guarantee. Map known error types to an allowlist and use a safe fallback for unknown values.