Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 39 additions & 0 deletions __tests__/shotRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = {}): Shot {
const shot = makeShot(overrides);
delete (shot as Partial<Shot>).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.
Expand Down
40 changes: 40 additions & 0 deletions __tests__/useSessionStore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> = {}): Shot {
const shot = makeShot(overrides);
delete (shot as Partial<Shot>).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');
});
});
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
19 changes: 13 additions & 6 deletions storage/shotRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions stores/useSessionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,14 @@ export const useSessionStore = create<SessionState>((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;
Expand Down
Loading