Sign up
View stargazers13Sign up

refactor(web): timeline row derivation, position restore and find reader live in hooks#22682

merged1
refactor/messages-timeline-assistant-event-rows
refactor/messages-timeline-scroll-find-hooks
opened3d agobyJJulius Marminge
5 files changed
View on GitHubTwo-way sync enabledPR last imported 3d ago

Part of the MessagesTimeline split. This layer moves the scroll and find machinery out of ConversationTimeline into hooks. Effect bodies, dependencies and effect order are unchanged.

  • useTimelinePositionRestore.ts: the list-opened tracker and the layout effect that restores a thread's remembered scroll position. setPositionedThreadKey is passed straight through as onPositioned.
  • useThreadFindPositionReader.ts: the layout effect that publishes the find position reader.
  • useTimelineRows.ts: useIncrementalTimelineRows, useStableRows, useStableHandoffRuns. The pure sameRowsInput and handoffRunsSignature go to MessagesTimeline.logic.ts.
FileBeforeAfter
MessagesTimeline.tsx1,6441,380

Measurements

Dev app on a seeded copy of real data, median of 3, React renders counted by the commit-root counter (only fibers React actually rendered). "Before" is main at 6a6c84f990; "after" is the top of this stack. Nested update peak comes from a probe in react-dom's getRootForUpdatedFiber.

InteractionBefore: commits / renders / timeline renders / nested peakAfter
Scroll an 8.5k-item thread up 40 × 150px43 / 1,656 / 70 / 044 / 1,670 / 70 / 0
Load earlier turns, then scroll up 60 × 300px107 / 5,316 / 356 / 0104 / 5,275 / 356 / 0
Agent streams a 20-line reply (561-item thread)63 / 3,092 / 164 / 155 / 2,728 / 139 / 0

The extractions keep every component, context, and hook boundary unchanged, so render counts match within run-to-run noise. Streaming numbers vary with reply length. The win is readability: each row family is now a file you can read on its own, and the React Compiler caches each one independently. The legend-list wiring is unchanged. That covers getItemType with its length buckets (#21269), TimelineRowCtx inside renderItem with extraData carrying sharedState (#21123), and scroll restore. No update loops appeared while loading earlier turns.

Tests

  • vp test run MessagesTimeline logic, active-tools, and browser tests, plus timelineScrollAnchoring
  • reactCompilerCoverage for every touched and new file (0 bailouts)
  • tsc-rs --noEmit for apps/web, vp lint on changed files

🤖 Generated with Claude Code

Checks

All checks passed

Added 1 commit

  • refactor(web): timeline row derivation, position restore and find reader live in hooks52061da
MmacroscopeappBot3d ago
Synced from GitHub

Approvability

Verdict: Approved at 52061da

Macroscope's review found this PR approvable — This PR extracts existing timeline derivation, position restoration, and find-reader logic into focused hooks without changing their call sites or behavior. The changes are a contained local refactor with no product-default, schema, deployment, security, billing, or authentication impact.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot3d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: daab09da-d590-414d-914f-d7e0f5d0bca1
📥 Commits

Reviewing files that changed from the base of the PR and between 1637084a952a326e21b87ba3a1d6d9a029f633de and 52061dad19113a8357711aec08affb1671de7624.

📒 Files selected for processing: 5
  • apps/web/src/components/chat/MessagesTimeline.logic.ts
  • apps/web/src/components/chat/MessagesTimeline.tsx
  • apps/web/src/components/chat/useThreadFindPositionReader.ts
  • apps/web/src/components/chat/useTimelinePositionRestore.ts
  • apps/web/src/components/chat/useTimelineRows.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

MessagesTimeline now delegates timeline row stabilization, position restoration, and thread-find position reading to extracted hooks. The row input contract is exported, and helper functions compare row inputs and create handoff-run signatures.

Changes: Timeline hooks

Layer / File(s)Summary
Timeline row derivation and stabilization
apps/web/src/components/chat/MessagesTimeline.logic.ts, apps/web/src/components/chat/useTimelineRows.ts, apps/web/src/components/chat/MessagesTimeline.tsx		The row input type is exported, and helpers compare row inputs and create handoff-run signatures. New hooks stabilize handoff runs and timeline rows. MessagesTimeline removes the inline row-stabilization logic.
Timeline position restoration
apps/web/src/components/chat/useTimelinePositionRestore.ts, apps/web/src/components/chat/MessagesTimeline.tsx		A hook restores remembered row or scroll positions, handles cancellation and citation positioning, and reports positioning and manual navigation. MessagesTimeline passes it the timeline state and callbacks.
Thread-find position reader
apps/web/src/components/chat/useThreadFindPositionReader.ts, apps/web/src/components/chat/MessagesTimeline.tsx		A hook builds and publishes a reader that resolves timeline rows and folds to thread-find positions. MessagesTimeline passes it the reader ref and timeline inputs.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 52061

This refactor moves timeline row, scroll-restore and find-reader logic into hooks without intended behavior change. No concrete merge-blocking risk was identified.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the implementation and provides detailed verification results, but it does not include the required Problem section or Scope and approval information.Add a Problem section that states the underlying problem and expected behavior. Add a Scope and approval section with the triaged issue or maintainer approval, including the approval comment, or explain why this focused refactor qualifies f…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly summarizes the main refactor: moving timeline row derivation, position restoration, and find-reader logic into hooks.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Resolution

Add a Problem section that states the underlying problem and expected behavior. Add a Scope and approval section with the triaged issue or maintainer approval, including the approval comment, or explain why this focused refactor qualifies for an exemption.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.