Sign up
View stargazers13Sign up

refactor(web): timeline work entry rows and tool icons live in their own files#22674

merged1
refactor/messages-timeline-minimap
refactor/messages-timeline-work-entry-rows
opened4d agobyJJulius Marminge
7 files changed
View on GitHubTwo-way sync enabledPR last imported 4d ago

Part of the MessagesTimeline split. This layer moves the work-entry rows out:

  • timelineRowContext.ts: the row contexts (TimelineRowCtx, TimelineRowActivityCtx, WorkGroupViewCtx) and their types, shared by every row file.
  • TimelineRowParts.tsx: small shared row pieces (author heading, timestamp, shimmer overlay).
  • TimelineToolActivityIcon.tsx: tool and work-entry icons.
  • TimelineWorkEntryRow.tsx: SimpleWorkEntryRow, WorkEntryLogRow, reasoning trace, question history, thread-read labels.
  • TimelineWorkEntryRow.logic.ts: the pure helpers (icon names, expanded tool-call body, remarkThoughtPreview).
FileBeforeAfter
MessagesTimeline.tsx5,4414,279

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 work entry rows and tool icons live in their own files39d56c9
MmacroscopeappBot4d ago
Synced from GitHub

Approvability

Verdict: Approved at 39d56c9

Macroscope's review found this PR approvable — This PR reorganizes existing timeline row, icon, context, and helper code into focused files without introducing new product behavior or changing existing render wiring. The only other change is a test import update, keeping the runtime impact limited to module boundaries.

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

CcoderabbitaiBot4d 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: 07375135-2a86-4520-8c7f-de2b196d6a1f
📥 Commits

Reviewing files that changed from the base of the PR and between 60e318155ef80a03a4d28cb4857a994e755d829f and 39d56c9f958192fdbf6fcc33cb3f0ec31832497c.

📒 Files selected for processing: 7
  • apps/web/src/components/chat/MessagesTimeline.browser.test.tsx
  • apps/web/src/components/chat/MessagesTimeline.tsx
  • apps/web/src/components/chat/TimelineRowParts.tsx
  • apps/web/src/components/chat/TimelineToolActivityIcon.tsx
  • apps/web/src/components/chat/TimelineWorkEntryRow.logic.ts
  • apps/web/src/components/chat/TimelineWorkEntryRow.tsx
  • apps/web/src/components/chat/timelineRowContext.ts

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

📝 Walkthrough

The chat timeline’s row state, shared UI, tool icons, work-entry logic, and work-entry rendering now reside in dedicated modules. MessagesTimeline imports these modules, and the browser test imports the reasoning preview from its new location.

Changes: Chat timeline row extraction

Layer / File(s)Summary
Timeline row contracts and shared UI
apps/web/src/components/chat/timelineRowContext.ts, apps/web/src/components/chat/TimelineRowParts.tsx		The new context module defines shared row state, activity state, work-group state, and timeline row types. The new UI module provides the author heading, timestamp, and activity overlay.
Tool icons and work-entry logic
apps/web/src/components/chat/TimelineToolActivityIcon.tsx, apps/web/src/components/chat/TimelineWorkEntryRow.logic.ts		Tool icon rendering handles fallback icons, favicons, themed logos, and native-app assets. Work-entry helpers select icons, build expanded content, and format reasoning previews.
Work-entry rendering and timeline integration
apps/web/src/components/chat/TimelineWorkEntryRow.tsx, apps/web/src/components/chat/MessagesTimeline.tsx, apps/web/src/components/chat/MessagesTimeline.browser.test.tsx		The new row module renders work-entry content and interactions. MessagesTimeline imports the extracted modules instead of defining those responsibilities locally. The browser test imports remarkThoughtPreview from its new module.

Priority: ⬇️ Low

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

Change: Refactor

Merge Risk: ⚪ Minimal · up to 39d56

This refactor moves timeline row code into separate files without intended behavior changes. No concrete merge-blocking risk was found.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the refactor and provides detailed verification results, but it omits the required Problem section and Scope and approval information. It also does not use the required templa…Add a Problem section with the underlying problem and expected behavior. Add Scope and approval with a triaged issue, maintainer approval, or a clear explanation of why this focused refactor is exempt. Keep the existing change and verificat…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main refactor: moving timeline work-entry rows and tool icons into separate files.
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

Explanation

The description explains the refactor and provides detailed verification results, but it omits the required Problem section and Scope and approval information. It also does not use the required template headings.

Resolution

Add a Problem section with the underlying problem and expected behavior. Add Scope and approval with a triaged issue, maintainer approval, or a clear explanation of why this focused refactor is exempt. Keep the existing change and verification details, and organize them under the template headings.

✨ 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.