plan 0016 (1/3): device sync foundations — stop tracks that can never sync - #381
Closed
TheAngryRaven wants to merge 4 commits into
Closed
plan 0016 (1/3): device sync foundations — stop tracks that can never sync#381TheAngryRaven wants to merge 4 commits into
TheAngryRaven wants to merge 4 commits into
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
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lapwing | 744ac9d | Commit Preview URL Branch Preview URL |
Aug 05 2026, 06:14 AM |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Coverage SummaryLines: 57.93% (7351/12688) · Statements: 57.1% · Functions: 54.61% · Branches: 55.04% Per-file coverage
|
This was referenced Aug 5, 2026
10 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
First of three PRs building the rename-on-connect flow (plan 0016). This one is all model and bug fixes, no UI — the wizard (#385) and the on-connect prompts (#386) sit on top.
The on-device course creator writes
/TRACKS[/SPRINT]/N260803_1432.json. Renaming that has to happen here, and the new name has to be written back, or the "please name this" prompt fires on every connect. Designing for that turned up four existing ways to produce a track that can never reachsynced— the same failure, reached without any of the new code:buildTrackJsonForUploademitted a bare JSON array. The firmware parses an array, but its array branch (BirdsEye/sd_functions.ino:475-484) blankslongName/shortName/defaultCourse, and every course falls back tolengthFt = 0(:507).lengthFtis 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 blankshortNamereached the DOVEX header'sshort_namecolumn. The two course-level writers inDeviceTracksTab.tsxhand-rolled the same array.parseDeviceCourseJsondiscardedlongName/shortName.handleDownloadToAppnever passed ashortNametoaddTrack, andbuildMergedTrackListskips app tracks that have none (if (!sn) continue) — so every downloaded track was invisible to the merge forever.N260803_1432.jsonbut declaresshortName: "08031432"— 8 chars, chosen by the firmware author precisely because that's this app'sTrack.shortNamebudget (BirdsEye/course_creator.h:36-57). The merge keyed on the filename, so an imported track could never match the file it came from.No firmware changes, and no capability gate. The firmware has parsed the object track format since well before any shipped release (field units are 3.0.1/3.1.0) and reads
longName,shortName,defaultCourse,typeand per-courselengthFt(sd_functions.ino:449-507). Switching the writer asks nothing new of any device in the field.New pure modules
Test env is
nodewith no testing-library — a dialog can't be rendered — so the whole decision surface lives outside the component:deviceGeneratedNames.tsN{YYMMDD}_{HHMM}/MMDDHHMM, with date + time validated so a real name that looks the part isn't treated as a placeholderdeviceSyncPlan.tsdeviceSyncNames.tsdeviceSyncOps.tsRows that can never converge are refused, not retried —
mixed_kind,too_many_courses(past the firmware'sMAX_LAYOUTS, whose parser silently ignores the tail), andsprint_unsupported(native IPC drops thekindarg). Each would otherwise report a difference on every connect forever. Not trimmed to fit — dropping a user's courses to turn a checkmark green is the worse failure, and truncation is explicitly deferred per the owner.Ordering is the load-bearing part. 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.
Related Issues
Builds on #380 (merged). Followed by #385 and #386.
Type of Change
Checklist
bun run lintpassesbun run typecheckpassesbun run test:runpasses (2663 tests, 186 files)bun run buildsucceedsdocs/plans/0016-device-track-sync-rename.md,CHANGELOG.mdNotes for Reviewers
Two tests were verified to actually bite, by breaking the fix and watching them fail:
app_only/device_onlysplit that made the prompt re-fire.shortNamefrom the stored track makes both settle tests reportdevice_only.That mattered, because my first draft of one of them was vacuous: it derived its input from the value under test and passed either way. It now spells the expectation out literally. Worth knowing since the same class of thing is how bug 1 survived review — a test named "emits a JSON array of courses (not a wrapping object)" had pinned the lossy shape as the contract. That test is replaced.
Two things I'd flag rather than have you find:
too_many_coursesis a real skip, not a no-op. If you have a track with >10 courses it will now be reported as un-syncable rather than silently half-written. That's the deliberate reading of "don't truncate yet" — say so, don't guess. Happy to change the shape once you've decided.kindargument on get/put/delete. I guarded around it (sprint rows are skipped there) rather than fixing it, since it's tracked as the plan 0015 Android IPC follow-up.