Skip to content

refactor(app): extract session page support owners#682

Merged
Astro-Han merged 3 commits into
devfrom
codex/session-page-support-owners
May 16, 2026
Merged

refactor(app): extract session page support owners#682
Astro-Han merged 3 commits into
devfrom
codex/session-page-support-owners

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

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

Summary

  • Extracted session page support owners from packages/app/src/pages/session.tsx: route prompt bootstrap, session diagnostics, deferred render scheduling, and revert support.
  • Kept session.tsx as orchestration for the route while moving side-effect helpers into focused files.
  • Reduced packages/app/src/pages/session.tsx from 691 LOC to 491 LOC.
  • Review follow-up: createSessionDeferredRender now uses createEffect for rAF/timeout side effects and has browser-condition behavior coverage.

Why

Owner lane: #601 message flow.

This is a flat, independently reviewable frontend governance slice. It lowers the session launch-path route component below the 500 LOC warning line without changing visible behavior or public contracts.

Related Issue

Related to #601 and #599.

Human Review Status

Ready once CI is fully green. Gemini review thread was fixed and resolved; latest fresh-eyes review found no actionable P0/P1/P2/P3 issues. At last gate check, GitHub CI was still waiting on perf-probe-baseline, so the PR was not merged.

Review Focus

  • Confirm the extracted helpers preserve the previous route prompt bootstrap, diagnostics emission, deferred render, and revert support side effects.
  • Confirm session.tsx remains orchestration-only and the PR does not depend on other open feature branches.
  • Confirm deferred render still starts non-deferred on initial session and only schedules rAF/timer after session key changes.

Topology

  • Base: dev
  • Dependency status: independent flat PR; no parent or child PR.
  • Review order: can be reviewed after chore: add frontend architecture governance baseline #667/chore(ui): add shared tool contract export #669 for governance context, but it is mergeable on its own once checks are green.
  • Touched files:
    • packages/app/src/pages/session.tsx
    • packages/app/src/pages/session/session-page-support-source.test.ts
    • packages/app/src/pages/session/use-session-deferred-render.ts
    • packages/app/src/pages/session/use-session-deferred-render.test.ts
    • packages/app/src/pages/session/use-session-page-diagnostics.ts
    • packages/app/src/pages/session/use-session-revert-support.ts
    • packages/app/src/pages/session/use-session-route-prompt-bootstrap.ts

Architecture Effect

  • Owner extracted: route prompt bootstrap, diagnostics, deferred render, and revert support now have focused owners.
  • LOC reduced: session.tsx 691 -> 491.
  • Verification added: source-boundary test keeps these support owners out of the route component; deferred render behavior test locks rAF/timer scheduling and cancellation.
  • Public contract: unchanged; no package exports or runtime routes changed.

Risk Notes

Behavior unchanged. Main risk is a wiring regression in session prompt bootstrap, diagnostics emission, deferred first-frame rendering, or revert toast/store snapshot handling.

How To Verify

Focused app tests: 1125 pass, 0 fail
Command: bun --cwd packages/app test --preload ./happydom.ts src/pages/session/session-page-support-source.test.ts src/pages/session/use-session-deferred-render.test.ts src/pages/session/use-session-hash-scroll.test.ts src/shell-frame-contract.test.ts

Typecheck: 8 successful, 8 total
Command: bun run typecheck

Lint: passed
Command: bun run lint:ci

Diff check: no whitespace errors
Command: git diff --check origin/dev...HEAD && git diff --cached --check

Session route E2E: 2 passed
Command: bun --cwd packages/app test:e2e e2e/app/session.spec.ts -g "can open an existing session|session composer matches"

Baseline E2E notes:
- `bun --cwd packages/app test:e2e e2e/app/session.spec.ts -g "session timeline visible content is not narrower than composer shell"` fails the same way on latest `dev` because the test selector `[data-component="session-prompt-dock"] [data-dock-surface="shell"]` is missing.
- `bun --cwd packages/app test:e2e e2e/session/session-undo-redo.spec.ts -g "slash undo sets revert"` fails the same way on latest `dev` waiting for a seeded `[data-message-id]`.

GitHub CI: all completed jobs were green at last gate check, but `perf-probe-baseline` was still pending, so this PR was not merged yet.

Screenshots or Recordings

Not run. This is a structural owner extraction with no intended visible UI, copy, style, or interaction change. Electron manual verification was not run; web session-route E2E and focused behavior tests are listed above.

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, and my PR title and commit messages use Conventional Commits in English

@Astro-Han Astro-Han added app Application behavior and product flows ui Design system and user interface tech-debt Internal cleanup and maintainability debt labels May 16, 2026
@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 40 minutes and 11 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: d9b4d945-d75f-4823-8ea5-cd1f8a818044

📥 Commits

Reviewing files that changed from the base of the PR and between 361d533 and c713b27.

📒 Files selected for processing (7)
  • packages/app/src/pages/session.tsx
  • packages/app/src/pages/session/session-page-support-source.test.ts
  • packages/app/src/pages/session/use-session-deferred-render.test.ts
  • packages/app/src/pages/session/use-session-deferred-render.ts
  • packages/app/src/pages/session/use-session-page-diagnostics.ts
  • packages/app/src/pages/session/use-session-revert-support.ts
  • packages/app/src/pages/session/use-session-route-prompt-bootstrap.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/session-page-support-owners

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 the P2 Medium priority 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 user-path files (packages/app/src/pages/session.tsx, packages/app/src/pages/session/session-page-support-source.test.ts, packages/app/src/pages/session/use-session-deferred-render.ts, packages/app/src/pages/session/use-session-page-diagnostics.ts, packages/app/src/pages/session/use-session-revert-support.ts, packages/app/src/pages/session/use-session-route-prompt-bootstrap.ts)).

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.

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 session.tsx page by extracting complex logic into several dedicated hooks: createSessionPageDiagnostics, createSessionDeferredRender, useSessionRoutePromptBootstrap, and createSessionRevertSupport. This modularization improves the maintainability of the main component and is accompanied by a new test file to verify the extraction. Feedback was provided regarding the createSessionDeferredRender hook, suggesting the use of createEffect instead of createComputed for managing side effects like timers and animation frames, which aligns better with SolidJS best practices.

Comment thread packages/app/src/pages/session/use-session-deferred-render.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 24 -> 24 (0) 48 -> 40 (-8) 76 -> 63 (-13) 26 -> 13 (-13) 16.8 -> 16.8 (0) 166.6 -> 166.6 (0) 4 -> 3 (-1) 0 -> 0 (0) pass
default / long-session-input-lag 48 -> 48 (0) 48 -> 64 (+16) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 40 -> 48 (+8) 64 -> 56 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 16 (0) 16 -> 16 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 16.8 -> 16.8 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 24 (0) 24 -> 32 (+8) 63 -> 62 (-1) 13 -> 12 (-1) 49.9 -> 33.3 (-16.6) 166.6 -> 166.7 (+0.1) 3 -> 2 (-1) 0 -> 0 (0) pass
default / terminal-side-panel-open 48 -> 48 (0) 56 -> 56 (0) 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 32 -> 24 (-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 64 -> 56 (-8) 64 -> 56 (-8) 59 -> 67 (+8) 13 -> 26 (+13) 16.8 -> 16.8 (0) 66.6 -> 66.7 (+0.1) 1 -> 2 (+1) 0.011 -> 0.011 (0) pass
low-end / session-timeline-recompute 112 -> 120 (+8) 120 -> 120 (0) 104 -> 104 (0) 149 -> 164 (+15) 83.3 -> 100 (+16.7) 166.7 -> 166.7 (0) 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.8 -> 16.7 (-0.1) 33.4 -> 16.8 (-16.6) 0 -> 0 (0) 0 -> 0 (0) pass

@Astro-Han Astro-Han merged commit 6c643a4 into dev May 16, 2026
28 checks passed
@Astro-Han Astro-Han deleted the codex/session-page-support-owners branch May 16, 2026 12:11
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