diff --git a/README.ja.md b/README.ja.md index 59a908a..9a0aa50 100644 --- a/README.ja.md +++ b/README.ja.md @@ -108,7 +108,7 @@ bot (`sender.login` が `[bot]` で終わる) と trim 後 10 文字未満の bo | `milestone` | string | milestone title で絞り込み。 | | `assignee` | string | assignee login で絞り込み。 | | `type` | 下記参照 | type で絞り込み (既定 `all`)。 | -| `top_k` | number | 最大件数 (既定 10、上限 50)。 | +| `top_k` | number | 最大件数 (既定 10、上限 50)。索引の行数ではなく実体の数で数えます。下記「実体単位の集約」参照。 | | `fusion` | `"rrf"` / `"dense_only"` / `"sparse_only"` | fusion 戦略 (既定 `rrf`)。scan モードでは無視。 | | `rerank` | boolean | cross-encoder rerank (既定 `true`)。scan モードでは無視。 | | `sort` | `"relevance"` / `"updated_desc"` / `"created_desc"` | 並び順。query ありの既定は `relevance`、query なしの既定は `updated_desc`。時系列指定は ranker score を上書きします。 | @@ -133,6 +133,12 @@ bot (`sender.login` が `[bot]` で終わる) と trim 後 10 文字未満の bo | `"pr_review_comment"` | PR の per-line インラインレビューコメント。 | | `"all"` | 上記すべての union (既定)。 | +#### 実体単位の集約 + +1 つの実体は複数行として索引されます。ファイルは `doc` 行 + それを触った commit の数だけの `diff` 行、issue / PR は本体 + そのコメントやレビュー、という形です。応答を `top_k` 件に切り詰める前にこれらを 1 件へ畳むので、`top_k` はそのまま独立した実体の数になります。畳む基準は「その行が何を指しているか」であって「どの作業がその行を生んだか」ではありません。同一 commit が触った別々のファイルは別々の結果として残り、issue とそれを閉じる PR も別々に残ります。 + +代表になるのはその group で最上位に来た行です。したがって「いつ変わったか」を問う検索では、現在の版ではなく該当する古い commit diff が返ります。他の行を吸収した結果には `same_entity` フィールドが付き(`count` は自身を含む件数、`others[]` は畳んだ各行の type / URL / 時刻 / score)、畳んだ分は捨てられません。完全な規則は [docs/0-requirements.ja.md](docs/0-requirements.ja.md) を参照してください。 + #### 使用例 特定トピックの意味検索: diff --git a/README.md b/README.md index dd3bfe2..a7c777d 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ Bot-authored comments (`sender.login` ending in `[bot]`) and comments shorter th | `milestone` | string | Filter by milestone title. | | `assignee` | string | Filter by assignee login. | | `type` | see below | Filter by type (default `all`). | -| `top_k` | number | Max results (default 10, max 50). | +| `top_k` | number | Max results (default 10, max 50). Counts distinct entities, not index rows — see Entity aggregation below. | | `fusion` | `"rrf"` \| `"dense_only"` \| `"sparse_only"` | Fusion strategy (default `rrf`). Ignored in scan mode. | | `rerank` | boolean | Cross-encoder rerank (default `true`). Ignored in scan mode. | | `sort` | `"relevance"` \| `"updated_desc"` \| `"created_desc"` | Result ordering. Default `relevance` with a query, `updated_desc` without. Time sorts override ranker score. | @@ -133,6 +133,12 @@ Bot-authored comments (`sender.login` ending in `[bot]`) and comments shorter th | `"pr_review_comment"` | PR inline review comments (per-line diff comments). | | `"all"` | Union of every type above (default). | +#### Entity aggregation + +One thing is indexed as several rows: a file is a `doc` row plus one `diff` row per commit that touched it, an issue or PR is its own row plus its comments and reviews. Those rows are collapsed into one result before the response is trimmed, so `top_k` returns that many distinct entities. Rows are grouped by what they point at, not by the work that produced them — different files touched by one commit stay separate results, and so do an issue and the PR that closes it. + +The representative is the highest-ranked row of the group, so a query about when something changed still returns the relevant old commit diff rather than the current version. A result that absorbed other rows carries a `same_entity` field (`count` including itself, plus `others[]` with the type, URL, timestamp and score of each collapsed row) so nothing is lost. See [docs/0-requirements.md](docs/0-requirements.md) for the full rule. + #### Examples Semantic search for a specific topic: diff --git a/docs/0-requirements.ja.md b/docs/0-requirements.ja.md index ccf8401..e9a0bf6 100644 --- a/docs/0-requirements.ja.md +++ b/docs/0-requirements.ja.md @@ -309,12 +309,13 @@ retrieval layer は hybrid search(dense + sparse)+ cross-encoder rerank + st 1. query の embedding を Workers AI BGE-M3 で生成 2. structured params から Vectorize filter(dense 側)と D1 SQL WHERE(sparse 側)を同時構築(repo, state, type, milestone は pre-filter) -3. labels / assignee フィルタ指定時、または reranker 有効時は内部 topK をオーバーフェッチ(requestedTopK × 5, max 50)。reranker は最大 50 件まで処理 +3. 内部 topK を常にオーバーフェッチ(requestedTopK × 5, max 50)。条件なしなのは、8 の entity 集約がどの経路でも複数行を 1 件に畳むため、rerank 無効時でも候補プールが top_k を上回っていなければ要求件数を満たせないからである。reranker は最大 50 件まで処理 4. dense (Vectorize.query) と sparse (D1 FTS5 MATCH + BM25) を並列実行 5. 両 ranker の結果を Reciprocal Rank Fusion(RRF、k=60)で合成 6. 合成後の rank 順に、labels(AND ロジック、個別フィールド + CSV フォールバック)と assignee を post-filter 7. reranker 有効時(default ON)は post-filter 後の候補を `@cf/baai/bge-reranker-base` で re-score し、reranker score 降順に並び替え -8. requestedTopK にトリムして structured context と共に返す +8. 同一 entity を指す行を畳み、最上位の行を代表にする(Entity Aggregation 参照) +9. requestedTopK にトリムして structured context と共に返す #### Reciprocal Rank Fusion (RRF) @@ -356,6 +357,47 @@ score(d) = sum_over_rankers ( 1 / (k + rank_r(d)) ) - Free tier 10,000 neurons/day で約 1,300 検索/day 上限 - neuron 実測値はレスポンスに `usage` フィールドが含まれる場合に取得し、理論試算と照合する(公式未文書化のため存在しない場合は黙ってスキップ) +### Entity Aggregation + +1 つの実体が複数行として索引される。ファイルは `doc` 行 + それを触った commit の数だけの `diff` 行、issue は本体 + コメント数だけの `issue_comment` 行、PR は本体 + `pr_review` / `pr_review_comment` 行。これらが同一の `top_k` プールで枠を奪い合う。本番索引での実測(2026-08-01、`top_k: 10`、rrf + rerank)では独立した情報は 10 枠中 6 前後、`dense_only` では 5 枠中 3 枠が同一ファイルだった。 + +diff を索引し続けるのは意図的である——diff は判断履歴そのものであり、変更を commit diff として持つ設計が索引の容量を抑えている。したがって畳むのは表示段(fusion / rerank / time sort の後、trim の手前)であって索引側ではない。 + +**同一実体の定義.** 実体とはその行が指している対象(referent)であって、その行を生んだ作業(event)ではない。 + +| 行 | key | 畳む | +|---|---|---| +| ファイルの `doc` 行 + 複数 commit の `diff` 行 | `file:{repo}:{doc_path ?? file_path}` | する | +| issue + その `issue_comment` 行 | `thread:{repo}:{number}` | する | +| PR + その `pr_review` / `pr_review_comment` 行 | `thread:{repo}:{number}` | する | +| 同一 commit が触った別々のファイル | — | **しない** | +| issue と、それを閉じる PR | — | **しない** | +| 同一ソースファイルの他 repo への複製 | — | **しない** | + +同じ対象の複数の版を畳んでも対象の数は減らないので、集約が独立した情報を隠すことはない。一方「作業」で畳むと、1 つの commit が触った別々のファイルが 1 枠になり、実際に独立した対象が隠れる。だから key はパスを持ち commit SHA を持たない。issue と、それを閉じる PR も 2 実体のまま残す——同一の作業単位ではあるが、両者を結ぶ `Closes #N` は索引に無く、入れるには索引側の変更が要る。 + +repo 横断の複製(Li+ source が各 user repo の `.claude/` に複製されている件)は対象外。同一と判定するには内容 hash を索引に持たせるか、パスの正規化ヒューリスティクスを置くかが要る。前者は索引側の変更で、後者は本当に別物のファイルを誤って畳む。加えて配布先が古い場合はその差異自体が情報である。 + +`wiki_doc` / `release` は 1 実体 1 行なので行の identity を key にし、畳まれることはない。 + +**代表の選び方.** fusion / rerank / time sort 後の順位が最上位の行。最新版は固定**しない**。これが「いつ変わったか」を問うクエリへの答えを残す: そのクエリでは該当する古い `diff` が最上位に来るので、それが代表として残る。最新版固定だと答えそのものが消える。 + +**返却形式.** `top_k` は実体の数で数えるので、10 を要求した呼び出し側には独立実体 10 件が返る。他の行を吸収した代表 item にはフィールドが 1 つ増える。畳んだ行は参照として付され、捨てられない: + +```json +{ + "...": "(代表 item の既存フィールド)", + "same_entity": { + "count": 3, + "others": [ + { "type": "diff", "url": "...", "updated_at": "...", "score": 0.0161, "commit_sha": "601aa38" } + ] + } +} +``` + +`count` は代表を含むので必ず 2 以上。1 件以上畳んだ場合のみ付く。フィールド追加であって既存フィールドの変更ではないので、無視する client には集約前と同じ形に見える。 + ### 切替オプション `search` の `fusion` パラメータで retrieval mode を切り替え可能: @@ -398,6 +440,7 @@ Returns: - repository、type、state、labels、milestone、assignees、URL、RRF fused score を含む ranked match - 追加 debug フィールド: `dense_score`、`sparse_score`、`dense_rank`、`sparse_rank`、`rerank_score`(rerank 無効時または fallback 時は null) +- 同一実体の他の行を吸収した結果には `same_entity`(Entity Aggregation 参照)。`top_k` は行数ではなく実体数で数える - top-level metadata: `fusion`、`dense_candidates`、`sparse_candidates`、`rerank_requested`、`rerank_applied` **scan mode(query 空).** Vectorize / FTS5 / reranker を経由せず、structured store の recency endpoint から集約する。`since` / `until` は store 側へ push down されるので、窓に行があれば、その窓がどれだけ古くても返る。`since` 省略時の既定は `until` の 7 日前(`until` も省略時は現在の 7 日前)。`until` だけ指定した問い合わせが「下限が上限より新しい空窓」に潰れないための既定である。 diff --git a/docs/0-requirements.md b/docs/0-requirements.md index 4243c00..7a96156 100644 --- a/docs/0-requirements.md +++ b/docs/0-requirements.md @@ -311,12 +311,13 @@ Expected retrieval behavior: 1. Generate an embedding for the query via Workers AI BGE-M3. 2. Build Vectorize metadata filter (dense side) and D1 SQL WHERE clause (sparse side) from the same structured params (repo, state, type, milestone are pre-filtered on both sides). -3. When labels or assignee filters are present, OR when the reranker is enabled, overfetch internally on both sides (requestedTopK × 5, max 50). The reranker processes at most 50 candidates per call. +3. Overfetch internally on both sides (requestedTopK × 5, max 50). Unconditional: entity aggregation (step 8) collapses several rows into one result on every path, so the candidate pool must exceed top_k even when the reranker is off. The reranker processes at most 50 candidates per call. 4. Query Vectorize (dense) and D1 FTS5 (sparse, BM25) in parallel. 5. Combine the two rankers via Reciprocal Rank Fusion (RRF, k=60). 6. Post-filter labels (AND logic, expanded fields + CSV fallback) and assignee over the fused view. 7. When the reranker is enabled (default ON), re-score the post-filtered candidates with `@cf/baai/bge-reranker-base` and reorder by reranker score, descending. -8. Trim to requestedTopK and return results with structured context. +8. Collapse rows that point at the same entity, keeping the highest-ranked row as the representative (see Entity Aggregation). +9. Trim to requestedTopK and return results with structured context. #### Reciprocal Rank Fusion (RRF) @@ -358,6 +359,47 @@ Cost estimate: - Free tier (10,000 neurons/day) supports ~1,300 searches/day at this rate. - Actual neuron usage is read from a `usage` field on the response when present and reconciled against the estimate. The field is not officially documented as of 2026-04, so absence is tolerated silently. +### Entity Aggregation + +One underlying thing is indexed as several rows: a file is a `doc` row plus one `diff` row per commit that touched it, an issue is its own row plus one `issue_comment` row per comment, a PR is its own row plus its `pr_review` / `pr_review_comment` rows. They all compete for slots in the same `top_k` pool. Measured on the production index (2026-08-01, `top_k: 10`, rrf + rerank), roughly 6 of 10 slots held independent information; a `dense_only` probe put 3 of 5 slots on one file. + +Keeping the diff rows indexed is deliberate — they are the judgment history, and storing changes as commit diffs is what keeps the index small — so the collapse happens at the presentation stage (after fusion, rerank and time sort; before the trim), not in the index. + +**What counts as one entity.** The entity is the *referent* a row points at, never the *event* that produced the row. + +| rows | key | collapse | +|---|---|---| +| a file's `doc` row + its `diff` rows across commits | `file:{repo}:{doc_path ?? file_path}` | yes | +| an issue + its `issue_comment` rows | `thread:{repo}:{number}` | yes | +| a PR + its `pr_review` / `pr_review_comment` rows | `thread:{repo}:{number}` | yes | +| different files touched by one commit | — | **no** | +| an issue and the PR that closes it | — | **no** | +| the same source file copied into another repo | — | **no** | + +Collapsing versions of one referent cannot hide an independent thing, because the number of referents is unchanged. Collapsing by event would: one commit touches several distinct files, and folding them into a single slot hides files that are genuinely independent. So the key carries the path and not the commit SHA, and an issue and the PR that closes it stay two entities — they are one unit of work, but the `Closes #N` link that would join them is not in the index, and putting it there is an index-side change. + +Cross-repo duplication (the same Li+ source file copied into every user repo's `.claude/`) is out of scope: deciding those are one entity needs either a content hash in the index or a path-normalizing heuristic — the first is an index change, the second folds genuinely different files, and a stale copy's difference is itself information. + +`wiki_doc` and `release` rows have exactly one row per referent, so they key on the row identity and never collapse. + +**Representative.** The highest-ranked row of the group after fusion / rerank / time sort — the newest version is *not* pinned. This is what keeps "when did this change" answerable: for such a query the relevant old `diff` ranks top, so it is the row that survives. Pinning the newest version would delete the answer. + +**Response shape.** `top_k` counts entities, so a caller asking for 10 gets 10 independent entities. A representative that absorbed other rows carries one additional field; the collapsed rows are referenced, never dropped: + +```json +{ + "...": "(the representative's existing fields)", + "same_entity": { + "count": 3, + "others": [ + { "type": "diff", "url": "...", "updated_at": "...", "score": 0.0161, "commit_sha": "601aa38" } + ] + } +} +``` + +`count` includes the representative, so it is at least 2. The field is present only when at least one row was collapsed, and it is additive — a client that ignores it sees the pre-aggregation shape. + ### Fusion mode toggle `search` accepts a `fusion` parameter: @@ -400,6 +442,7 @@ Returns: - ranked matches with repository, type, state, labels, milestone, assignees, URL, and RRF fused `score` - additional debug fields per result: `dense_score`, `sparse_score`, `dense_rank`, `sparse_rank`, `rerank_score` (null when rerank disabled or when graceful fallback engaged) +- `same_entity` on results that absorbed other rows of the same entity (see Entity Aggregation); `top_k` counts entities, not rows - top-level metadata: `fusion`, `dense_candidates`, `sparse_candidates`, `rerank_requested`, `rerank_applied` **Scan mode (empty query).** Vectorize / FTS5 / reranker are skipped and the result set is aggregated from the structured store's recency endpoints. `since` / `until` are pushed down to the store, so a window returns rows whenever it holds rows, however far back it sits. `since` defaults to 7 days before `until` (before now when `until` is omitted), so an `until`-only query does not degenerate into an empty window above its own ceiling. diff --git a/mcp-server/server/tools.js b/mcp-server/server/tools.js index 39d5c82..2c80678 100644 --- a/mcp-server/server/tools.js +++ b/mcp-server/server/tools.js @@ -24,7 +24,10 @@ export const TOOLS = [ "optionally narrow via since / until; " + "(3) doc content fetch — include_content: true inlines raw content on top doc and wiki_doc results. " + "Structured filters (repo, state, labels, milestone, assignee, type) apply across all modes; " + - "type: \"wiki_doc\" narrows to GitHub Wiki pages only.", + "type: \"wiki_doc\" narrows to GitHub Wiki pages only. " + + "Results are aggregated per underlying entity: a file's doc row and its commit diffs are one result, " + + "an issue or PR and its comments / reviews are one result. top_k therefore counts distinct entities, " + + "and a result that absorbed others carries same_entity { count, others[] } with links to them.", inputSchema: { type: "object", properties: { @@ -81,7 +84,9 @@ export const TOOLS = [ }, top_k: { type: "number", - description: "Max results (default: 10, max: 50)", + description: + "Max results (default: 10, max: 50). Counts distinct entities, not index rows " + + "(a file's doc row and its commit diffs collapse into one result).", }, fusion: { type: "string", diff --git a/src/aggregate.test.ts b/src/aggregate.test.ts new file mode 100644 index 0000000..3c6153b --- /dev/null +++ b/src/aggregate.test.ts @@ -0,0 +1,245 @@ +import { describe, it, expect } from "vitest"; +import { entityKey, groupByEntity, type EntityRow } from "./aggregate.js"; + +/** + * Binding-independent unit tests for entity aggregation (node pool). + * + * Focus = the issue #189 surface: `top_k` was being spent on several rows of + * one referent, and the line that must hold while fixing it is referent ≠ + * event. Two files touched by one commit are two referents and must survive + * as two results; a file's `doc` row and its `diff` rows are one referent and + * collapse into one. + */ + +function row(over: Partial & Pick): EntityRow { + return { repo: "Liplus-Project/Li-plus", number: 0, ...over }; +} + +/** Rank-ordered pool → the entity keys of the surviving representatives. */ +function keysAfterAggregation(rows: EntityRow[], topK: number): string[] { + return groupByEntity(rows, entityKey) + .slice(0, topK) + .map((g) => g.key); +} + +describe("entityKey: file class (doc + diff of the same file)", () => { + it("gives a doc row and every diff row of that file one key", () => { + const doc = row({ vectorId: "v-doc", type: "doc", docPath: "skills/x/SKILL.md" }); + const diffA = row({ vectorId: "v-a", type: "diff", filePath: "skills/x/SKILL.md" }); + const diffB = row({ vectorId: "v-b", type: "diff", filePath: "skills/x/SKILL.md" }); + + expect(entityKey(doc)).toBe("file:Liplus-Project/Li-plus:skills/x/SKILL.md"); + expect(entityKey(diffA)).toBe(entityKey(doc)); + expect(entityKey(diffB)).toBe(entityKey(doc)); + }); + + it("keeps two files touched by ONE commit as two entities (referent, not event)", () => { + // The line the whole design rests on: folding by commit would hide a file + // that is genuinely independent. The SHA is deliberately absent from the key. + const fileA = row({ vectorId: "v-a", type: "diff", filePath: "rules/model/absolute.md" }); + const fileB = row({ vectorId: "v-b", type: "diff", filePath: "rules/task/task.md" }); + + expect(entityKey(fileA)).not.toBe(entityKey(fileB)); + expect(keysAfterAggregation([fileA, fileB], 10)).toHaveLength(2); + }); + + it("keeps the same path in two different repos apart (cross-repo copies are out of scope)", () => { + const upstream = row({ vectorId: "v-a", type: "doc", docPath: "skills/x/SKILL.md" }); + const copy = row({ + vectorId: "v-b", + type: "doc", + repo: "Liplus-Project/github-webhook-mcp", + docPath: "skills/x/SKILL.md", + }); + + expect(entityKey(upstream)).not.toBe(entityKey(copy)); + }); +}); + +describe("entityKey: thread class (issue / PR + its comments)", () => { + it("gives an issue and its top-level comments one key", () => { + const issue = row({ vectorId: "v-i", type: "issue", number: 1317 }); + const comment = row({ vectorId: "v-c", type: "issue_comment", number: 1317 }); + + expect(entityKey(comment)).toBe(entityKey(issue)); + expect(entityKey(issue)).toBe("thread:Liplus-Project/Li-plus:1317"); + }); + + it("gives a PR, its reviews and its inline review comments one key", () => { + const pr = row({ vectorId: "v-p", type: "pull_request", number: 1318 }); + const review = row({ vectorId: "v-r", type: "pr_review", number: 1318 }); + const inline = row({ + vectorId: "v-rc", + type: "pr_review_comment", + number: 1318, + filePath: "src/mcp.ts", + }); + + expect(entityKey(review)).toBe(entityKey(pr)); + expect(entityKey(inline)).toBe(entityKey(pr)); + }); + + it("keeps an issue and the PR that closes it as two entities", () => { + // One unit of work, two referents. The `Closes #N` link that would join + // them is not in the index, so joining here would be a guess. + const issue = row({ vectorId: "v-i", type: "issue", number: 1317 }); + const pr = row({ vectorId: "v-p", type: "pull_request", number: 1318 }); + + expect(entityKey(issue)).not.toBe(entityKey(pr)); + }); +}); + +describe("entityKey: rows that must never fold", () => { + it("keeps wiki pages out of the file namespace", () => { + const wiki = row({ vectorId: "v-w", type: "wiki_doc", docPath: "Decision-Structure" }); + const doc = row({ vectorId: "v-d", type: "doc", docPath: "Decision-Structure" }); + + expect(entityKey(wiki)).toBe("row:v-w"); + expect(entityKey(wiki)).not.toBe(entityKey(doc)); + }); + + it("falls back to the row identity when the keying field is missing", () => { + const pathless = row({ vectorId: "v-a", type: "diff" }); + const numberless = row({ vectorId: "v-b", type: "issue" }); + const release = row({ vectorId: "v-c", type: "release" }); + + expect(entityKey(pathless)).toBe("row:v-a"); + expect(entityKey(numberless)).toBe("row:v-b"); + expect(entityKey(release)).toBe("row:v-c"); + }); +}); + +describe("groupByEntity", () => { + it("keeps the highest-ranked row as representative and the rest as others", () => { + const rows = [ + row({ vectorId: "v-1", type: "diff", filePath: "skills/x/SKILL.md" }), + row({ vectorId: "v-2", type: "doc", docPath: "docs/other.md" }), + row({ vectorId: "v-3", type: "doc", docPath: "skills/x/SKILL.md" }), + row({ vectorId: "v-4", type: "diff", filePath: "skills/x/SKILL.md" }), + ]; + + const groups = groupByEntity(rows, entityKey); + + expect(groups).toHaveLength(2); + expect(groups[0].representative.vectorId).toBe("v-1"); + expect(groups[0].others.map((o) => o.vectorId)).toEqual(["v-3", "v-4"]); + expect(groups[1].representative.vectorId).toBe("v-2"); + expect(groups[1].others).toEqual([]); + }); + + it("does not pin the newest version — an older diff ranked top stays the answer", () => { + // Negative control for "when did this change": the query ranks the old + // diff first, so the collapse must not replace it with the live doc row. + const oldDiff = row({ vectorId: "v-old-diff", type: "diff", filePath: "rules/model/absolute.md" }); + const liveDoc = row({ vectorId: "v-doc", type: "doc", docPath: "rules/model/absolute.md" }); + + const groups = groupByEntity([oldDiff, liveDoc], entityKey); + + expect(groups).toHaveLength(1); + expect(groups[0].representative.vectorId).toBe("v-old-diff"); + expect(groups[0].others.map((o) => o.vectorId)).toEqual(["v-doc"]); + }); + + it("preserves rank order of the representatives", () => { + const rows = [ + row({ vectorId: "v-1", type: "issue", number: 10 }), + row({ vectorId: "v-2", type: "issue", number: 20 }), + row({ vectorId: "v-3", type: "issue_comment", number: 10 }), + row({ vectorId: "v-4", type: "issue", number: 30 }), + ]; + + expect(groupByEntity(rows, entityKey).map((g) => g.representative.vectorId)).toEqual([ + "v-1", + "v-2", + "v-4", + ]); + }); + + it("returns an empty array for an empty pool", () => { + expect(groupByEntity([], entityKey)).toEqual([]); + }); +}); + +describe("duplicate rate regression (issue #189 measurements, 2026-08-01)", () => { + /** + * The two pools below reproduce the duplication actually observed on the + * production index at `top_k: 10` (rrf + rerank). Before aggregation each + * pool spent ~10 slots on ~6 independent things. The assertions fix both + * directions: the known duplicates collapse, and the known non-duplicates + * (same commit / issue+PR / cross-repo copy) do not. + */ + + it("`subagent への委譲はいつ必須になるのか`: doc + diff of one file collapse", () => { + const repo = "Liplus-Project/Li-plus"; + const pool: EntityRow[] = [ + row({ vectorId: "d1", type: "doc", docPath: "skills/task-subagent-delegation/SKILL.md" }), + // same file, commit 45deef8 — collapses into the doc row above + row({ vectorId: "d2", type: "diff", filePath: "skills/task-subagent-delegation/SKILL.md" }), + // different file of the SAME commit 45deef8 — stays independent + row({ vectorId: "d3", type: "diff", filePath: "rules/task/task.md" }), + // the github-webhook-mcp copy of the same source — stays independent (axis (c)) + row({ + vectorId: "d4", + type: "doc", + repo: "Liplus-Project/github-webhook-mcp", + docPath: ".claude/skills/task-subagent-delegation/SKILL.md", + }), + row({ vectorId: "d5", type: "issue", number: 919, repo }), + row({ vectorId: "d6", type: "doc", docPath: "rules/operations/operations.md", repo }), + row({ vectorId: "d7", type: "wiki_doc", docPath: "Subagent-Delegation", repo }), + row({ vectorId: "d8", type: "issue", number: 1180, repo }), + row({ vectorId: "d9", type: "pull_request", number: 1181, repo }), + row({ vectorId: "d10", type: "diff", filePath: "docs/G.-Sheepdog-Engineering.md", repo }), + row({ vectorId: "d11", type: "release", repo }), + row({ vectorId: "d12", type: "issue_comment", number: 1180, repo }), + ]; + + const groups = groupByEntity(pool, entityKey); + + // 12 rows → 10 entities: d1+d2 collapse, d8+d12 collapse. + expect(groups).toHaveLength(10); + expect(groups[0].others.map((o) => o.vectorId)).toEqual(["d2"]); + + const top10 = keysAfterAggregation(pool, 10); + expect(top10).toHaveLength(10); + expect(new Set(top10).size).toBe(10); + }); + + it("`wiki sync sidebar integrity check`: one file across two commits collapses", () => { + const repo = "Liplus-Project/Li-plus"; + const pool: EntityRow[] = [ + // same commit afae460, two files — stays two entities + row({ vectorId: "w1", type: "diff", filePath: "skills/operations-on-wiki-sync/SKILL.md", repo }), + row({ vectorId: "w2", type: "diff", filePath: "scripts/wiki-sync.sh", repo }), + // issue #1317 and PR #1318, one unit of work — stays two entities + row({ vectorId: "w3", type: "issue", number: 1317, repo }), + row({ vectorId: "w4", type: "pull_request", number: 1318, repo }), + // docs/Decision-Structure.md in two different commits — collapses + row({ vectorId: "w5", type: "diff", filePath: "docs/Decision-Structure.md", repo }), + row({ vectorId: "w6", type: "diff", filePath: "docs/Decision-Structure.md", repo }), + row({ vectorId: "w7", type: "doc", docPath: "docs/Decision-Structure.md", repo }), + row({ vectorId: "w8", type: "wiki_doc", docPath: "Wiki-Sync", repo }), + row({ vectorId: "w9", type: "pr_review", number: 1318, repo }), + row({ vectorId: "w10", type: "issue", number: 1290, repo }), + row({ vectorId: "w11", type: "doc", docPath: "docs/4.-Operations.md", repo }), + row({ vectorId: "w12", type: "diff", filePath: "docs/4.-Operations.md", repo }), + ]; + + const groups = groupByEntity(pool, entityKey); + const keys = groups.map((g) => g.key); + + // 12 rows → 8 entities: w5+w6+w7 collapse, w4+w9 collapse, w11+w12 collapse. + expect(groups).toHaveLength(8); + expect(new Set(keys).size).toBe(8); + + const decisionStructure = groups.find( + (g) => g.key === `file:${repo}:docs/Decision-Structure.md`, + ); + expect(decisionStructure?.representative.vectorId).toBe("w5"); + expect(decisionStructure?.others.map((o) => o.vectorId)).toEqual(["w6", "w7"]); + + // Negative controls: the pairs that must NOT be one entity. + expect(entityKey(pool[0])).not.toBe(entityKey(pool[1])); // one commit, two files + expect(entityKey(pool[2])).not.toBe(entityKey(pool[3])); // issue #1317 vs PR #1318 + }); +}); diff --git a/src/aggregate.ts b/src/aggregate.ts new file mode 100644 index 0000000..17cdec9 --- /dev/null +++ b/src/aggregate.ts @@ -0,0 +1,129 @@ +/** + * Entity aggregation for the retrieval surface — collapse rows that point at + * the same referent into one representative. + * + * Layer = L4 Operations (retrieval surface) + * + * Why this exists (issue #189 / #180 fact 2): + * One underlying thing is indexed as several rows. A single file is a `doc` row + * plus one `diff` row per commit that touched it; a single issue is an `issue` + * row plus one `issue_comment` row per comment; a single PR is a `pull_request` + * row plus its `pr_review` / `pr_review_comment` rows. All of them compete for + * slots in the same `top_k` pool, so `top_k: 10` was measured returning roughly + * 6 independent things. Keeping the diff rows indexed is deliberate (they are + * the judgment history, and the commit-diff backup design saves storage), so + * the collapse belongs at the presentation stage, not the index. + * + * What is a "same entity" (the load-bearing line): + * The entity is the REFERENT a row points at, never the EVENT that produced the + * row. Collapsing several versions of one referent cannot hide an independent + * thing — the count of referents is unchanged. Collapsing by event would: one + * commit touches several distinct files, and folding them into a single slot + * hides files that are genuinely independent. So `file:` keys on the path and + * NOT on the commit SHA, and issue #1317 / PR #1318 stay two entities even + * though they are one unit of work (the `Closes #N` link that would join them + * is not in the index, and putting it there is an index-side change). + * + * Out of scope: cross-repo duplication of the same Li+ source file (the same + * SKILL.md copied into every user repo's `.claude/`). Deciding those are one + * entity needs a content hash in the index or a path-normalizing heuristic — + * the first is an index change, the second folds genuinely different files. + * Tracked separately. + */ + +/** + * The fields of a resolved search row that the entity key is derived from. + * Deliberately a narrow structural subset: the caller resolves them from + * Vectorize metadata / the FTS row and passes the projection, so this module + * stays free of retrieval-layer types and is unit-testable on plain objects. + */ +export interface EntityRow { + /** Vectorize vector id — the per-row identity, used as the no-fold fallback. */ + vectorId: string; + repo: string; + type: string; + /** Issue / PR number. 0 when the row type has none. */ + number: number; + /** `doc` rows: repo-relative path of the document. */ + docPath?: string; + /** `diff` rows: repo-relative path of the file inside the commit. */ + filePath?: string; +} + +/** One entity: the highest-ranked row plus the rows collapsed into it. */ +export interface EntityGroup { + key: string; + representative: T; + others: T[]; +} + +/** + * Entity key for one row. + * + * - `file:{repo}:{path}` — a `doc` row and every `diff` row of the same file. + * The referent is the file; the commit is the event and is not in the key. + * - `thread:{repo}:{number}` — an `issue` / `pull_request` row and its + * comments / reviews / inline review comments. Issues and PRs share one + * number space per repo, so the number alone is unambiguous. + * - `row:{vectorId}` — everything else, and any row missing the field its + * class keys on. A unique key means the row never folds, which is the + * safe direction: a missed collapse costs a slot, a wrong collapse hides + * an independent result. + * + * `wiki_doc` and `release` rows fall through to `row:` on purpose. Each has + * exactly one row per referent, so there is nothing to collapse, and keying + * a wiki page into the `file:` namespace would risk folding it together with + * a repo doc that happens to share its path. + */ +export function entityKey(row: EntityRow): string { + switch (row.type) { + case "doc": + case "diff": { + const path = (row.type === "doc" ? row.docPath : row.filePath) ?? ""; + if (path === "") break; + return `file:${row.repo}:${path}`; + } + case "issue": + case "pull_request": + case "issue_comment": + case "pr_review": + case "pr_review_comment": { + if (!row.number) break; + return `thread:${row.repo}:${row.number}`; + } + } + return `row:${row.vectorId}`; +} + +/** + * Group an already-ordered candidate list by entity key, preserving rank. + * + * The first row seen for a key becomes the representative, so the + * representative is whatever the fusion / rerank / time sort put highest — + * the newest version is NOT pinned. That is what keeps a "when did this + * change" query answerable: for such a query the relevant old diff ranks + * top, so it is the row that survives the collapse. Pinning the newest + * version would delete the answer. + * + * Groups come back in representative order, so slicing to `top_k` yields + * `top_k` independent entities. + */ +export function groupByEntity( + rows: readonly T[], + keyOf: (row: T) => string, +): Array> { + const byKey = new Map>(); + const ordered: Array> = []; + for (const row of rows) { + const key = keyOf(row); + const existing = byKey.get(key); + if (existing) { + existing.others.push(row); + continue; + } + const group: EntityGroup = { key, representative: row, others: [] }; + byKey.set(key, group); + ordered.push(group); + } + return ordered; +} diff --git a/src/mcp.ts b/src/mcp.ts index 2335c18..46d2e1f 100644 --- a/src/mcp.ts +++ b/src/mcp.ts @@ -33,6 +33,7 @@ import { import { rerankCandidates, rerankWasApplied, RERANK_MAX_CANDIDATES } from "./rerank.js"; import { queryNeighbors, getDocsByVectorIds } from "./graph.js"; import { runScan, type ScanRow } from "./scan.js"; +import { entityKey, groupByEntity } from "./aggregate.js"; /** User context passed via props from OAuth layer */ interface McpProps extends Record { @@ -63,6 +64,111 @@ function githubHeaders(token: string): Record { */ const INCLUDE_CONTENT_MAX_DOCS = 5; +/** + * Per-candidate payload assembled after fusion: the dense metadata, the sparse + * FTS row, and each ranker's raw score. Either side may be missing — a hit can + * come from one ranker only. + */ +interface RowPayload { + meta: VectorMetadata | undefined; + ftsRow: FtsHit | undefined; + denseScore: number | undefined; + sparseScore: number | undefined; +} + +/** + * A candidate's fields resolved from whichever side saw it. Single resolution + * site for the three consumers that need them: the entity key (aggregation), + * the result item, and the `same_entity.others` references. + */ +interface ResolvedRow { + repo: string; + number: number; + type: string; + state: string; + labelsCsv: string; + milestone: string; + assigneesCsv: string; + updatedAt: string; + tagName: string; + docPath: string; + wikiPath: string; + wikiExtension: string; + commitSha: string; + filePath: string; + fileStatus: string; + commitDate: string; + commitAuthor: string; + author: string; + commentId: number; + reviewId: number; + line: number; +} + +/** + * Resolve one candidate's fields. Dense metadata wins when present (it carries + * the dedicated fields), the FTS row fills the gaps for sparse-only hits. + */ +function resolveRow(p: RowPayload | undefined): ResolvedRow { + const meta = p?.meta; + const ftsRow = p?.ftsRow; + const type = meta?.type ?? (ftsRow?.type as VectorMetadata["type"] | undefined) ?? ""; + return { + repo: meta?.repo ?? ftsRow?.repo ?? "", + number: meta?.number ?? ftsRow?.number ?? 0, + type, + state: meta?.state ?? ftsRow?.state ?? "", + labelsCsv: meta?.labels ?? ftsRow?.labels ?? "", + milestone: meta?.milestone ?? ftsRow?.milestone ?? "", + assigneesCsv: meta?.assignees ?? ftsRow?.assignees ?? "", + updatedAt: meta?.updated_at ?? ftsRow?.updatedAt ?? "", + tagName: meta?.tag_name ?? ftsRow?.tagName ?? "", + docPath: meta?.doc_path ?? ftsRow?.docPath ?? "", + // wiki_doc rows reuse the FTS5 `doc_path` column for the page slug — the + // schema-level field is unified across "where did this come from", + // distinguished by the row's `type`. Vectorize metadata carries the + // dedicated `wiki_path` / `wiki_extension` fields so we prefer them when + // present and fall back to the FTS row when the dense hit lost. + wikiPath: meta?.wiki_path ?? (type === "wiki_doc" ? ftsRow?.docPath ?? "" : ""), + wikiExtension: meta?.wiki_extension ?? "", + commitSha: meta?.commit_sha ?? ftsRow?.commitSha ?? "", + filePath: meta?.file_path ?? ftsRow?.filePath ?? "", + fileStatus: meta?.file_status ?? ftsRow?.fileStatus ?? "", + commitDate: meta?.commit_date ?? ftsRow?.commitDate ?? "", + commitAuthor: meta?.commit_author ?? ftsRow?.commitAuthor ?? "", + author: meta?.author ?? "", + commentId: meta?.comment_id ?? 0, + reviewId: meta?.review_id ?? 0, + line: meta?.line ?? 0, + }; +} + +/** Canonical GitHub URL for a resolved row, by type. */ +function buildResultUrl(r: ResolvedRow): string { + if (r.type === "release" && r.tagName) { + return `https://github.com/${r.repo}/releases/tag/${r.tagName}`; + } + if (r.type === "doc" && r.docPath) { + return `https://github.com/${r.repo}/blob/main/${r.docPath}`; + } + if (r.type === "wiki_doc" && r.wikiPath) { + return `https://github.com/${r.repo}/wiki/${encodeURIComponent(r.wikiPath)}`; + } + if (r.type === "diff" && r.commitSha) { + return `https://github.com/${r.repo}/commit/${r.commitSha}`; + } + if (r.type === "issue_comment" && r.commentId) { + return `https://github.com/${r.repo}/issues/${r.number}#issuecomment-${r.commentId}`; + } + if (r.type === "pr_review" && r.reviewId) { + return `https://github.com/${r.repo}/pull/${r.number}#pullrequestreview-${r.reviewId}`; + } + if (r.type === "pr_review_comment" && r.commentId) { + return `https://github.com/${r.repo}/pull/${r.number}#discussion_r${r.commentId}`; + } + return `https://github.com/${r.repo}/issues/${r.number}`; +} + /** * Legacy class retained solely to satisfy Cloudflare's "class must exist in * script for classes declared in past migrations (v1 new_sqlite_classes)" @@ -119,7 +225,10 @@ export class RagMcpAgentV2 extends McpAgent { "Use type: \"diff\" to retrieve judgment history preserved in commit diffs — including changes to deleted files " + "and non-.md files that are not present in the live document index. " + "Use type: \"issue_comment\" / \"pr_review\" / \"pr_review_comment\" to retrieve comment-level judgment history " + - "(Master's feedback, AI responses, self-review now/later/accepted classifications).", + "(Master's feedback, AI responses, self-review now/later/accepted classifications).\n" + + "Results are aggregated per underlying entity: a file's doc row and its commit diffs are one result, " + + "an issue or PR and its comments / reviews are one result. top_k therefore counts distinct entities, " + + "and a result that absorbed others carries same_entity { count, others[] } with links to them.", { query: z .string() @@ -332,17 +441,17 @@ export class RagMcpAgentV2 extends McpAgent { } // ── Search mode (non-empty query): existing hybrid pipeline ─ - // Overfetch on both sides when label/assignee post-filter is needed. - // Also overfetch when the reranker is enabled, so the cross-encoder - // sees enough candidates (issue #91 default: top_k × 5, capped at 50). + // Overfetch on both sides (issue #91 default: top_k × 5, capped at 50). // RERANK_MAX_CANDIDATES is the AI-side upper bound; we mirror it here // so dense and sparse fetch enough rows to feed the reranker. - const needsPostFilter = (labels && labels.length > 0) || !!assignee; - const needsTimeFilter = !!since || !!until; - const internalTopK = - needsPostFilter || needsTimeFilter || rerankEnabled - ? Math.min(requestedTopK * 5, RERANK_MAX_CANDIDATES) - : requestedTopK; + // + // The overfetch used to be conditional (label / assignee post-filter, + // time window, or reranker enabled). Entity aggregation (#189) removes + // that condition: every search-mode query now collapses several rows of + // one referent into a single result, so the candidate pool must exceed + // top_k on every path — including `rerank: false` — or the caller gets + // fewer than the top_k it asked for. + const internalTopK = Math.min(requestedTopK * 5, RERANK_MAX_CANDIDATES); // ── Dense path: Vectorize embedding query ──────────────── const densePromise: Promise<{ @@ -448,15 +557,7 @@ export class RagMcpAgentV2 extends McpAgent { // Build a vector_id → payload lookup combining dense metadata and sparse rows. // Dense metadata wins when both sides see a vector; sparse hits fill in the gaps // (e.g., when BM25 surfaces a row that dense missed entirely). - const payload = new Map< - string, - { - meta: VectorMetadata | undefined; - ftsRow: FtsHit | undefined; - denseScore: number | undefined; - sparseScore: number | undefined; - } - >(); + const payload = new Map(); for (const h of denseResult.hits) { payload.set(h.vectorId, { meta: h.meta, @@ -672,8 +773,37 @@ export class RagMcpAgentV2 extends McpAgent { }); } - // Trim to requested top-K after fusion + post-filter (+ rerank / time sort). - filtered = filtered.slice(0, requestedTopK); + // ── Entity aggregation (issue #189) ────────────────────── + // Several rows can point at one referent: a file is a `doc` row plus a + // `diff` row per commit, an issue or PR is its own row plus its + // comments / reviews. They crowd out independent results in the same + // top_k pool. Collapse them here — after every reordering stage, before + // the trim — so `top_k` counts referents and the representative is + // whichever row the final order ranked highest (see aggregate.ts for + // why the newest version is deliberately NOT pinned). + const groups = groupByEntity(filtered, (f) => { + const r = resolveRow(payload.get(f.vectorId)); + return entityKey({ + vectorId: f.vectorId, + repo: r.repo, + type: r.type, + number: r.number, + docPath: r.docPath, + filePath: r.filePath, + }); + }); + + // vector_id of a representative → the rows folded into it, in rank order. + const collapsedInto = new Map(); + for (const g of groups.slice(0, requestedTopK)) { + if (g.others.length > 0) { + collapsedInto.set(g.representative.vectorId, g.others); + } + } + + // Trim to requested top-K after fusion + post-filter (+ rerank / time + // sort / entity aggregation). + filtered = groups.slice(0, requestedTopK).map((g) => g.representative); // ── Format results ─────────────────────────────────────── type ResultItem = { @@ -709,67 +839,56 @@ export class RagMcpAgentV2 extends McpAgent { content?: string; graph_hop?: number; graph_from?: string; + /** + * Present only when this item is the representative of an entity that + * had other rows in the candidate pool (issue #189). `count` includes + * the representative, so it is always ≥ 2 when the field is present. + * Additive: a client that ignores it sees the pre-#189 shape. + */ + same_entity?: { + count: number; + others: Array<{ + type: string; + url: string; + updated_at: string; + score: number; + commit_sha?: string; + }>; + }; }; const items: ResultItem[] = filtered.map((f) => { const p = payload.get(f.vectorId); - const meta = p?.meta; - const ftsRow = p?.ftsRow; - - const itemRepo = meta?.repo ?? ftsRow?.repo ?? ""; - const number = meta?.number ?? ftsRow?.number ?? 0; - const itemType = meta?.type ?? (ftsRow?.type as VectorMetadata["type"] | undefined) ?? ""; - const itemState = meta?.state ?? ftsRow?.state ?? ""; - const labelsCsv = meta?.labels ?? ftsRow?.labels ?? ""; - const milestoneVal = meta?.milestone ?? ftsRow?.milestone ?? ""; - const assigneesCsv = meta?.assignees ?? ftsRow?.assignees ?? ""; - const updatedAt = meta?.updated_at ?? ftsRow?.updatedAt ?? ""; - const tagName = meta?.tag_name ?? ftsRow?.tagName ?? ""; - const docPath = meta?.doc_path ?? ftsRow?.docPath ?? ""; - // wiki_doc rows reuse the FTS5 `doc_path` column for the page slug — - // the schema-level field is unified across "where did this come from", - // distinguished by the row's `type`. Vectorize metadata carries the - // dedicated `wiki_path` / `wiki_extension` fields so we prefer them - // when present and fall back to the FTS row when the dense hit lost. - const wikiPath = (meta?.wiki_path as string | undefined) ?? (itemType === "wiki_doc" ? ftsRow?.docPath ?? "" : ""); - const wikiExtension = (meta?.wiki_extension as string | undefined) ?? ""; - const commitSha = meta?.commit_sha ?? ftsRow?.commitSha ?? ""; - const filePath = meta?.file_path ?? ftsRow?.filePath ?? ""; - const fileStatus = meta?.file_status ?? ftsRow?.fileStatus ?? ""; - const commitDate = meta?.commit_date ?? ftsRow?.commitDate ?? ""; - const commitAuthor = meta?.commit_author ?? ftsRow?.commitAuthor ?? ""; - const author = meta?.author ?? ""; - const commentId = meta?.comment_id ?? 0; - const reviewId = meta?.review_id ?? 0; - const line = meta?.line ?? 0; + const r = resolveRow(p); - let url: string; - if (itemType === "release" && tagName) { - url = `https://github.com/${itemRepo}/releases/tag/${tagName}`; - } else if (itemType === "doc" && docPath) { - url = `https://github.com/${itemRepo}/blob/main/${docPath}`; - } else if (itemType === "wiki_doc" && wikiPath) { - url = `https://github.com/${itemRepo}/wiki/${encodeURIComponent(wikiPath)}`; - } else if (itemType === "diff" && commitSha) { - url = `https://github.com/${itemRepo}/commit/${commitSha}`; - } else if (itemType === "issue_comment" && commentId) { - url = `https://github.com/${itemRepo}/issues/${number}#issuecomment-${commentId}`; - } else if (itemType === "pr_review" && reviewId) { - url = `https://github.com/${itemRepo}/pull/${number}#pullrequestreview-${reviewId}`; - } else if (itemType === "pr_review_comment" && commentId) { - url = `https://github.com/${itemRepo}/pull/${number}#discussion_r${commentId}`; - } else { - url = `https://github.com/${itemRepo}/issues/${number}`; - } + // Rows folded into this representative. Kept as references (never + // dropped) so the caller can still reach every version / comment. + const folded = collapsedInto.get(f.vectorId) ?? []; + const sameEntity = + folded.length > 0 + ? { + count: folded.length + 1, + others: folded.map((o) => { + const or = resolveRow(payload.get(o.vectorId)); + return { + type: or.type, + url: buildResultUrl(or), + updated_at: or.updatedAt, + score: o.fusedScore, + ...(or.type === "diff" ? { commit_sha: or.commitSha } : {}), + }; + }), + } + : undefined; return { - number, + number: r.number, title: "", // Enriched below - state: itemState, - type: itemType, - labels: labelsCsv ? labelsCsv.split(",").filter(Boolean) : [], - milestone: milestoneVal, - assignees: assigneesCsv ? assigneesCsv.split(",").filter(Boolean) : [], + state: r.state, + type: r.type, + labels: r.labelsCsv ? r.labelsCsv.split(",").filter(Boolean) : [], + milestone: r.milestone, + assignees: r.assigneesCsv ? r.assigneesCsv.split(",").filter(Boolean) : [], score: f.fusedScore, dense_score: p?.denseScore ?? null, sparse_score: p?.sparseScore ?? null, @@ -780,47 +899,48 @@ export class RagMcpAgentV2 extends McpAgent { // otherwise. `rerank_applied: true` guarantees at least one row in // the response carries a non-null score. rerank_score: rerankScores.get(f.vectorId) ?? null, - url, - updated_at: updatedAt, - repo: itemRepo, - ...(itemType === "release" ? { tag_name: tagName } : {}), - ...(itemType === "doc" ? { doc_path: docPath } : {}), - ...(itemType === "wiki_doc" + url: buildResultUrl(r), + updated_at: r.updatedAt, + repo: r.repo, + ...(r.type === "release" ? { tag_name: r.tagName } : {}), + ...(r.type === "doc" ? { doc_path: r.docPath } : {}), + ...(r.type === "wiki_doc" ? { - wiki_path: wikiPath, - ...(wikiExtension ? { wiki_extension: wikiExtension } : {}), + wiki_path: r.wikiPath, + ...(r.wikiExtension ? { wiki_extension: r.wikiExtension } : {}), } : {}), - ...(itemType === "diff" + ...(r.type === "diff" ? { - commit_sha: commitSha, - file_path: filePath, - file_status: fileStatus, - commit_date: commitDate, - commit_author: commitAuthor, + commit_sha: r.commitSha, + file_path: r.filePath, + file_status: r.fileStatus, + commit_date: r.commitDate, + commit_author: r.commitAuthor, } : {}), - ...(itemType === "issue_comment" + ...(r.type === "issue_comment" ? { - author, - comment_id: commentId, + author: r.author, + comment_id: r.commentId, } : {}), - ...(itemType === "pr_review" + ...(r.type === "pr_review" ? { - author, - review_id: reviewId, + author: r.author, + review_id: r.reviewId, } : {}), - ...(itemType === "pr_review_comment" + ...(r.type === "pr_review_comment" ? { - author, - comment_id: commentId, - file_path: filePath, - line, - commit_sha: commitSha, + author: r.author, + comment_id: r.commentId, + file_path: r.filePath, + line: r.line, + commit_sha: r.commitSha, } : {}), + ...(sameEntity ? { same_entity: sameEntity } : {}), }; });