Skip to content

Refresh persisted chat history once after an idle reattachment - #856

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
Maya-Kid:fix/chat-history-reattach-handoff
Sep 23, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
Maya-Kid:fix/chat-history-reattach-handoff

Conversation

@Maya-Kid

@Maya-Kid Maya-Kid commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Review follow-up (2026-09-23)

Current head: 188100410dd4ea2acd8c10c406d1fbac02e4b1c4. Comment-only follow-up to #854 review: document the new-conversation route/ownership/live-entry handoff, correct the old early-return comment, and provide a complete optional Playwright installation and browser-test invocation at the top of chat-view.test.mjs. These browser probes are explicitly separate from Vitest and CI; no new CI scope is introduced. The changes are carried through the #854 → #856 → #855 stack.

The documented command passed on #854 (11 subtests, 12 including the parent); the final stack passed all 27 browser subtests (29 including parents). Runtime code and assertions are unchanged. Prior validation below refers to its stated historical heads.


Post-merge synchronization (2026-09-22)

Rebased onto dev@7cfeedfeb11c3a1c42d5091c5941a51afaaeb161 after #848, #851, #849, #850 and #852 merged. Current head: ba964f3aaa94ae514f9e585c04329f642708299d.

The UI dependency order remains #854 → #856 → #855. Retained upstream stream-interruption strings alongside the history/pagination keys when resolving i18n conflicts. The complete UI tree is byte-for-byte identical to the previous validated integration tree. Maintainer visual review remains outstanding.

Validation on combined tree d06f4ad0199a382635a6bbf94210b9421cbeba66 (675 tracked Linux inputs hash-matched):

Earlier evidence below refers to its stated historical heads.


Depends on #854 (view-request ownership). This branch includes that prerequisite so stale refreshes cannot overwrite a later navigation or send. The additional commits for this behavior are cb82751 and 9afbe39; after #854 merges, GitHub's diff will reduce to those changes.

An answer can commit after the browser reads a user-only history but before it reattaches to the live producer. The server then correctly returns idle, while the page remains stuck with the unanswered question. On that initial idle handoff, read the persisted history once and apply it only while the initiating view request still owns the page. If it is still unanswered, display a bounded idle status; a failed read offers an explicit retry. This never re-POSTs the question or recursively reattaches.

Validation, based on dev@ea0557b plus #854, final head 9afbe39:

  • Four new history-handoff browser regressions fail before this change while the 11 prerequisite cases pass.
  • All 16 browser subtests pass after the fix: the 11 prerequisite cases plus completed-between-reads, bounded user-only history, late refresh after navigation, retry after refresh failure, and a new send taking ownership before the old refresh arrives. The tests render the real Chat/router/query/live-answer implementation with controlled HTTP responses.
  • Frontend 116 module tests and build/typecheck/guard pass; the final additional send-race case and typecheck pass as well.
  • Chrome with the actual Linux backend and PostgreSQL held reattachment until a locally scripted model's answer had committed and the producer had unregistered. The real idle response caused exactly one extra detail GET, rendered the saved answer, and caused zero chat POSTs.
  • Integration tree f7bcd0878bf490d6efb122b71d6f13d379df4709, including Hand off exhausted tool runs to an evidence-only final answer #845 and Preserve exact chat outcomes across stream closure and reattachment #851's strict stream terminal handling, passes Rust fmt, strict workspace Clippy, 1,013 tests (one pre-existing external-HTTPS RSS test ignored), workspace build, 129 frontend module tests, 27 browser subtests and frontend build.

Run node --test tests/chat-view.test.mjs from web, with optional CHAT_PLAYWRIGHT_PATH and CHAT_CHROMIUM_PATH. No new dependency, polling loop, model retry, or server generation policy is introduced. All model integration evidence uses a local scripted endpoint, not a paid model.

WaylandYang
WaylandYang previously approved these changes Sep 23, 2026

@WaylandYang WaylandYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed as the second commit pair of the stack (#854 ⊂ #856 ⊂ #855), so only the
+231 on top of #854.

The idle handoff is the right shape: reattachChat answering idle means nothing is
running now, which says nothing about whether the answer committed between the
history read and the attach, and the one re-read closes exactly that window. The
guards are the ones that matter: checkedIdle so it can never recurse, handle
null so a snapshot that did arrive wins, ownsView(owner) before and after the
await so a navigation or a new send() (which claims a new owner and clears
idleHistoryKey) makes the late read a no-op. historyTurns pulled out so the two
readers can't drift is a good small move.

Two things to know, neither blocking:

  • This is one extra GET .../detail on every open of a conversation whose last turn
    is the user's, whether or not anything was ever running. That is the honest cost
    of not trusting the first read; fine at chat scale.
  • The "no active answer" line keys on idleHistoryKey === loadedKey === viewKey(...)
    and !streaming, so it disappears the moment a new send begins. Good; just noting
    it is the only place the flag is read, in case someone later reuses it.

Same caveat as #854: the 197 test lines are on-demand browser probes, not CI. LGTM.

Signed-off-by: dada-yan <BinjunYann@gmail.com>
Signed-off-by: dada-yan <BinjunYann@gmail.com>
@WaylandYang
WaylandYang dismissed their stale review September 23, 2026 18:45

The merge-base changed after approval.

@WaylandYang
WaylandYang force-pushed the fix/chat-history-reattach-handoff branch from 6549f40 to cd87e88 Compare September 23, 2026 18:45
@WaylandYang

Copy link
Copy Markdown
Contributor

Rebased this branch onto dev as a maintainer edit so it can merge: #854 landed as a squash (9fa75e4), which left the two original #854 commits here conflicting with their own squashed copy. The branch is now exactly your two commits (083f4ed → 192a1fa, 1881004 → cd87e88) on top of current dev; authorship and sign-offs are unchanged, and pnpm guard / typecheck / test (132) / build pass on the result. If you have local work on this branch, git fetch && git reset --hard origin/fix/chat-history-reattach-handoff before continuing. #855 will get the same treatment after this merges.

@WaylandYang
WaylandYang merged commit 315f46c into deeplethe:dev Sep 23, 2026
7 checks passed
@WaylandYang WaylandYang mentioned this pull request Sep 25, 2026
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