Dashboard › sentry › Session 13tDQDnEAWLV
13tDQDnEAWLVQDhzgYou are reviewing PR #598 in BYK/loreai. This is a small follow-up PR (30 lines changed across 2 files). Review it thoroughly and critically.
PR #595 added a "Dismiss" button to entity merge suggestions in the dashboard. The dismiss handler recorded reject feedback in dedup_feedback, but the dashboard never filtered dismissed pairs — they kept reappearing on every page load. This PR fixes that.
packages/core/src/entities.ts (+20 lines)New exported function added between existing constants:
/**
* Return a Set of "nameA\x1fnameB" keys for entity pairs that have been
* explicitly dismissed (accepted=0) via the dashboard. Both orderings are
* included so callers can do a single `has()` check.
*/
export function getDismissedEntityPairs(): Set<string> {
const rows = db()
.query(
`SELECT entry_a_title, entry_b_title FROM dedup_feedback
WHERE kind = 'entity' AND accepted = 0 AND source = 'dashboard'`,
)
.all() as Array<{ entry_a_title: string; entry_b_title: string }>;
const dismissed = new Set<string>();
for (const r of rows) {
dismissed.add(`${r.entry_a_title}\x1f${r.entry_b_title}`);
dismissed.add(`${r.entry_b_title}\x1f${r.entry_a_title}`);
}
return dismissed;
}
packages/gateway/src/ui.ts (+10, -1 lines)In pageEntities(), after the deduplicateEntities() dry-run call, filters out dismissed pairs:
// Before (old):
const clusters = [...dupes.merged, ...dupes.suggested];
// After (new):
const dismissed = entities.getDismissedEntityPairs();
const clusters = [...dupes.merged, ...dupes.suggested]
.map((c) => ({
...c,
merged: c.merged.filter(
(m) => !dismissed.has(`${m.name}\x1f${c.surviving.name}`),
),
}))
.filter((c) => c.merged.length > 0);
I'll start by examining the actual PR and the relevant code to give a thorough review.
{"additions":30,"body":"## Summary\n\nFollow-up to #595. The dismiss button recorded reject feedback in `dedup_feedback` but the dashboard never checked it — dismissed pairs kept reappearing on every page load.\n\n### Changes\n\n**`packages/core/src/entities.ts`**\n- Added `getDismissedEntityPairs()`: queries `dedup_feedback` for dashboard-rejected entity pairs and returns a Set of bidirectional `\"nameA\\x1fnameB\"` keys for O(1) lookup.\n\n**`packages/gateway/src/ui.ts`**\n- After `deduplicateEntities()` dry-run, filters out pairs present in the dismissed set before rendering. Empty clusters are dropped entirely so the banner disappears when all suggestions are dismissed.","changedFiles":2,"commits":[{"authoredDate":"2026-06-07T14:19:07Z","authors":[{"email":"ben@byk.im","id":"MDQ6VXNlcjEyNjc4MA==","login":"BYK","name":"Burak Yigit Kaya"}],"committedDate":"2026-06-07T14:45:33Z","messageBody":"","messageHeadline":"fix(entities): filter dismissed pairs from dashboard dedup ...
diff --git a/packages/core/src/entities.ts b/packages/core/src/entities.ts
index 688fa22..c5f9d56 100644
--- a/packages/core/src/entities.ts
+++ b/packages/core/src/entities.ts
@@ -1697,6 +1697,26 @@ export type EntityDedupFeedbackSource =
const MIN_ENTITY_CALIBRATION_SAMPLES = 20;
/** Only record auto-signals for pairs with similarity >= this floor. */
const ENTITY_AUTO_SIGNAL_MIN_SIMILARITY = 0.8;
+/**
+ * Return a Set of "nameA\x1fnameB" keys for entity pairs that have been
+ * explicitly dismissed (accepted=0) via the dashboard. Both orderings are
+ * included so callers can do a single `has()` check.
+ */
+export function getDismissedEntityPairs(): Set<string> {
+ const rows = db()
+ .query(
+ `SELECT entry_a_title, entry_b_title FROM dedup_feedback
+ WHERE kind = 'entity' AND accepted = 0 AND source = 'dashboard'`,
+ )
+ .all() as Array<{ entry_a_title: string; entry_b_title: string }>;
+ const dismissed = new Set<string>();
+ for (const r of rows) {
+ ...
Now let me examine the surrounding code for full context — the dedup_feedback schema, how dismiss is recorded, the cluster/merged types, and how entity names flow through the system.
<path>/home/byk/Code/opencode-lore/packages/core/src/entities.ts</path> <type>file</type> <content> 1680: } 1681: 1682: // --------------------------------------------------------------------------- 1683: // Entity dedup adaptive threshold calibration (#462) 1684: // 1685: // Reuses the shared `dedup_feedback` table with kind='entity' so entity and 1686: // knowledge feedback coexist without a second table. All queries here scope to 1687: // kind='entity'; the knowledge calibration path (ltm.ts) implicitly operates on 1688: // kind='knowledge' (the column default). 1689: // --------------------------------------------------------------------------- 1690: 1691: export type EntityDedupFeedbackSource = 1692: | "auto_dedup" 1693: | "cli_yes" 1694: | "cli_interactive" 1695: | "dashboard"; 1696: 1697: const MIN_ENTITY_CALIBRATION_SAMPLES = 20; 1698: /** Only record auto-signals for pairs with similarity >= this floor. */ 1699: const ENTITY_AUTO_SIGNAL_MIN_SIMILARITY = 0.8; 1700: ...
<path>/home/byk/Code/opencode-lore/packages/gateway/src/ui.ts</path>
<type>file</type>
<content>
2780:
2781: let body = breadcrumb([
2782: { label: "Dashboard", href: "/ui" },
2783: { label: "Entities" },
2784: ]);
2785: body += `<h1>Entities (${all.length})</h1>`;
2786:
2787: if (!all.length) {
2788: body += `<p class="empty">No entities found. Entities are created automatically when the curator detects recurring people, services, tools, and other named references in conversations.</p>`;
2789: return layout("Entities", body);
2790: }
2791:
2792: // Merge suggestions (#462): surface duplicate candidates as a dry-run banner.
2793: // Only compute when embeddings are available (the primary signal); cheap for
2794: // typical registry sizes. Non-fatal — failures just omit the section.
2795: if (embedding.isAvailable() && all.length >= 2) {
2796: try {
2797: const dupes = await entities.deduplicateEntities(undefined, {
2798: dryRun: ...
Now let me check the dismiss handler to understand how feedback is recorded, and look at the cluster/merged types:
Found 2 matches
/home/byk/Code/opencode-lore/packages/gateway/src/ui.ts:
Line 2830: body += `<form method="POST" action="/ui/api/dismiss/entity/${esc(m.id)}/${esc(c.surviving.id)}" style="display:inline;">`;
Line 3189: "/ui/api/dismiss/entity/:entityAId/:entityBId",
Found 11 matches
/home/byk/Code/opencode-lore/packages/core/src/entities.ts:
Line 1380: export type EntityDedupCluster = {
Line 1385: export type EntityDedupResult = {
Line 1387: merged: EntityDedupCluster[];
Line 1389: suggested: EntityDedupCluster[];
Line 1431: ): Promise<EntityDedupResult> {
Line 1436: const empty: EntityDedupResult = {
Line 1604: const merged: EntityDedupCluster[] = [];
Line 1605: const suggested: EntityDedupCluster[] = [];
Line 1650: const mergeMembers: EntityDedupCluster["merged"] = [];
Line 1651: const suggestMembers: EntityDedupCluster["merged"] = [];
Line 1759: result: EntityDedupResult,
No files found
<path>/home/byk/Code/opencode-lore/packages/core/src/entities.ts</path>
<type>file</type>
<content>
1375: const ENTITY_NAME_JACCARD_THRESHOLD = 0.5;
1376: /** Small additive boost applied to a pair's score when a softer signal fires. */
1377: const ENTITY_SIGNAL_BOOST = 0.05;
1378:
1379: /** A cluster of duplicate entities sharing one survivor. */
1380: export type EntityDedupCluster = {
1381: surviving: { id: string; name: string };
1382: merged: Array<{ id: string; name: string; similarity: number }>;
1383: };
1384:
1385: export type EntityDedupResult = {
1386: /** Auto-merge tier (similarity ≥ ENTITY_AUTO_MERGE_THRESHOLD or alias overlap). */
1387: merged: EntityDedupCluster[];
1388: /** Suggestion tier (≥ dedup threshold, < auto-merge threshold). */
1389: suggested: EntityDedupCluster[];
1390: /** All pairwise similarities (key = entityPairKey) for calibration. ...
<path>/home/byk/Code/opencode-lore/packages/gateway/src/ui.ts</path>
<type>file</type>
<content>
3185:
3186: // Dismiss entity merge suggestion: record reject feedback (#462)
3187: const dismissEntity = matchRoute(
3188: pathname,
3189: "/ui/api/dismiss/entity/:entityAId/:entityBId",
3190: );
3191: if (dismissEntity) {
3192: const entityA = entities.get(dismissEntity.entityAId);
3193: const entityB = entities.get(dismissEntity.entityBId);
3194: const formData = await req.formData();
3195: const similarity = Number.parseFloat(
3196: (formData.get("similarity") as string) || "",
3197: );
3198: if (Number.isFinite(similarity) && entityA && entityB) {
3199: entities.recordEntityDedupFeedback({
3200: projectId: null,
3201: entryATitle: entityA.canonical_name,
3202: entryBTitle: entityB.canonical_name,
3203: similarity,
3204: accepted: false,
3205: ...
Now let me check the DB schema for dedup_feedback and see if there's a project_id filter concern, and look at existing tests:
Found 5 matches /home/byk/Code/opencode-lore/packages/core/src/db.ts: Line 568: CREATE TABLE IF NOT EXISTS dedup_feedback ( Line 578: CREATE INDEX IF NOT EXISTS idx_dedup_feedback_project Line 579: ON dedup_feedback(project_id); Line 988: -- knowledge.embedding) and a 'kind' discriminator on dedup_feedback so the Line 993: ALTER TABLE dedup_feedback ADD COLUMN kind TEXT NOT NULL DEFAULT 'knowledge';
No files found
/home/byk/Code/opencode-lore/packages/core/test/entities.test.ts /home/byk/Code/opencode-lore/packages/core/test/entity-dedup.test.ts
<path>/home/byk/Code/opencode-lore/packages/core/src/db.ts</path> <type>file</type> <content> 565: -- on embedding-based duplicate pairs for per-project threshold calibration. 566: -- Titles stored instead of FK IDs because entries are deleted during dedup; 567: -- the similarity float is the actual calibration input. 568: CREATE TABLE IF NOT EXISTS dedup_feedback ( 569: id INTEGER PRIMARY KEY AUTOINCREMENT, 570: project_id TEXT, 571: entry_a_title TEXT NOT NULL, 572: entry_b_title TEXT NOT NULL, 573: similarity REAL NOT NULL, 574: accepted INTEGER NOT NULL, 575: source TEXT NOT NULL DEFAULT 'manual', 576: created_at INTEGER NOT NULL 577: ); 578: CREATE INDEX IF NOT EXISTS idx_dedup_feedback_project 579: ON dedup_feedback(project_id); 580: `, 581: ` 582: -- Version 26: Persist sub-agent parent–child session relationships. ...
No files found
Now let me check whether the dismiss handler stores project_id: null and whether the query should filter on that, plus check how recordEntityDedupFeedback is called elsewhere:
Found 11 matches
/home/byk/Code/opencode-lore/packages/gateway/src/ui.ts:
Line 3171: entities.recordEntityDedupFeedback({
Line 3199: entities.recordEntityDedupFeedback({
/home/byk/Code/opencode-lore/packages/core/src/entities.ts:
Line 1726: export function recordEntityDedupFeedback(input: {
Line 1798: recordEntityDedupFeedback({
/home/byk/Code/opencode-lore/packages/core/test/entity-dedup.test.ts:
Line 223: entities.recordEntityDedupFeedback({
Line 237: entities.recordEntityDedupFeedback({
Line 253: entities.recordEntityDedupFeedback({
Line 261: entities.recordEntityDedupFeedback({
/home/byk/Code/opencode-lore/packages/gateway/src/cli/entity.ts:
Line 704: entities.recordEntityDedupFeedback({
Line 716: entities.recordEntityDedupFeedback({
Line 781: entities.recordEntityDedupFeedback({
No files found
Let me check how the merge handler records feedback (for comparison) and whether cli_interactive reject also uses project_id: null:
<path>/home/byk/Code/opencode-lore/packages/gateway/src/ui.ts</path>
<type>file</type>
<content>
3160: selfPersonSet.has(source.entity_type)));
3161: if (target && source && target.id !== source.id && typesCompatible) {
3162: // Parse form data before merge — source entity is deleted by merge().
3163: const formData = await req.formData();
3164: const similarity = Number.parseFloat(
3165: (formData.get("similarity") as string) || "",
3166: );
3167: entities.merge(target.id, source.id);
3168: // Record accept feedback — the similarity score is the real cosine
3169: // value from the dedup dry-run, passed through as a hidden form field.
3170: if (Number.isFinite(similarity)) {
3171: entities.recordEntityDedupFeedback({
3172: projectId: null,
3173: entryATitle:
3174: (formData.get("nameA") as string) || source.canonical_name,
3175: ...
<path>/home/byk/Code/opencode-lore/packages/gateway/src/cli/entity.ts</path>
<type>file</type>
<content>
695: );
696: }
697: const choice = await promptChoice(
698: "\n [a]ccept merge / [r]eject (keep all) / [s]kip? ",
699: ["a", "r", "s"],
700: );
701: if (choice === "a") {
702: for (const m of c.merged) {
703: entities.merge(c.surviving.id, m.id);
704: entities.recordEntityDedupFeedback({
705: projectId: pid,
706: entryATitle: c.surviving.name,
707: entryBTitle: m.name,
708: similarity: m.similarity,
709: accepted: true,
710: source: "cli_interactive",
711: });
712: mergedCount++;
713: }
714: } else if (choice === "r") {
715: for (const m of c.merged) {
716: entities.recordEntityDedupFeedback({
717: projectId: pid,
718: entryATitle: c.surviving.name,
719: ...
Now let me check the dismiss handler more carefully — specifically the order of names in entryATitle/entryBTitle versus how the filter constructs the key:
<path>/home/byk/Code/opencode-lore/packages/gateway/src/ui.ts</path>
<type>file</type>
<content>
2820: shown++;
2821: body += `<div style="display:flex;align-items:center;gap:8px;flex-wrap:wrap;">`;
2822: body += `<span><a href="/ui/entities/${esc(m.id)}">${esc(truncate(m.name, 40))}</a> → <a href="/ui/entities/${esc(c.surviving.id)}">${esc(truncate(c.surviving.name, 40))}</a></span>`;
2823: body += `<span class="muted" style="color:#888;">[sim: ${m.similarity.toFixed(3)}]</span>`;
2824: body += `<form method="POST" action="/ui/api/merge/entity/${esc(c.surviving.id)}/${esc(m.id)}" style="display:inline;" onsubmit="return confirm('Merge "${esc(m.name)}" into "${esc(c.surviving.name)}"?');">`;
2825: body += `<input type="hidden" name="similarity" value="${m.similarity}">`;
2826: body += `<input type="hidden" name="nameA" value="${esc(m.name)}">`;
2827: body += `<input ...
Good. Now I have full context. Let me trace the exact name flow:
Dismiss handler (line 3199): stores entryATitle: entityA.canonical_name, entryBTitle: entityB.canonical_name — where entityA is looked up by dismissEntity.entityAId (the m.id from the form action at line 2830) and entityB by entityBId (the c.surviving.id).
Filter (line 2805-2806): checks m.name\x1fc.surviving.name — where m is from the cluster's merged array and c.surviving is the cluster survivor.
Both orderings are in the Set (lines 1714-1715), so the name order in the dismiss handler vs the filter doesn't matter. Correct.
Now let me verify whether m.name in the cluster equals canonical_name from the entity:
<path>/home/byk/Code/opencode-lore/packages/core/src/entities.ts</path>
<type>file</type>
<content>
1640: ?.find((h) => h.id === survivor.id);
1641: if (fromMember)
1642: return { score: fromMember.score, force: fromMember.forceMerge };
1643: // Fallback: use the combined/boosted score (not raw cosine) to preserve
1644: // the Jaccard/knowledge boosts when the survivor differs from the center.
1645: const pk = entityPairKey(survivor.id, memberId);
1646: const s = pairScores.get(pk) ?? 0;
1647: return { score: s, force: s >= ENTITY_AUTO_MERGE_THRESHOLD };
1648: };
1649:
1650: const mergeMembers: EntityDedupCluster["merged"] = [];
1651: const suggestMembers: EntityDedupCluster["merged"] = [];
1652: for (const m of sorted.slice(1)) {
1653: const { score, force } = scoreFor(m.id);
1654: const entry = { id: m.id, name: m.canonical_name, similarity: score };
1655: if (force) ...
Good — line 1654 confirms name: m.canonical_name, so cluster m.name is indeed the entity's canonical_name. The names match end-to-end. Now I have everything I need.
Here's my review of PR #598:
The PR does exactly what it says — it reads dismissed entity pairs from dedup_feedback and filters them out of the dashboard's dedup suggestion banner. The implementation is clean, correct, and appropriately scoped. I have no critical findings.
None.
1. Missing project_id filter in getDismissedEntityPairs() (packages/core/src/entities.ts:1708-1709)
The query has no WHERE project_id ... clause. The dismiss handler stores projectId: null (line 3200 of ui.ts), so this works today — all dashboard dismissals have project_id IS NULL. But if a future caller records dashboard dismissals with a non-null project ID, this query would return those too, potentially cross-contaminating. More importantly, the query pattern is inconsistent with other dedup_feedback reads in the codebase (e.g., calibrateEntityDedupThreshold always filters by project_id).
Low risk since the dashboard is project-agnostic, but adding AND project_id IS NULL would make the intent explicit and future-proof.
2. No test coverage for getDismissedEntityPairs()
entity-dedup.test.ts already tests recordEntityDedupFeedback and calibration, but has zero coverage for the new function. A minimal test — record a dismiss, call getDismissedEntityPairs(), assert both orderings are present — would take ~10 lines and would catch regressions if the query or key format changes. Given this is the sole mechanism preventing dismissed suggestions from resurfacing (the user-facing bug this PR fixes), it warrants a test.
**3. ...