fix(desktop): memoize mission inbox snapshots on inputs, not the wall clock - #138
Conversation
Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Oscar Le <oscar.lehuu@gmail.com>
Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Oscar Le <oscar.lehuu@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
CI status: required lanes green, red lanes are preexistingGreen here: Desktop Fast (job Red/cancelled, all preexisting on
One PR-side smoke failure was in the feature area this PR touches, so it got its own check rather than a hand-wave:
|
Summary
Root cause.
deriveMissionInboxSectionsderivesconst now = input.now ?? Date.now()and then folds thatnowinto its module-level memo key:When the caller supplies no
now— which the only caller,useMissionInboxSections, never does — each invocation samples the wall clock independently. Two calls with the same input object therefore produce different keys whenever they straddle a millisecond tick, the memo misses, and a fresh object is returned. That is exactly whatmissionInbox.test.mjs:654catches: it calls the function twice back-to-back and asserts reference equality. The failure rate is simplyderive duration / 1ms. Nothing in the key is order- or hash-dependent; the clock is the only nondeterministic component.Discriminating evidence (2000-iteration probe through the repo loader on unmodified
main): 8/2000 iterations returned non-reference-equal snapshots, and 8 of those 8 coincided with the twoDate.now()samples differing, while all 1992 reference-equal iterations had identical samples.Fix. The memo key is now a pure function of caller-provided inputs: an explicitly supplied
input.nowstays part of the key, the implicitDate.now()fallback does not.Identical inputs ⇒ cache hit ⇒ identical object, always — which is what a
getSnapshot-shaped selector must guarantee. A caller that wants clock-driven recomputation passesnowexplicitly, and a new test pins that direction of the contract too.Accepted consequence, recorded as D-039: without an explicit clock, row
agevalues are fixed at snapshot time and refresh only when a real input changes. This is not a behavior regression — the sole caller passes no clock and the home surface has no ticker, so ages already only updated when a store changed.The flaky test itself is untouched: no skip, no retry, no tolerance, no weakened assertion.
DECISIONS.md numbering: took D-039 per the coordination note (D-028 through D-038 are allocated to in-flight PRs #120, #124, #127, #129, #134, #128 and reserved 037/038). No
STATE.mdchange — this fixes existing shipped behavior rather than changing the shipped slice.Related issue
Fixes #135.
Testing
Narrowed named test, run through the repo harness with a validity guard (a run that matches 0 tests is counted INVALID, not a pass, so no tally is vacuous):
origin/main@35af74019Representative failure on
main:Full gates on this branch, all clean:
pnpm --filter buzz test—ℹ pass 5046,ℹ fail 0,ℹ skipped 1pnpm --filter buzz check— Biome, 2195 files, no fixes neededpnpm --filter buzz typecheck— cleanNo UI change, so no screenshots.
Link to Devin session: https://app.devin.ai/sessions/c247a65930ba49ed8471a10d38931037
Requested by: @oscarlehuu