[Review] computeVaultMoodTrend anchors window on newest entry, but computeMoodTrend (#171) anchors on Date.now() — opposite choices for the same signal #181

Closed
opened 2026-09-14 14:07:30 +00:00 by Mark · 3 comments
Owner

Finding

src/vault/vault-metrics-engine.ts:127-133 (vault variant):

const newest = scored[0].date.getTime();
const seven = scored.filter(
  (s) => Math.abs(newest - s.date.getTime()) <= 7 * DAY_MS,
);
const thirty = scored.filter(
  (s) => Math.abs(newest - s.date.getTime()) <= 30 * DAY_MS,
);

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 on Date.now():

const now = new Date();
const sevenDayCutoff = new Date(now.getTime() - 7 * DAY_MS);
const thirtyDayCutoff = new Date(now.getTime() - 30 * DAY_MS);
const last7 = scored.filter((s) => s.date >= sevenDayCutoff);
const last30 = scored.filter((s) => s.date >= thirtyDayCutoff);

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.ts is 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:

const now = new Date();
const sevenDayCutoff = new Date(now.getTime() - 7 * DAY_MS);
const thirtyDayCutoff = new Date(now.getTime() - 30 * DAY_MS);
const seven = scored.filter((s) => s.date >= sevenDayCutoff);
const thirty = scored.filter((s) => s.date >= thirtyDayCutoff);

The Math.abs(...) windowing (which counted entries in either direction from newest) is also wrong for "the last 7 days" — only forward-in-time from now is 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.

## Finding `src/vault/vault-metrics-engine.ts:127-133` (vault variant): ```ts const newest = scored[0].date.getTime(); const seven = scored.filter( (s) => Math.abs(newest - s.date.getTime()) <= 7 * DAY_MS, ); const thirty = scored.filter( (s) => Math.abs(newest - s.date.getTime()) <= 30 * DAY_MS, ); ``` 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 on `Date.now()`: ```ts const now = new Date(); const sevenDayCutoff = new Date(now.getTime() - 7 * DAY_MS); const thirtyDayCutoff = new Date(now.getTime() - 30 * DAY_MS); const last7 = scored.filter((s) => s.date >= sevenDayCutoff); const last30 = scored.filter((s) => s.date >= thirtyDayCutoff); ``` 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.ts` is 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`: ```ts const now = new Date(); const sevenDayCutoff = new Date(now.getTime() - 7 * DAY_MS); const thirtyDayCutoff = new Date(now.getTime() - 30 * DAY_MS); const seven = scored.filter((s) => s.date >= sevenDayCutoff); const thirty = scored.filter((s) => s.date >= thirtyDayCutoff); ``` The `Math.abs(...)` windowing (which counted entries in either direction from `newest`) is also wrong for "the last 7 days" — only forward-in-time from `now` is 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.
Mark closed this issue 2026-09-14 14:07:30 +00:00
Author
Owner

Routing: direct (user handles). Per the kin-app triage convention, quickfix would 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 labeled needs-triage.

The fix is documented in the issue body: replace the Math.abs(newest - s.date.getTime()) <= 7 * DAY_MS windowing in src/vault/vault-metrics-engine.ts:127-133 with s.date >= new Date(now.getTime() - 7 * DAY_MS)-style cutoff, mirroring computeMoodTrend at src/lib/intimate-state-engine.ts:206-211.

Routing: **direct** (user handles). Per the kin-app triage convention, `quickfix` would 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 labeled `needs-triage`. The fix is documented in the issue body: replace the `Math.abs(newest - s.date.getTime()) <= 7 * DAY_MS` windowing in `src/vault/vault-metrics-engine.ts:127-133` with `s.date >= new Date(now.getTime() - 7 * DAY_MS)`-style cutoff, mirroring `computeMoodTrend` at `src/lib/intimate-state-engine.ts:206-211`.
Author
Owner

Closing as already-fixed.

Commit ed706ec (fix(vault): anchor computeVaultMoodTrend window on Date.now(), not newest entry (#181)) 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 `ed706ec` (`fix(vault): anchor computeVaultMoodTrend window on Date.now(), not newest entry (#181)`) 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.
Author
Owner

Closing as already-fixed.

Commit ed706ec (fix(vault): anchor computeVaultMoodTrend window on Date.now(), not newest entry (#181)) landed on main at 2026-09-14 07:29:06 — before this triage session reviewed the file.

The chat-agent code review read against a stale HEAD (assumed 4fe2244 from Sep 13) but HEAD was actually 0854652 (the #184 fix), which already contained the clock-anchored cutoff. The agent failed to re-check git rev-parse HEAD between the initial git log -1 and 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 HEAD immediately before each source read, and (b) account for stash@{0} and dirty files in the working tree, which can also contain fixes that aren't yet on main.

No work to do.

Closing as already-fixed. Commit `ed706ec` (`fix(vault): anchor computeVaultMoodTrend window on Date.now(), not newest entry (#181)`) landed on `main` at 2026-09-14 07:29:06 — before this triage session reviewed the file. The chat-agent code review read against a stale HEAD (assumed `4fe2244` from Sep 13) but HEAD was actually `0854652` (the #184 fix), which already contained the clock-anchored cutoff. The agent failed to re-check `git rev-parse HEAD` between the initial `git log -1` and 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 HEAD` immediately before each source read, and (b) account for `stash@{0}` and dirty files in the working tree, which can also contain fixes that aren't yet on `main`. 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#181
No description provided.