diff --git a/__tests__/shotRepository.test.ts b/__tests__/shotRepository.test.ts index ba2c501..47b1c5a 100644 --- a/__tests__/shotRepository.test.ts +++ b/__tests__/shotRepository.test.ts @@ -183,6 +183,45 @@ describe('shotRepository', () => { expect(sessions[0].lastShotAt).toBe('2026-09-14T09:30:00Z'); }); + // Swing-speed mode reuses the `shot` event but serializes a different key + // set: swing_speed_to_shot_dict() omits shot_number entirely rather than + // sending it as null (server.py:4322-4374). An absent key deserializes to + // undefined, which is not === null, so the identity guard below takes the + // wrong branch and undefined reaches the bound SQL parameter. The factory + // above always supplies shot_number, so no existing test reaches this path. + describe('a shot the server could not number', () => { + function makeUnnumberedShot(overrides: Partial = {}): Shot { + const shot = makeShot(overrides); + delete (shot as Partial).shot_number; + return shot; + } + + it('records every swing of a swing-speed session', async () => { + // The whole session was being lost, not one shot: undefined reached a + // bound parameter, threw, and saveShot's catch swallowed it. Nothing + // surfaced, because the live view never reads back from storage. + const repo = createShotRepository(openTestDatabase()); + await repo.init(); + + await repo.saveShot('session-1', makeUnnumberedShot({ timestamp: '2026-09-14T10:00:00Z' })); + await repo.saveShot('session-1', makeUnnumberedShot({ timestamp: '2026-09-14T10:01:00Z' })); + + expect(await repo.loadShots('session-1')).toHaveLength(2); + }); + + it('stores an absent shot number as null, the same as an explicit one', async () => { + // Normalising on the way in keeps one representation in the database, so + // a reloaded shot is indistinguishable from one the server sent as null. + const repo = createShotRepository(openTestDatabase()); + await repo.init(); + + await repo.saveShot('session-1', makeUnnumberedShot({ timestamp: '2026-09-14T10:00:00Z' })); + + const [stored] = await repo.loadShots('session-1'); + expect(stored.shot_number).toBeNull(); + }); + }); + it('can be initialised twice without losing what is already stored', async () => { // init() runs on every launch; a migration that re-ran destructively would // wipe the player's history. diff --git a/__tests__/useSessionStore.test.ts b/__tests__/useSessionStore.test.ts index 7a74ec2..2215e1f 100644 --- a/__tests__/useSessionStore.test.ts +++ b/__tests__/useSessionStore.test.ts @@ -157,3 +157,43 @@ describe('an enriched shot replacing its provisional version', () => { expect(useSessionStore.getState().shots).toHaveLength(2); }); }); + +// Swing-speed mode reuses the `shot` event but serializes a different key set: +// swing_speed_to_shot_dict() omits shot_number entirely rather than sending it +// as null (server.py:4322-4374). The null guards below are correct; what was +// missing is normalising an absent key, which arrives as undefined and is not +// === null, so it slipped past them. The factories above always supply +// shot_number, so no existing test reaches this path — these build the payload +// the way the server actually sends it. +describe('a shot the server could not number', () => { + // Strip the key rather than setting it undefined, so the object matches a + // JSON.parse of the real payload. + function makeUnnumberedShot(overrides: Partial = {}): Shot { + const shot = makeShot(overrides); + delete (shot as Partial).shot_number; + return shot; + } + + it('follows the same append-never-merge rule as an explicit null', () => { + // An absent key and an explicit null both mean "the server could not + // number this shot", so both must append. Before this was normalised, + // undefined slipped past the null guard and matched the first unnumbered + // shot on the list — overwriting a different swing. + useSessionStore.getState().addShot(makeUnnumberedShot({ timestamp: 't1', club: 'driver' })); + + useSessionStore.getState().replaceShot(makeUnnumberedShot({ timestamp: 't2', club: '7 iron' })); + + expect(useSessionStore.getState().shots).toHaveLength(2); + }); + + it('leaves a numbered shot alone when an unnumbered one arrives', () => { + // A swing-speed rep must never land on a launch-monitor shot. + useSessionStore.getState().addShot(makeShot({ shot_number: 7, club: 'driver' })); + + useSessionStore.getState().replaceShot(makeUnnumberedShot({ timestamp: 't9', club: '7 iron' })); + + const shots = useSessionStore.getState().shots; + expect(shots).toHaveLength(2); + expect(shots.find((shot) => shot.shot_number === 7)?.club).toBe('driver'); + }); +}); diff --git a/package.json b/package.json index 01fde81..8f9472d 100644 --- a/package.json +++ b/package.json @@ -60,9 +60,11 @@ "collectCoverageFrom": [ "app/**/*.{ts,tsx}", "components/**/*.{ts,tsx}", + "data/**/*.{ts,tsx}", "services/**/*.{ts,tsx}", "storage/**/*.{ts,tsx}", "stores/**/*.{ts,tsx}", + "utils/**/*.{ts,tsx}", "types.ts" ], "coverageReporters": [ diff --git a/storage/shotRepository.ts b/storage/shotRepository.ts index c0b0f3e..2623afe 100644 --- a/storage/shotRepository.ts +++ b/storage/shotRepository.ts @@ -205,17 +205,24 @@ export function createShotRepository(db: ShotDatabase): ShotRepository { // The single write path for both `shot` and `shot_update`. The server // re-emits an enriched shot under the same shot_number, so a shot already - // filed is updated in place. A shot the server could not number — nullable - // on the wire, though a supported server always sets it — is appended, - // which is the old behaviour and cannot collide with anything. + // filed is updated in place. A shot the server could not number is + // appended, which cannot collide with anything. + // + // shot_number is normalised before use: swing-speed mode omits the key + // from its payload entirely rather than sending null + // (swing_speed_to_shot_dict, server.py:4322-4374). The absent key arrives + // as undefined, which is not === null, so it slipped past the guard below + // and reached a bound SQL parameter — throwing, and being swallowed by the + // catch, which discarded every shot of the session. async saveShot(sessionId, shot) { try { + const shotNumber = shot.shot_number ?? null; const existing = - shot.shot_number === null + shotNumber === null ? null : await db.getFirstAsync<{ id: number }>( 'SELECT id FROM shots WHERE session_id = ? AND shot_number = ?', - [sessionId, shot.shot_number], + [sessionId, shotNumber], ); if (existing) { @@ -231,7 +238,7 @@ export function createShotRepository(db: ShotDatabase): ShotRepository { await db.runAsync( `INSERT INTO shots (${columns.join(', ')}) VALUES (${columns.map(() => '?').join(', ')})`, - [sessionId, shot.shot_number, ...measurementValues(shot)], + [sessionId, shotNumber, ...measurementValues(shot)], ); } catch { // A dropped write loses one shot from history; the live view is unaffected. diff --git a/stores/useSessionStore.ts b/stores/useSessionStore.ts index b8812a5..d59e54c 100644 --- a/stores/useSessionStore.ts +++ b/stores/useSessionStore.ts @@ -52,10 +52,14 @@ export const useSessionStore = create((set) => ({ addShot: (shot) => set((prev) => ({ shots: [shot, ...prev.shots] })), replaceShot: (shot) => set((prev) => { + // Normalised because swing-speed payloads omit shot_number rather than + // sending null; undefined is not === null and slipped past this guard, + // matching every other unnumbered shot on the list. + const shotNumber = shot.shot_number ?? null; const index = - shot.shot_number === null + shotNumber === null ? -1 - : prev.shots.findIndex((existing) => existing.shot_number === shot.shot_number); + : prev.shots.findIndex((existing) => existing.shot_number === shotNumber); if (index === -1) return { shots: [shot, ...prev.shots] }; const shots = [...prev.shots]; shots[index] = shot;