3 Commits

Include renames

Author	SHA1	Message	Date
YYClaw	d62048f447	test(handler): use database clock for timestamp fallback bounds (#8570)	2026-09-19 17:10:48 +08:00
3 people	aac4aa3f44	MUL-7248: fix agent log duration timestamps (#8271) ... * fix(transcript): preserve agent event timing Co-authored-by: multica-agent <[email protected]> * fix(transcript): keep timestamp fallbacks consistent Co-authored-by: multica-agent <[email protected]> * fix(migrations): resolve duplicate 458 prefix Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Lambda <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-09-10 22:18:16 +08:00
3 people	115bfab759	MUL-6523: persist task message batches in one statement (#7377) ... * perf(server): persist task message batches in one statement /api/daemon/tasks/{id}/messages is the hottest write path in the system: every running task POSTs its buffered transcript every 500ms. The handler was paying 3 + N database round trips per batch — GetAgentTask, the workspace lookup inside requireDaemonTaskAccess, a SECOND identical GetIssue/GetChatSession in the handler body, and then one `INSERT ... RETURNING` per message, each in its own implicit transaction. Two changes, both on that path: - Take the workspace id from requireDaemonTaskAccessWithWorkspace, which already resolved it to authorize the daemon, instead of re-querying it. Broadcast reach is deliberately unchanged: the id is still only used for issue- and chat-backed tasks, so autopilot / quick-create tasks keep their current (non-)streaming behaviour. - Add CreateTaskMessages, which expands the whole batch from one jsonb document via jsonb_to_recordset and inserts it with a single statement. A jsonb document rather than parallel arrays because `input` is itself jsonb and a text[]/uuid[] column cannot express a per-row NULL from Go; it also stays clear of the ~64k bind-parameter ceiling a multi-row VALUES list would hit. Per batch this takes 3 + N round trips down to 3, and collapses N commits into 1. Measured locally against Postgres 17 (200 iterations per shape, same rows): 2.1ms -> 0.7ms at 3 messages, 7.0ms -> 0.8ms at 10, 80ms -> 1.1ms at 50. Atomicity is the other half of the fix, not a side effect: the daemon does not retry this endpoint, so a loop that failed halfway left a permanent hole in the transcript. One statement makes a batch all-or-nothing. The NUL sanitizer calls stay exactly where they were and matter more now — jsonb rejects \u0000 for the whole document, so an unsanitized byte would fail the entire batch rather than one row (GH #7098). Co-authored-by: multica-agent <[email protected]> * fix(server): guarantee task message publish order, test the real query Review follow-ups on the batch insert. Blocking: - Realtime ordering had no contract. `INSERT ... RETURNING` defines no row order, but the publish loop walked the result as-is, so a batch could reach subscribers scrambled where the per-message loop always published in request order. CreateTaskMessages now returns through an ordered CTE (`WITH inserted AS (INSERT ... RETURNING *) SELECT * FROM inserted ORDER BY seq`), putting the ordering in the query rather than asking every client to re-sort. Verified it is not a no-op: with the ORDER BY removed, the new test reports the published order as [3 1 2] for a request listing seq 3, 1, 2 — i.e. the bug was real, not theoretical. - The atomicity test copied the batch SQL by hand, so it could have passed while CreateTaskMessages was broken. It now calls the generated query. Both tests were also rewritten onto the repo's shared fixtures — dbfx for rows, testutil.JSONRequest / WithURLParams / Call(...).Want(...) for the handler — instead of open-coded INSERT/cleanup pairs and recorder quartets. - New TestReportTaskMessagesPublishesInSeqOrder subscribes to the event bus and asserts the published seq order, with the request deliberately listing messages out of seq order so array order cannot pass for seq order. Non-blocking: - A failed `json.Marshal` of a message's input is no longer swallowed. A tool_use row that silently loses its arguments is worse than a 500 the daemon logs, because nothing anywhere records the loss. - Corrected the atomicity claim in the query and test comments: one statement buys CONSISTENCY, not completeness. Without a retry a failed batch is now lost whole rather than leaving a prefix; actually closing the gap needs a retry plus a (task_id, seq) uniqueness rule, which is out of scope here. The CTE makes sqlc name the result CreateTaskMessagesRow rather than reusing the table model, so the publish site converts explicitly — a compiler-checked cast that fails loudly if the query ever stops returning the whole row. Co-authored-by: multica-agent <[email protected]> * perf(server): pass task message batches as arrays, not a JSON envelope Second review round. The jsonb-document parameter was a regression on large messages: content and output had just been decoded out of the request, and wrapping them back into JSON made the server escape every byte a second time and Postgres parse the envelope out again. Measured against the per-row insert it replaced, at one message: ~1.0x at 8KB, 1.46-1.48x at 128KB, ~3.0x at 1MB — worst exactly where the old path was cheapest, and those messages are the least likely to be batched. CreateTaskMessages now takes parallel arrays. NULLIF(value, '') is what makes a per-row NULL expressible through the non-nullable []string sqlc gives a text[] parameter, and it reproduces the single-row query's pgtype.Text{Valid: x != ""} mapping exactly. input stays a text parameter cast to jsonb in SQL — it is the one column that genuinely has to be parsed. The arrays are zipped by several single-argument unnest calls in one SELECT list, which expand in lockstep (PG10+ SRF semantics). The tidier unnest(a, b, ...) form is equivalent but sqlc's analyzer only knows the single-argument signature and fails to generate for it. Measured after the change, same harness, two runs: one message, 8KB 0.98-0.90x of the per-row insert one message, 128KB 0.99-1.04x (was 1.46-1.48x) one message, 1MB 1.59-1.67x (was 2.99-3.02x) 3 messages, 512B 2.7-3.2x faster 10 messages, 512B 7.7-8.2x faster 128KB is now parity. 1MB is still above parity because any batch shape makes Postgres materialize the array and unnest it before the insert, which a direct single-row parameter skips; closing that needs a len == 1 fast path, which review explicitly preferred not to have for the extra code path. A real 1MB single message is also unlikely: tool_result output is truncated to 8KB in the daemon (internal/daemon/daemon.go). The durable answer for oversized payloads is a body/batch cap on this endpoint, which is still a follow-up. Also from review: - The conflicting row in the atomicity test is built with dbfx.Insert instead of CreateTaskMessage, so test setup no longer shares code with the code under test. - The ordering test runs on its own event bus and restores the shared one, so it stops leaving a listener behind on a bus that has no unsubscribe. - The per-batch probe is Debug, not Info. At Info it was ~2 lines/second/task in production; it was added at Info in #7371 only to count batch sizes, which is answered. Re-verified the ordering test still fails without the ORDER BY (reports [3 1 2]) against the new query shape. Co-authored-by: multica-agent <[email protected]> --------- Co-authored-by: Sol-Boy <[email protected]> Co-authored-by: multica-agent <[email protected]>	2026-08-21 15:17:02 +08:00