plan 0016 (2/3): the two-screen sync + rename wizard - #385
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
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lapwing | 7af0c31 | Commit Preview URL Branch Preview URL |
Aug 05 2026, 06:27 AM |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Coverage SummaryLines: 58.23% (7458/12806) · Statements: 57.37% · Functions: 55.06% · Branches: 55.13% Per-file coverage
|
TheAngryRaven
changed the title
TheAngryRaven
marked this pull request as ready for review
August 6, 2026 00:45
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
Second of three (plan 0016). Replaces #382, which GitHub had locked into a stack. Targets
BETAdirectly — merge after #387 and this diff shrinks to just its own work.The wizard itself:
Gating is as you specified: track names always required (circuit and sprint — a venue is permanent), course names required for circuit only (a sprint venue re-lays its course every event, so the walked date is genuinely the useful label).
New modules
deviceSyncWizard.tsdeviceSyncRunner.tsdeviceSyncFetch.tsbuildDeviceSyncSnapshotDeviceSyncWizard.tsxuseStateand the markuptrackStorage.saveSyncedTrackis new becauseaddTrack/addCourseonly ever add — they backfill a short name only when absent and never remove a course. A partial write leaves the two sides disagreeing, which is the loop this is trying to end.Failure handling
runSyncOperationskeeps 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.Related Issues
Supersedes #382. Merge after #387.
Type of Change
Checklist
bun run lintpassesbun run typecheckpassesbun run test:runpasses (2705 tests, 189 files — +42 here)bun run buildsucceedsCHANGELOG.mdNotes for Reviewers
The wizard has no launcher yet — that's #386. You asked for it to be connect-only, so PR 3 wires it to the connection event alongside the firmware prompt. The
DeviceSyncPromptyes/no lives here so PR 3 is purely "when does this fire".Two things worth a look:
DeviceTracksTablost ~35 lines — its device-read loop now calls the sharedfetchDeviceTrackFiles. Behaviour is identical (same order, same per-file error swallowing, same sprint-capability skip); the reason to share it is that a second copy keying files by filename would quietly reintroduce the bug plan 0016 (1/3): device sync foundations — stop tracks that can never sync #387 fixes. The tab's UI is otherwise untouched, as you asked.initialCourseDraft.The i18n is hand-written across 6 languages (no API key for
i18n:seed), matching each file's existing terms for track/course. Japanese is the one I'd second-guess: existing keys use コース for both track and course, so I used トラック for track in the new strings to keep the two boxes distinguishable side by side.