Fix/1984 synthetic compaction - #2036
Conversation
|
…loudflare#1984) getHistory() synthesizes a compaction_<id> overlay on read; when a client echoed it back it was filed as a real row, duplicating the summary in every later read. Session now refuses to persist reserved compaction_ ids, and applyCompactions filters such rows so existing sessions self-heal on read.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| --- | ||
| "agents": patch | ||
| --- | ||
|
|
||
| Fix `useAgentChat` dropping an in-flight optimistic send on reconnect. When a message was sent while the socket was down, PartySocket buffers the frame for delivery, but the idle-connect transcript the server replays on reconnect (`cf_agent_chat_messages`) doesn't include it yet — so the whole-array `setMessages` replace erased the just-sent message from the UI until the turn completed and the server rebroadcast. The reconnect replay now preserves a trailing local-only `user` message that was buffered while disconnected, while still letting the server snapshot win for a delivered send it deliberately rolls back (e.g. `messageConcurrency: "drop"`) and for a regenerate's assistant replacement. Also fixes `@cloudflare/ai-chat` and `@cloudflare/think`, which re-export the hook unchanged. |
There was a problem hiding this comment.
🟡 Compaction duplication fix ships without a release note, so users never get it
The bug fix that stops compaction summaries being stored and duplicated is added to a published package without an accompanying changeset entry (only .changeset/reconnect-optimistic-send.md exists, which describes an unrelated reconnect fix), so the fix is not released or documented.
Impact: The fix may not be published in a version bump and won't appear in the changelog users read.
Repository rule: changesets required for package bug fixes
AGENTS.md ("Contributing → Changesets") states: "Changes to packages/ that affect the public API or fix bugs need a changeset". Commit ffaafdc4 changes packages/agents/src/experimental/memory/session/session.ts:624-635, packages/agents/src/experimental/memory/session/providers/agent.ts:535-541 and packages/agents/src/experimental/memory/session/providers/postgres.ts:269-272 — a user-visible bug fix (#1984) — but adds no .changeset/*.md. The only changeset in the PR (.changeset/reconnect-optimistic-send.md:1-5) documents the useAgentChat reconnect fix from the other commit.
Prompt for agents
The PR contains two independent fixes but only one changeset. Add a second changeset markdown file under .changeset/ (patch bump for the "agents" package) describing the #1984 fix: Session no longer persists messages using the reserved compaction_ id prefix, and both the SQLite (AgentSessionProvider) and Postgres session providers filter compaction-prefixed rows out of the read projection so previously corrupted sessions self-heal on read.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Did you remove the changeset? It’s important to include one otherwise the change won’t be released when we merge.
There was a problem hiding this comment.
oh wait I now see that was an unrelated changeset. Can you add a new one for this change @justanotherbyte ?
ffaafdc to
33b7b89
Compare
agents
@cloudflare/ai-chat
@cloudflare/codemode
create-think
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
cjol
left a comment
cjol
left a comment
There was a problem hiding this comment.
Overall shape is good, thank you! Definitely need a changeset, a few nits otherwise in comments.
| if (isCompactionMessage(message)) { | ||
| console.warn( | ||
| `[Session] Refusing to persist a message with the reserved compaction id "${message.id}" — synthetic compaction overlays are computed on read and are never stored (#1984).` | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Is this necessary? Receiving a compaction message from the browser is expected, so we’re logging a warning under normal, happy-path conditions (potentially even on every single turn after a compaction?)
| expect(compactions).toHaveLength(1); | ||
| }); | ||
|
|
||
| it("does not persist synthetic compaction overlays echoed back by a client (#1984)", async () => { |
There was a problem hiding this comment.
For completeness, it might be good to add a test for the postgres provider as well
| // Simulate an already-corrupted session: a synthetic overlay was filed as | ||
| // a real row on a prior turn (before the intake guard existed), parented | ||
| // into the live chain via the raw insert seam. | ||
| await agent.rawInsertChildForTest("m5", overlay!.id); |
There was a problem hiding this comment.
For completeness, I might test that a child of the overlay message is restored and re-parented correctly
2 participants
What this fixes
Fixes #1984 where a Think session's compaction summary got persisted as a real message row and duplicated on every later read — sending the summary to the model twice, breaking keyed rendering on the client, and compounding with each further compaction.
How
Fixed at the shared chokepoint every provider and caller routes through, rather than patching Think's intake.
Session._appendMessagenow refuses to persist any message using the reserved compaction_ prefix, and applyCompactions in both the agent (SQLite) and Postgres providers filters compaction-prefixed rows out of the read projection so already-affected sessions self-heal on read with no migration — a no-op for healthy sessions.Added testing coverage in the agents package (intake-persistence repro, pre-filed-row read-dedup repro, Session write-guard unit tests) plus a full Think test.
Why
getHistory()substitutes a syntheticcompaction_<id>overlay for the compacted range on read — it has no row of its own. The browser transport echoes the full transcript back on the next turn, so that overlay arrived as an incoming message and got filed as a real assistant_messages row; from then ongetHistory()returned the id twice (once as the substituted overlay, once as the stored row).