Skip to content

fix(chat): dedupe uploaded-file pending turns - #2906

Merged
1 commit merged into
nesquena:masterfrom
ai-ag2026:fix/upload-pending-user-dedupe
May 25, 2026
Merged

1 commit merged into
nesquena:masterfrom
ai-ag2026:fix/upload-pending-user-dedupe

Conversation

@ai-ag2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Hermes WebUI keeps active chat turns coherent across the persisted transcript, pending turn metadata, and browser-side optimistic state.
  • Uploaded-file turns can render an optimistic user bubble before the server returns the pending message text with an [Attached files: ...] suffix.
  • fix(chat): dedupe uploaded-file optimistic user turns #2723 already normalized that suffix for the INFLIGHT tail merge path.
  • The pending-session merge helper used during active-session restore still inserted the server pending message without checking transcript identity first.
  • This PR applies the same _sameTranscriptMessage identity check to that pending merge path so the same uploaded-file user turn is rendered once.

What Changed

  • Reuses _sameTranscriptMessage(existing, pendingMsg) in _mergePendingSessionMessage() before inserting a server pending user message.
  • Extends the existing uploaded-file dedupe regression to pin the pending-session merge path, not only the INFLIGHT tail merge.

Why It Matters

Verification

  • python3 -m pytest -q tests/test_regressions.py::test_inflight_merge_dedupes_uploaded_user_message tests/test_issue2341_pending_user_reattach.py -o addopts='' -> 3 passed
  • node --check static/sessions.js
  • git diff --check origin/master...HEAD
  • Added-line public hygiene/secret scan -> 0 hits

Risks / Follow-ups

  • Low risk: one guard in the pending merge helper, using the same transcript identity helper already used by the adjacent INFLIGHT merge logic.
  • This does not attempt a broader session-state cleanup; it only closes the missing pending-merge dedupe seam.

Model Used

  • AI-assisted via OpenAI GPT-5.5 in Hermes/TARS with terminal, file-edit, git, and GitHub CLI tooling.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Reading the diff at cron-pr-2906 (single line added at static/sessions.js:650) and cross-referencing the _mergeInflightTailMessages dedupe at static/sessions.js:1307-1324: this is the symmetric companion to #2723's INFLIGHT-tail dedupe. Where #2723 covered the live-stream reattach path, this PR covers the persisted-session-restore path that loadSession() runs when no INFLIGHT cache is present (e.g. switch-away-and-back to a session whose pending turn has already been committed server-side).

Code reference

The change at static/sessions.js:646-657:

function _mergePendingSessionMessage(session,messages){
  if(!Array.isArray(messages)) return false;
  const pendingMsg=typeof getPendingSessionMessage==='function'?getPendingSessionMessage(session,messages):null;
  if(!pendingMsg) return false;
  if(messages.some(existing=>_sameTranscriptMessage(existing,pendingMsg))) return false;
  const liveAssistantIdx=messages.findIndex(m=>m&&m.role==='assistant'&&m._live);
  if(liveAssistantIdx>=0) messages.splice(liveAssistantIdx,0,pendingMsg);
  else messages.push(pendingMsg);
  return true;
}

The dedupe key is _sameTranscriptMessage (static/sessions.js:1294-1305), which strips [Attached files: ...] for user-role comparisons — exactly the suffix introduced at static/messages.js:439:

else if(uploaded.length)msgText=`${text}\n\n[Attached files: ${uploadedPaths.join(', ')}]`;

So a server-pending message containing the suffix and a transcript copy without it (or vice versa) compare equal at the identity level.

Diagnosis / Recommendation

The fix is correct and minimal. Specific observations:

  1. Coverage symmetry. _mergeInflightTailMessages at static/sessions.js:1320 already filters against _sameTranscriptMessage for the INFLIGHT-tail path, but _mergePendingSessionMessage was inserting the server pending blindly. After this PR both paths apply the same identity rule, which is the right shape.

  2. Search-window scope. The new check uses messages.some(...) — full-array scan — while _mergeInflightTailMessages uses a bounded tail window (merged.slice(-Math.max(5,tail.length+2))). The full scan is actually fine here because there's only ever one pending message per session at a time, and loadSession() runs on transcript restore (not in a streaming hot path). No perf concern.

  3. Test guard at tests/test_regressions.py:668-673 is targeted:

pending_idx = src.find("function _mergePendingSessionMessage")
assert pending_idx >= 0, "pending session merge helper not found"
pending_block = src[pending_idx:pending_idx+500]
assert "_sameTranscriptMessage(existing,pendingMsg)" in pending_block, ...

A 500-char window is tight enough that future refactors of the helper would have to keep the dedupe lookup adjacent, which is the invariant being preserved. Good.

  1. One edge case to confirm manually: if getPendingSessionMessage returns a different pending message than what's in the transcript (e.g. user edited mid-restore, or pending text differs from final-persisted user content beyond just the attachments suffix), the new check would correctly let it through. This matches the policy of fix(chat): dedupe uploaded-file optimistic user turns #2723. Worth a quick sanity check that no other normalization in getPendingSessionMessage would break the identity comparison.

Test plan

The regression assertion in tests/test_regressions.py catches future drift. Manual verification:

  • Open a chat, attach a file, send. Switch to another session before the assistant responds, then switch back. Confirm the user turn appears exactly once (no duplicate with/without the [Attached files: ...] suffix).
  • Same flow but on a fresh browser load (no INFLIGHT cache): confirms the _mergePendingSessionMessage path is exercised rather than _mergeInflightTailMessages.
  • Send a non-attachment message and switch away/back. Confirms the dedupe doesn't false-positive on identical pure-text turns.

LGTM. Small, surgical, and exactly mirrors #2723's contract on the other restore path.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Merged in Release DG / v0.51.135 (stage-batch17, batch with PRs #2906 #2912 #2917 #2919 #2921 #2922 #2927 #2936 #2940).

Thanks @ai-ag2026! 🚢

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants