feat(v2): add no-migration feature support - #663
Conversation
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughV2画面を実データに接続しました。ジョブ管理、AIヘルスチェック、ソース同期状態、メディア件数、リスト表示、メディア詳細の前後移動、Tauriルート変換を追加しました。 Changesgh-stack運用文書
V2機能とAPI拡張
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant V2JobsScreen
participant V2JobsRoute
participant jobsRouter
participant Database
V2JobsScreen->>V2JobsRoute: request refresh or retry
V2JobsRoute->>jobsRouter: call jobs API
jobsRouter->>Database: query or update job
Database-->>jobsRouter: return job DTO data
jobsRouter-->>V2JobsRoute: return jobs response
V2JobsRoute-->>V2JobsScreen: update list and inspector
sequenceDiagram
participant DirectorySyncService
participant RealtimeEventBus
participant useSourcesEvents
participant V2SourceList
DirectorySyncService->>RealtimeEventBus: publish source-sync-status
RealtimeEventBus->>useSourcesEvents: validate and handle event
useSourcesEvents->>V2SourceList: invalidate query and update status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
de3e087 to
a331c5c
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (4)
packages/ui/src/screens/v2-jobs-screen.tsx (1)
404-407: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value型アサーションの理由をコメントで示してください。
value as JobFilterは外部ライブラリ境界での最小スコープのアサーションです。コーディングガイドラインは、やむを得ない場合に理由をコメントで示すことを求めています。♻️ 提案する変更
<Tabs - onChange={(value) => setActiveFilter(value as JobFilter)} + // Kobalte の onChange は string を渡す。値は JOB_FILTERS の value に限定される。 + onChange={(value) => setActiveFilter(value as JobFilter)} value={activeFilter()} >コーディングガイドラインに基づく指摘です(「外部ライブラリ境界では……型アサーションがやむを得ない場合は最小スコープに限定して理由をコメントする」)。
🤖 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/v2-jobs-screen.tsx` around lines 404 - 407, Tabs の onChange コールバックにある value as JobFilter の型アサーション直前へ、外部ライブラリの値が JobFilter として型付けされていないため最小スコープでアサーションしている理由を示すコメントを追加してください。Source: Coding guidelines
packages/ui/src/query-options/jobs-query.ts (1)
7-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueキーのプレフィックスを
all()から派生させてください。
"jobs"のリテラルが3箇所に重複しています。all()を基点にすると、プレフィックスの不整合を防げます。無効化はall()のプレフィックス一致に依存するため、この整合は重要です。♻️ 提案する変更
export const jobsQueryKeys = { all: () => ["jobs"] as const, - list: (input: JobListRequest) => ["jobs", "list", input] as const, - detail: (jobId: string) => ["jobs", "detail", jobId] as const, + list: (input: JobListRequest) => + [...jobsQueryKeys.all(), "list", input] as const, + detail: (jobId: string) => [...jobsQueryKeys.all(), "detail", jobId] as const, };🤖 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/query-options/jobs-query.ts` around lines 7 - 11, Update jobsQueryKeys so list and detail derive their shared prefix from all() instead of repeating the "jobs" literal, while preserving their existing key shapes and all() prefix-matching behavior..agents/skills/gh-stack/SKILL.md (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winmarkdownlint の警告を解消してください。
静的解析が MD040 を十個のコードフェンスで、MD028 を Line 321 で検出しています。ASCII 図とコマンド構文には
text、実行例にはbashなどの言語識別子を付けてください。Line 321 の blockquote 内の空行は削除してください。Also applies to: 84-84, 321-321, 416-416, 453-453, 502-502, 575-575, 629-629, 670-670, 803-803, 839-839
🤖 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 @.agents/skills/gh-stack/SKILL.md at line 17, SKILL.md 内の10個のコードフェンスに適切な言語識別子を追加し、ASCII図とコマンド構文にはtext、実行例にはbashなどを指定してください。あわせて、blockquote内の空行を削除してMD028警告を解消してください。Source: Linters/SAST tools
apps/server/src/application/services/directory-sync-service.ts (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
globalThisの型アサーションを除去してください。Line 27 は拡張したグローバル型を型アサーションで作成しています。
declare globalで__SOLID_IMAGER_SOURCE_SYNC_STATES__を宣言し、直接参照してください。型アサーションを維持する場合は、必要な理由をコメントしてください。As per coding guidelines, 「型アサーションがやむを得ない場合は最小スコープに限定して理由をコメントする。」
🤖 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/application/services/directory-sync-service.ts` around lines 27 - 33, Remove the globalThis type assertion around syncStateGlobal and add a declare global declaration for __SOLID_IMAGER_SOURCE_SYNC_STATES__ with type Map<string, SourceSyncStatus> | undefined. Update the sourceSyncStates initialization to reference globalThis.__SOLID_IMAGER_SOURCE_SYNC_STATES__ directly while preserving its existing fallback and assignment behavior.Source: Coding guidelines
🤖 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 @.agents/skills/gh-stack/SKILL.md:
- Around line 56-57: Update the gh stack submit behavior description in the
relevant numbered guidance to state that omitting --auto opens the full-screen
editor for the PR title, body, and draft/ready status; retain the recommendation
to always use --auto, explaining that it skips this editor.
- Line 322: Update the gh stack sync guidance to state that automation must not
proceed based solely on a successful exit code: in non-interactive divergence,
detect “ℹ Sync aborted” on stderr and stop processing. For flows that continue,
require validating the current stack state with gh stack view --json; direct
resolution toward unstacking and recreating the stack.
In `@apps/server/public/openapi.json`:
- Around line 6826-6831:
OpenAPI生成で、/jobs/list、/jobs/get、/jobs/retryの200レスポンスに具体的なスキーマを設定し、空のanyOf({} と
not
{})を除去してください。jobListResponseSchemaにはjobListResponseSchema、個別ジョブ応答にはjobDtoSchemaの契約を参照させ、生成クライアントがitems、total、ジョブDTOの型を取得できるようにします。
In `@apps/server/src/application/services/directory-sync-service.ts`:
- Around line 168-182: Make sync execution exclusive per source ID in
syncMediaSource by adding a per-mediaSourceId mutex or in-flight Promise guard
before the synchronization work begins. Skip or await duplicate calls according
to the existing flow, and ensure the guard is always released in a finally
block, including errors and early returns.
- Around line 267-271: Prevent internal error details from reaching clients: in
apps/server/src/application/services/directory-sync-service.ts:267-271, update
publishSyncStatus to use a fixed safe directory-sync failure message instead of
error.message, while retaining detailed errors in server logs. In
apps/server/src/infrastructure/api/routers/jobs-router.ts:53-65, map job.error
to an explicitly public error code or fixed safe message when constructing the
Safe Job DTO.
In `@apps/server/src/infrastructure/api/routers/jobs-router.ts`:
- Around line 111-123: Update the retry operation in the jobs router to
atomically update only when the job ID matches and its status is "failed" by
adding that status predicate to the update condition. When no row is returned,
preserve the not-found response for missing jobs and return the appropriate
current-state error for jobs that are no longer failed.
- Around line 72-125: 公開ハンドラの list、get、retry に共有レスポンススキーマを os.output(...)
で設定し、toJobDto の戻り値と list の items/total
を実行時に検証できるようにしてください。既存の入力検証とハンドラの処理内容は維持し、各エンドポイントで適切な共通出力スキーマを再利用してください。
- Around line 111-124: 再試行成功時にジョブ更新イベントが発行されていません。`requeued`
の存在確認後、`toJobDto(requeued)` で作成した成功 DTO を既存のジョブイベント契約で `RealtimeEventBus`
に発行してから返却し、未検出時は現在の `NOT_FOUND` 処理を維持してください。
In `@apps/server/src/routes/v2/jobs.tsx`:
- Around line 20-25: Update the useJobEvents invalidation callback so
high-frequency job-progress events do not invalidate jobsQueryKeys.all() on
every event. Restrict invalidation to job completion/failure events, or throttle
all invalidations to approximately 500ms–1s while preserving timely list
refreshes.
In `@apps/server/src/routes/v2/sources/`$mediaSourceId/$mediaId/index.tsx:
- Around line 51-62: Update navigateToNeighbor so neighbor navigation replaces
the current detail-page history entry by enabling replace: true on navigate.
Preserve the existing neighbor lookup, route parameters, and destination.
In `@apps/tauri/src/routes/v2/`$.tsx:
- Around line 11-16: Update the segment decoding in segments() to safely handle
decodeURIComponent failures, including malformed percent escapes and invalid
UTF-8. Return the original segment whenever decoding throws, so route resolution
can continue and NotFoundScreen remains reachable.
In `@packages/ui/src/hooks/use-job-events.ts`:
- Around line 10-18: Update useJobEvents so enabled is an Accessor<boolean>
matching use-media-source-events.ts, and evaluate enabled() inside its
createEffect guard. Preserve the existing server check and ensure the effect
starts or stops the event subscription reactively when the signal changes.
In `@packages/ui/src/screens/v2-jobs-screen.tsx`:
- Around line 143-200: Add the missing accessibility attributes in JobsTable:
provide a descriptive table caption, add scope="col" to each column header, and
set the row-selection button’s aria-pressed based on whether job.id matches
selectedJobId.
- Around line 56-63: Replace the local formatDate helper in the jobs screen with
the shared formatDate from v2-manager/utils, reusing its Date | string handling
and removing the duplicate dateFormatter definition.
---
Nitpick comments:
In @.agents/skills/gh-stack/SKILL.md:
- Line 17: SKILL.md
内の10個のコードフェンスに適切な言語識別子を追加し、ASCII図とコマンド構文にはtext、実行例にはbashなどを指定してください。あわせて、blockquote内の空行を削除してMD028警告を解消してください。
In `@apps/server/src/application/services/directory-sync-service.ts`:
- Around line 27-33: Remove the globalThis type assertion around syncStateGlobal
and add a declare global declaration for __SOLID_IMAGER_SOURCE_SYNC_STATES__
with type Map<string, SourceSyncStatus> | undefined. Update the sourceSyncStates
initialization to reference globalThis.__SOLID_IMAGER_SOURCE_SYNC_STATES__
directly while preserving its existing fallback and assignment behavior.
In `@packages/ui/src/query-options/jobs-query.ts`:
- Around line 7-11: Update jobsQueryKeys so list and detail derive their shared
prefix from all() instead of repeating the "jobs" literal, while preserving
their existing key shapes and all() prefix-matching behavior.
In `@packages/ui/src/screens/v2-jobs-screen.tsx`:
- Around line 404-407: Tabs の onChange コールバックにある value as JobFilter
の型アサーション直前へ、外部ライブラリの値が JobFilter
として型付けされていないため最小スコープでアサーションしている理由を示すコメントを追加してください。
🪄 Autofix
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: 0ddf43c7-d909-42b1-869e-c64e40294d39
📒 Files selected for processing (52)
.agents/skills/gh-stack/SKILL.mdAGENTS.mdREPORT.mdapps/server/public/openapi.jsonapps/server/src/application/services/directory-sync-service.tsapps/server/src/components/media/legacy-media-grid-item.tsxapps/server/src/components/media/v2-media-grid-item.tsxapps/server/src/components/v2/v2-source-list.tsxapps/server/src/hooks/use-media-source-events.tsapps/server/src/infrastructure/api-clients/queries/index.tsapps/server/src/infrastructure/api/routers/ai-router.tsapps/server/src/infrastructure/api/routers/characters-router.tsapps/server/src/infrastructure/api/routers/entity-media-counts.tsapps/server/src/infrastructure/api/routers/ips-router.tsapps/server/src/infrastructure/api/routers/jobs-router.tsapps/server/src/infrastructure/api/routers/projects-router.tsapps/server/src/infrastructure/api/routers/sources-router.tsapps/server/src/routes/sources/$mediaSourceId/components/source-media-page.tsxapps/server/src/routes/sources/$mediaSourceId/components/v2-source-media-page.tsxapps/server/src/routes/v2/config.tsxapps/server/src/routes/v2/jobs.tsxapps/server/src/routes/v2/media-context.tsapps/server/src/routes/v2/search.tsxapps/server/src/routes/v2/sources/$mediaSourceId/$mediaId/index.tsxapps/tauri/src/hooks/use-media-source-events.tsapps/tauri/src/routes/v2.tsxapps/tauri/src/routes/v2/$.tsxpackages/core/src/domain/characters/schemas.tspackages/core/src/domain/contract/ai.contract.tspackages/core/src/domain/contract/jobs.contract.tspackages/core/src/domain/ips/schemas.tspackages/core/src/domain/jobs/schemas.tspackages/core/src/domain/projects/schemas.tspackages/core/src/domain/sources/events.tspackages/core/src/domain/sources/schemas.tspackages/core/src/domain/tagging/schemas.tspackages/ui/src/hooks/use-job-events.tspackages/ui/src/hooks/use-media-source-events.tspackages/ui/src/hooks/use-sources-events.tspackages/ui/src/query-options/index.tspackages/ui/src/query-options/jobs-query.tspackages/ui/src/screens/search-screen.types.tspackages/ui/src/screens/source-media-screen.types.tspackages/ui/src/screens/v2-config-screen.tsxpackages/ui/src/screens/v2-config-state-screen.tsxpackages/ui/src/screens/v2-jobs-screen.tsxpackages/ui/src/screens/v2-manager/entity-panel.tsxpackages/ui/src/screens/v2-search-screen.tsxpackages/ui/src/screens/v2-source-media-screen.tsxpackages/ui/src/source-media-grid.tsxpackages/ui/src/source-media-page.tsxpackages/ui/src/v2/search-toolbar.tsx
a331c5c to
2b52b07
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/server/src/tests/unit/application/services/directory-sync-service.test.ts (1)
180-187: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win公開イベント全体で機密文字列の非露出を検証してください。
現在の
expect.objectContainingはmessageとstatusだけを検証します。将来、別のフィールドに元の例外メッセージを追加しても、このテストは成功します。
RealtimeEventBus.publishSourceの全呼び出しを検査し、"/secret/source-path"と"password=secret"が公開ペイロードに存在しないことを検証してください。修正例
expect(RealtimeEventBus.publishSource).toHaveBeenCalledWith( "source-1", "source-sync-status", expect.objectContaining({ message: "Directory sync failed", status: "error", }), ); + +expect( + JSON.stringify(vi.mocked(RealtimeEventBus.publishSource).mock.calls), +).not.toContain("/secret/source-path"); +expect( + JSON.stringify(vi.mocked(RealtimeEventBus.publishSource).mock.calls), +).not.toContain("password=secret");🤖 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/application/services/directory-sync-service.test.ts` around lines 180 - 187, Update the test around RealtimeEventBus.publishSource to inspect every published event, not only selected fields of the failure payload. Assert that the public payload never contains "/secret/source-path" or "password=secret", while preserving the existing message and status assertions.
🤖 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 @.agents/skills/gh-stack/SKILL.md:
- Line 17: SKILL.md のすべての Markdown fenced code block に適切な言語識別子を追加し、図には
text、コマンド例には bash を指定してください。連続する blockquote の間には空行を入れず、markdownlint の MD040 と
MD028 を解消してください。
- Around line 831-859: Update the gh stack unstack documentation to add a
stopping condition for stacks containing queued or auto-merge-enabled pull
requests. In the unstack workflow and examples around gh stack unstack, instruct
users not to run gh stack init until the GitHub stack has disappeared; if
unstack leaves the stack or returns HTTP 422 when all PRs are affected, remove
the PRs from the merge queue or disable auto-merge, then rerun unstack.
- Line 322: Update the command-specific exit-code table and divergence recovery
guidance in the gh stack documentation: define gh stack sync as exit 0 even when
it emits Sync aborted and makes no changes, require stderr inspection before
continuing, clarify that exit code 2 means either no stack membership or a
missing specified stack depending on the command, and state that unknown gh
stack unstack <number> values must be reported without running gh stack init.
Extend divergence recovery to account for queued or auto-merge-enabled PRs,
where unstack does not fully remove the stack and local tracking remains.
In `@apps/tauri/src/routes/v2/`$.tsx:
- Around line 29-59: Update the route matches in the v2 path handler so
redirects require the exact supported segments().length for each route, rather
than matching only first(). Keep the existing sources length-specific handling,
and ensure unsupported subpaths such as search/unknown or sources/.../extra fall
through to NotFoundScreen instead of redirecting.
In `@packages/ui/src/screens/v2-manager/utils.ts`:
- Around line 50-55: Update the date-formatting logic around the
Intl.DateTimeFormat call to create the Date value first, validate it with
date.getTime(), and return "—" for invalid strings or Invalid Date values before
formatting. Preserve the existing Japanese locale and includeTime-dependent
formatting for valid dates.
---
Nitpick comments:
In
`@apps/server/src/tests/unit/application/services/directory-sync-service.test.ts`:
- Around line 180-187: Update the test around RealtimeEventBus.publishSource to
inspect every published event, not only selected fields of the failure payload.
Assert that the public payload never contains "/secret/source-path" or
"password=secret", while preserving the existing message and status assertions.
🪄 Autofix
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: 41d8f395-2d29-442a-a62d-b2a1d16ebb3e
📒 Files selected for processing (13)
.agents/skills/gh-stack/SKILL.mdapps/server/public/openapi.jsonapps/server/src/application/services/directory-sync-service.tsapps/server/src/infrastructure/api/routers/jobs-router.tsapps/server/src/routes/v2/jobs.tsxapps/server/src/routes/v2/sources/$mediaSourceId/$mediaId/index.tsxapps/server/src/tests/unit/application/services/directory-sync-service.test.tsapps/tauri/src/routes/v2/$.tsxpackages/core/src/domain/sources/events.tspackages/ui/src/hooks/use-batch-job-events.tspackages/ui/src/hooks/use-job-events.tspackages/ui/src/screens/v2-jobs-screen.tsxpackages/ui/src/screens/v2-manager/utils.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/server/src/routes/v2/sources/$mediaSourceId/$mediaId/index.tsx
- apps/server/src/infrastructure/api/routers/jobs-router.ts
- apps/server/src/routes/v2/jobs.tsx
- apps/server/src/application/services/directory-sync-service.ts
- packages/core/src/domain/sources/events.ts
- packages/ui/src/screens/v2-jobs-screen.tsx
2b52b07 to
d07bc2a
Compare
概要
REPORT.mdで不足していた機能のうち、DB migrationなしで自然に提供できる機能をV2へ接続します。このPRは既存のperf/v2-media-gallery-performance向けPR #662に積み上げています。
変更内容
未対応
検証
Summary by CodeRabbit
新機能
ドキュメント