Skip to content

🤖 fix: chat layout tears — sticky in-flow composer dock + first-paint readiness barrier#3501

Open
ammar-agent wants to merge 7 commits into
mainfrom
chat-layout-jyvs
Open

🤖 fix: chat layout tears — sticky in-flow composer dock + first-paint readiness barrier#3501
ammar-agent wants to merge 7 commits into
mainfrom
chat-layout-jyvs

Conversation

@ammar-agent

@ammar-agent ammar-agent commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Eliminates the chat view's post-paint layout shifts ("tears"/"flashes") with two architectural changes: (1) the composer dock becomes in-flow sticky scroll content, so transcript clearance is reserved by flow layout in the same pass as any dock height change; and (2) a first-paint readiness barrier makes the chat view reveal once, fully formed — decorations (background bashes, compaction warning, concurrent-local warning, chat instructions, …) can no longer "load in" a few frames after the transcript is visible and shift it.

Background

Two distinct flash classes were observed after hydration:

  1. Measurement-lag tears — the composer floated out of flow and clearance was reserved via a ResizeObserver-published --composer-h padding. The measurement channel lags layout by ≥1 frame, so any decoration mount painted the last message under the composer for a frame, then snapped.
  2. Pop-in shifts — decoration data sources deliver their initial snapshot via async IPC after first paint. Their stores could not distinguish "not yet loaded" from "known empty", so the UI painted "unknown" as "absent" and corrected later — e.g. BackgroundProcessesBanner popped in on every workspace visit because BackgroundBashStore deleted its cache on unsubscribe.

Implementation

Sticky in-flow composer dock (kills class 1)

The dock is now the last child of the transcript scrollport (position: sticky; bottom: 0), after the bottom-anchoring sentinel. Clearance is the dock itself — exact, and reserved in the same layout pass as any height change. The measured --composer-h channel, reanchorBottom(), and all LayoutStackLane height-reservation machinery are deleted (~250 LoC of correction logic). Dock growth happens below the sentinel where native anchoring can't compensate, so useAutoScroll's safety-net ResizeObserver now observes the scrollport and each direct child, re-pinning pre-paint while bottom-locked.

First-paint readiness barrier (kills class 2)

New contract in useChatViewDataReady"unknown is not empty": every async source that can change the chat view's initial layout must expose a latched, synchronous "state known" signal and register it in the barrier. computeChatViewReveal then derives a single reveal decision: the hydration skeleton holds (and the decoration lane stays empty) until history is caught up and all sources are known, so transcript + decorations mount in one commit. Readiness is monotonic per mounted workspace — the barrier only ever delays the initial mount, never unmounts visible decorations. A 2s resilience deadline force-reveals if a source hangs (every source also self-heals to "known" on error), so a broken backend degrades to the old pop-in behavior instead of a stuck skeleton.

Sources registered:

  • Background bashesBackgroundBashStore gains a state-known signal; caches are now retained across unsubscribe so revisits render last-known state synchronously.
  • Cross-workspace activity (concurrent-local warning) — known once the initial activity.list() snapshot applies.
  • Providers configuseProvidersConfig is rebacked by a new app-wide ProvidersConfigStore (one fetch + one onConfigChanged subscription per session instead of one per mount across ~12 consumers). Fixes CompactionWarning/CodexOauthWarningBanner popping in on every mount, and makes optimistic updates visible to all consumers.
  • Session usage (compaction warning) — per-workspace known-flag on fetch settle.
  • Chat instructions scratchpad — now hydrated at workspace open (previously only when the Instructions tab was opened, so the decoration silently never showed until then; it now appears at open when saved instructions exist).

data-loaded (perf tests + story play helpers) now includes readiness, so anything waiting on it observes the final post-reveal layout.

Validation

  • New unit tests: BackgroundBashStore known-signal/retention/self-heal, ProvidersConfigStore shared-fetch/self-heal/optimistic-vs-stale, computeChatViewReveal branch matrix, safety-net observer coverage in useAutoScroll.
  • tests/ui: chat (15 suites), config, layout, workspaces, compaction, review — green.
  • tests/e2e: composerLayoutStability (clientHeight invariant across send + pinned bottom with grown dock) and all 6 perf.workspaceOpen profiles (proves data-loaded reaches true with the barrier in a real Electron app).

Risks

  • ChatPane reveal gating is on the hottest UI path. The skeleton now also waits for decoration readiness; a wedged source delays reveal by up to 2s (bounded, self-healing, and almost always masked by history replay which is slower than the one-round-trip sources). Mitigated by error-path self-heal on every source + deadline fallback.
  • ProvidersConfigStore changes config consumption app-wide (Settings, model selector, costs). Semantics preserved (loading until first fetch settles; explicit refresh() after saves; onConfigChanged for external edits), but any consumer relying on a per-mount refetch would now see session-cached data until an event fires.
  • Scroll behavior around the sticky dock is covered by the e2e layout gate; happy-dom suites assert the DOM/style contract.

Pains

  • happy-dom cannot evaluate native scroll anchoring or sticky layout, so layout invariants are split across tests/ui (structural contract) and tests/e2e (pixels), and e2e requires xvfb-run in this environment.

Generated with mux • Model: anthropic:claude-fable-5 • Thinking: xhigh • Cost: $30.66

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

ℹ️ 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".

@ammar-agent ammar-agent changed the title 🤖 fix: reserve composer clearance via in-flow sticky dock to kill chat layout tears 🤖 fix: chat layout tears — sticky in-flow composer dock + first-paint readiness barrier Jun 9, 2026
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Pushed a second architectural layer on top of the sticky dock: a first-paint readiness barrier (useChatViewDataReady) so decoration data sources that arrive over async IPC (background bashes, activity, providers config, session usage, chat instructions) can never mount after the transcript reveals and shift layout. See the updated PR description for the full picture.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6e83ed982

ℹ️ 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".

Comment thread src/browser/stores/WorkspaceStore.ts Outdated
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed: the session-usage "known" flip now runs after the stale-response version guard (both success and failure paths), so only the latest request can mark usage known and the reveal can't race a pending refresh.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb357392ee

ℹ️ 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".

Comment thread src/browser/stores/WorkspaceStore.ts
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

Re: thread PRRT_kwDOPxxmWM6IT5Ol (reset usage readiness per refresh) — keeping the latch is intentional, now documented at the sessionUsageKnown declaration:

  • Resetting readiness when setActiveWorkspaceId re-fetches would flip useChatViewDataReady to false on every workspace switch, unmounting and remounting the entire composer decoration lane — a guaranteed flash on every revisit, traded against a rare one (usage changed while inactive and crossed the compaction threshold).
  • That rare case is a genuine live state change; the barrier's contract is "never paint unknown as absent", not "freeze data that legitimately updates". Post-reveal updates land like any other live event (and the sticky-dock + scrollport-children ResizeObserver keep the transcript pinned through them).

Resolving the thread on that basis.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Please take another look. The session-usage latch rationale is now documented inline; no behavior change in this round.

@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Two commits since the last review: (1) documented the deliberate per-session latch on session-usage readiness (no behavior change), (2) wired ProvidersConfigStore in the useRouting hook test which bypasses AppLoader.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3bfe06b9a0

ℹ️ 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".

Comment thread src/browser/stores/ProvidersConfigStore.ts
@ammar-agent

Copy link
Copy Markdown
Collaborator Author

@codex review

Addressed: the live onConfigChanged iterator is now tracked on the store instance and force-closed (return()) in setClient on client swap — including the subscribe-in-flight race — mirroring the old hook's cleanup and BackgroundBashStore's bookkeeping.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

ℹ️ 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".

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