perf(a2a): push ListTasks all-sessions query down to SQL#1
Closed
QuentinBisson wants to merge 1 commit into
Closed
perf(a2a): push ListTasks all-sessions query down to SQL#1QuentinBisson wants to merge 1 commit into
QuentinBisson wants to merge 1 commit into
Conversation
collectUserTasks loaded every session for a user, then every task per session, then filtered, sorted, and paginated in memory (an N+1 query whose latency and memory scaled with the user's total task count even for a small page). Replace it with a single ListUserTasks sqlc query that joins session to task, filters, orders by task id, and paginates server-side, returning COUNT(*) OVER() as the total. Status and statusTimestampAfter are filtered in SQL via a jsonb cast on task.data (no schema migration). Rows exist in two shapes — v1 (protocol_version 'v1', state "TASK_STATE_WORKING") and legacy (protocol_version NULL, state "working") — so both spellings are passed and matched; the vocabularies never overlap. The timestamp cast is guarded by a CASE so a malformed value cannot error the query. The single-session path keeps its fail-closed GetSession ownership check; the join scopes every path to the caller's own sessions. Adds a Postgres integration test covering both row shapes, cross-user isolation, the session predicate, status/timestamp filters, and pagination.
Owner
Author
|
Superseded by kagent-dev#2302 (opened on upstream as a draft). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to kagent-dev#2187, addressing issue kagent-dev#2197 (Copilot review finding). Stacked on the kagent-dev#2187 branch (
feat/a2a-store-backed-task-queries, mirrored here as basestack/2197-base) — review only the singleperf(a2a)commit; retarget tomainonce kagent-dev#2187 merges.What
collectUserTasksserved an all-sessions ListTasks query by loading every session for the user, then every task per session, then filtering, sorting, and paginating in memory — an N+1 query whose latency and memory scale with the user's total task count even for a small page.This replaces that path with a single
ListUserTaskssqlc query that joinssessiontotask, filters, orders bytask.id, and paginates server-side, returningCOUNT(*) OVER()as the total.Status filtering (issue option 1)
statusandstatusTimestampAfterare filtered in SQL via ajsonbcast ontask.data— no schema migration (option 1 from the issue, per the maintainer's request to "start with option 1 for status for now").Rows exist in two persisted shapes and both are matched:
protocol_version'v1'TASK_STATE_WORKINGNULLworkingBoth spellings are passed (
status_v1,status_legacy) and matched withIN (...); the two vocabularies never overlap, so no row is silently dropped.datais always a JSON object (json.Marshaloutput) so::jsonbnever errors; the timestamp cast is guarded by aCASE(ordered evaluation) so a present-but-malformed value can't error the query.Option 2 (generated column + index) remains the durable path for a later migration.
Behavior preserved
The single-session path keeps its fail-closed
GetSessionownership check (a missing/other-user context is an error, not an empty page); the join scopes every path to the caller's own sessions.Tests
Adds a Postgres integration test (
TestListUserTasks) covering both row shapes, cross-user isolation, the single-session predicate, status and timestamp filters, and LIMIT/OFFSET pagination with a stable total. Existingtask_query_storeunit tests pass unchanged against a fake that mirrors the query semantics.