feat: live OB / stroke-and-distance (#839)

[cner-smith]

Closes #839. Also fixes #797.

Makes out-of-bounds work as a live action. Before this, shots.ob was true on 2 rows out of 886 in production — not because players didn't hit it OB, but because there was no way to say so: the only control lived in the end-of-hole review sheet behind a "+ result" chip, and saveHoleSummary hardcoded the flag back to false on every save (#797). The flag wasn't merely unset — it was actively erased.

The live affordance

A chip in the PLACE_BALL chrome, beside "⛳ On the green":

⚠ Last shot went OB          →  tap again to undo

One tap. It sets ob on the last logged shot, snaps the ball back to that shot's origin, and adds the penalty stroke.

This is cheap because of a property of the existing capture flow: persistShot writes the shot row on confirmAim/skipAim — before the ball is struck — so when a shot flies OB the player is still standing at that shot's origin. Stroke-and-distance placement costs nothing. It also stays correct for a lost ball: walk up, fail to find it, walk back, and the chip is still valid because no new shot was logged in between.

Online-first, like its siblings (delete_shot, projectShotMove): flush pending → shots.update → fail visibly on a 0-row response.

Strokes gained: the rule, not an inference

if (shot.ob) endExpected = startExpected   // stroke and distance: replay from here

Two earlier designs derived the −2 from the next row happening to share coordinates with the OB shot's origin. That's a coincidence, not a rule, and it produced a sign-inverted result: with no following row, isLastShot is positional, endExpected fell to 0, and an OB booked a large positive SG. Reachable by normal use — "Skip all, just track location" is always on screen, so a player who drops without logging the recovery hits exactly that shape.

Stating the rule makes the terms cancel, so −2 is exact and table-independent, and it closes three things at once: the sign flip, web's inexact −2 (two taps never match to the yard), and a silent coupling on off_tee/approach sharing a baseline table.

Numbering: struck shots + a badge

Markers stay 1, 2, 3, 4. The OB shot wears a red badge; the re-hit is the next number from the same origin; the scorecard shows 5.

The issue asks for the re-hit "marked as 4th" — this deliberately differs. Two reasons. A marker in this app is a shot's START, and a penalty stroke has no start→end vector, so numbering it is a category error in our own model. And renumbering would render 1, 2, 4 with a visible gap unless a penalty pip filled it — which is the abandoned design's visual, reintroduced. The stroke count is right everywhere it matters; only the marker label differs.

Score: struck rows + OB rows

One obCount helper in @oga/core, applied at every site where a row count becomes a score — 8 on mobile, 2 on web. One of the mobile sites was missed by the plan and found only because the implementer searched by derivation shape (hs.score = shotCount) rather than write shape (score:).

Why this isn't the trap that killed the first design. That one added a phantom row ~10 consumers had to include in some pipelines and exclude from others; getting it wrong deleted the wrong shot or erased SG silently. This adds one additive term from a boolean every row already carries — every consumer still sees exactly the struck rows it always saw, and the failure mode is a visible off-by-one on a score, not silent structural corruption.

Rejected: a DB trigger recomputing score from rows. It would own the invariant in one place but destroy the end-of-hole ticker's manual override and break the 767 production holes that carry a score with no shot rows at all.

Also fixed

• hole_scores.gir was inflated by the hole's OB count. inferGir reads shot_number as a stroke count, and preserving struck-shot numbering makes those differ. This is persisted from three sites, so it was corrupting GIR, not just displaying it wrong.
• A second SG loop. approachByDistance in stats.ts is a hand-rolled duplicate of the SG inner loop powering the Stats approach chart. It had penaltyAdjust for ob but not the stroke-and-distance rule, so a no-recovery OB booked ≈ +1 there while calculateRoundSG reported −2 — two surfaces disagreeing by ~3 strokes, the wrong one positive. Caught only by the whole-branch review; no task-scoped reviewer had reason to open that file.
• Web could silently destroy a mobile-set flag. saveReviewedHole hardcoded ob: false, so editing a round on the web wiped it. Web's save is delete-then-recreate, so preservation uses a snapshot guarded on activeHoleShots.length === rows.length — positional keying is only sound when the position space is unchanged, and on a mismatch it deliberately drops the flag rather than moving it to a different shot.

Migration

0054_delete_shot_ob.sql — deleting an ob row drops 2 strokes, not 1, since the penalty has no row of its own. A faithful create or replace of 0046: three deltas total, every auth/ownership guard, the re-tally, and the deferrable-constraint renumbering preserved verbatim.

Sequencing: 0053 belongs to open PR #868 and is not on this branch. Production is still at 0046, and 0047–0052 are on dev awaiting the next release — apply in order.

Unapplied and unexecuted. No local Postgres and oga-dev is paused, so this SQL has never been parsed by a Postgres. Apply to dev first.

Known follow-ups (not fixed here)

• Mobile's review sheet doesn't move the Score ticker when a row is flipped OB→non-OB, where web does. The stale value sits in a user-editable ticker on the same screen.
• getShotCategory gates off_tee on shotNumber === 1, so an OB re-tee is categorized approach. Pre-existing, but this feature is what makes it reachable.
• Web's ShotEntryModal sets/clears ob without touching hole_scores.score, so "score = struck + OB" is not an invariant on the web scorecard path.
• Web's review sheet never seeds rows from stored shot data, so re-opening a saved hole shows no OB chip.
• Single-slot OB override protects only the most recent OB on a hole.

Verification

pnpm test 658 core (+7 new) · web 27 · supabase 7 — all green. pnpm typecheck, pnpm --filter @oga/web build, apps/mobile npm run typecheck clean.

Every task got an independent spec + quality review; the branch then got a whole-branch review that returned one Critical and three Important, all fixed and re-verified. Migration 0054 is the one file whose task-scoped reviewer died mid-run — it was checked by hand and then given a genuine pass in the whole-branch review.

Behaviour is device/browser-only and unverified. Nothing here has run on a phone. The load-bearing QA checks: the ball snapping and staying put under GPS, double-tap charging exactly one stroke, delete dropping 2 through the RPC, GIR on an OB hole, and a mobile→web round-trip preserving the flag.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
opengolfapp-web	Ready	Preview	Sep 2, 2026 10:48pm UTC

[cner-smith]

Review — feat: live OB / stroke-and-distance (#839)

Concerns

• apps/web/src/pages/rounds/hooks/useRoundActions.ts:722-723 — web persists ob: (existingByShotNumber.get(...)?.ob ?? false) || row.shotResult === 'ob'. This is the existing.ob || fallback that the mobile side deliberately rejects for the same fact, with a long comment at apps/mobile/components/round/hole/useShotActions.ts:894: because the result picker is single-select, a stored ob=true row that the player re-touches with a non-OB result (e.g. pull) persists ob=true alongside shot_result='pull' — SG charges −2 on a row the label and map badge show as a normal shot, and there's no way to clear ob from the sheet. Combined with the disclosed follow-up that web never re-seeds shotResult from stored data, a plain web re-save of an OB hole also seeds the ticker at the struck count (obCount(merged) is 0 there, HoleReviewSheet.tsx:206) and persists a score one stroke low while ob stays set, so score and SG then disagree. I think the durable fix is the re-seed follow-up (once the sheet hydrates shotResult from stored ob, mobile's no-fallback ob: row.shotResult === 'ob' would be sound on web too and both the flag and the score stay consistent). Until then this || trades the feature(mobile): end-of-hole review — location-now, details-at-EOH (#791) #797 flag-loss for a possible flag/result/score divergence — the two platforms taking opposite stances on the same write is worth a note in the diff at least.
• packages/core/src/holeInference.ts:27-28 — ShotLike.ob and .shotResult are both optional, and the interface comment itself notes a caller supplying neither silently reverts inferGir to the pre-feat(scoring): Allow more explicit marking of Penalty Strokes (for example OB) #839 inflated thresholds with a clean typecheck. All four current callers pass one (the fourth, apps/mobile/app/(app)/round/[id]/index.tsx:886, is safe only because it selects *), but nothing in the type stops a fifth from forgetting — which is exactly the bug class this PR fixes. A required union ({ ob: boolean | null } | { shotResult: string | null }) would let the compiler enforce it. Minor, since it's documented and currently safe.

Nits

• supabase/migrations/0054_delete_shot_ob.sql:1 — this branch's migrations run 0052 → 0054, skipping 0053. CONTRIBUTING.md's "Database migrations" says "No date prefixes, no skipped numbers" and "the next migration goes in the next free number." The PR body explains 0053 is reserved for open PR feat(db): gate user-submitted courses behind approval (0053) #868, but both are unmerged: whichever lands second will need to renumber, and if feat(db): gate user-submitted courses behind approval (0053) #868 slips, dev is left with a 0052→0054 gap. Coordination note, not a code defect.

Suggestions (out of scope for this PR)

• The web score/flag divergence above overlaps the PR's own follow-ups chore: gitignore .env.development / .production / .staging #2 and chore(mobile): remove unused react-native-worklets #4; landing the re-seed would close it. Flagging only so it doesn't get lost — the follow-up list is honest and thorough.

I did not line-diff 0054 against 0046 to verify the "faithful create or replace" claim, so I can't confirm the unchanged portions verbatim; the introduced change (score decrement of 2 for an ob row) reads correct, and the auth/ownership guards are present in the version on this branch.

---

Looked good: delete_shot keeps its auth.uid()/ownership guards and authenticated-only grant; obCount is a new @oga/core export and has tests; the stroke-and-distance rule cancels to an exact −2 and both SG loops (sg-calculator.ts and stats.ts approachByDistance) were brought back in step; the three per-shot arrays in useHoleData share one filter so index alignment is structural.

---

Generated by Claude Code