Skip to content

refactor(app): extract session turn changes#674

Merged
Astro-Han merged 3 commits into
devfrom
codex/message-timeline-turn-changes
May 16, 2026
Merged

refactor(app): extract session turn changes#674
Astro-Han merged 3 commits into
devfrom
codex/message-timeline-turn-changes

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

@Astro-Han Astro-Han commented May 16, 2026

Topology Update (2026-05-16)

Final base: dev after #672 was squash merged.
Dependency status: true stack dependency on the message-timeline owner context from #670/#671/#672; this PR has been restacked to latest dev.
Review order: review and merge after #672, before #675.
Latest head after restack and review follow-up: 8981175f0.

Summary

Extracted MessageTimeline turn-change ownership into packages/app/src/pages/session/session-turn-changes.tsx and added focused tests for the extracted pure helpers.

Why

This continues the #601 message-flow architecture stack after #672. MessageTimeline was still carrying turn-change fetch/cache/retry/dialog state inside the render component, which made the launch-path timeline harder for another agent to read or safely split before virtualization work.

Related Issue

Human Review Status

Fresh-eyes review clean. The follow-up review after 8981175f0 found no P0-P3 issues and confirmed the restack diff only contains this PR's turn-change extraction changes.

Review Focus

  • Confirm the PR only moves the turn-change owner out of MessageTimeline and keeps SessionTurn props/behavior unchanged.
  • Check the new session-turn-changes.tsx boundary, especially conflict dialog retry flow and blocked toast descriptions.
  • Confirm the restack diff contains only this PR's turn-change extraction changes.

Risk Notes

Behavior is intended unchanged. Main risk is wiring regression in undo/redo turn-change fetch or conflict-confirm retry because that logic moved modules. No platform, dependency, persistence, or migration change.

Architecture Boundary

Owner lane: #601 message flow.

Base/depends on: dev. #667, #669, #670, #671, and #672 are merged into dev; #675 depends on this PR.

Touched files:

  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session/session-turn-changes.tsx
  • packages/app/src/pages/session/session-turn-changes.test.ts

Architecture effect:

  • owner extracted
  • verification added
  • message-timeline.tsx remains large but sheds turn-change ownership in this slice

Review Follow-up

  • Gemini blocked-copy separator threads fixed and resolved. 730de8916 added the missing separator in the focused fake translator path.
  • Fresh-eyes P2 locale separator finding fixed in 8981175f0; real locales continue to own their separator prefix, avoiding duplicate punctuation in English and Chinese.

How To Verify

Focused tests: bun test --preload ./happydom.ts src/pages/session/session-turn-changes.test.ts src/pages/session/session-message-comments.test.ts -> 8 pass, 0 fail
bun run typecheck -> 8 successful, 8 total
Targeted #600 timeline perf: bun script/e2e-local.ts -- e2e/perf/perf-probe.spec.ts --grep "long-session-input-lag|session-timeline-recompute" -> 1 passed, 1 skipped
git diff --check origin/dev...HEAD && git diff --check -> pass
GitHub required checks on 8981175f0: all passed, including perf-probe-baseline
Review threads: all resolved

Screenshots or Recordings

Not included. This PR is a behavior-preserving controller extraction with no intended visible UI or copy change, so Electron manual visual verification was not run.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has type, primary area, and priority labels, or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev after the merged stack base, and my PR title and commit messages use Conventional Commits in English

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 52 minutes and 43 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3107c955-53d5-4abd-a91e-fa9aeafe19a4

📥 Commits

Reviewing files that changed from the base of the PR and between 4dfea3f and 8981175.

📒 Files selected for processing (3)
  • packages/app/src/pages/session/message-timeline.tsx
  • packages/app/src/pages/session/session-turn-changes.test.ts
  • packages/app/src/pages/session/session-turn-changes.tsx
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/message-timeline-turn-changes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@Astro-Han Astro-Han added P2 Medium priority ui Design system and user interface app Application behavior and product flows tech-debt Internal cleanup and maintainability debt labels May 16, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the MessageTimeline component by extracting turn-change management logic—including fetching, caching, retries, and conflict dialogs—into a new createSessionTurnChanges controller. This change reduces the complexity of the timeline component, updates the frontend architecture manifest, and introduces unit tests for the extracted logic. Feedback identifies a missing separator in the blockedTurnChangeDescription function that results in poorly formatted file lists, and suggests updating the associated test expectations to reflect the corrected string concatenation.

Comment thread packages/app/src/pages/session/session-turn-changes.tsx
Comment thread packages/app/src/pages/session/session-turn-changes.test.ts Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 16, 2026

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / homepage-cold 40 -> 32 (-8) 56 -> 72 (+16) 88 -> 65 (-23) 38 -> 15 (-23) 33.4 -> 33.3 (-0.1) 133.4 -> 100 (-33.4) 3 -> 3 (0) 0 -> 0 (0) pass
default / long-session-input-lag 40 -> 48 (+8) 48 -> 48 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.8 -> 16.7 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 48 -> 40 (-8) 72 -> 56 (-16) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 50 -> 16.8 (-33.2) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 24 (+8) 16 -> 24 (+8) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.7 -> 16.7 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 24 (0) 32 -> 24 (-8) 65 -> 66 (+1) 15 -> 16 (+1) 33.4 -> 33.4 (0) 150 -> 166.6 (+16.6) 1 -> 2 (+1) 0 -> 0 (0) pass
default / terminal-side-panel-open 48 -> 48 (0) 48 -> 56 (+8) 0 -> 0 (0) 0 -> 0 (0) 33.3 -> 33.4 (+0.1) 33.3 -> 33.4 (+0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 24 -> 24 (0) 32 -> 32 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls
low-end / session-scroll-reading-long 48 -> 48 (0) 48 -> 64 (+16) 72 -> 88 (+16) 35 -> 49 (+14) 16.8 -> 16.8 (0) 83.4 -> 83.3 (-0.1) 2 -> 3 (+1) 0.011 -> 0.011 (0) pass
low-end / session-timeline-recompute 120 -> 120 (0) 128 -> 120 (-8) 105 -> 102 (-3) 160 -> 170 (+10) 83.4 -> 83.4 (0) 166.7 -> 183.3 (+16.6) 3 -> 3 (0) 0.081 -> 0.081 (0) pass
low-end / concurrent-shimmer-extreme 0 -> 0 (0) 0 -> 0 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass

@Astro-Han Astro-Han force-pushed the codex/message-timeline-turn-changes branch from db3cbf5 to 7a78f36 Compare May 16, 2026 13:47
@Astro-Han Astro-Han changed the base branch from codex/message-comment-strip-contract to dev May 16, 2026 13:47
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Suggested priority: P2 (includes user-path files (packages/app/src/pages/session/message-timeline.tsx, packages/app/src/pages/session/session-turn-changes.test.ts, packages/app/src/pages/session/session-turn-changes.tsx)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han merged commit ec854d7 into dev May 16, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows P2 Medium priority tech-debt Internal cleanup and maintainability debt ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant