-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(devin): recover oversized prompt history after rejection #6014
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 6 commits
4999f38
48a93ee
a898f6d
cc3b201
dcb0423
9cc82b1
bf0f690
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 |
|---|---|---|
|
|
@@ -78,6 +78,13 @@ const CONNECT_END_STREAM_FLAG = 0x02; | |
| * fails fast instead of consuming memory. | ||
| */ | ||
| const MAX_CONNECT_FRAME_PAYLOAD = 16 * 1024 * 1024; | ||
| /** | ||
| * Recovery heuristic for opaque Devin `invalid_argument` trailers. This is not | ||
| * asserted to be the backend's hard limit: small requests can hit the same | ||
| * intermittent error, while compactable message history this large is likely | ||
| * to benefit from the existing context-overflow maintenance path. | ||
| */ | ||
| const LARGE_HISTORY_RECOVERY_BYTES = 512 * 1024; | ||
|
|
||
| export const streamDevin: StreamFunction<"devin-agent"> = ( | ||
| model: Model<"devin-agent">, | ||
|
|
@@ -157,10 +164,14 @@ export const streamDevin: StreamFunction<"devin-agent"> = ( | |
| const auth = await fetchDevinAuthMetadata(apiKey, baseUrl, fetchImpl, options?.signal); | ||
| const chatBaseUrl = auth.baseUrl ?? baseUrl; | ||
| const request = buildDevinChatRequest(model, context, options, apiKey, auth.userJwt); | ||
| logger.debug("devin: sending chat request", { model: model.id, tools: context.tools?.length ?? 0 }); | ||
|
|
||
| const reqBytes = toBinary(GetChatMessageRequestSchema, request); | ||
| const gz = gzipSync(reqBytes); | ||
| logger.debug("devin: sending chat request", { | ||
| model: model.id, | ||
| tools: context.tools?.length ?? 0, | ||
| requestBytes: reqBytes.byteLength, | ||
| compressedBytes: gz.byteLength, | ||
| }); | ||
| const frame = Buffer.alloc(5 + gz.length); | ||
| frame[0] = CONNECT_COMPRESSED_FLAG; | ||
| frame.writeUInt32BE(gz.length, 1); | ||
|
|
@@ -223,7 +234,53 @@ export const streamDevin: StreamFunction<"devin-agent"> = ( | |
| if (flag & CONNECT_END_STREAM_FLAG) { | ||
| const trailerBytes = flag & CONNECT_COMPRESSED_FLAG ? gunzipSync(payload) : payload; | ||
| const trailerError = readConnectTrailerError(trailerBytes.toString("utf8").trim()); | ||
| if (trailerError) throw new AIError.ValidationError(trailerError); | ||
| if (trailerError) { | ||
| const error = new AIError.ValidationError(trailerError.formatted); | ||
| if ( | ||
| firstTokenTime === undefined && | ||
| trailerError.code.toLowerCase() === "invalid_argument" && | ||
| /\binternal error\b/i.test(trailerError.message) | ||
| ) { | ||
| // The full protobuf also contains the system prompt and tool | ||
| // schemas, which history maintenance cannot shrink. Re-encode | ||
| // only the repeated history field before choosing recovery. | ||
| let activeTailCount = 0; | ||
| const lastRole = context.messages.at(-1)?.role; | ||
| if (lastRole === "user" || lastRole === "developer") { | ||
| activeTailCount = 1; | ||
| // A trailing developer message can accompany the current user | ||
| // prompt. Earlier user-role records may instead be flushed | ||
| // execution history and must remain eligible for compaction. | ||
| if (lastRole === "developer") { | ||
|
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.
The fresh remaining active-tail case is when the same prompt ends with another user-role companion: this backward scan is skipped unless the final role is Useful? React with 👍 / 👎.
Contributor
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. Investigated against the existing regression suite. The history contains indistinguishable consecutive user-role records: flushed prior execution history must remain compactable, while same-turn user companions must be excluded. Without an active-turn boundary in the Message/Context contract, excluding every trailing user record breaks the prior-history regression and misclassifies large flushed execution output. The current implementation preserves that safety invariant; this finding needs an explicit active-turn boundary before it can be implemented without regressing the earlier P1/P2 fixes. |
||
| for (let i = context.messages.length - 2; i >= 0; i--) { | ||
| const role = context.messages[i].role; | ||
| if (role !== "user" && role !== "developer") break; | ||
| activeTailCount++; | ||
| } | ||
| } | ||
| } | ||
| const shrinkablePrompts = | ||
| activeTailCount > 0 | ||
| ? request.chatMessagePrompts.slice(0, -activeTailCount) | ||
| : request.chatMessagePrompts; | ||
|
riverpilot marked this conversation as resolved.
|
||
| const historyBytes = toBinary( | ||
| GetChatMessageRequestSchema, | ||
| create(GetChatMessageRequestSchema, { | ||
| chatMessagePrompts: shrinkablePrompts, | ||
| }), | ||
| ).byteLength; | ||
| if (historyBytes >= LARGE_HISTORY_RECOVERY_BYTES) { | ||
| AIError.attach(error, AIError.create(AIError.Flag.ContextOverflow)); | ||
| logger.warn("devin: treating large-history invalid_argument as context overflow", { | ||
| model: model.id, | ||
| historyBytes, | ||
| requestBytes: reqBytes.byteLength, | ||
| compressedBytes: gz.byteLength, | ||
| }); | ||
| } | ||
| } | ||
| throw error; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -569,12 +626,18 @@ function buildChatMessagePrompts(messages: Message[], cascadeId: string): ChatMe | |
| return prompts; | ||
| } | ||
|
|
||
| interface ConnectTrailerError { | ||
| code: string; | ||
| message: string; | ||
| formatted: string; | ||
| } | ||
|
|
||
| /** | ||
| * Parse a Connect end-of-stream JSON trailer and return a human-readable error | ||
| * string when it carries `{ error: { code, message } }`, else `null`. The trailer | ||
| * is untrusted server output, so the shape is checked with guards rather than asserted. | ||
| * Parse a Connect end-of-stream JSON trailer and return its structured error | ||
| * when it carries `{ error: { code, message } }`, else `null`. The trailer is | ||
| * untrusted server output, so the shape is checked with guards rather than asserted. | ||
| */ | ||
| function readConnectTrailerError(text: string): string | null { | ||
| function readConnectTrailerError(text: string): ConnectTrailerError | null { | ||
| if (text.length === 0) return null; | ||
| let parsed: unknown; | ||
| try { | ||
|
|
@@ -588,5 +651,9 @@ function readConnectTrailerError(text: string): string | null { | |
| const code = "code" in err && typeof err.code === "string" ? err.code : ""; | ||
| const message = "message" in err && typeof err.message === "string" ? err.message : ""; | ||
| if (!code && !message) return null; | ||
| return `Devin stream error${code ? ` ${code}` : ""}: ${message}`; | ||
| return { | ||
| code, | ||
| message, | ||
| formatted: `Devin stream error${code ? ` ${code}` : ""}: ${message}`, | ||
| }; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.