Repository navigation
Conversation
fix(search): AI処理の可観測性と類似検索の復旧操作を改善
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive logging for AI tagging, vector similarity search, and job execution, capturing duration and execution contexts. It also adds a feature to clear the vector search anchor in the UI. The review feedback highlights three main areas for improvement: synchronizing the session storage when clearing the vector search anchor to prevent state inconsistency, avoiding optional chaining on required API schema fields, and handling invalid data records gracefully by logging and returning null instead of using fallback values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+64
to
+70
| export const clearVectorSearchAnchor = () => { | ||
| setSearchState({ | ||
| similarityAnchorMediaId: null, | ||
| offset: 0, | ||
| scrollY: 0, | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
activateVectorSearch では sessionStorage に similarityAnchorMediaId などの状態を保存していますが、clearVectorSearchAnchor では sessionStorage の更新や削除が行われていません。これにより、ページをリロードした際などにセッションストレージから古いデータが復元され、状態の不整合が発生する可能性があります。clearVectorSearchAnchor 内でも sessionStorage の同期（または削除）を行うことを検討してください。
Suggested change
| export const clearVectorSearchAnchor = () => { | |
| setSearchState({ | |
| similarityAnchorMediaId: null, | |
| offset: 0, | |
| scrollY: 0, | |
| }); | |
| }; | |
| export const clearVectorSearchAnchor = () => { | |
| setSearchState({ | |
| similarityAnchorMediaId: null, | |
| offset: 0, | |
| scrollY: 0, | |
| }); | |
| if (typeof sessionStorage !== "undefined") { | |
| const stored = sessionStorage.getItem("current-all"); | |
| if (stored) { | |
| try { | |
| const parsed = JSON.parse(stored); | |
| parsed.similarityAnchorMediaId = null; | |
| sessionStorage.setItem("current-all", JSON.stringify(parsed)); | |
| } catch { | |
| sessionStorage.removeItem("current-all"); | |
| } | |
| } | |
| } | |
| }; |
| { | ||
| ...logContext, | ||
| execution: "remote", | ||
| detectionCount: result.detections.length, |
Contributor
There was a problem hiding this comment.
リモートサーバーからのレスポンス result の構造を検証する際、APIスキーマで必須とされているフィールドに対してオプショナルチェーニング（?.）やNull合体演算子（??）を使用することは避けてください。これらはスキーマ契約違反を隠蔽し、予期しない挙動の原因となります。代わりに、Zodなどのスキーマ定義を用いて受信時に厳密なバリデーションを行うか、型安全なガードを実装してください。
const parsedResult = aiResponseSchema.parse(result);
const detectionCount = parsedResult.detections.length;References
- Do not use optional chaining (?.) or nullish coalescing (??) on fields that are defined as required/mandatory in the API contract or schema.
Comment on lines
+54
to
+56
| tagCount: result ? Object.keys(result.general).length : 0, | ||
| characterCount: result ? Object.keys(result.character).length : 0, | ||
| ipCount: result?.ips.length ?? 0, |
Contributor
There was a problem hiding this comment.
データレコードのパースにおいて、result 内の必須フィールドが欠落している場合にオプショナルチェーニングやデフォルト値（0）を使用すると、スキーマ違反やデータ破損が隠蔽されてしまいます。破損したレコードは、例外をスローしてバッチ全体を失敗させるのではなく、エラーをログに記録した上で null を返して安全にスキップ（または後続処理で flatMap 等で除外）するように実装してください。
try {
if (!result || !result.general || !result.character) {
throw new Error("Invalid record structure");
}
const tagCount = Object.keys(result.general).length;
const characterCount = Object.keys(result.character).length;
} catch (error) {
console.error("Failed to parse record:", error);
return null;
}References
- When parsing data records (e.g., from a database), handle invalid records gracefully by returning null and logging an error, rather than throwing an exception.
- Do not use optional chaining (?.) or nullish coalescing (??) on fields that are defined as required/mandatory in the API contract or schema.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughサーバーのログ出力を構造化し、著者を platform/account ベースで扱うデータモデルと永続化を追加しました。LanceDBダンプとバックアップ復元は新しい著者構造と画像取得に対応し、検索UIとXtracterも新しい著者表示とFanbox/Danbooru対応へ更新されています。 Changesサーバー構造化ログ
著者プラットフォームとバックアップ
検索UIの著者表示
XtracterのFanbox/Danbooru対応
レビュー自動化設定
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 (1)
apps/server/src/infrastructure/api/routers/media-router.ts (1)
46-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
logContextを共通化してstartedAtもそのまま流してください。同じキーを 3 箇所で手書きしているので、今後どれか 1 箇所だけ項目がずれるリスクがあります。
const logContext = { startedAt, anchorMediaId: input.anchorMediaId, mediaSourceId: input.mediaSourceId, topK: input.topK }を作って各ログで展開しておくと、このコホートで揃えたい構造化ログの形を維持しやすいです。差分イメージ
- const startedAt = Date.now(); - logger.info( - { - anchorMediaId: input.anchorMediaId, - mediaSourceId: input.mediaSourceId, - topK: input.topK, - }, - "Vector similarity search started", - ); + const startedAt = Date.now(); + const logContext = { + startedAt, + anchorMediaId: input.anchorMediaId, + mediaSourceId: input.mediaSourceId, + topK: input.topK, + }; + logger.info(logContext, "Vector similarity search started"); try { const result = await ccipVectorService.searchSimilar( input.anchorMediaId, input.topK, input.mediaSourceId, ); logger.info( { - anchorMediaId: input.anchorMediaId, - mediaSourceId: input.mediaSourceId, - topK: input.topK, + ...logContext, resultCount: result.media.length, durationMs: Date.now() - startedAt, }, "Vector similarity search completed", ); return result; } catch (error) { logger.error( { + ...logContext, err: error, - anchorMediaId: input.anchorMediaId, - mediaSourceId: input.mediaSourceId, - topK: input.topK, durationMs: Date.now() - startedAt, }, "Vector similarity search failed", );🤖 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/api/routers/media-router.ts` around lines 46 - 84, The Vector similarity search logging in media-router repeats the same fields across the start, success, and error paths, which can drift over time. In the search handler around ccipVectorService.searchSimilar, introduce a shared logContext that includes startedAt plus anchorMediaId, mediaSourceId, and topK, and reuse it in logger.info/logger.error so every log entry stays structurally consistent. Keep the timing data flowing through the same shared context and only add resultCount or err where needed.
🤖 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/api/routers/ai-router.ts`:
- Around line 191-197: The upload logging in ai-router still records
user-provided file names, which can leak PII into start/complete/failure logs.
Update the logContext built in the file-input branch of the AI upload flow so it
no longer includes input.file.name; use a non-identifying alternative such as a
hashed identifier or keep only fileSize. Make the same change anywhere the
upload lifecycle logs reference the file name, including the later log block
noted in the review, so all related logs stay consistent and privacy-safe.
- Around line 246-251: callRemoteTagging() と callRemoteCrop()
の戻り値をそのまま型アサーションせず、ai-router.ts の該当処理で taggingResponseSchema と
detectAndCropResponseSchema を使って parse してから result を扱うように修正してください。ORPC 境界から来る
unknown をスキーマで検証し、result.detections.length などの参照やログ/返却は parse 後の値に限定してください。関連箇所は
callRemoteTagging、callRemoteCrop、result.detections を扱う処理です。
In `@packages/ui/src/stores/search-store.ts`:
- Around line 64-70: `clearVectorSearchAnchor` only resets the in-memory search
state, so the cleared anchor is restored after reload from the persisted
`"current-all"` value. Update this function to also clear or null out the saved
`similarityAnchorMediaId` in sessionStorage, matching the persistence behavior
used by `activateVectorSearch` and the restore logic in
`use-current-search-persistence`, so the解除 state remains after refresh.
---
Nitpick comments:
In `@apps/server/src/infrastructure/api/routers/media-router.ts`:
- Around line 46-84: The Vector similarity search logging in media-router
repeats the same fields across the start, success, and error paths, which can
drift over time. In the search handler around ccipVectorService.searchSimilar,
introduce a shared logContext that includes startedAt plus anchorMediaId,
mediaSourceId, and topK, and reuse it in logger.info/logger.error so every log
entry stays structurally consistent. Keep the timing data flowing through the
same shared context and only add resultCount or err where needed.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4805d8f-9c2e-4591-b41a-ff7310b0cfd9
📒 Files selected for processing (10)
apps/server/src/infrastructure/api/routers/ai-router.tsapps/server/src/infrastructure/api/routers/media-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/api/rpc.$.tsapps/server/src/tests/unit/infrastructure/jobs/ccip-jobs.test.tsapps/server/src/tests/unit/infrastructure/jobs/tagging-jobs.test.tspackages/ui/src/search-control-panel.tsxpackages/ui/src/stores/search-store.ts
…ng from DB
createDump lancedb mode now checks for an existing cache directory
(.cache/lancedb-cache/source-{id}/). When found, the cache is
tarred directly without DB queries or writeToLanceDB rebuild.
Falls back to the original rebuild path when cache is unavailable.
- Check manifest.json instead of just directory existence - Wrap cache path in try-catch for safe fallback to DB rebuild - Parallelize image downloads with Promise.all - Only readMediaFilePaths when includeImages is true
perf(backup): LanceDB dump で既存キャッシュを直接使用
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/application/services/backup-service.ts`:
- Around line 1548-1597: The cache-read fallback in the TAR dump path is
ineffective because the outer try/catch around the cache branch only covers
synchronous setup in backup-service’s cacheValid block, while the real work
happens later inside the async IIFE after nodeStreamToWebReadable(passThrough)
has already been returned. Move the cache validation and archive population work
(including readMediaFilePaths, archive.directory, and image reads) to complete
before returning the stream, or otherwise surface async failures back to the
cacheValid decision point so the code can actually log the warning and fall back
to DB rebuild instead of returning a broken TAR. Use the cacheValid branch,
archive.finalize, and nodeStreamToWebReadable as the key locations to refactor.
- Around line 1557-1578: In backup-service’s archive creation flow, the
combination of archive.directory(cacheDir, false) and the includeImages block
can add the same images/ entries twice. Update the logic around the
archive.append path to skip files already included via archive.directory, or
exclude images/ from the directory walk, using the readMediaFilePaths and
driver.get handling in backup-service to avoid duplicate tar entries.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 438c6dbe-49e2-4f8b-ae17-e4ee85dfbe75
📒 Files selected for processing (4)
apps/server/src/application/services/backup-service.tsapps/server/src/application/services/lancedb-dump-service.tspackages/application/src/ports/lancedb-dump-service.tspackages/application/src/services/lancedb-dump-service.ts
Comment on lines
+1557
to
+1578
| archive.directory(cacheDir, false); | ||
|
|
||
| if (includeImages) { | ||
| const { readMediaFilePaths } = await import( | ||
| "~/application/services/lancedb-dump-service" | ||
| ); | ||
| const mediaFilePaths = await readMediaFilePaths(cacheDir); | ||
|
|
||
| await Promise.all( | ||
| mediaFilePaths.map(async (item) => { | ||
| if (!item.filePath) return; | ||
| try { | ||
| const buffer = await driver.get(item.filePath); | ||
| archive.append(buffer, { | ||
| name: `images/${item.filePath}`, | ||
| }); | ||
| } catch { | ||
| // ignore missing files | ||
| } | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# キャッシュ/ダンプ dir に images が書き込まれる経路があるか確認
rg -nP '"images"|images/|join\([^)]*images' --type=ts -C2 packages/application/src/services/lancedb-dump-service.ts | head -40Repository: hmjn023/solid-imager
Length of output: 332
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 関連箇所の前後を確認
sed -n '1538,1590p' apps/server/src/application/services/backup-service.ts
printf '\n---\n'
sed -n '790,850p' packages/application/src/services/lancedb-dump-service.tsRepository: hmjn023/solid-imager
Length of output: 3501
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 画像パスの読み出し元も確認
rg -n "readMediaFilePaths|archive\.directory\(|archive\.append\(" apps/server/src/application/services/backup-service.ts packages/application/src/services/lancedb-dump-service.ts -C 3Repository: hmjn023/solid-imager
Length of output: 5622
images/ の重複格納を避けてください
cacheDir では images/<filePath> を別経路で扱っているため、archive.directory(cacheDir, false) の後に同じ画像を archive.append() すると tar に重複エントリが入り得ます。images/ を除外するか、追加側をスキップしてください。
🤖 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/backup-service.ts` around lines 1557 -
1578, In backup-service’s archive creation flow, the combination of
archive.directory(cacheDir, false) and the includeImages block can add the same
images/ entries twice. Update the logic around the archive.append path to skip
files already included via archive.directory, or exclude images/ from the
directory walk, using the readMediaFilePaths and driver.get handling in
backup-service to avoid duplicate tar entries.
Add FANBOX image extraction, platform-scoped author accounts, metadata persistence across JSON and LanceDB, and deterministic media filenames.
feat(xtracter): pixivFANBOX対応を追加
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)
apps/xtracter/src/content/danbooru.ts (1)
269-281: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win単一の
twitterAccountIdを全 author に無条件でplatform: "twitter"として適用してしまう。複数の artist タグが存在する投稿で、ページ全体から検出された単一の
twitterAccountIdが、無関係な author にもplatform: "twitter"として一律で付与されます。accountIdの代入は!author.accountIdの条件付きですが、platformの代入はガードされていないため、実際には twitter に紐づかない author にも誤ったプラットフォームラベルが付きます。🐛 提案（accountId 代入と platform 代入を同一条件に揃える）
if (twitterAccountId) { for (const author of authors) { if (!author.accountId) { author.accountId = twitterAccountId; + author.platform = "twitter"; } - author.platform = "twitter"; } } else {🤖 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/xtracter/src/content/danbooru.ts` around lines 269 - 281, The author mapping in danbooru.ts applies twitterAccountId to every author and sets author.platform to "twitter" even when the author already has its own accountId, so align the platform assignment with the same conditional used in the author loop. Update the logic around the authors processing block so only authors that actually receive the fallback twitterAccountId are labeled with the twitter platform, while authors with existing accountId values keep their own platform handling; use the existing authors iteration and twitterAccountId check as the main locator.
🧹 Nitpick comments (2)
apps/server/src/components/media/pro-search-builder.tsx (1)
331-332: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value著者ラベルからプラットフォーム表示が削除された。
(twitter)のようなプラットフォーム表示がなくなり、name: accountIdのみの表示になりました。複数プラットフォーム（twitter/pixiv-fanbox/danbooru）対応が追加された今、同一 accountId 表記でもプラットフォームが区別できずユーザーが混同する可能性があります。♻️ 提案（platform を再表示）
const getAuthorLabel = (author: Author) => - author.accountId ? `${author.name}: ${author.accountId}` : author.name; + author.accountId + ? `${author.name}: ${author.accountId}${author.platform ? ` (${author.platform})` : ""}` + : author.name;🤖 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/pro-search-builder.tsx` around lines 331 - 332, The getAuthorLabel helper in pro-search-builder.tsx no longer shows the author platform, so update it to include the platform identifier again alongside name and accountId. Use the existing Author fields in getAuthorLabel to append the platform label in the displayed author text, keeping the format clear enough to distinguish twitter, pixiv-fanbox, danbooru, and similar sources. Ensure any places rendering author labels through this helper pick up the revised format without changing unrelated search builder behavior.packages/core/src/domain/authors/schemas.ts (1)
14-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
z.string().uuid()/z.string().url()を新しい API に寄せる
Zod 4 ではまだ動きますが非推奨なので、この新規スキーマだけでもz.uuid()/z.url()に統一すると移行が楽です。🤖 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/core/src/domain/authors/schemas.ts` around lines 14 - 24, The new authorAccountSchema still uses deprecated z.string().uuid() and z.string().url() calls, so update the schema in authorAccountSchema to use Zod 4’s top-level z.uuid() and z.url() helpers instead. Keep the same field behavior for id, authorId, and profileUrl, and leave the rest of the schema and AuthorAccount type inference unchanged.
🤖 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/application/services/backup-service.ts`:
- Around line 414-506: The author restore loop in backup-service’s author
recovery path is causing N+1 queries by awaiting multiple DB lookups per author
inside the `for (const input of authorInputs)` block. Refactor this section to
batch work like `_ensureMasterData`: prefetch all relevant `authors` and
`authorAccounts` rows for the collected `authorData`, resolve reusable/linked
IDs in memory, and perform inserts in bulk before building `authorMap` and
`authorRestoreKey` mappings. Keep the existing dedupe/legacy matching behavior,
but move DB round-trips out of the per-author loop.
In `@packages/db/src/repositories/author-repository.ts`:
- Around line 36-41: `findOrCreateAuthorsBulk` in `author-repository.ts` is only
checking `input.name.length > 0`, so whitespace-only author names can bypass the
same validation enforced by `create()`. Update the bulk filter to apply the same
trimmed-name rule used by `create` (reject names whose trimmed length is zero)
before deduping with `authorInputKey`, so bulk paths like `registerAuthors`
behave consistently with single-author creation.
- Around line 32-172: findOrCreateAuthorsBulk currently does a non-atomic
read/insert flow that can create duplicate authors under concurrent requests and
also lets whitespace-only names through because it only checks
input.name.length. Update the bulk path in author-repository to match the
stricter create() validation by trimming names before accepting them, and change
the author/author_accounts persistence to an atomic strategy (for example,
author_accounts-driven upsert or locking plus UPSERT) so concurrent `(platform,
accountId)` / `name` lookups cannot create orphaned duplicate rows. Use the
existing findOrCreateAuthorsBulk, authors, and authorAccounts logic as the main
place to apply the fix.
In `@packages/ui/src/pro-search-builder.tsx`:
- Line 494: `ProSearchBuilder` の選択値の契約がずれており、`optionValue` が `item.id` なのに保存・復元は
`value.name` / `item.name` 基準になっています。`ProSearchBuilder` 内の該当ロジックを揃えて、検索条件を name
ベースで維持するなら `optionValue` も name に統一し、ID ベースにするなら保存・復元・検索判定をまとめて `id`
基準へ変更してください。特に選択肢の識別、保存処理、復元処理が同じキーを使うように修正してください。
---
Outside diff comments:
In `@apps/xtracter/src/content/danbooru.ts`:
- Around line 269-281: The author mapping in danbooru.ts applies
twitterAccountId to every author and sets author.platform to "twitter" even when
the author already has its own accountId, so align the platform assignment with
the same conditional used in the author loop. Update the logic around the
authors processing block so only authors that actually receive the fallback
twitterAccountId are labeled with the twitter platform, while authors with
existing accountId values keep their own platform handling; use the existing
authors iteration and twitterAccountId check as the main locator.
---
Nitpick comments:
In `@apps/server/src/components/media/pro-search-builder.tsx`:
- Around line 331-332: The getAuthorLabel helper in pro-search-builder.tsx no
longer shows the author platform, so update it to include the platform
identifier again alongside name and accountId. Use the existing Author fields in
getAuthorLabel to append the platform label in the displayed author text,
keeping the format clear enough to distinguish twitter, pixiv-fanbox, danbooru,
and similar sources. Ensure any places rendering author labels through this
helper pick up the revised format without changing unrelated search builder
behavior.
In `@packages/core/src/domain/authors/schemas.ts`:
- Around line 14-24: The new authorAccountSchema still uses deprecated
z.string().uuid() and z.string().url() calls, so update the schema in
authorAccountSchema to use Zod 4’s top-level z.uuid() and z.url() helpers
instead. Keep the same field behavior for id, authorId, and profileUrl, and
leave the rest of the schema and AuthorAccount type inference unchanged.
🪄 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: 2d69cdfa-3921-4c32-ae02-119126ed361c
📒 Files selected for processing (31)
apps/server/drizzle/0017_ambitious_gideon.sqlapps/server/drizzle/meta/0017_snapshot.jsonapps/server/drizzle/meta/_journal.jsonapps/server/src/application/services/backup-service.tsapps/server/src/components/media/pro-search-builder.tsxapps/server/src/components/media/search-filters.tsxapps/server/src/infrastructure/jobs/download-jobs.tsapps/server/src/tests/integration/backup/backup-service.test.tsapps/server/src/tests/integration/repository/author-dedupe.test.tsapps/server/src/tests/unit/application/services/lancedb-dump-service.test.tsapps/server/src/tests/unit/application/services/media-processing-service.test.tsapps/server/src/tests/unit/domain/media/schemas.test.tsapps/server/src/tests/unit/infrastructure/jobs/download-jobs.test.tsapps/xtracter/README.mdapps/xtracter/public/manifest.jsonapps/xtracter/src/content/danbooru.tsapps/xtracter/src/content/fanbox.tsapps/xtracter/src/content/index.tsapps/xtracter/src/content/twitter.tsapps/xtracter/src/schema.tspackages/application/src/services/lancedb-dump-service.tspackages/application/src/services/media-processing-service.tspackages/core/src/domain/authors/schemas.tspackages/core/src/domain/media/schemas.tspackages/core/src/domain/media/utils/filename-utils.tspackages/core/src/domain/repositories/author-repository.tspackages/core/tests/filename-utils.test.tspackages/db/src/repositories/author-repository.tspackages/db/src/schema.tspackages/ui/src/pro-search-builder.tsxpackages/ui/src/search-filters.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/server/src/infrastructure/jobs/download-jobs.ts
- apps/xtracter/README.md
| : "" | ||
| } | ||
| optionValue={(item) => item.name} | ||
| optionValue={(item) => item.id} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
optionValue と保存される検索値を揃えてください。
Line 494 は選択肢を item.id で識別しますが、Line 475 は value.name を保存し、Line 497-499 も item.name で復元しています。同名の作者・関連項目があると、選んだ ID ではなく最初に一致した名前へ戻る可能性があります。検索条件が name ベースのままなら optionValue も name に戻し、ID ベースにするなら保存・復元・検索契約をまとめて更新してください。
name ベース契約を維持する場合の修正案
- optionValue={(item) => item.id}
+ optionValue={(item) => item.name}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| optionValue={(item) => item.id} | |
| optionValue={(item) => item.name} |
🤖 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/pro-search-builder.tsx` at line 494, `ProSearchBuilder`
の選択値の契約がずれており、`optionValue` が `item.id` なのに保存・復元は `value.name` / `item.name`
基準になっています。`ProSearchBuilder` 内の該当ロジックを揃えて、検索条件を name ベースで維持するなら `optionValue` も
name に統一し、ID ベースにするなら保存・復元・検索判定をまとめて `id`
基準へ変更してください。特に選択肢の識別、保存処理、復元処理が同じキーを使うように修正してください。
fix(lancedb): use SQL transform for schema migration
fix(search): 詳細検索の対象選択を更新できるよう修正
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/components/media/pro-search-builder.tsx (1)
499-499: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win著者検索の値契約と
optionValueを揃えてください。
optionValueをitem.idにすると、著者条件が ID ベースになります。一方、下流のpackages/db/src/repositories/media-repository.tsはauthor条件をauthors.nameと比較しているため、選択した著者で検索が一致しなくなります。name 契約を維持するならitem.nameに戻し、ID 契約にするなら DB 側もauthors.id比較へ更新してください。name 契約を維持する場合の修正案
- optionValue={(item) => item.id} + optionValue={(item) => item.name}🤖 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/pro-search-builder.tsx` at line 499, The author search contract is inconsistent because ProSearchBuilder is using optionValue={(item) => item.id} while the downstream media repository compares author filters against authors.name. Update the author selection flow in pro-search-builder.tsx to keep the same contract end-to-end: either restore optionValue to item.name, or if ID-based selection is intended, change the author filtering logic in media-repository.ts to compare against authors.id instead of names. Use the existing author-related fields and filter handling in ProSearchBuilder and MediaRepository to keep the chosen contract consistent.
🧹 Nitpick comments (1)
apps/server/src/components/media/pro-search-builder.tsx (1)
273-287: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win不要な型キャストを型分岐に置き換えてください。
child.typeと"accountId" in itemで絞り込めるため、as SearchCriterion/as SearchGroup/as Authorは不要です。修正案
- <Show - fallback={ - <CriterionBuilder + {child.type === "group" ? ( + <GroupBuilder + authors={props.authors} + characters={props.characters} + depth={props.depth + 1} + group={child} + ips={props.ips} + onChange={(g) => updateChild(index(), g)} + onRemove={() => removeChild(index())} + projects={props.projects} + tags={props.tags} + /> + ) : ( + <CriterionBuilder authors={props.authors} characters={props.characters} - criterion={child as SearchCriterion} + criterion={child} ips={props.ips} onChange={(c) => updateChild(index(), c)} onRemove={() => removeChild(index())} projects={props.projects} tags={props.tags} /> - } - when={isSearchGroup(child)} - > - <GroupBuilder - authors={props.authors} - characters={props.characters} - depth={props.depth + 1} - group={child as SearchGroup} - ips={props.ips} - onChange={(g) => updateChild(index(), g)} - onRemove={() => removeChild(index())} - projects={props.projects} - tags={props.tags} - /> - </Show> + )}- const label = - props.criterion.target === "author" - ? getAuthorLabel(item as Author) - : (item as { name: string }).name; + const label = "accountId" in item ? getAuthorLabel(item) : item.name;As per coding guidelines,
**/*.{ts,tsx}: “In application code, do not useany, unnecessaryunknown,as unknown as ..., oras any; prefer public types, type guards, Zod schemas, and explicit mappers, and only use unavoidable casts in the smallest possible scope with a comment.”Also applies to: 360-363
🤖 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/pro-search-builder.tsx` around lines 273 - 287, Remove the unnecessary type assertions in the SearchBuilder rendering logic by relying on the existing type guards instead of casting. In the branch that renders the criterion and group children, use the narrowed result from `isSearchCriterion(child)` and `isSearchGroup(child)` directly in the `SearchCriterionBuilder` and `GroupBuilder` paths, and for the author-related branch use the `"accountId" in item` check to narrow to `Author` without `as Author`. Keep the fix localized to the child-dispatch code in `pro-search-builder.tsx` so `child.type`-based narrowing and the helper guards do the typing work.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.
Outside diff comments:
In `@apps/server/src/components/media/pro-search-builder.tsx`:
- Line 499: The author search contract is inconsistent because ProSearchBuilder
is using optionValue={(item) => item.id} while the downstream media repository
compares author filters against authors.name. Update the author selection flow
in pro-search-builder.tsx to keep the same contract end-to-end: either restore
optionValue to item.name, or if ID-based selection is intended, change the
author filtering logic in media-repository.ts to compare against authors.id
instead of names. Use the existing author-related fields and filter handling in
ProSearchBuilder and MediaRepository to keep the chosen contract consistent.
---
Nitpick comments:
In `@apps/server/src/components/media/pro-search-builder.tsx`:
- Around line 273-287: Remove the unnecessary type assertions in the
SearchBuilder rendering logic by relying on the existing type guards instead of
casting. In the branch that renders the criterion and group children, use the
narrowed result from `isSearchCriterion(child)` and `isSearchGroup(child)`
directly in the `SearchCriterionBuilder` and `GroupBuilder` paths, and for the
author-related branch use the `"accountId" in item` check to narrow to `Author`
without `as Author`. Keep the fix localized to the child-dispatch code in
`pro-search-builder.tsx` so `child.type`-based narrowing and the helper guards
do the typing work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 82097b48-ff09-441d-be7e-ae9c926fd254
📒 Files selected for processing (4)
apps/server/src/components/media/pro-search-builder.tsxapps/server/src/tests/unit/application/services/lancedb-dump-service.test.tspackages/application/src/services/lancedb-dump-service.tspackages/ui/src/pro-search-builder.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/server/src/tests/unit/application/services/lancedb-dump-service.test.ts
- packages/application/src/services/lancedb-dump-service.ts
…rvice.test.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
fix(lancedb): dump manifest version判定を共通化
fix(server): サムネイル生成中の404ログを抑制
- Refactor extractMetadataFromUrl to remove duplicate URL parsing (reuse extractTwitterAuthorIdFromStatusUrl) - Batch author display-name updates into single CASE UPDATE instead of per-author loop - Normalize Twitter accountId at save time (strip @ + lowercase) for index-friendly storage - Merge upstream conflict resolution for duplicate author account inserts
- Normalize legacy authors.accountId on lookup: build legacyByAccount map with normalized keys (strip @ + lowercase) for Twitter account matching - Search DB for both raw and @-prefixed accountId variants - Fix CASE WHEN SQL to compare authors.id instead of bare UUID cast
fix(authors): refresh display names by account identity
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.
Summary by CodeRabbit
New Features
Bug Fixes