perf(ai): 大規模tagging/CCIP batch投入を非同期dispatch化 - #572
Merged
Conversation
- scan APIを対象件数(count)のみ返すように変更 - start APIをfilter/source/forceのみ受け取り、親job + dispatcher jobを即時作成 - dispatcherがkeyset paginationで500件ずつ子jobをbulk INSERT - 親job payloadからprocessedJobIdsを廃止し、processed/failed countのみ管理 - 子job resultにparentProcessedフラグを書き、再実行時の二重カウントを防止 - CCIP用batch_ccip_dispatch job typeを追加 - 進捗・失敗を構造化ログとrealtime eventへ出力 - UIからscan結果の選択グリッドを廃止 Closes #569
📝 WalkthroughWalkthroughtagging と CCIP のバッチ開始処理を、全件ID送信と即時大量ジョブ作成から、親ジョブとdispatchジョブによる非同期投入へ変更した。スキャン結果は件数返却に変わり、進捗集計は BatchProgress ベースに統一された。UI とクライアント配線も新しい契約に追従している。 Changesバッチ投入非同期dispatch化
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant AiRouter
participant JobDispatchService
participant CcipJobs
participant CcipVectorService
participant JobRepository
AiRouter->>JobRepository: batch_ccip_parent / batch_ccip_dispatch を作成
JobDispatchService->>CcipJobs: processBatchCcipDispatchJob(job)
CcipJobs->>JobRepository: 既存子ジョブを除外して medias をページング取得
CcipJobs->>CcipVectorService: listRecords で既存ベクトルを照合
CcipJobs->>JobRepository: extract_ccip_vector 子ジョブをchunk挿入
CcipJobs->>JobRepository: 親payload.total更新と job-progress 発行
JobDispatchService->>CcipJobs: processCcipExtractionJob(childJob)
CcipJobs->>JobRepository: incrementProgress / incrementFailedCount
CcipJobs->>JobRepository: finalizeBatchParent で completed / failed を確定
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4
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/hooks/use-manager-page.ts (1)
421-438: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
result.successが偽のときのフィードバックが欠落しています。
handleStartBatchTagging（440-461）には失敗時のelse分岐がありますが、handleStartBatchCcipExtractionにはありません。result.successまたはresult.jobIdが偽の場合、ステータスが「Starting...」のまま残り、ユーザーにエラーが伝わりません。整合性を合わせてください。🛠 修正案
if (result.success && result.jobId) { toast.success(result.message); setTaggingStatus("Batch CCIP extraction in progress..."); setActiveJobId(result.jobId); + } else { + toast.error("Failed to start batch CCIP extraction."); + setTaggingStatus("Failed to start batch CCIP extraction."); }🤖 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 421 - 438, `handleStartBatchCcipExtraction` is missing the failure path that `handleStartBatchTagging` already has, so when `actions.startBatchCcipExtraction` returns a non-success result or no `jobId`, the UI stays on “Starting...” with no user feedback. Update `handleStartBatchCcipExtraction` to mirror the `handleStartBatchTagging` pattern by adding an explicit `else` branch after the `result.success && result.jobId` check, and set both a toast error and an error `setTaggingStatus` message using the returned result/message so the user sees the failure state.
🧹 Nitpick comments (3)
apps/tauri/src/routes/manager.tsx (1)
61-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win新設したラッパー関数を再利用してインライン定義の重複を解消することを推奨します。
apps/tauri/src/infrastructure/api-clients/ai-api.tsにscanBatchCcipTargets/startBatchCcipExtractionのラッパーが追加されていますが、ここではclient.ai.*を直接呼ぶインライン定義になっており重複しています。加えてインライン側はforce: boolean（必須）で、ラッパー側はforce?: boolean（任意）と入力型が食い違っています。server 側manager.tsxと同様にラッパーを import して利用する形に揃えると、契約の一貫性と保守性が向上します。♻️ 提案: ラッパーを import して利用
import { scanBatchCcipTargets, scanBatchTaggingTargets, + startBatchCcipExtraction, startBatchTagging, } from "~/infrastructure/api-clients/ai-api";scanBatchTaggingTargets, startBatchTagging, - scanBatchCcipTargets: (input: { force: boolean; mediaSourceId?: string }) => - client.ai.scanBatchCcipTargets(input), - startBatchCcipExtraction: (input: { - force: boolean; - mediaSourceId?: string; - }) => client.ai.startBatchCcipExtraction(input), + scanBatchCcipTargets, + startBatchCcipExtraction, findDuplicateMedia,🤖 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/tauri/src/routes/manager.tsx` around lines 61 - 66, Reuse the newly added AI API wrapper functions instead of keeping inline client calls in manager.tsx: replace the direct client.ai.scanBatchCcipTargets and client.ai.startBatchCcipExtraction definitions with imports from ai-api.ts. Align the input contract to the wrapper signatures, especially the force field being optional in the wrapper, so the manager route and the shared API client stay consistent. Use the scanBatchCcipTargets and startBatchCcipExtraction symbols from ai-api.ts as the single source of truth and remove the duplicated inline definitions.packages/ui/src/screens/manager-screen.tsx (1)
250-253: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
renderMediaCardを削除
ManagerScreenPropsでしか使われていないため、renderMediaCardは不要です。ManagerScreenPropsから外し、併せてMediaSafeの import も整理してください。🤖 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/manager-screen.tsx` around lines 250 - 253, `renderMediaCard` is only used for `ManagerScreenProps`, so remove that prop from the `ManagerScreen` component contract and clean up the related `MediaSafe` import. Update the `ManagerScreen` type/implementation so it no longer expects or references `renderMediaCard`, and ensure any now-unused imports or symbols are removed from the same module.apps/server/src/infrastructure/jobs/ccip-jobs.ts (1)
118-134: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
ccipVectorService.listRecordsがループの各反復で全件取得されています。
forceが false の場合、whileループの反復ごとにlistRecords(mediaSourceId)を呼び、全レコードをMapに構築しています。対象が大きいほど反復回数が増え、同じ全件取得を繰り返すため負荷が高くなります。ループ外で一度取得して使い回すか、対象mediaIdに絞って取得する方法をご検討ください。🤖 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/infrastructure/jobs/ccip-jobs.ts` around lines 118 - 134, The `ccipVectorService.listRecords(mediaSourceId)` call inside the `while`-loop in `ccip-jobs.ts` is re-fetching all records on every iteration when `force` is false. Move the record lookup out of the loop in the job flow around `targetRows`/`rows.filter`, cache the `Map` once per media source, and reuse it across iterations; if possible, further narrow the fetch to only the `mediaId`s being processed.
🤖 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/jobs/ccip-jobs.ts`:
- Around line 74-83: The existing `existingChild` query in `ccip-jobs.ts`
compares `jobs.payload->>'mediaId'` as text against `medias.id` as uuid, which
can fail in PostgreSQL. Update the SQL expression in the
`db.select(...).where(and(...))` block to explicitly cast the extracted
`mediaId` to uuid before comparing it to `medias.id`, and keep the change
localized to the `existingChild` condition.
- Around line 163-179: The parent job finalize logic in ccip-jobs should not
depend on dispatch timing, because parent total is still 0 while child jobs are
created as pending and can cause premature completion/failed transitions. Update
the flow around jobRepo.update, RealtimeEventBus.publishJob, and the batch
dispatch completion path to separate “dispatch finished” from “parent can
finalize,” and use the parent job state (parentJob / parentId) to explicitly
mark the parent completed when dispatchedCount is 0 so it does not remain
in_progress.
In `@apps/server/src/infrastructure/jobs/tagging-jobs.ts`:
- Around line 249-263: Update the parent job bookkeeping in tagging-jobs around
bulk_tagging_dispatch so the parent payload total is set before any auto_tagging
progress can evaluate completion. In the flow using jobRepo.update,
RealtimeEventBus.publishJob, and the finalize logic around
bulk_tagging_dispatch/auto_tagging, ensure dispatch updates the parent first and
suppress finalize while dispatching is still in progress. Also handle
dispatchedCount === 0 by explicitly marking the parent job as completed instead
of leaving it in_progress.
In `@packages/db/src/repositories/job-repository.ts`:
- Around line 610-611: The progressKey binding in the job repository update
query should normalize missing values to null instead of letting undefined pass
through. Update the SQL in the same place that uses WHERE id = ${id} and AND
(${progressKey} IS NULL OR EXISTS (...)) so the progressKey interpolation
matches the child CTE handling by using the null-coalescing form. Keep the fix
localized around the incrementProgress path in job-repository.ts so the IS NULL
branch behaves consistently.
---
Outside diff comments:
In `@packages/ui/src/hooks/use-manager-page.ts`:
- Around line 421-438: `handleStartBatchCcipExtraction` is missing the failure
path that `handleStartBatchTagging` already has, so when
`actions.startBatchCcipExtraction` returns a non-success result or no `jobId`,
the UI stays on “Starting...” with no user feedback. Update
`handleStartBatchCcipExtraction` to mirror the `handleStartBatchTagging` pattern
by adding an explicit `else` branch after the `result.success && result.jobId`
check, and set both a toast error and an error `setTaggingStatus` message using
the returned result/message so the user sees the failure state.
---
Nitpick comments:
In `@apps/server/src/infrastructure/jobs/ccip-jobs.ts`:
- Around line 118-134: The `ccipVectorService.listRecords(mediaSourceId)` call
inside the `while`-loop in `ccip-jobs.ts` is re-fetching all records on every
iteration when `force` is false. Move the record lookup out of the loop in the
job flow around `targetRows`/`rows.filter`, cache the `Map` once per media
source, and reuse it across iterations; if possible, further narrow the fetch to
only the `mediaId`s being processed.
In `@apps/tauri/src/routes/manager.tsx`:
- Around line 61-66: Reuse the newly added AI API wrapper functions instead of
keeping inline client calls in manager.tsx: replace the direct
client.ai.scanBatchCcipTargets and client.ai.startBatchCcipExtraction
definitions with imports from ai-api.ts. Align the input contract to the wrapper
signatures, especially the force field being optional in the wrapper, so the
manager route and the shared API client stay consistent. Use the
scanBatchCcipTargets and startBatchCcipExtraction symbols from ai-api.ts as the
single source of truth and remove the duplicated inline definitions.
In `@packages/ui/src/screens/manager-screen.tsx`:
- Around line 250-253: `renderMediaCard` is only used for `ManagerScreenProps`,
so remove that prop from the `ManagerScreen` component contract and clean up the
related `MediaSafe` import. Update the `ManagerScreen` type/implementation so it
no longer expects or references `renderMediaCard`, and ensure any now-unused
imports or symbols are removed from the same module.
🪄 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: 8f149915-25da-442b-b50a-20a48817a377
📒 Files selected for processing (21)
apps/server/public/openapi.jsonapps/server/src/application/services/job-dispatch-service.tsapps/server/src/infrastructure/api-clients/ai-api.tsapps/server/src/infrastructure/api/routers/ai-router.tsapps/server/src/infrastructure/jobs/ccip-jobs.tsapps/server/src/infrastructure/jobs/job-worker.tsapps/server/src/infrastructure/jobs/tagging-jobs.tsapps/server/src/routes/manager.tsxapps/server/src/tests/unit/application/services/media-service.test.tsapps/server/src/tests/unit/infrastructure/jobs/ccip-jobs.test.tsapps/server/src/tests/unit/infrastructure/jobs/job-worker.test.tsapps/server/src/tests/unit/infrastructure/jobs/tagging-jobs.test.tsapps/tauri/src/infrastructure/api-clients/ai-api.tsapps/tauri/src/routes/manager.tsxpackages/core/src/domain/contract/ai.contract.tspackages/core/src/domain/repositories/job-repository.tspackages/core/src/domain/tagging/schemas.tspackages/db/src/repositories/job-repository.tspackages/ui/src/hooks/use-manager-page.tspackages/ui/src/media-sidebar.tsxpackages/ui/src/screens/manager-screen.tsx
💤 Files with no reviewable changes (1)
- packages/ui/src/media-sidebar.tsx
…uidにキャスト\n- 子job handlerでtotal === 0時はfinalizeをスキップ\n- dispatchedCount === 0時に親jobを即completedに\n- ccip listRecordsをループ外で1回取得\n- tauri manager.tsxでai-apiラッパーを再利用\n- ManagerScreenから未使用のrenderMediaCard propを削除\n- handleStartBatchCcipExtractionに失敗パス追加
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)
apps/server/src/infrastructure/jobs/tagging-jobs.ts (1)
249-268: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
total更新後に finalize を再判定してください
totalをここで確定したあとにprocessed + failed >= totalを見直してfinalizeBatchParentを呼ばないと、先に完了した子ジョブがいても親ジョブがin_progressのまま残ります。job-progressのprocessed: 0も、実際のparentPayload.processedとズレます。🤖 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/infrastructure/jobs/tagging-jobs.ts` around lines 249 - 268, After setting the batch parent payload’s total via jobRepo.update in tagging-jobs, re-evaluate whether the parent is now complete and call finalizeBatchParent when parentPayload.processed + parentPayload.failed is greater than or equal to the newly updated total. Also update the job-progress publish in the non-empty branch to use the current parent progress instead of hardcoding processed: 0, so the state stays aligned with parentPayload and the parent job does not remain stuck in_progress.
🤖 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 `@apps/server/src/infrastructure/jobs/tagging-jobs.ts`:
- Around line 249-268: After setting the batch parent payload’s total via
jobRepo.update in tagging-jobs, re-evaluate whether the parent is now complete
and call finalizeBatchParent when parentPayload.processed + parentPayload.failed
is greater than or equal to the newly updated total. Also update the
job-progress publish in the non-empty branch to use the current parent progress
instead of hardcoding processed: 0, so the state stays aligned with
parentPayload and the parent job does not remain stuck in_progress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: baea4aa9-6d04-47b1-ad68-2cead5827794
📒 Files selected for processing (7)
apps/server/src/infrastructure/jobs/ccip-jobs.tsapps/server/src/infrastructure/jobs/tagging-jobs.tsapps/server/src/routes/manager.tsxapps/tauri/src/routes/manager.tsxpackages/db/src/repositories/job-repository.tspackages/ui/src/hooks/use-manager-page.tspackages/ui/src/screens/manager-screen.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/db/src/repositories/job-repository.ts
- packages/ui/src/hooks/use-manager-page.ts
- apps/server/src/infrastructure/jobs/ccip-jobs.ts
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.
概要
Issue #569 に対応し、大規模tagging/CCIP batch開始時の30秒timeoutやPostgreSQL parameter上限問題を解消します。
変更内容
技術詳細
Closes #569
Summary by CodeRabbit