plan 0016 (3/3): prompt for firmware and tracks on connect - #386
Merged
Conversation
buildTrackJsonForUpload emitted a bare JSON array of courses. The firmware parses that — but its array branch (sd_functions.ino parseTrackFile) blanks longName, shortName and defaultCourse, and every course falls back to lengthFt = 0. lengthFt is what CourseDetector ranks courses by, so a track uploaded from this app could never be course-detected and dropped straight to Lap Anything, and the blank shortName reached the DOVEX header's short_name column. Emit the object form instead — the same shape the app's own track files and the on-device course creator already write, and one the firmware has parsed since well before any shipped release, so no version gate is needed. Also add parseDeviceTrackFile(), which keeps the wrapper's longName/shortName/ type/defaultCourse rather than discarding them; parseDeviceCourseJson stays as a thin wrapper over it for the callers that only want courses. The rename flow needs longName, and needs shortName because for a device-authored track the FILENAME is the 12-char longName (N260803_1432.json) while the 8-char shortName the sync merge keys on lives inside the file. The old "emits a JSON array of courses (not a wrapping object)" test asserted the lossy shape as the contract, which is how this survived review; it is replaced with assertions on the metadata the device actually consumes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
A track the on-device course creator wrote is stored at N260803_1432.json but declares shortName "08031432" — 8 characters, chosen by the firmware precisely because that is this app's Track.shortName budget and the key its sync merge uses. buildMergedTrackList keyed on the FILENAME instead, so a track imported from the device could never be matched to the file it came from: it stayed "device_only" forever and the sync kept re-offering it. Separate the two concepts. DeviceTrackFile.shortName is now the identity (the declared shortName, falling back to the filename base only for legacy bare-array files that declare nothing), and the new fileName / deviceFileName carry the location. deviceTrackFileFrom() owns that rule so it is unit-tested rather than buried in the tab, and every write path now targets the real file instead of `shortName + ".json"` — which would otherwise orphan the original and leave two copies on the card. Also fixes the other half of the same nag: handleDownloadToApp never passed a shortName to addTrack, and buildMergedTrackList skips app tracks that have none, so downloaded tracks were invisible to the merge whatever the key was. It now carries the shortName over and names the track from the file's longName. The two course-level writers went through rebuildDeviceTrackJson so editing one course stops stripping the file's wrapper metadata and resetting every lengthFt — the same loss the bare-array uploader caused, reached from a different button. Verified by reverting the identity rule and watching the round-trip test report 2 merged entries instead of 1 — literally the app_only/device_only split that made the prompt re-fire. The first draft of that test derived its input from the value under test and passed either way; it now spells the expectation out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Four modules, all Arduino-free pure logic with no React, because the test
environment is "node" with no testing-library — a dialog cannot be rendered, so
anything worth asserting has to live outside the component.
- deviceGeneratedNames: recognises the on-device course creator's
N{YYMMDD}_{HHMM} names and MMDDHHMM short names. The date and time parts are
validated, so a real name that merely looks the part isn't mistaken for a
placeholder and the user pushed to rename something they already named.
- deviceSyncPlan: decides what a sync would offer and in which direction.
Synced tracks are dropped; app tracks the user didn't create are never pushed
(the two we ship are reference data, not "unknown tracks"); a mismatch uploads
the app's version after importing any course walked on the device.
Crucially it also refuses to offer rows that could never converge — mixed
circuit+sprint tracks, tracks past the firmware's MAX_LAYOUTS (whose tail its
parser silently ignores, so the file can never read back as written), and
sprint tracks on a transport that can't reach /TRACKS/SPRINT. Each of those
would otherwise report a difference on every connect forever. They are
surfaced with a reason rather than trimmed to fit: dropping a user's courses
to turn a checkmark green is the worse failure.
- deviceSyncNames: the edit rules and the save gate. A short name follows the
long name until the user takes it over, and editing the long name takes it
back; a course name follows its track's name the same way. Track names are
required for both kinds — a venue is permanent. Course names are required for
circuit only: a sprint venue re-lays its course every event, so the date it
was walked genuinely is the most useful label.
- deviceSyncOps: the ordered operation list. Put before delete, so a failure
between them leaves the track on the card twice rather than nowhere; device
before app, so a failure after the write leaves a correctly-named file the
next connect offers as a plain download, instead of stranding a renamed app
track beside its old device file. FAT is case-insensitive, so a case-only
filename change is not a rename — deleting "the old file" would delete the one
just written.
The load-bearing tests replay a plan back through deviceTrackFileFrom and
buildMergedTrackList and assert "synced". If that ever fails, the on-connect
prompt re-fires on every connect, which is the whole thing this is avoiding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Captures why the sync path had four separate ways to produce a track that could never reach "synced", the identity-vs-location split that fixes the worst of them, why operation order (put before delete, device before app) is the load-bearing part, and the decisions taken with the owner — including the ones about what NOT to build: no truncation, no firmware capability layer, no new opcodes. Also records that no capability gate is needed here, with the evidence: the firmware has parsed the object track format since before any shipped release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
addTrack/addCourse deliberately only ever ADD: they backfill a short name only when there isn't one, and never remove a course. That is right for "add this one thing" and wrong for a sync, which has already worked out what the track must look like for both sides to agree — anything less than a wholesale write leaves them disagreeing, and the next connect re-offers the same track. Courses are marked user-defined on the way in because saveUserTracks drops any that aren't, which would silently discard a course walked on the device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
deviceSyncRunner walks the operation list with injected executors, so the risky part is testable without a radio or a browser. It keeps going after a failure, but a failed track ABANDONS ITS OWN remaining operations — once the new file didn't write, deleting the old one destroys the only copy, which for a course walked in the field is unrecoverable. Other tracks still run: one track failing is no reason to leave the other nine untouched, and the plan orders each track's work contiguously so that split is clean. deviceSyncFetch owns reading both folders. The Device → Tracks tab now uses it instead of its own copy of the loop, so there is exactly one place that applies deviceTrackFileFrom's identity rule — a second copy that keyed files by filename would quietly reintroduce the bug where an imported track never matches the file it came from. Behaviour is unchanged: same order, same per-file error swallowing, same sprint-capability skip. buildDeviceSyncSnapshot also collects the short names of tracks the plan is NOT touching, so a rename can't be pointed at an already-synced track's file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Two screens, the selection, and the validation that decides whether "Save & import" can fire — all as pure functions, because the test environment is node with no testing-library and logic left in a component is logic nobody checks. Three rules worth naming: - Unchecking a row stops it being validated. Otherwise one track you don't want to name blocks the entire sync with no way past it. - A course name follows its track's name until the user types in it, and going Back to rename the track re-points every course that is still following. A name they typed themselves is never overwritten. - canSave re-checks the TRACK screen, not just the course screen. Going forward and then back and clearing a track name must not leave Save live. Duplicate short names are caught across the whole selection and against tracks outside the plan, per kind — two tracks sharing a short name are one file on the device, so the second write silently overwrites the first, while the same short name in the circuit and sprint folders is two legitimate files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Screen 1 names the tracks: a checkbox per row, an upload/download bubble, a sprint bubble, and — only on rows the logger named itself — a full-name box with a narrower short-name box beside it that fills in as you type. Screen 2 does the same for course names with a circuit/sprint bubble. Back/Next, then Back and "Save & import". Rows the sync refuses to attempt are listed underneath with the reason, rather than silently missing. The component holds one useState and the markup; every decision comes from the pure modules. Problem and skip-reason strings are spelled out in switch statements because the i18n keys are literal-union typed — a computed key won't typecheck, which is the point. 37 new keys across all seven locales, matching each language's existing terminology for track and course. The walked-on date renders in UTC: the name encodes the GPS clock, so a local-time render would show the wrong minute. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
… itself checkForUpdates toasted on every non-update outcome — "up to date", "couldn't read the version", "check failed". That is right when the user pressed the button and wrong for a check that fires on connect: they didn't ask, and being told nothing happened is worse than being told nothing. It now takes a `silent` option, and returns whether an update was actually offered so an automatic caller knows whether it still owns the screen. The snooze is keyed by DEVICE AND VERSION, and checked against the version actually on offer rather than up front. Two consequences, both wanted: the owner's two loggers snooze independently, and a NEW release asks again immediately instead of inheriting yesterday's "tomorrow". Storage is a standalone module in the pendingCheckout.ts shape — a private localStorage key and a pure parse(raw, now) the getters wrap, so the 24h rule is testable without touching the clock. Deliberately NOT AppSettings: that is cloud-synced, and "I'll do it tomorrow" is a decision about this browser and this logger, not something to push to every device the user owns. A future timestamp is treated as live rather than discarded, so winding the clock back can't silently un-snooze everything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
The firmware dialog used to be inline in FirmwareUpdateSection, which mounts only when the drawer is open, on the Device tab, on the settings sub-tab, while connected over BLE. A check that fires on connect had nowhere to show itself. Split the dialog out and give a FirmwareUpdateProvider ownership of the single useFirmwareUpdate instance, rendered once high in the tree. Mounting a second instance instead would mean two device-version reads over GATT and two dialogs racing to be the one on screen. DeviceConnectFlow then runs the two questions in order on each new connection: - FIRMWARE FIRST, because accepting it reboots the device and drops the link — anything queued behind it would be thrown away. It is also the rarer of the two, and "remind me tomorrow" keeps it rare. - THEN TRACKS, and only when the sync plan actually has rows. A prompt that appears on every connect to say "nothing to sync" is worse than silence, and the plan already excludes the rows that could never converge. Declining the track prompt is scoped to the connection: no stored suppression, and the flow only re-runs when a new connection appears. Being asked again after deliberately reconnecting is expected; being asked twice on one connection is nagging. Offline, the firmware step is skipped rather than surfaced — the manifest is a network fetch and that is not this connection's problem. A failure reading the card is logged, not toasted, for the same reason the auto-check went silent: the user didn't ask, and the Device → Tracks tab is still there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ESkRRtF4vRrANPL6huSgmD
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lapwing | e59adfd | Commit Preview URL Branch Preview URL |
Aug 05 2026, 06:37 AM |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Coverage SummaryLines: 58.28% (7485/12841) · Statements: 57.43% · Functions: 55.17% · Branches: 55.2% Per-file coverage
|
This was referenced Aug 6, 2026
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Last of three (plan 0016). Replaces #383, which GitHub had locked into a stack. Targets
BETAdirectly — merge after #387 and #385 and this diff shrinks to just its own work.Connect a logger and two questions run in order:
Declining the track prompt is scoped to the connection: no stored suppression, and the flow only re-runs when a new connection appears. Being asked again after deliberately reconnecting is expected; twice on one connection is nagging.
The snooze
Keyed by device and version, and checked against the version actually on offer rather than up front. So your two loggers snooze independently, and a brand-new release asks again immediately instead of inheriting yesterday's "tomorrow".
Storage is a standalone
src/lib/firmwareUpdateReminder.tsin thependingCheckout.tsshape — private localStorage key, pureparse(raw, now)the getters wrap. Deliberately notAppSettings: that's cloud-synced viaaccountExport.ts, and "I'll do it tomorrow" is a decision about this browser and this logger, not something to push to every device you own.Two changes that made it possible
checkForUpdatesgainedsilentandsuppress, and now returns whether an update was offered. It toasted on every non-update outcome — "up to date", "couldn't read the version", "check failed". Right for a button press, wrong for a check the user didn't ask for.FirmwareUpdateSection, which mounts only with the drawer open, on the Device tab, on the settings sub-tab, over BLE — so an on-connect check had nowhere to render.FirmwareUpdateProvidernow owns the singleuseFirmwareUpdateinstance and rendersFirmwareUpdateDialogonce, high in the tree.Related Issues
Supersedes #383. Merge after #387 and #385.
Type of Change
Checklist
bun run lintpassesbun run typecheckpassesbun run test:runpasses (2719 tests, 190 files)bun run buildsucceedsuseOnlineStatusCHANGELOG.mdNotes for Reviewers
The dialog hoist is the riskiest thing in this PR and it's worth a look even though it's mechanical.
FirmwareUpdateSectionwent from ~200 lines to ~55 — everything removed is now inFirmwareUpdateDialog.tsxunchanged apart from the new snooze button. The alternative (a seconduseFirmwareUpdateinstance for the auto-check) would have meant two GATT version reads per connect and two dialogs racing to be on screen, so I took the refactor instead.Things I'd flag:
nodewith no testing-library — soDeviceConnectFlowand the dialog have no unit tests, only the purefirmwareUpdateRemindermodule does (14 tests). The connect sequencing genuinely needs hardware to verify. The one behaviour I'd specifically watch: after you dismiss or snooze the firmware dialog, the track prompt should follow — that transition keys offconfirmOpengoing back down, which is the least robust thing here.DeviceProvideris mounted twice inIndex.tsx(landing and main), soFirmwareUpdateProvider+DeviceConnectFloware mounted twice too — never simultaneously, since the branches are exclusive, but worth knowing.DeviceDetails.supportsSprintTracksis the seam if you ever want one.