Skip to content

fix: resolve race between the chat greeting and history restore - #275

Open
cpsievert wants to merge 2 commits into
mainfrom
fix/greeting-history-restore-race
Open

fix: resolve race between the chat greeting and history restore#275
cpsievert wants to merge 2 commits into
mainfrom
fix/greeting-history-restore-race

Conversation

@cpsievert

Copy link
Copy Markdown
Collaborator

Problem

When both a greeting and chat history are enabled, the greeting could show at the wrong time.

  • Reloading a page that restores an existing conversation would briefly flash the app's greeting before the restored transcript replaced it.
  • Starting a new chat in a session that began by restoring an existing conversation would never show a greeting again for the rest of that session.

To reproduce (before this fix):

Chat(
    id="chat",
    client=my_client,
    greeting=chat_greeting("## Welcome!", persistent=True),
    history=HistoryOptions(...),
)
  1. Load the app, send a message (this saves a conversation).
  2. Reload the page — the conversation restores, but the greeting flashes briefly first.
  3. Open the history drawer and click "New chat" — no greeting ever appears again this session.

Root cause

The greeting used to resolve via an independent client-side signal ({id}_greeting_requested, which fires whenever the chat is visible, empty, and has no greeting) that raced the server's async history-restore decision. Whichever finished first determined the (inconsistent) outcome.

Fix

Greeting resolution now defers to the same "is this conversation restored?" decision HistoryController already computes — once at startup, and now also on every new_chat() — instead of racing the client's independent request:

  • HistoryController.replay_ui() (used both on initial restore and when switching conversations from the drawer) now explicitly clears any greeting, since a restored conversation is never a "new chat".
  • HistoryController.new_chat() now re-resolves the greeting directly, so a fresh conversation always gets one — even in a session that began by restoring an existing conversation.

Applied symmetrically in pkg-py and pkg-r.

Testing

  • New Playwright e2e coverage: no greeting flash on restore, greeting clears when switching to an old conversation, and greeting reappears after "New chat" in a session that started by restoring a conversation.
  • Unit tests for the new HistoryController.on_settled/notify_settled hook in both languages.
  • Existing greeting e2e tests were switched to use persistent=True greetings — a plain-string greeting is auto-dismissed by the client regardless of this fix, so those tests passed even against the buggy code (a false negative).

Resolving the app greeting used to race an independent client-side
`{id}_greeting_requested` request against the server's async history
restore: reloading into a restored conversation could flash the
greeting before the restore replaced it, and a restored conversation's
transcript had no way to suppress a stale/incoming greeting.

Greeting resolution now defers to the same "is this conversation
restored?" decision `HistoryController` already computes on init and
on `new_chat()`, instead of a separate client-driven signal. Restoring
a conversation (whether on initial load or via the history drawer)
now explicitly clears any greeting, since a restored conversation is
never a "new chat".
Mirrors the pkg-py fix: resolving the app greeting used to race an
independent client-side `{id}_greeting_requested` request against the
server's async history restore, which could flash the greeting on a
restored conversation, or leave a fresh conversation with no greeting
at all if the session began by restoring one.

Greeting resolution now defers to the same "is this conversation
restored?" decision `HistoryController` already computes on init and
on `new_chat()`. Restoring a conversation now explicitly clears any
greeting, since a restored conversation is never a "new chat".
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.

1 participant