Skip to content

ref(persistence): a turn is persisted twice and reconciled by turn_id #2089

Description

@rogercloud

One user turn is written to two independent stores. Neither derives from the
other, so they can disagree — and the code that exists to stop them
disagreeing is the clearest evidence of the problem.

Evidence

A shared key whose stated purpose is reconciliation. TaskChatMessage.turn_id (src/xagent/web/models/chat_message.py:36) is documented as:

Stable per-user-turn identity shared with user_message trace events. Used by historical replay to reconcile trace rows with transcript rows without collapsing distinct turns that happen to share text/attachments.

Two deduplication guards. _is_duplicate_user_message_turn exists twice — src/xagent/web/api/trace_handlers.py:1036 and src/xagent/web/api/websocket.py:912 — both querying by turn_id to stop the same user message being recorded or displayed twice.

The same failure recorded twice, at different fidelities, both reaching the model. On a failed turn:

  • assistant_history_values_for_persistence (src/xagent/web/services/assistant_history_safety.py:27) stores the constant string "Task execution failed." into the transcript.
  • The trace event keeps the real detail, which summarize_execution_failure_event (src/xagent/web/services/task_execution_context_service.py:223) renders as - Previous execution failed: step=X; reason=Y.

Both land in the same list: ExecutionAdapter._initial_messages() (src/xagent/core/agent/execution_adapter.py:354) concatenates execution_context_messages (the trace-derived summary) with conversation_history (the transcript rows). The model sees one failure described twice, and one of the two descriptions carries zero information — the content is a constant, so it says strictly less than the other.

A shared key, two dedup guards, and a reconciliation step are all costs paid to keep two stores agreeing. None of them would need to exist with one source.

What each store legitimately owns today

This is not "delete one of them". The transcript table carries responsibilities an append-only event log does not serve well:

  • delivery_status (chat_message.py:39) — a durable, transactional claim for retry-safe handoff.
  • UNIQUE (task_id, role, turn_id) (chat_message.py:14) — a uniqueness constraint.
  • attachments (chat_message.py:44) — per-message file metadata for replay.
  • Curated-for-display content: sanitized on write, and gated again on read by client_safe_assistant_history_content.

And the read-time gate is applied on both paths — UI (src/xagent/web/api/websocket.py:7940) and model context (src/xagent/web/services/chat_history_service.py:926). That is worth stating plainly, because it looks like a layering mistake and is not: applying the same gate to both is what keeps the model and the user looking at the same conversation. A reference like "what you said earlier" resolves consistently only because of it.

Conversely, tool exchanges exist only in trace events. Making the transcript the single source would mean persisting tool payloads into the user-visible chat table, which is the wrong direction.

So both stores have real reasons to exist. The problem is that they are two independent writes of overlapping facts, not that either is unnecessary.

Shape of a fix

The principled form is one record of truth with the other as a maintained projection: trace events as the log, the transcript as a materialized view built from it. The redundancy then becomes an intentional projection rather than two writes that drift, and turn_id reconciliation plus both dedup guards can be deleted.

Honest cost: the transactional responsibilities above do not project cleanly. Either they move into the projection layer — at which point it is no longer purely derived — or they stay put and there are still two sources. That makes this an architectural change touching persistence, the UI feed, and delivery, not a refactor.

Not asking for that now

Two smaller things are worth doing first, and they are cheap:

  1. Stop replaying the constant failure row into model context. Its content is a fixed string, so it carries no information the trace-derived summary does not carry better, and two conflicting levels of detail about one event is worse than one. The row should stay in the transcript — it is a meaningful event on the user's timeline — it just should not occupy a slot in the model's history.
  2. Sequence this after fix(agent): resume rebuilds conversation context by clipping tool results mid-JSON #1598's step 4. That step rewrites the conversation_history path, which is exactly where (1) lands, so doing it afterwards puts the change on code that was just touched rather than on code about to be replaced.

Filing this now mainly to keep the evidence while it is fresh. The three artefacts above are easy to read past individually; together they are unambiguous.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions