🤖 feat: show chat history loading above the composer - #4125
Conversation
This comment has been minimized.
This comment has been minimized.
ibetitsmike
changed the title
|
@codex review |
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e5c7c8874
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed all three findings in 42747ad:
Each production-only revert failed the corresponding regression assertion; restored code passed. Full local static checks and targeted tests passed, as did both real Storybook plays. The completed Codex status summary and clean security report are being collapsed because the existing CI gate mistakes them for unresolved findings. No substantive finding is hidden by that cleanup. |
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42747ad0eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed the duplicate loading announcement in 6e3559e. The skeleton owns the status role during initial hydration; the dock row owns it during cached replay. Desktop and phone plays assert exactly one exposed status while loading and none after catch-up. Reverting only the production role made both plays fail with two statuses, then restoration returned them green. Full static checks and targeted tests pass. |
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e3559e523
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed retry backoff in da65e70. Hydration remains active between attempts, including repeated failures before catch-up and disconnects after cached history. Retry cleanup also invalidates buffered stream/barrier state. Seven behavioral cases cover empty and cached transcripts, iterator ends, transport errors, client reconnects, initial missing clients, and deactivation during backoff. Reverting only production code failed all seven; restoration passed the full store/transport suites and static checks. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
## Summary Fix the transcript layout flash caused by the “Loading messages” indicator during history catch-up. Keep loading feedback out of the composer’s normal layout flow so its appearance/removal cannot resize the dock or move cached messages. ## Background Regression introduced by #4125, **“🤖 feat: show chat history loading above the composer”** (commit `d8d368d4bfd49f7b761e6b19d2923b0865458f07`). That change inserted a transient 24px row into the in-flow composer dock, changing transcript geometry on workspace switches and catch-up completion. ## Implementation - Position the compact, non-interactive loading indicator above the dock without consuming layout space. Keep a permanent transcript gutter so the badge cannot cover compact tail rows; the gutter does not toggle with loading. - Preserve editable/read-only replay feedback and existing stream/monitor suppression. Yield to “Jump to bottom” while scrolled up so the controls cannot overlap on phones. - Extend existing desktop and pinned-phone stories with long cached history, scroll navigation, non-overlap checks against the transcript content boundary, and assertions that catch-up preserves dock height, message position, and scroll height. ## Validation - `make static-check` passed. - Hydration/reveal and layout-stack unit tests: 8 passed. - Chromium Storybook Replay and Phone plays passed, including editable/read-only geometry and phone navigation. - Red/green check: returning the loading indicator to normal flow fails both stories on the 24px dock-height change (176 → 152); restoring the fix passes both. - Storybook coverage contract passed (38 tests). The separate snapshot-budget tests fail unchanged on the baseline as well (509 estimated snapshots versus a 305 cap); this change adds no stories or viewport variants. ## Risks The indicator is now a small overlay above the composer rather than an extra row. History/subscription state and composer behavior are unchanged. Desktop-agent visual inspection was unavailable in this SSH environment; responsive checks ran in real Chromium via the pinned-phone story. --- _Generated with `xum` • Model: `openai:gpt-6-astra` • Thinking: `medium` • Cost: `$9.15`_ <!-- mux-attribution: model=openai:gpt-6-astra thinking=medium costs=9.15 -->
1 participant
Summary
Show a compact loading indicator above the composer while chat history catches up, including when cached messages remain visible and when a workspace is transcript-only. Existing initialization, stream-preparation, streaming, and background-monitor barriers suppress the loading row.
Implementation
Render the indicator once in the shared composer dock, before asynchronous decorations are ready. Keep transcript hydration active across subscription retries and their backoff, so cached messages and loading feedback stay visible until catch-up completes or the workspace is deactivated.
Validation
Remaining remote UAT gaps
Two bounded remote UAT rounds on the initial revision support initial loading, cached-history retention, draft preservation, loader removal, rapid switching, and initialization. They did not establish unobscured phone layout, actual streaming tokens, preparation state, empty history, or delayed reconnect. The final round stalled at an unanswered project-trust dialog, not a proven product defect.
Proceeding with PR review as requested. Follow-up review fixes were validated locally, including browser plays, without another remote UAT round.
Remote recovery chat
Risks
The changed subscription state controls transient hydration UI. Regression coverage checks repeated retries, cached-row preservation, deactivation, and competing progress indicators.