[Review] computeVaultMoodTrend anchors window on newest entry, but computeMoodTrend (#171) anchors on Date.now() — opposite choices for the same signal #181
Labels
No labels
blocked:depends-p1
blocked:gate-research
blocked:lint-failed
blocked:needs-decision
blocked:needs-human
blocked:verify-failed
bug
data-loss
documentation
enhancement
feature-request
in-progress
needs-decompose
needs-triage
pwa
quickfix
security
security/high
security/medium
test
test-coverage
triage-approved
vault
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Mark/kimi_agent_kin_app_sync#181
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Finding
src/vault/vault-metrics-engine.ts:127-133(vault variant):This is the deliberate "anchor on data, not on clock" choice.
But the main-DB
computeMoodTrend(src/lib/intimate-state-engine.ts:206-211) anchors onDate.now():This is exactly the fix #171 just landed —
feat: anchor 7-day mood trend on actual current date; add regression tests. The PR body explicitly rejected the data-anchor choice because with stale data, the trend silently never rendered.vault-metrics-engine.tsis using the data-anchor that #171 explicitly rejected for the main path. If a user unlocks the vault with stale data, the vault mood trend says "no data in the last 7 days" even when there is — same bug shape, opposite anchor choice.Suggested fix
Mirror the #171 fix in
vault-metrics-engine.ts:The
Math.abs(...)windowing (which counted entries in either direction fromnewest) is also wrong for "the last 7 days" — only forward-in-time fromnowis meaningful.Severity
Medium. Same bug shape as #171, opposite fix. Inconsistent behavior between main-DB mood trend and vault mood trend.
Discovered by
Manual code review pass over #169 + #171, 2026-09-14.
Routing: direct (user handles). Per the kin-app triage convention,
quickfixwould route this to the kilo quickfix lane, but this is a one-line mirror of the fix just landed in #171 (same bug shape, opposite anchor choice) — the user wants to apply the fix themselves rather than have a lane run it. No lane should pick this up while labeledneeds-triage.The fix is documented in the issue body: replace the
Math.abs(newest - s.date.getTime()) <= 7 * DAY_MSwindowing insrc/vault/vault-metrics-engine.ts:127-133withs.date >= new Date(now.getTime() - 7 * DAY_MS)-style cutoff, mirroringcomputeMoodTrendatsrc/lib/intimate-state-engine.ts:206-211.Closing as already-fixed.
Commit
ed706ec(fix(vault): anchor computeVaultMoodTrend window on Date.now(), not newest entry (#181)) landed onmainat 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
ed706ec(fix(vault): anchor computeVaultMoodTrend window on Date.now(), not newest entry (#181)) landed onmainat 2026-09-14 07:29:06 — before this triage session reviewed the file.The chat-agent code review read against a stale HEAD (assumed
4fe2244from Sep 13) but HEAD was actually0854652(the #184 fix), which already contained the clock-anchored cutoff. The agent failed to re-checkgit rev-parse HEADbetween the initialgit log -1and reading the source. The fix is identical to what the agent would have written.Root cause to record: code-review sessions must (a) re-check
git rev-parse HEADimmediately before each source read, and (b) account forstash@{0}and dirty files in the working tree, which can also contain fixes that aren't yet onmain.No work to do.