Skip to content

[fix] Stop dropping session record batches on duplicate record_ids - #6086

Open
mmabrouk wants to merge 1 commit into
mainfrom
fix/records-batch-dedupe
Open

[fix] Stop dropping session record batches on duplicate record_ids#6086
mmabrouk wants to merge 1 commit into
mainfrom
fix/records-batch-dedupe

Conversation

@mmabrouk

Copy link
Copy Markdown
Member

Context

Agent session transcripts silently lost tool calls. The session itself ran fine, but when you reopened it in the UI, stretches of the conversation were missing: a tool result with no matching call, or a whole call/result pair gone. On the bighetzner deployment one session lost 28 frames across 8 batches, and the same worker log shows 36 dropped batches across 9 sessions. The records were deleted from the Redis stream after the failure, so the loss was permanent.

Root cause: RecordsDAO.append_many writes one multi-row INSERT ... ON CONFLICT DO UPDATE, and Postgres rejects such a statement when two rows share the conflict key. Duplicate keys inside one batch are not a bug in the producer; they are by design (see the walkthrough). When it happened, the whole batch failed, and the worker had already marked the messages processed, so they were ACKed and deleted.

Closes #6085

How the bug unfolds, step by step

Follow one tool call through the pipeline, as if stepping through a debugger:

  1. Runner (sandbox-agent). The harness starts tool call toolu_011f.... The runner emits a partial tool_call frame immediately, with input: {}, so the UI can show the call as pending. It derives a stable record_id from the call id (uuid5), so later frames for the same call upsert onto the same row. It POSTs the frame to the API.

  2. API ingest. The API appends the frame to the streams:records Redis stream and returns. ([sessions/persist] ingest OK ... idx=31 type=tool_call.)

  3. Runner again, ~5 seconds later. The tool input is complete. The runner emits the completed tool_call frame with the full input. Same call id, therefore the same record_id. This is intentional: the completed frame is meant to overwrite the partial row.

  4. RecordsWorker (worker-streams). process_batch reads up to 50 stream messages in one XREADGROUP window. Both frames from steps 1 and 3 land in the same window because they are only seconds apart. Every message id goes into processed_ids as it is deserialized.

  5. RecordsDAO.append_many. All frames become one statement:

    INSERT INTO records (...) VALUES
      (..., 'ac3f1a4e-...', 31, ..., '{"input": {}}', ...),      -- partial
      (..., 'ac3f1a4e-...', 32, ..., '{"input": {...}}', ...),   -- completed, SAME record_id
      ...
    ON CONFLICT (project_id, record_id) DO UPDATE SET ...
  6. Postgres. The statement violates a hard rule of ON CONFLICT DO UPDATE: one command may not affect the same row twice. Postgres raises CardinalityViolationError ("ON CONFLICT DO UPDATE command cannot affect row a second time") and the whole insert rolls back. Not just the duplicate pair: the usage and tool_result frames that shared the batch roll back with it.

  7. RecordsWorker, except branch. The exception is logged ("Failed to append event batch") and swallowed. process_batch returns processed_ids unchanged, the consumer ACKs and deletes the messages, and the frames are gone for good.

Note the race that decides whether you lose data: if the partial and completed frames land in different flush windows, the second one upserts cleanly onto the first one's row and everything works. The bug only fires when the agent is fast enough (or the worker slow enough) for both frames to share a window. That is why it looked flaky and only bit sessions with rapid back-to-back tool calls.

Changes

append_many now collapses in-batch duplicates before building the statement, in a new RecordsDAO._dedupe_values helper. The collapse reproduces exactly what sequential per-event upserts would have produced:

  • columns the upsert updates on conflict (record_type, record_source, timestamp, attributes, turn_id, span_id) take the last occurrence,
  • insert-only columns (record_index, session_id, audit columns) keep the first occurrence, because ON CONFLICT DO UPDATE never touches them.

Stepping through the same run after the fix: steps 1 to 4 are unchanged. In step 5 the two frames for ac3f1a4e-... merge into one row (record_index 31 from the partial, full attributes from the completed frame) before the SQL is built. Postgres sees each conflict key once, the batch commits, and step 7's except branch never runs.

Before, the DB ended the day with the whole batch missing. After, it holds the same rows a slow, one-by-one ingest would have produced.

Tradeoffs and notes

  • Dedupe in Python, not SQL. Postgres has no "last row wins" variant of multi-VALUES upsert, so the merge has to happen client-side. The helper mirrors the set_ column list; if someone adds a column to the upsert they must add it to _UPSERT_UPDATED_COLUMNS. The two sit next to each other in the same file to keep that visible.
  • Keep-first for record_index is deliberate, not an accident of implementation: it matches what the sequential path produced all along (the completed frame never moved the row's index), so UI ordering behavior does not change.
  • The worker still ACKs on failure. This PR removes the only known systematic cause of batch failure, but a transient DB outage would still drop batches permanently, because process_batch adds message ids to processed_ids before the insert succeeds. Fixing that means separating "deserialized" from "persisted" and letting failed batches redeliver, which needs idempotency thinking (the upsert gives us most of it). Left as a follow-up on (bug) Session transcripts silently lose tool calls when frames land in one ingestion batch #6085.
  • No migration, no API change. The fix is contained in the DAO.

Tests

  • New unit tests in api/oss/tests/pytest/unit/sessions/test_records_batch_dedupe.py: duplicate pair collapses to one row with first index and last attributes; distinct ids pass through in order; the same record_id in two projects does not collapse.
  • Full sessions unit suite: 474 passed.
  • Validated the failure mode against the live bighetzner deployment first: reproduced the CardinalityViolationError batches in worker-streams logs and confirmed each dropped batch contained a partial/completed frame pair sharing a record_id.

What to QA

  • Run an agent session that fires several quick tool calls in a row (any Terminal-heavy prompt). Open the transcript: every tool call shows its input and its result, no orphan results.
  • Watch worker-streams logs during the run: no CardinalityViolationError, no "Failed to append event batch".
  • Regression: resume an existing session and send one message. The transcript still renders in order (ordering uses timestamps; this change does not touch reads).

https://claude.ai/code/session_01AxbkMmS2VvpaxhXPAWzTbH

…he upsert

One batched INSERT ... ON CONFLICT DO UPDATE cannot touch the same
(project_id, record_id) twice — Postgres raises CardinalityViolationError and
the records worker drops the whole batch. The runner legitimately re-sends a
record_id within one flush window (partial tool_call frame, then the completed
one), so collapse in-batch duplicates into the end state sequential upserts
would have produced.

Closes #6085

Claude-Session: https://claude.ai/code/session_01AxbkMmS2VvpaxhXPAWzTbH
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug python Pull requests that update Python code tests labels Aug 17, 2026
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Agenta Team seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@vercel

vercel Bot commented Aug 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Ready Ready Preview Aug 17, 2026 4:22pm

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug python Pull requests that update Python code size:M This PR changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(bug) Session transcripts silently lose tool calls when frames land in one ingestion batch

2 participants