Skip to content

refactor(ui): extract session turn changes panel#676

Merged
Astro-Han merged 4 commits into
devfrom
codex/session-turn-change-panel
May 16, 2026
Merged

refactor(ui): extract session turn changes panel#676
Astro-Han merged 4 commits into
devfrom
codex/session-turn-change-panel

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

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

Topology Update (2026-05-16)

Final base: dev.
Dependency status: independent flat PR and parent of the short SessionTurn stack. #677 is stacked on this PR because both slices modify packages/ui/src/components/session-turn.tsx, and reviewing the diff-summary extraction is cleaner after this panel extraction lands.
Review order: review and merge #676 before #677. This PR is independent from #667/#669 and from the message-timeline stack #670 -> #671 -> #672 -> #674 -> #675.
Latest head after review follow-up: d50f897bd.

Summary

Extracted the SessionTurn turn-change summary/action/file panel into packages/ui/src/components/session-turn-changes-panel.tsx and moved the turn-change action type into the existing session-turn-changes.ts contract helper.

Why

This continues the #601 message-flow architecture work. session-turn.tsx still mixed assistant rendering, legacy diff summary, and turn-change UI. This PR gives the turn-change panel a clear owner before the next #601 slice tackles the remaining session-turn.tsx over-500 debt.

Related Issue

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

  • Confirm the extracted panel preserves the existing data-slot names, action confirmation behavior, file expansion behavior, and open/show-in-folder callbacks.
  • Confirm the legacy diff summary still appears when turnChanges exists but has no visible entry for the current turn.
  • Confirm the turn-change file expansion state remains owned by SessionTurn so it survives panel hide/show transitions.
  • Confirm visibleTurnChange is initialized after working so initial renders with turn-change data cannot hit a Solid memo temporal-dead-zone error.
  • Confirm this PR does not touch assistant content rendering or product behavior beyond the behavior-preserving extraction and review fixes.

Risk Notes

Behavior is intended unchanged. Main risk is a UI wiring regression in turn-change file rows, legacy diff fallback visibility, or the two-click undo/redo confirmation because the panel moved to a separate component. No dependency, persistence, platform, or public package export change.

Architecture Boundary

Owner lane: #601 message flow.

Base/depends on: dev. #677 depends on this PR.

Touched files:

  • packages/ui/src/components/session-turn.tsx
  • packages/ui/src/components/session-turn-changes.ts
  • packages/ui/src/components/session-turn-changes-panel.tsx
  • packages/ui/src/components/session-turn-parent.test.ts

Architecture effect:

Review Follow-up

  • CodeRabbit legacy diff fallback thread fixed in e11b0cbce and resolved. The fallback now gates on hasVisibleTurnChanges(turnChange()) instead of the presence of the whole turnChanges map.
  • Fresh-eyes P3 expansion-state finding fixed in b0b187e35; turn-change expansion state is owned by SessionTurn and passed into the extracted panel.
  • Fresh-eyes P1 Solid memo initialization finding fixed in d50f897bd; visibleTurnChange is declared after working.
  • Added focused regression assertions covering the fallback gate, expansion-state ownership, and memo declaration order.

How To Verify

bun install --frozen-lockfile: ok in new worktree
Focused UI tests: bun test src/components/session-turn-turn-changes.test.ts src/components/session-turn-parent.test.ts -> 7 pass, 0 fail
bun run typecheck -> 8 successful, 8 total
Targeted perf gate: 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: rerunning after d50f897bd

Screenshots or Recordings

Not included. This PR is intended as a behavior-preserving component extraction with no visual or copy changes; 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 the declared 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

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: extracting the SessionTurn turn-changes panel into a new component.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is comprehensive and well-structured, following the template with all major sections completed including Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, Architecture Boundary, How To Verify, and Checklist.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/session-turn-change-panel

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 SessionTurn component by extracting the turn-change panel logic and UI into a new standalone component, SessionTurnChangesPanel. This change includes the creation of the new component file, updating type definitions in session-turn-changes.ts, and cleaning up session-turn.tsx by removing redundant logic and state. The architecture manifest was also updated to reflect this extraction. I have no feedback to provide as there were no review comments.

@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 32 -> 24 (-8) 64 -> 40 (-24) 65 -> 63 (-2) 15 -> 13 (-2) 33.3 -> 33.3 (0) 166.7 -> 116.8 (-49.9) 2 -> 3 (+1) 0 -> 0 (0) pass
default / long-session-input-lag 48 -> 48 (0) 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 -> 48 (0) 64 -> 64 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 33.4 -> 16.8 (-16.6) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 16 (0) 24 -> 16 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.7 (-0.1) 16.8 -> 16.7 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 32 (+8) 24 -> 32 (+8) 64 -> 65 (+1) 14 -> 17 (+3) 50 -> 50 (0) 99.9 -> 116.7 (+16.8) 3 -> 3 (0) 0 -> 0 (0) pass
default / terminal-side-panel-open 48 -> 48 (0) 56 -> 56 (0) 0 -> 0 (0) 0 -> 0 (0) 33.4 -> 33.3 (-0.1) 33.4 -> 33.3 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 24 -> 32 (+8) 32 -> 32 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.7 -> 16.8 (+0.1) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls
low-end / session-scroll-reading-long 72 -> 56 (-16) 96 -> 56 (-40) 90 -> 73 (-17) 63 -> 38 (-25) 16.8 -> 16.8 (0) 233.3 -> 166.7 (-66.6) 4 -> 3 (-1) 0.016 -> 0.011 (-0.005) pass
low-end / session-timeline-recompute 120 -> 120 (0) 136 -> 136 (0) 108 -> 106 (-2) 178 -> 175 (-3) 100 -> 100 (0) 183.3 -> 183.3 (0) 4 -> 3 (-1) 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.7 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass

@Astro-Han Astro-Han force-pushed the codex/message-timeline-dead-header branch from 0bca5d3 to aad77f6 Compare May 16, 2026 06:33
@Astro-Han Astro-Han force-pushed the codex/session-turn-change-panel branch from 77d1b82 to f0958b4 Compare May 16, 2026 06:33
@Astro-Han Astro-Han changed the base branch from codex/message-timeline-dead-header to dev May 16, 2026 06:34
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/ui/src/components/session-turn.tsx`:
- Around line 450-451: The current visibility gate uses props.turnChanges ===
undefined which hides the legacy diff fallback when turnChanges exists but lacks
this turn; update the condition to check the per-turn entry instead. Replace the
leading check props.turnChanges === undefined with a check that the per-turn
entry is missing (e.g., turnChange() == null or !turnChange()) so the whole
condition becomes something like: when={ (turnChange() == null ||
(turnChange()?.files.length ?? 0) === 0) && edited() > 0 && !working() }; keep
references to the existing helpers turnChange(), edited(), and working() when
making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 224ee396-19bd-46a3-ba20-7c2e7c14a063

📥 Commits

Reviewing files that changed from the base of the PR and between 03140f7 and f0958b4.

📒 Files selected for processing (3)
  • packages/ui/src/components/session-turn-changes-panel.tsx
  • packages/ui/src/components/session-turn-changes.ts
  • packages/ui/src/components/session-turn.tsx

Comment thread packages/ui/src/components/session-turn.tsx Outdated
@github-actions github-actions Bot removed the app Application behavior and product flows label May 16, 2026
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 non-doc, non-test paths outside the low-risk bucket).

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 e8a40d4 into dev May 16, 2026
28 of 29 checks passed
@Astro-Han Astro-Han deleted the codex/session-turn-change-panel branch May 16, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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