-
Notifications
You must be signed in to change notification settings - Fork 257
Keep active-turn prompts visible in budgeted timelines #1639
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ import type { | |
| } from "@bb/server-contract"; | ||
| import { | ||
| findStoredTimelineWindowByteBudgetFloor, | ||
| findFirstStoredTurnInputAcceptedRow, | ||
| findTimelineWindowBudgetFloorSequence, | ||
| getStoredEventRowsByParentToolCallIdsDataBytes, | ||
| getEnvironment, | ||
|
|
@@ -37,6 +38,7 @@ import { | |
| listLatestBackgroundTaskStateRowsByItemIds, | ||
| listLatestGoalEventRowsByThreadIds, | ||
| listLatestOpenBackgroundTaskStateRowsForThread, | ||
| listStoredClientTurnRequestRowsByKeys, | ||
| listStoredTimelineWindowEventRows, | ||
| listTodoSnapshotEventRowsForThread, | ||
| listStoredToolCallRowsByItemIds, | ||
|
|
@@ -1021,6 +1023,8 @@ interface ResolveTimelineSegmentWindowArgs { | |
| } | ||
|
|
||
| interface ResolvedTimelineSegmentWindow { | ||
| /** Unfinished turn whose work is cut by this sequence window. */ | ||
| activeTurnId: string | null; | ||
| beforeSequence: number | undefined; | ||
| byteWindowSequenceStart: number | null; | ||
| /** | ||
|
|
@@ -1061,8 +1065,15 @@ function applyTimelineWindowByteBudget( | |
| if (floor.kind === "single-event-too-large") { | ||
| const hasOlderRows = | ||
| floor.hasOlderRows || args.window.knownHasOlderSegments === true; | ||
| const activeTurnId = | ||
| args.window.activeTurnId ?? | ||
| findUnfinishedTurnCoveringSequence(db, { | ||
| sequence: floor.sequenceStart, | ||
| threadId: args.threadId, | ||
| }); | ||
| return { | ||
| ...args.window, | ||
| activeTurnId, | ||
| byteWindowSequenceStart: floor.sequenceStart, | ||
| knownHasOlderSegments: hasOlderRows, | ||
| oversizedEventPlaceholder: { | ||
|
|
@@ -1093,8 +1104,16 @@ function applyTimelineWindowByteBudget( | |
| return args.window; | ||
| } | ||
|
|
||
| const activeTurnId = | ||
| args.window.activeTurnId ?? | ||
| findUnfinishedTurnCoveringSequence(db, { | ||
| sequence: floor.sequenceStart, | ||
| threadId: args.threadId, | ||
| }); | ||
|
|
||
| return { | ||
| ...args.window, | ||
| activeTurnId, | ||
| byteWindowSequenceStart: floor.sequenceStart, | ||
| requiresWholeItemClosure: true, | ||
| sequenceWindowStart: { | ||
|
|
@@ -1169,7 +1188,10 @@ function resolveTimelineWindowBounds( | |
| args: ResolveTimelineWindowBoundsArgs, | ||
| ): Pick< | ||
| ResolvedTimelineSegmentWindow, | ||
| "effectiveSegmentLimit" | "sequenceStart" | "sequenceWindowStart" | ||
| | "activeTurnId" | ||
| | "effectiveSegmentLimit" | ||
| | "sequenceStart" | ||
| | "sequenceWindowStart" | ||
| > & { affordableAnchorCount: number } { | ||
| const { anchors, budgetFloorSequence, segmentLimit, threadId } = args; | ||
| const affordable = countAffordableAnchors( | ||
|
|
@@ -1194,6 +1216,7 @@ function resolveTimelineWindowBounds( | |
| }) | ||
| ) { | ||
| return { | ||
| activeTurnId: unfinishedTurnId, | ||
| affordableAnchorCount: 0, | ||
| effectiveSegmentLimit: segmentLimit, | ||
| sequenceWindowStart: { | ||
|
|
@@ -1209,6 +1232,7 @@ function resolveTimelineWindowBounds( | |
| // empty thread rather than a slow one. | ||
| const segmentCount = Math.max(1, affordable); | ||
| return { | ||
| activeTurnId: null, | ||
| affordableAnchorCount: segmentCount, | ||
| effectiveSegmentLimit: segmentCount, | ||
| sequenceWindowStart: null, | ||
|
|
@@ -1235,6 +1259,7 @@ function resolveTimelineSegmentWindow( | |
| ): ResolvedTimelineSegmentWindow { | ||
| const { eventBudget, page, threadId } = args; | ||
| const noAnchors: ResolvedTimelineSegmentWindow = { | ||
| activeTurnId: null, | ||
| beforeSequence: undefined, | ||
| byteWindowSequenceStart: null, | ||
| requiresWholeItemClosure: false, | ||
|
|
@@ -1314,6 +1339,7 @@ function resolveTimelineSegmentWindow( | |
| threadId, | ||
| }); | ||
| return { | ||
| activeTurnId: bounds.activeTurnId, | ||
| // Every cursor names the first sequence the page that issued it covered, | ||
| // so this page ends exactly there. Reading up to the *next anchor* past | ||
| // the cursor instead — and trimming that segment off after projecting it | ||
|
|
@@ -1353,6 +1379,7 @@ function resolveTimelineSegmentWindow( | |
| threadId, | ||
| }); | ||
| return { | ||
| activeTurnId: bounds.activeTurnId, | ||
| beforeSequence: undefined, | ||
| byteWindowSequenceStart: null, | ||
| requiresWholeItemClosure: bounds.sequenceWindowStart !== null, | ||
|
|
@@ -1367,6 +1394,33 @@ function resolveTimelineSegmentWindow( | |
| }; | ||
| } | ||
|
|
||
| function ensureTimelineWindowActiveTurnInputRows( | ||
| db: DbConnection, | ||
| args: TimelineWindowRowsArgs & { activeTurnId: string | null }, | ||
| ): StoredEventRow[] { | ||
| if (args.activeTurnId === null) { | ||
| return [...args.rows]; | ||
| } | ||
|
|
||
| const acceptedInputRow = findFirstStoredTurnInputAcceptedRow(db, { | ||
| threadId: args.threadId, | ||
| turnId: args.activeTurnId, | ||
| }); | ||
| if (!acceptedInputRow) { | ||
| return [...args.rows]; | ||
| } | ||
|
|
||
| const clientRequestId = parseAcceptedInputClientRequestId(acceptedInputRow); | ||
| const requestRows = listStoredClientTurnRequestRowsByKeys(db, { | ||
| keys: [{ requestId: clientRequestId, threadId: args.threadId }], | ||
| }); | ||
| return mergeStoredEventRowsById([ | ||
| ...requestRows, | ||
| acceptedInputRow, | ||
| ...args.rows, | ||
| ]); | ||
| } | ||
|
|
||
| function selectStandardTimelineEventRows( | ||
| db: DbConnection, | ||
| thread: Thread, | ||
|
|
@@ -1402,15 +1456,23 @@ function selectStandardTimelineEventRows( | |
| threadId: thread.id, | ||
| }; | ||
| const windowRows = listStoredTimelineWindowEventRows(db, windowArgs); | ||
| const windowRowsWithActiveTurnInput = ensureTimelineWindowActiveTurnInputRows( | ||
|
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 later page transform adds a different The new client test uses one prompt ID for both pages. Actual server responses use different prompt IDs. Please keep the restored prompt ID stable across byte pages. Add a test that merges real latest-page and older-page IDs. |
||
| db, | ||
| { | ||
| activeTurnId: window.activeTurnId, | ||
| rows: windowRows, | ||
| threadId: thread.id, | ||
| }, | ||
| ); | ||
| const wholeItemWindowRows = window.requiresWholeItemClosure | ||
| ? ensureSequenceWindowWholeItemRows(db, { | ||
| beforeSequence, | ||
| maxInlineOutputChars, | ||
| rows: windowRows, | ||
| rows: windowRowsWithActiveTurnInput, | ||
| sequenceStart, | ||
| threadId: thread.id, | ||
| }) | ||
| : windowRows; | ||
| : windowRowsWithActiveTurnInput; | ||
| const selectedRowsWithTurnStarts = ensureTimelineWindowTurnStartedRows(db, { | ||
| threadId: thread.id, | ||
| rows: wholeItemWindowRows, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -888,6 +888,11 @@ export interface ListOpenTurnInputAcceptedRowsByThreadIdsArgs { | |
| threadIds: readonly string[]; | ||
| } | ||
|
|
||
| export interface FindFirstStoredTurnInputAcceptedRowArgs { | ||
| threadId: string; | ||
| turnId: string; | ||
| } | ||
|
|
||
| export interface ThreadClientTurnRequestKey { | ||
| requestId: ClientTurnRequestId; | ||
| threadId: string; | ||
|
|
@@ -1229,6 +1234,27 @@ export function listOpenTurnInputAcceptedRowsByThreadIds( | |
| ); | ||
| } | ||
|
|
||
| export function findFirstStoredTurnInputAcceptedRow( | ||
| db: DbQueryConnection, | ||
| args: FindFirstStoredTurnInputAcceptedRowArgs, | ||
| ): StoredEventRow | null { | ||
| return ( | ||
| db | ||
| .select(storedEventRowFields) | ||
| .from(events) | ||
| .where( | ||
| and( | ||
| eq(events.threadId, args.threadId), | ||
| eq(events.turnId, args.turnId), | ||
| eq(events.type, "turn/input/accepted"), | ||
| ), | ||
| ) | ||
| .orderBy(events.sequence) | ||
|
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. 🚨 SQLite selects the thread/type/sequence index to satisfy this order. It then filters accepted rows until it reaches the active turn. A 100,000-turn check used about 500,000 VM steps here. The request-row lookup used about 600,000 more steps per page. Please make this lookup selective. Constrain the accepted row by |
||
| .limit(1) | ||
| .get() ?? null | ||
| ); | ||
| } | ||
|
|
||
| export function listStoredClientTurnRequestRowsByKeys( | ||
| db: DbQueryConnection, | ||
| args: ListStoredClientTurnRequestRowsByKeysArgs, | ||
|
|
||
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.
🚨
slopcop/review— High: the prompt backfill bypasses the timeline byte limit.The four-megabyte check runs before this query. The code then adds the complete request row to every active-turn page.
Prompt text has no size limit. A large prompt can make each refresh consume excessive memory and make the server unavailable.
Please include these rows in the byte calculation. Use the existing oversized-event result when the prompt exceeds the remaining limit.