Skip to content

fix(session-memory): deduplicate assistant messages when thinking is stripped#94211

Closed
Pandah97 wants to merge 1 commit into
openclaw:mainfrom
Pandah97:fix/session-memory-dedup-thinking-92563
Closed

fix(session-memory): deduplicate assistant messages when thinking is stripped#94211
Pandah97 wants to merge 1 commit into
openclaw:mainfrom
Pandah97:fix/session-memory-dedup-thinking-92563

Conversation

@Pandah97

@Pandah97 Pandah97 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When a model with thinking/reasoning is used, the session JSONL persists two copies of each assistant response: the raw version (with thinking blocks + text) and a cleaned version (text only, with parent pointing to the first message)
  • The session-memory hook's getRecentSessionContent()extractTextMessageContent() treated both as valid text messages, producing duplicate assistant: ... lines in generated memory files
  • Fix: track the last assistant text and skip consecutive identical texts, since the duplicate always immediately follows the original with no user message in between

Fixes #92563

Linked context

Real behavior proof

Behavior addressed: Session memory files no longer contain duplicate assistant messages when using models with thinking/reasoning enabled

Real setup tested:

  • Runtime: Node v24.13.1 on Linux x86_64
  • Repository: OpenClaw git worktree at commit ab1ef1cd62
  • Verification method: Imported the actual getRecentSessionContent() function from src/hooks/bundled/session-memory/transcript.ts via tsx (TypeScript runtime), created a real session JSONL file with duplicate assistant messages (simulating the thinking-stripped scenario), and verified the output correctly deduplicates.

Exact steps or command run after fix:

import { getRecentSessionContent } from "./src/hooks/bundled/session-memory/transcript.ts";

// Create session JSONL with duplicate assistant (thinking-stripped)
const sessionLines = [
  '{"type":"message","message":{"role":"user","content":...}}',
  '{"type":"message","message":{"role":"assistant","content":...,"text":"Hi there!"}}',
  '{"type":"message","message":{"role":"assistant","content":...,"text":"Hi there!"}}', // duplicate
  '{"type":"message","message":{"role":"user","content":...}}',
  '{"type":"message","message":{"role":"assistant","content":...,"text":"Let me check."}}',
];

const output = await getRecentSessionContent(sessionFile, 10);

After-fix evidence:

=== Real Behavior Proof: PR #94211 ===
Runtime: Node v24.13.1 on linux x64
Repository: OpenClaw worktree (commit ab1ef1cd62)

--- Test: Session with duplicate assistant messages ---
Created session file with 5 entries
  (entry[1]: assistant 'Hi there!')
  (entry[2]: DUPLICATE assistant 'Hi there!')
  (entry[4]: unique assistant 'Let me check...')

Output:
user: Hello
assistant: Hi there! How can I help?
user: What is the weather?
assistant: Let me check the weather for you.

=== Verification ===
Duplicate 'Hi there!' lines: 1 (expected 1) ✅
Unique 'Let me check' preserved: ✅

--- Regression test: Clean session (no duplicates) ---
Output lines: 4 (expected 4)
All messages preserved: ✅

Before/after comparison:

Scenario Before fix After fix
2 consecutive identical assistant texts Both included (duplicate) Only first included ✅
Normal interleaved user/assistant Unchanged Unchanged ✅
Clean session (no duplicates) All messages preserved All messages preserved ✅

What was not tested: End-to-end test with a real model that produces thinking content (requires credentials). The logic is straightforward string comparison operating on the actual getRecentSessionContent() function imported from OpenClaw source.

Tests and validation

  • pnpm test -- --run src/hooks/bundled/session-memory/handler.test.ts — 23/23 passed
  • Additionally verified by importing and executing getRecentSessionContent() directly from OpenClaw source on Node v24.13.1 with a real session JSONL file containing duplicates

Risk checklist

Did user-visible behavior change? (Yes)

  • Session memory files no longer contain duplicate assistant entries when thinking is enabled

Did config, environment, or migration behavior change? (No)

Did security, auth, secrets, network, or tool execution behavior change? (No)

What is the highest-risk area?

  • The dedup could theoretically skip a legitimate consecutive assistant message

How is that risk mitigated?

  • Dedup is scoped to role === "assistant" && text === lastAssistantText — only fires on exact text match of consecutive assistant messages
  • The lastAssistantText is reset on each non-assistant message
  • Direct runtime verification confirms clean sessions are unaffected

Current review state

What is the next action?

  • Maintainer review

Which bot or reviewer comments were addressed?

  • ClawSweeper: needs real behavior proof from a real setup

…stripped

When a model with thinking/reasoning is used, the session JSONL persists
two copies of each assistant response: the raw version (with thinking
blocks + text) and a cleaned version (text only). The session-memory
hook's getRecentSessionContent() treated both as valid, producing
duplicate lines in generated memory files.

Fix by tracking the last assistant text and skipping consecutive
duplicates. This handles the dedup at the consumer level without
changing the storage layer.

Fixes openclaw#92563.
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. size: XS labels Jun 17, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the linked session-memory bug is real, but this text-equality dedupe branch is conflicting and broader than the transcript-mirror shape; the open, clean, proof-positive semantic candidate at #94401 now owns the safe landing path.

Root-cause cluster
Relationship: superseded
Canonical: #94401
Summary: This PR is an overlapping text-dedupe candidate for the same session-memory duplicate assistant-lines bug; the clean semantic delivery-mirror candidate is the safer canonical landing path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this overlapping text-dedupe branch, land the semantic delivery-mirror filter in #94401 or an equivalent narrow fix, then close the canonical issue after merge.

So I’m closing this here and keeping the remaining discussion on #94401.

Review details

Best possible solution:

Close this overlapping text-dedupe branch, land the semantic delivery-mirror filter in #94401 or an equivalent narrow fix, then close the canonical issue after merge.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main and v2026.6.9 extract every assistant text row into session-memory output, so a raw thinking row followed by a delivery-mirror text row duplicates assistant lines.

Is this the best way to solve the issue?

No. The consumer-layer location is plausible, but plain text equality is broader than the reported raw/cleaned transcript artifact; delivery-mirror metadata or parent-lineage filtering is narrower and safer.

Security review:

Security review cleared: Security review cleared: the diff only changes local session-memory transcript filtering and adds no dependency, workflow, secret, auth, network, or command-execution surface.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • Vincent Koc: Current blame and symbol history point to recent work on the session-memory transcript helper and shared transcript-only assistant surfaces. (role: recent area contributor; confidence: high; commits: 52251261ca0f, c109a7623b13, c645ec4555c0; files: src/hooks/bundled/session-memory/transcript.ts, src/shared/transcript-only-openclaw-assistant.ts, src/config/sessions/transcript.ts)
  • Peter Steinberger: The internal hooks system introduced the bundled session-memory boundary, and delivered-message mirroring is part of the transcript shape behind this bug. (role: historical feature contributor; confidence: medium; commits: faba508fe0ae, fdaeada3ec76; files: src/hooks/bundled/session-memory/handler.ts, src/hooks/bundled/session-memory/transcript.ts, src/config/sessions/transcript.ts)
  • Andy: Recent adjacent history added redundant delivery-mirror dedupe in the session transcript facade, which is the same transcript artifact family this PR is trying to consume safely. (role: adjacent session transcript contributor; confidence: medium; commits: e95efa437381; files: src/config/sessions/transcript.ts)

Codex review notes: model internal, reasoning high; reviewed against cf67d8dded29.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 18, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@Pandah97

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #94401. ClawSweeper determined the semantic transcript-mirror filter in #94401 is the canonical fix path.

@Pandah97

Copy link
Copy Markdown
Contributor Author

Superseded by #94401

@Pandah97 Pandah97 closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

session-memory hook duplicates assistant messages when thinking is stripped

1 participant