技術書っぽさ検索機能の実装 - #91
Conversation
|
コードレビューしてください |
コードレビュー: 技術書検索の再ランキング全体として、スコアリングをドメイン層に切り出し、ルールベースで拡張しやすい設計になっています。ユニットテスト・結合テストもあり、 一方で、日本語の技術書タイトルに対するネガティブキーワードの誤判定は、本 PR の目的と矛盾するため、マージ前に対応した方がよいです。 良い点1. 責務分離が明確
2. API 統合が最小限 const sortedResults = rankTechBooks(results, query, {
includeReasons: debug === "1" || debug === "true",
});楽天 API 取得後に再ランキングするだけで、既存フローを壊しにくいです。 3. ASCII キーワードの境界マッチfunction hasKeyword(text: string, keyword: string): boolean {
if (!text || !keyword) return false;
const normalizedKeyword = keyword.toLowerCase();
if (isAsciiKeyword(normalizedKeyword)) {
return new RegExp(`(^|[^a-z0-9])${escapeRegExp(normalizedKeyword)}([^a-z0-9]|$)`).test(
text.toLowerCase(),
);
4. テストカバレッジ
14 件すべてパスを確認しました。 要修正（マージ前に対応推奨）1. ネガティブキーワードが技術書を不当に減点するconst NEGATIVE_KEYWORDS = [
"漫画",
...
"攻略本",
"ゲーム攻略",
"雑誌",
"ムック",
];
日本の技術書では「○○ムック」「○○攻略本」はよく使われます。PR の目的（技術書を上位に）と真っ向から矛盾します。 提案: 改善推奨（マージ後でも可）2. レイヤー依存の向きimport type { BookSearchResult } from "../services/rakuten";ドメインがサービス層に依存しています。 提案: 共通型を 3. リクエストごとの RegExp 生成ASCII キーワードごとに 提案: モジュールロード時に 4. キーワードの無制限加算マッチするキーワードごとに満点加算されます。例: タイトルに 提案: フィールドあたりの上限（例: タイトルは最大 3 件）や逓減を検討。 5. 日本語キーワードの部分一致
6. ページ単位の再ランキングと UI の不整合
setResults((prev) => [...prev, ...data.data!.results]);各ページは独立に再ランキングされるため、2 ページ目の高スコア本が 1 ページ目の低スコア本より下に表示される可能性があります。以前の「出版日ソート」でも同様でしたが、スコア差が大きいとより目立ちます。 7. タイブレークのテストが意図とずれている it("should prefer newer books when technical scores are otherwise similar", () => {
...
expect(ranked[0].techScore).toBeGreaterThan(ranked[1].techScore);改訂版は出版日ボーナス（+12）でスコアが上がるため、「スコア同点時の出版日タイブレーク」ではなく「鮮度ボーナスのテスト」になっています。 8.
|
| 項目 | 評価 |
|---|---|
debug パラメータ |
認証必須・スキーマ検証あり。本番でも利用可能だが、スコアリング詳細の露出とレスポンス肥大化の可能性あり。MVP では許容、将来的に本番無効化を検討 |
techScore の常時返却 |
問題なし。フロント未使用だが将来の UI 表示に使える |
| ISBN 検索 | 再ランキングなし。単件取得なので現状は妥当 |
総合評価
| 観点 | 評価 |
|---|---|
| 設計 | 良い（ドメイン分離・デバッグ設計） |
| 正確性 | 要修正（ムック / 攻略本 の誤減点） |
| テスト | 良好だが、上記エッジケースが不足 |
| パフォーマンス | MVP では許容、RegExp 事前コンパイルは改善余地あり |
| マージ判断 | 条件付き LGTM — ネガティブキーワード修正後ならマージ可 |
マージ前にやること（必須）
ムック/攻略本のネガティブキーワード見直し- 技術書タイトルでの回帰テスト追加
マージ後のフォローアップ
- RegExp 事前コンパイル
- 共通型の整理
- ページ跨ぎソート（必要なら）
- タイブレークテストの修正
ネガティブキーワードの修正 PR を出す場合は、その対応も進められます。
|
いくつかなおしたので再レビューたのむ |
再レビュー: 技術書検索の関連度による並び替え前回の指摘の多くが反映されています。4 コミット分（ 前回指摘への対応状況
今回の変更で良くなった点1. NG キーワードの見直しconst NEGATIVE_KEYWORDS = [
"漫画",
...
"ゲーム攻略",
"雑誌",
];
2. キーワード重複の解消
3. 並び替えロジックの共通化
4. テストの充実ISBN 正規化、出版社の空白無視、境界値の出版日、入力配列の非破壊、 残っている軽微な点（マージ阻害ではない）1.
|
| 観点 | 前回 | 今回 |
|---|---|---|
| 正確性 | 要修正 | OK |
| テスト | 不足 | 十分 |
| UX（ページ跨ぎ） | 不整合 | 修正済 |
| マージ判断 | 条件付き | LGTM |
前回ブロッカーだった NG キーワード問題は解消され、テストもしっかり追随しています。設計・実装ともにマージして問題ない水準です。
任意のフォローアップ（マージ後で可）
techScore→relevanceへのリネーム（命名の好み）parsePublishedDateの完全統一- RegExp 事前コンパイル
- PR 説明文の用語更新（タイトルは「関連度による並び替え」に変わっているが、本文はまだ
techScore/ 再ランキング表記）
…arch-reranking # Conflicts: # app/islands/BookSearchForm.tsx # app/server/api/search.test.ts # app/types/api.ts
1 participant
概要
techScoreを付与し、検索 API の返却順を再ランキングするように変更debug=1/debug=trueのときだけscoreReasonsを返し、スコア調整時に理由を確認できるようにした確認
補足
gh認証トークンが無効だったため、PR 作成は GitHub コネクタ経由で行っています。.codex/,AGENTS.md,SECURITY_REVIEW.mdなど）は PR に含めていません。