fix(web): stabilize route loading and CCIP state - #593
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 (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughQuery設定と状態モデルを共通化し、Server/Tauriのloader、検索・一覧画面、ジョブイベント、CCIP処理、SSR/hydration検証を更新しました。 Changes非同期Query・SSR状態モデル
ジョブイベントとCCIP
SSR検証と開発支援
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
packages/ui/src/hooks/use-search-page.ts (1)
127-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
enabled: !isServerの共通化packages/ui/src/hooks/use-search-page.ts:127-150で同じガードを6回書いているので、clientOnlyQueryOptions相当の共通ヘルパーに切り出してまとめたいです。🤖 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/hooks/use-search-page.ts` around lines 127 - 150, 重複している enabled: !isServer の設定を共通化してください。use-search-page.ts の tags、sources、allProjects、allIps、allCharacters、allAuthors 各 createQuery 呼び出しで利用できる clientOnlyQueryOptions 相当のヘルパーを定義または既存のものを再利用し、各クエリの設定に適用してください。packages/ui/src/hooks/use-manager-page.ts (1)
227-242: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
clientOnlyQueryOptionsを共通化してください
enabled: !isServerの付与がuse-manager-page.tsとuse-search-page.tsで重複しています。source-media-page.tsxのローカルヘルパーをそのまま再利用する形ではなく、共通ユーティリティとして切り出して揃えると保守しやすくなります。🤖 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/hooks/use-manager-page.ts` around lines 227 - 242, use-manager-page.ts と use-search-page.ts に重複している enabled: !isServer の設定を共通化してください。source-media-page.tsx のローカルヘルパーは流用せず、clientOnlyQueryOptions を共通ユーティリティとして切り出し、projects、ipsQuery、characters、sourcesQuery などの createQuery 呼び出しで利用するよう統一してください。apps/server/src/components/media/media-sidebar.tsx (1)
109-147: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
getCcipVectorStatusにsignalを伝播する
apps/server/src/components/media/media-sidebar.tsxのAbortControllerは、apps/server/src/infrastructure/api-clients/ai-api.tsのgetCcipVectorStatusがsignalを受け取らないため実リクエストまで届いていません。signal?: AbortSignalを追加してorpc.ai.ccipVectorStatus(..., { signal })に渡すと、切り替え時やポーリング重複時の不要な通信を抑えられます。🤖 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/components/media/media-sidebar.tsx` around lines 109 - 147, Propagate cancellation from refreshCcipStatus through getCcipVectorStatus by adding an optional signal parameter to the API client method and passing it in the orpc.ai.ccipVectorStatus request options; update the caller to provide the AbortController signal so aborted refreshes cancel the underlying request.packages/application/src/services/ccip-vector-service.ts (1)
224-237: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
extractedAt === modifiedAtをテストに追加したい LanceDB 側はmediaModifiedAt/extractedAtをz.coerce.date()でDateに戻しているので型の懸念はないです。>=で equality を current 扱いにする意図だけ、ユニットテストで固定しておきたい。🤖 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/application/src/services/ccip-vector-service.ts` around lines 224 - 237, Add a unit test for the isCurrent method covering the case where record.extractedAt and media.modifiedAt are exactly equal, asserting the record is considered current. Keep the existing >= comparison in isCurrent unchanged and use the existing test fixtures/helpers for the CCIP model, embedding version, and matching media source ID.apps/server/src/tests/unit/infrastructure/events/realtime-event-bus.test.ts (1)
67-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win新規テストのアサーションが弱く、他テストからのイベント漏れを検出できない。
expect(listener).toHaveBeenCalledWith(event)は「一致する呼び出しが1つでもあればパス」するため、直前のテスト(46-66行目)で発行されたjob-progressイベントがrecentJobEventsに残っていて一緒に再送されても失敗しません。toHaveBeenCalledTimes(1)等で厳密化するか、テスト前にrecentJobEventsをクリアする仕組みを追加することを推奨します。🤖 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/tests/unit/infrastructure/events/realtime-event-bus.test.ts` around lines 67 - 79, Strengthen the “replays recent job events to a late subscriber” test by asserting the listener is called exactly once before verifying its argument, or clear the shared recent event history before publishing. Use the existing RealtimeEventBus.publishJob, subscribeToJobs, and cleanup setup so events from preceding tests cannot be accepted as valid replay results.packages/ui/src/screens/sources-screen.tsx (1)
60-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
state().phase === "empty"専用の表示がなく、"data" フェーズと区別されない。
QueryUiStateはemptyフェーズを定義していますが(PRの目的にも「pending/data/empty/error/offline/background-fetching の共通状態定義」とある)、このコンポーネントでは "empty" 時にグリッドが空のまま表示されるだけで、専用の空状態メッセージがありません。ユーザーには「読み込み中なのか、単に0件なのか」が分かりにくくなります。💡 追加案
+ <Show when={props.state().phase === "empty"}> + <div class="mt-8 text-center" role="status"> + <p class="text-muted-foreground">登録済みのメディアソースがありません</p> + </div> + </Show> + <Show when={props.state().phase === "pending"}>🤖 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/screens/sources-screen.tsx` around lines 60 - 99, QueryUiState の "empty" フェーズに専用表示がなく、空グリッドと区別できない。sources-screen.tsx の mediaSources() グリッド付近に state().phase === "empty" 用の Show を追加し、0件であることと必要な案内を表示する。既存の pending・error・offline 表示と重ならない条件にし、data フェーズの通常表示は維持する。
🤖 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 `@apps/server/src/infrastructure/events/realtime-event-bus.ts`:
- Around line 133-148: subscribeToJobs の deliveredEvents が購読中に無制限に増加しています。初回の
recentJobEvents リプレイ完了後、for ループ直後に deliveredEvents.clear()
を呼び出し、購読開始時の競合防止にのみSetを使用するよう修正してください。
In `@packages/ui/src/media-sidebar.tsx`:
- Around line 115-146: The refreshCcipStatus logic can indefinitely ignore a
“missing” response while the same active job ID remains, leaving polling pending
forever. Add bounded fallback state associated with the active job—such as a
retry counter or elapsed timeout—and in refreshCcipStatus mark the CCIP status
as failed, clear the active job ID, and set isCcipJobPending(false) once the
limit is reached, while resetting the counter when a valid status or different
job is observed.
In `@packages/ui/src/screens/sources-screen.tsx`:
- Around line 32-35: 日本語UI内に英語のフォールバック・ローディング文言が残っています。sources-screen.tsx の
errorMessage() にある "API connection failed" を周辺の表現に合わせた日本語へ変更し、保留状態を表示する箇所の
"Loading sources..." も日本語化してください。
---
Nitpick comments:
In `@apps/server/src/components/media/media-sidebar.tsx`:
- Around line 109-147: Propagate cancellation from refreshCcipStatus through
getCcipVectorStatus by adding an optional signal parameter to the API client
method and passing it in the orpc.ai.ccipVectorStatus request options; update
the caller to provide the AbortController signal so aborted refreshes cancel the
underlying request.
In `@apps/server/src/tests/unit/infrastructure/events/realtime-event-bus.test.ts`:
- Around line 67-79: Strengthen the “replays recent job events to a late
subscriber” test by asserting the listener is called exactly once before
verifying its argument, or clear the shared recent event history before
publishing. Use the existing RealtimeEventBus.publishJob, subscribeToJobs, and
cleanup setup so events from preceding tests cannot be accepted as valid replay
results.
In `@packages/application/src/services/ccip-vector-service.ts`:
- Around line 224-237: Add a unit test for the isCurrent method covering the
case where record.extractedAt and media.modifiedAt are exactly equal, asserting
the record is considered current. Keep the existing >= comparison in isCurrent
unchanged and use the existing test fixtures/helpers for the CCIP model,
embedding version, and matching media source ID.
In `@packages/ui/src/hooks/use-manager-page.ts`:
- Around line 227-242: use-manager-page.ts と use-search-page.ts に重複している enabled:
!isServer の設定を共通化してください。source-media-page.tsx
のローカルヘルパーは流用せず、clientOnlyQueryOptions
を共通ユーティリティとして切り出し、projects、ipsQuery、characters、sourcesQuery などの createQuery
呼び出しで利用するよう統一してください。
In `@packages/ui/src/hooks/use-search-page.ts`:
- Around line 127-150: 重複している enabled: !isServer
の設定を共通化してください。use-search-page.ts の
tags、sources、allProjects、allIps、allCharacters、allAuthors 各 createQuery
呼び出しで利用できる clientOnlyQueryOptions 相当のヘルパーを定義または既存のものを再利用し、各クエリの設定に適用してください。
In `@packages/ui/src/screens/sources-screen.tsx`:
- Around line 60-99: QueryUiState の "empty"
フェーズに専用表示がなく、空グリッドと区別できない。sources-screen.tsx の mediaSources() グリッド付近に
state().phase === "empty" 用の Show を追加し、0件であることと必要な案内を表示する。既存の
pending・error・offline 表示と重ならない条件にし、data フェーズの通常表示は維持する。
🪄 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: e8a3b857-269e-440e-8a28-36b7f4f5954a
📒 Files selected for processing (67)
.agents/skills/tanstack-ssr/SKILL.md.agents/skills/tanstack-ssr/agents/openai.yamlAGENTS.mdapps/server/src/components/media/media-sidebar.tsxapps/server/src/hooks/use-batch-job-events.tsapps/server/src/infrastructure/api-clients/error-policy.tsapps/server/src/infrastructure/api-clients/queries/index.tsapps/server/src/infrastructure/api-clients/search-api.tsapps/server/src/infrastructure/events/realtime-event-bus.tsapps/server/src/router.tsxapps/server/src/routes/__root.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/$mediaSourceId/components/source-media-page.tsxapps/server/src/routes/sources/$mediaSourceId/index.tsxapps/server/src/routes/sources/index.tsxapps/server/src/tests/e2e/pages.spec.tsapps/server/src/tests/unit/application/services/ccip-vector-service.test.tsapps/server/src/tests/unit/infrastructure/events/realtime-event-bus.test.tsapps/tauri/src/collections/authors-collection.tsapps/tauri/src/collections/characters-collection.tsapps/tauri/src/collections/ips-collection.tsapps/tauri/src/collections/projects-collection.tsapps/tauri/src/collections/query-keys.tsapps/tauri/src/collections/sources-collection.tsapps/tauri/src/collections/tags-collection.tsapps/tauri/src/hooks/use-batch-job-events.tsapps/tauri/src/infrastructure/api-clients/projects-api.tsapps/tauri/src/infrastructure/api-clients/search-api.tsapps/tauri/src/queries/index.tsapps/tauri/src/router.tsxapps/tauri/src/routes/config.tsxapps/tauri/src/routes/manager.tsxapps/tauri/src/routes/search.tsxapps/tauri/src/routes/sources/$mediaSourceId/$mediaId/index.tsxapps/tauri/src/routes/sources/$mediaSourceId/index.tsxapps/tauri/src/routes/sources/index.tsxpackages/application/src/services/ccip-vector-service.tspackages/client/src/api-error.test.tspackages/client/src/api-error.tspackages/client/src/create-client.test.tspackages/client/src/create-client.tspackages/client/src/index.tspackages/ui/src/hooks/use-batch-job-events.test.tspackages/ui/src/hooks/use-batch-job-events.tspackages/ui/src/hooks/use-manager-page.tspackages/ui/src/hooks/use-search-page.tspackages/ui/src/hooks/use-source-media-page.tspackages/ui/src/media-sidebar-content.tsxpackages/ui/src/media-sidebar.tsxpackages/ui/src/query-options/index.tspackages/ui/src/query-options/prefetch.tspackages/ui/src/query-options/query-client.test.tspackages/ui/src/query-options/query-client.tspackages/ui/src/query-options/search-query.test.tspackages/ui/src/query-options/search-query.tspackages/ui/src/query-options/sources-query.tspackages/ui/src/query-state.test.tspackages/ui/src/query-state.tspackages/ui/src/screens/manager-screen.tsxpackages/ui/src/screens/media-detail-screen.tsxpackages/ui/src/screens/search-screen.tsxpackages/ui/src/screens/source-media-screen.tsxpackages/ui/src/screens/sources-screen.tsxpackages/ui/src/source-media-page.tsx
💤 Files with no reviewable changes (2)
- apps/server/src/infrastructure/api-clients/error-policy.ts
- apps/server/src/routes/sources/$mediaSourceId/index.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/media-sidebar.tsx (1)
145-154: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win例外継続時にポーリングが停止しない(
missingと同種の無限ループ)
getCcipVectorStatus()が例外を投げ、かつactiveCcipJobId()が残っている場合、isCcipJobPendingが解除されないため 215-223 のインターバルが上限もバックオフもなく回り続けます。missing側はCCIP_MISSING_STATUS_LIMITで救済されましたが、例外側には同等の逃げ道がありません。同じカウンタで上限を設けて終端させることを推奨します。🛡️ 例外側にも上限を設ける修正案
} catch { if (ccipStatusRequestId() !== requestId) { return; } if (!activeCcipJobId()) { setCcipStatus("failed"); setActiveCcipJobId(null); setIsCcipJobPending(false); + return; + } + const missingStatusCount = ccipMissingStatusCount() + 1; + setCcipMissingStatusCount(missingStatusCount); + if (missingStatusCount >= CCIP_MISSING_STATUS_LIMIT) { + setCcipStatus("failed"); + setActiveCcipJobId(null); + setIsCcipJobPending(false); } }🤖 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/media-sidebar.tsx` around lines 145 - 154, 例外発生時に activeCcipJobId() が残るとポーリングが無限継続します。getCcipVectorStatus() の catch 処理に missing と同じカウンタ方式の上限(CCIP_MISSING_STATUS_LIMIT 相当)を追加し、上限到達時は setCcipStatus("failed")、setActiveCcipJobId(null)、setIsCcipJobPending(false) で終了させてください。
🤖 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.
Outside diff comments:
In `@packages/ui/src/media-sidebar.tsx`:
- Around line 145-154: 例外発生時に activeCcipJobId()
が残るとポーリングが無限継続します。getCcipVectorStatus() の catch 処理に missing
と同じカウンタ方式の上限(CCIP_MISSING_STATUS_LIMIT 相当)を追加し、上限到達時は
setCcipStatus("failed")、setActiveCcipJobId(null)、setIsCcipJobPending(false)
で終了させてください。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 088815fe-2bb0-4589-ac3e-a8cafc6c24bf
📒 Files selected for processing (4)
apps/server/src/components/media/media-sidebar.tsxapps/server/src/infrastructure/events/realtime-event-bus.tspackages/ui/src/media-sidebar.tsxpackages/ui/src/screens/sources-screen.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/ui/src/screens/sources-screen.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/server/src/infrastructure/events/realtime-event-bus.ts
- apps/server/src/components/media/media-sidebar.tsx
概要
Query-heavy routeのSSR/CSR境界と画面状態を整理し、メディア詳細の直接アクセス・リロード、CCIP完了反映、Find Similarを安定化します。
Closes #578
変更内容
検証
Summary by CodeRabbit