Skip to content

fix(resume-library): add deterministic tiebreaker for same-millisecond saves #908

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
s-annam merged 4 commits into from
Aug 27, 2026
Merged

fix(resume-library): add deterministic tiebreaker for same-millisecond saves #908

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
24f4b44
fix(resume-library): add deterministic tiebreaker for same-millisecon…
shubhransh-gupta Aug 27, 2026
811a39f
fix(storage): route softDeleteRecord through monotonicNow and add for…
shubhransh-gupta Aug 27, 2026
b0812a9
test(storage): add regression test for softDeleteRecord monotonic tim…
shubhransh-gupta Aug 27, 2026
a6cfae9
test(resume-library): stub getAllResumes in reverse order to verify t…
shubhransh-gupta Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/lib/resume-library.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import "fake-indexeddb/auto";
import { deleteDB } from "idb";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { DB_NAME, closeDB, saveResume } from "./storage/index.ts";
import { putRecord } from "./storage/crud.ts";
import type { ResumeRecord } from "./storage/types.ts";
import {
saveResumeToLibrary,
listLibrary,
Expand Down Expand Up @@ -91,6 +93,49 @@ describe("resume-library: save + list", () => {
expect(list.map((e) => e.filename)).toEqual(["tailored.pdf", "general.pdf"]);
expect(list[0]).toMatchObject({ scoreOverall: 84, sourceKind: "pdf", hasCachedParse: true });
});

it("breaks ties deterministically on same-millisecond savedAt", async () => {
const tiedSavedAt = 1_700_000_000_000;
await putRecord<ResumeRecord>(
"resumes",
{
id: "id-b",
filename: "b.pdf",
blob: new Blob([bytes()]),
parse: { result: result(), score: score(70), sourceKind: "pdf", shapeVersion: "1:1" },
createdAt: tiedSavedAt,
updatedAt: tiedSavedAt,
},
{ touch: false },
);
await putRecord<ResumeRecord>(
"resumes",
{
id: "id-a",
filename: "a.pdf",
blob: new Blob([bytes()]),
parse: { result: result(), score: score(80), sourceKind: "pdf", shapeVersion: "1:1" },
createdAt: tiedSavedAt,
updatedAt: tiedSavedAt,
},
{ touch: false },
);
const list = await listLibrary();
expect(list).toHaveLength(2);
expect(list.map((e) => e.id)).toEqual(["id-a", "id-b"]);
});
Comment on lines +97 to +127

@s-annam s-annam Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this test still doesn't verify the tiebreaker. Reverted only a.id.localeCompare(b.id) in resume-library.ts and re-ran this test 5x — still passes every time.

Root cause: getAllResumes() → getAllRecords() → db.getAll(store) (crud.ts:244) has no index, so IndexedDB itself already returns tied records in ascending primary-key order. The test's ids ("id-a", "id-b") are already ascending, so Array.prototype.sort's stability alone reproduces ["id-a", "id-b"] with or without the explicit tiebreak — the test can't distinguish "the comparator works" from "IDB's own order happened to already match it."

Fix: stub getAllResumes/db.getAll to return the tied pair in descending id order, then assert listLibrary() still comes back ["id-a", "id-b"] — that actually exercises the comparator instead of agreeing with the store's incidental order.


it("preserves newest-first save order when saves occur in the same clock millisecond", async () => {
vi.spyOn(Date, "now").mockReturnValue(1_700_000_000_000);
try {
await save("first.pdf", 70);
await save("second.pdf", 80);
const list = await listLibrary();
expect(list.map((e) => e.filename)).toEqual(["second.pdf", "first.pdf"]);
} finally {
vi.restoreAllMocks();
}
});
});

describe("resume-library: load", () => {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/resume-library.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export async function listLibrary(): Promise<ResumeLibraryEntry[]> {
hasCachedParse: snap !== null,
};
})
.sort((a, b) => b.savedAt - a.savedAt);
.sort((a, b) => b.savedAt - a.savedAt || a.id.localeCompare(b.id));
}

/**
Expand Down
16 changes: 14 additions & 2 deletions src/lib/storage/crud.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,18 @@ export async function putRecordIntoExisting<T extends StoredRecord>(
return putRecordVia(getExistingDB, store, record, options);
}

let lastTimestamp = 0;

function monotonicNow(): number {
const now = Date.now();
if (now <= lastTimestamp) {
lastTimestamp += 1;
return lastTimestamp;
}
lastTimestamp = now;
return now;
}

async function putRecordVia<T extends StoredRecord>(
opener: () => Promise<IDBPDatabase<any>>,
store: StoreName,
Expand All @@ -177,7 +189,7 @@ async function putRecordVia<T extends StoredRecord>(
options: { touch?: boolean } = {},
): Promise<T> {
const db = await looseDB(opener);
const now = Date.now();
const now = monotonicNow();

@s-annam s-annam Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secondary — putRecordVia now stamps via this new module-global monotonicNow(), which can run ahead of the real wall clock under contention (each tie bumps lastTimestamp +1ms, doesn't fall back until Date.now() catches up). softDeleteRecord (unchanged, line 340) still stamps with a fresh Date.now() — it backs jobs.ts:134 and letters.ts:98/176.

Concrete failure: several jobs saved back-to-back under contention drift lastTimestamp ~2ms ahead of real time → a job is soft-deleted right after (raw Date.now(), still ~2ms "behind") → the tombstone's updatedAt ends up smaller than the just-written job's. listRecordsUpdatedSince's own docblock says the extension replicator's cursor depends on strictly-increasing updatedAt across writes — a cursor that already passed the job's updatedAt would never see the tombstone. Before this PR both paths shared one raw clock, so this specific inversion wasn't reachable; patching only one of the two writers introduces it.

Suggest routing softDeleteRecord's stamp through the same clock (export monotonicNow(), or hoist one shared helper) instead of a second, independent Date.now() call.

const existing = (await db.get(store, record.id)) as T | undefined;
const written = {
...record,
Expand Down Expand Up @@ -325,7 +337,7 @@ export async function softDeleteRecord(
const db = await looseDB();
const existing = (await db.get(store, id)) as StoredRecord | undefined;
if (existing === undefined || !isLive(existing)) return false;
const now = Date.now();
const now = monotonicNow();
await db.put(store, { ...existing, deletedAt: now, updatedAt: now });
emitChange(store);
return true;
Expand Down
22 changes: 20 additions & 2 deletions src/lib/storage/storage.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import "fake-indexeddb/auto";
import { deleteDB } from "idb";
import { beforeEach, describe, expect, it } from "vitest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { DB_NAME, getDB, closeDB } from "./db.ts";
import {
saveResume,
Expand All @@ -19,7 +19,8 @@ import {
deleteResume,
listResumeChoices,
} from "./resumes.ts";
import { saveJob, getAllJobs } from "./jobs.ts";
import { saveJob, getAllJobs, deleteJob } from "./jobs.ts";
import { getRecord } from "./crud.ts";
import { exportAll, exportToJson, importAll, importFromJson } from "./backup.ts";
import { captureJob } from "./capture.ts";
import { requestStoragePersistence, isStoragePersisted } from "./persist.ts";
Expand Down Expand Up @@ -123,6 +124,23 @@ describe("storage: jobs CRUD", () => {
expect(job.title).toBe("SWE");
expect(await getAllJobs()).toHaveLength(1);
});

it("monotonicNow guarantees softDeleteRecord timestamp is strictly greater than preceding writes in the same millisecond", async () => {
vi.spyOn(Date, "now").mockReturnValue(1_700_000_000_000);
try {
const job1 = await saveJob({ title: "SWE 1" });
const job2 = await saveJob({ title: "SWE 2" });
expect(job2.updatedAt).toBeGreaterThan(job1.updatedAt);

await deleteJob(job1.id);

const record = await getRecord<JobRecord>("jobs", job1.id);
expect(record?.deletedAt).toBeDefined();
expect(record!.updatedAt).toBeGreaterThan(job2.updatedAt);
} finally {
vi.restoreAllMocks();
}
});
});

describe("storage: export / import", () => {
Expand Down
Loading