[Review] behaviorPatternInsights slice(0, 20) is applied AFTER dismissed-state propagation — dismissed ghosts consume slice budget #182

Closed
opened 2026-09-14 14:07:30 +00:00 by Mark · 1 comment
Owner

Finding

src/lib/store.ts:1025-1027 (analyzeBehaviorPatterns):

const merged = [...behavior, ...intimate].map((insight) => ({
  ...insight,
  dismissed: dismissedIds.has(insight.id) ? true : insight.dismissed,
}));
set({ behaviorPatternInsights: merged.slice(0, 20) });

The slice is applied after dismissed-state propagation. So if a user has dismissed 15 insights over time, and only 10 active insights remain, merged has 25 entries (10 active + 15 dismissed), and slice(0, 20) holds the first 10 active + 10 dismissed. The user's view shows 10 dismissed insights crowding out active ones.

Suggested fix

Separate the visible bucket from the dismissed-bucket before slicing:

const visible = merged.filter((i) => !i.dismissed).slice(0, 20);
// Keep dismissed insights separately if needed for "show dismissed" toggle
set({ behaviorPatternInsights: visible });

Or, if a "show dismissed" toggle is needed in the UI, store both arrays:

set({
  activeInsights: merged.filter((i) => !i.dismissed).slice(0, 20),
  dismissedInsights: merged.filter((i) => i.dismissed),
});

The UI change in PersonDetail.tsx should follow the data model decision.

Severity

Medium. UX-visible bug — users see fewer actionable insights as they dismiss more.

Discovered by

Manual code review pass over #168, 2026-09-14.

## Finding `src/lib/store.ts:1025-1027` (`analyzeBehaviorPatterns`): ```ts const merged = [...behavior, ...intimate].map((insight) => ({ ...insight, dismissed: dismissedIds.has(insight.id) ? true : insight.dismissed, })); set({ behaviorPatternInsights: merged.slice(0, 20) }); ``` The slice is applied **after** dismissed-state propagation. So if a user has dismissed 15 insights over time, and only 10 active insights remain, `merged` has 25 entries (10 active + 15 dismissed), and `slice(0, 20)` holds the first 10 active + 10 dismissed. The user's view shows 10 dismissed insights crowding out active ones. ## Suggested fix Separate the visible bucket from the dismissed-bucket before slicing: ```ts const visible = merged.filter((i) => !i.dismissed).slice(0, 20); // Keep dismissed insights separately if needed for "show dismissed" toggle set({ behaviorPatternInsights: visible }); ``` Or, if a "show dismissed" toggle is needed in the UI, store both arrays: ```ts set({ activeInsights: merged.filter((i) => !i.dismissed).slice(0, 20), dismissedInsights: merged.filter((i) => i.dismissed), }); ``` The UI change in PersonDetail.tsx should follow the data model decision. ## Severity Medium. UX-visible bug — users see fewer actionable insights as they dismiss more. ## Discovered by Manual code review pass over #168, 2026-09-14.
Mark closed this issue 2026-09-14 14:07:30 +00:00
Author
Owner

Closing as already-fixed.

Commit 04720d8 (#182: split active vs dismissed before slice(0, 20) in analyzeBehaviorPatterns) landed on main at 2026-09-14 (today, before this triage session reviewed the file).

The chat-agent code review read against a stale HEAD and reported this issue as open. The fix was already in place when the review started — the agent failed to re-check HEAD between reads.

No work to do.

Closing as already-fixed. Commit `04720d8` (`#182: split active vs dismissed before slice(0, 20) in analyzeBehaviorPatterns`) landed on `main` at 2026-09-14 (today, before this triage session reviewed the file). The chat-agent code review read against a stale HEAD and reported this issue as open. The fix was already in place when the review started — the agent failed to re-check HEAD between reads. No work to do.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Mark/kimi_agent_kin_app_sync#182
No description provided.