Skip to content

fix(core): stable id tiebreaker in conversation list sort (kills the CI flake)#69

Merged
matze4u merged 1 commit into
mainfrom
fix/v1-0-conversation-list-stable-sort
Jun 3, 2026
Merged

fix(core): stable id tiebreaker in conversation list sort (kills the CI flake)#69
matze4u merged 1 commit into
mainfrom
fix/v1-0-conversation-list-stable-sort

Conversation

@matze4u

@matze4u matze4u commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

The flake

handler-list.test.ts failed ~25-50% of CI node jobs — it forced a
re-run on PR #66 and PR #68. Root cause: BaseConversationStore.list
sorted newest-first by updatedAt alone:

.sort((a, b) => b.updatedAt.localeCompare(a.updatedAt))

Two conversations created/touched in the same millisecond compare equal,
so the resulting order is non-deterministic (['c1','c2'] vs
['c2','c1']).

Fix

Add id as a stable secondary key:

.sort((a, b) => b.updatedAt.localeCompare(a.updatedAt) || b.id.localeCompare(a.id))

Ids carry a monotonic counter (conv-<ts>-<n>-<rand>), so id-descending
≈ creation order — the tiebreak stays newest-first. The sort lives in the
shared BaseConversationStore, so both InMemoryConversationStore and
FileConversationStore are covered.

Verification

  • New contract test freezes the clock so both conversations share
    updatedAt, then asserts deterministic order (id-2 before id-1) —
    runs for both stores.
  • The previously-flaky handler-list.test.ts now passes 10/10 in a
    loop (was intermittently failing).
  • Full core suite: 1157 passed / 1 skipped.

…flake)

`BaseConversationStore.list` sorted newest-first by `updatedAt` alone, so
two conversations touched in the same millisecond compared equal and the
order was non-deterministic — the `handler-list.test.ts` flake that
tripped ~25-50% of CI node jobs (a forced re-run on PRs #66 and #68).

Add `id` as a stable secondary key (`|| b.id.localeCompare(a.id)`). Ids
carry a monotonic counter (`conv-<ts>-<n>-<rand>`), so id-descending ≈
creation order — the tiebreak stays newest-first. Covers both stores
(the sort lives in the shared base). New contract test freezes the clock
so both conversations share `updatedAt` and asserts deterministic order;
the previously-flaky handler-list test now passes 10/10 in a loop.
@matze4u matze4u merged commit a534a59 into main Jun 3, 2026
8 checks passed
@matze4u matze4u deleted the fix/v1-0-conversation-list-stable-sort branch June 3, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant