Skip to content

技術書っぽさ検索の落穂拾い - #112

Merged
Ojoxux merged 3 commits into
stagingfrom
codex/search-result-followups
Jul 23, 2026
Merged

技術書っぽさ検索の落穂拾い#112
Ojoxux merged 3 commits into
stagingfrom
codex/search-result-followups

Conversation

@Ojoxux

@Ojoxux Ojoxux commented Jul 23, 2026

Copy link
Copy Markdown
Owner

概要

  • 「もっと見る」で検索結果を結合する際、同一ISBNの書籍を重複排除するように変更
  • 楽天サービスに残っていた未使用の出版日並び替え処理とテストを削除
  • 検索APIのデバッグ指定について、debug=1debug=true の両方を結合テストで確認

確認

  • pnpm test(245件成功)
  • pnpm typecheck
  • pnpm lint
  • pnpm format:check
  • pnpm build

@Ojoxux Ojoxux changed the title 技術書検索結果の重複排除と関連処理の整理 関連度検索の落穂拾い Jul 23, 2026
@Ojoxux
Ojoxux marked this pull request as ready for review July 23, 2026 02:51
@Ojoxux Ojoxux changed the title 関連度検索の落穂拾い 技術書っぽさ検索の落穂拾い Jul 23, 2026
@Ojoxux

Ojoxux commented Jul 23, 2026

Copy link
Copy Markdown
Owner Author

@cursoragent

レビューしてください
ref: #91

@cursor

cursor Bot commented Jul 23, 2026

Copy link
Copy Markdown

PR #91 レビュー: 技術書っぽさ検索の落穂拾い

総評: Approve(マージ可)

小さく目的が明確なフォローアップ PR で、変更の意図と実装が一致しています。ローカルで pnpm test(245 passed / 1 skipped)、pnpm typecheckpnpm lint も問題ありません。


変更の整理

変更 評価
mergeRankedSearchResults で ISBN 重複排除 妥当。「もっと見る」時のページ跨ぎ重複に対する正しい対処
rakuten.tssortByPublishedDateDesc 削除 妥当。未使用のデッドコード(並び替えは rankTechBooks / compareRankedSearchResults に集約済み)
debug=1 / debug=true の結合テスト 妥当。スキーマと実装の両方をカバー

良い点

1. 重複排除の「先勝ち」戦略が適切

  return [...currentResults, ...additionalResults]
    .filter((result) => {
      const isbn = result.isbn.replace(/[-\s]/g, "");
      if (!isbn) return true;
      if (seenIsbns.has(isbn)) return false;

      seenIsbns.add(isbn);
      return true;
    })
    .toSorted(compareRankedSearchResults);

[...currentResults, ...additionalResults] の順で走査するため、既に表示済みの 1 ページ目を優先して残します。ページ境界での重複に対して自然な挙動です。

2. テストが実用的

  • ハイフン有無の正規化(978-4-1234-5678-9 vs 9784123456789
  • ISBN 空文字の場合は重複排除しない(誤って潰さない)
  • 入力配列の非破壊性

3. デッドコード削除が安全

sortByPublishedDateDesc の参照はリポジトリ内に残っておらず、削除は問題ありません。なお book-search-ranking.ts 側の parsePublishedDate の方が柔軟(月・日省略対応)なので、残すべきはこちらです。


指摘事項

Minor: ISBN 正規化ロジックの不一致

重複排除:

result.isbn.replace(/[-\s]/g, "")

スコアリング側 (tech-book-search.ts):

value.replace(/[^0-9xX]/g, "").toUpperCase()

楽天 API ではほぼ問題になりませんが、理論上は次のケースで重複が残る可能性があります。

  • ISBN-10 のチェックディジット X / x の大小文字差
  • ハイフン・スペース以外の区切り文字

提案: normalizeIsbnapp/lib/ などに切り出して両方から使うと一貫性が上がります(現状は tech-book-searchbook-search-ranking の依存があるため、直接 import すると循環になります)。

Minor: ISBN-10 と ISBN-13 の同一書籍

同一書籍が ISBN-10 / ISBN-13 の両方で返る場合は重複排除されません。楽天 API では稀ですが、将来的な改善候補です。今回のスコープ外として許容できます。

Nit: 重複排除の「先勝ち」がスコアより優先される点

テストでは techScore が高い後続ページのエントリ(Duplicate, score 20)より、先に出ていたエントリ(Current, score 10)が残ることを確認しています。これはページ跨ぎ重複の文脈では正しい設計ですが、意図をコメントで 1 行補足しておくと、将来「高スコア側を残すべきでは?」という議論を防げます。

Nit: debug=0 / debug=false の明示テストなし

スキーマは '0' | 'false' を許容していますが、結合テストは有効値のみです。既存テストで scoreReasons 未付与は暗黙的に確認されているため、ブロッカーではありません。


確認した動作

  • BookSearchForm の「もっと見る」は mergeRankedSearchResults を正しく利用
  • ScoredBookSearchResultDtoisbn / techScore / publishedDate を持ち、RankedSearchResult 拡張と整合
  • 削除した sortByPublishedDateDesc の参照はなし

結論

マージして問題ない品質です。必須の修正はありません。

任意のフォローアップとしては、ISBN 正規化の共通化(Minor)が最も価値が高いです。必要であれば、その修正用のコミットも対応できます。

Open in Web Open in Cursor 

@Ojoxux
Ojoxux merged commit b5b46b1 into staging Jul 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant