fix(session-memory): deduplicate assistant messages when thinking is stripped#94211
fix(session-memory): deduplicate assistant messages when thinking is stripped#94211Pandah97 wants to merge 1 commit into
Conversation
…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.
|
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 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 detailsBest 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:
Codex review notes: model internal, reasoning high; reviewed against cf67d8dded29. |
|
Superseded by #94401 |
Summary
parentpointing to the first message)getRecentSessionContent()→extractTextMessageContent()treated both as valid text messages, producing duplicateassistant: ...lines in generated memory filesFixes #92563
Linked context
message_endevent handler persists both raw and cleaned versions when thinking content is present — this design choice is preserved; the fix handles the dedup at the consumer levelReal behavior proof
Behavior addressed: Session memory files no longer contain duplicate assistant messages when using models with thinking/reasoning enabled
Real setup tested:
ab1ef1cd62getRecentSessionContent()function fromsrc/hooks/bundled/session-memory/transcript.tsviatsx(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:
After-fix evidence:
Before/after comparison:
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 passedgetRecentSessionContent()directly from OpenClaw source on Node v24.13.1 with a real session JSONL file containing duplicatesRisk checklist
Did user-visible behavior change? (
Yes)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?
How is that risk mitigated?
role === "assistant" && text === lastAssistantText— only fires on exact text match of consecutive assistant messageslastAssistantTextis reset on each non-assistant messageCurrent review state
What is the next action?
Which bot or reviewer comments were addressed?