fix(web): stabilize reload and add E2E quality gates (#584) - #595
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughE2E実行基盤をdev/production対応に再構成し、動的ポート・隔離ランタイム・シードデータを導入しました。主要ルートのSSR/CSR描画、CCIP状態管理、イベント購読を更新し、通信障害・復旧・SSE・レスポンシブ表示などのテストを追加しました。 ChangesE2EとUI動作
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Playwright
participant E2EApp
participant Database
Browser->>Playwright: ルート遷移・API操作
Playwright->>E2EApp: 遅延・失敗・SSE応答を制御
E2EApp->>Database: シードデータ・CCIP状態を取得
Database-->>E2EApp: メディア・ベクトル情報を返却
E2EApp-->>Browser: ローディング・復旧・検索結果を表示
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/ui/src/event-stream.test.ts (1)
44-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBFCache時にストリームを維持する挙動を正しく検証。
persisted=trueの場合に abort されないこと、unsubscribe()呼び出し時には abort されることを確認できています。ただしwindow.removeEventListenerが実際に呼ばれたかまでは検証していません（event-stream.ts側の 90-92 行目のクリーンアップ処理）。vi.spyOn(page, "removeEventListener")などで呼び出しを確認するテストを追加すると、リスナーリーク回帰の検出力が上がります。🤖 Prompt for 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. In `@packages/ui/src/event-stream.test.ts` around lines 44 - 59, Extend the test for BFCache handling around subscribeToEventStream by spying on page.removeEventListener before subscribing, then assert it is called during unsubscribe(). Keep the existing assertions that persisted=true does not abort the stream immediately and that unsubscribe() aborts it.apps/server/src/routes/sources/$mediaSourceId/$mediaId/index.tsx (1)
46-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
MediaRouteContent内のClientOnlyは冗長な可能性があります
Media()のisMounted/ShowによってMediaRouteContentはすでにクライアント側マウント後にのみ描画されます。その内側で再度ClientOnly fallback={<MediaRouteFallback />}を挟んでいますが、search.tsxの同種パターン（SearchRouteContentはマウントゲートのみでClientOnlyを使わない）とは異なり、二重のガードになっています。動作上の問題はありませんが、両ルートでパターンを揃えると保守性が上がります。🤖 Prompt for 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. In `@apps/server/src/routes/sources/`$mediaSourceId/$mediaId/index.tsx around lines 46 - 55, MediaRouteContent 内の冗長な ClientOnly ガードを削除し、MediaContent を直接レンダリングしてください。Media() の isMounted/Show によるマウント後の描画は維持し、SearchRouteContent と同じ単一のマウントゲートパターンに揃えてください。apps/server/playwright.config.ts (1)
3-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
E2eMode型と環境変数スナップショット処理の重複。
E2eMode型定義とgetE2eMode/getEnvironment相当のロジックが、本ファイルとrun-e2e.tsのcreateEnvironment、e2e-server.tsのgetMode/serverEnvironmentにほぼ同一の実装で重複しています。すでにgetE2eModeとe2e-server.tsのgetModeはエラーメッセージが異なるなど僅かに乖離しており、今後さらに分岐する恐れがあります。共通モジュール(例:fixture.tsや新規e2e-env.ts)に集約することを推奨します。♻️ 共通化の方向性
// apps/server/src/tests/e2e/support/e2e-env.ts (新規) export type E2eMode = "dev" | "production"; export function getE2eMode(): E2eMode { const mode = process.env.E2E_MODE; if (mode === "dev" || mode === "production") return mode; throw new Error("E2E_MODE must be set to dev or production."); } export function snapshotEnv(): Record<string, string> { return Object.fromEntries( Object.entries(process.env).flatMap(([key, value]) => value === undefined ? [] : [[key, value]], ), ); }🤖 Prompt for 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. In `@apps/server/playwright.config.ts` around lines 3 - 21, 重複しているE2eMode定義と環境変数処理を共通モジュールへ集約し、playwright.config.tsのgetE2eMode/getEnvironment、run-e2e.tsのcreateEnvironment、e2e-server.tsのgetMode/serverEnvironmentをその共有実装へ置き換えてください。モード検証と環境スナップショットの既存動作を維持し、エラーメッセージも共通化してください。
🤖 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 @.github/workflows/e2e.yml:
- Line 21: Update the actions/checkout@v4 step in the workflow to explicitly
disable persisted Git credentials by setting persist-credentials to false, while
preserving the existing checkout behavior.
In `@apps/server/scripts/e2e-server.ts`:
- Around line 251-260: Update linkProductionDependencies to handle an existing
outputDir/node_modules path without failing on symlink’s EEXIST error. Preserve
the current symlink behavior when the destination is absent, and use an
existence check or equivalent force-safe handling so production E2E startup
continues when Nitro already created the directory.
In `@apps/server/vite.config.ts`:
- Around line 16-17: Update the E2E port initialization near e2ePort and
e2eHmrPort to obtain E2E_PORT and E2E_HMR_PORT through
getRequiredE2eEnvironment, matching the validation used for E2E_ROUTE_TREE_PATH,
E2E_OUTPUT_DIR, and E2E_RUNTIME_DIR. Remove the silent 3100/3101 fallbacks while
preserving numeric parsing of the validated values.
---
Nitpick comments:
In `@apps/server/playwright.config.ts`:
- Around line 3-21:
重複しているE2eMode定義と環境変数処理を共通モジュールへ集約し、playwright.config.tsのgetE2eMode/getEnvironment、run-e2e.tsのcreateEnvironment、e2e-server.tsのgetMode/serverEnvironmentをその共有実装へ置き換えてください。モード検証と環境スナップショットの既存動作を維持し、エラーメッセージも共通化してください。
In `@apps/server/src/routes/sources/`$mediaSourceId/$mediaId/index.tsx:
- Around line 46-55: MediaRouteContent 内の冗長な ClientOnly ガードを削除し、MediaContent
を直接レンダリングしてください。Media() の isMounted/Show によるマウント後の描画は維持し、SearchRouteContent
と同じ単一のマウントゲートパターンに揃えてください。
In `@packages/ui/src/event-stream.test.ts`:
- Around line 44-59: Extend the test for BFCache handling around
subscribeToEventStream by spying on page.removeEventListener before subscribing,
then assert it is called during unsubscribe(). Keep the existing assertions that
persisted=true does not abort the stream immediately and that unsubscribe()
aborts it.
🪄 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: 30f65b89-e573-4757-9733-1b3e11007378
📒 Files selected for processing (30)
.github/workflows/e2e.yml.gitignoreapps/server/package.jsonapps/server/playwright.config.tsapps/server/scripts/e2e-server.tsapps/server/scripts/run-e2e.tsapps/server/src/components/media/media-sidebar.tsxapps/server/src/infrastructure/api/routers/ai-router.tsapps/server/src/router.tsxapps/server/src/routes/config.tsxapps/server/src/routes/manager.tsxapps/server/src/routes/search.tsxapps/server/src/routes/sources/$mediaSourceId/$mediaId/index.tsxapps/server/src/routes/sources/index.tsxapps/server/src/tests/e2e/ccip-flow.spec.tsapps/server/src/tests/e2e/loading-recovery.spec.tsapps/server/src/tests/e2e/pages.spec.tsapps/server/src/tests/e2e/realtime-preservation.spec.tsapps/server/src/tests/e2e/route-reload.spec.tsapps/server/src/tests/e2e/search.responsive.spec.tsapps/server/src/tests/e2e/support/fixture.tsapps/server/src/tests/e2e/support/test.tsapps/server/src/tests/integration/ai/lancedb-ccip-vector-store.test.tsapps/server/tsconfig.jsonapps/server/vite.config.tsmise.tomlpackages/ui/src/event-stream.test.tspackages/ui/src/event-stream.tspackages/ui/src/media-sidebar.tsxpackages/ui/src/screens/search-screen.tsx
💤 Files with no reviewable changes (1)
- apps/server/src/tests/e2e/pages.spec.ts
This was referenced Jul 12, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
SSR/F5・CCIP反映を安定化し、#576 の後続となる Web/Chromium 品質ゲートを追加します。
変更内容
ローカル検証
bun run --cwd apps/server test
このコマンドはunit、integration、dev E2E、fresh production E2Eを順に実行します。
実行結果
Fixes #584
Summary by CodeRabbit
新機能
バグ修正
テスト