Skip to content

Waypoint altitudes: zero is never missing, OziExplorer is feet, and a waypoints page you can use from a hill - #700

Merged
pokle merged 12 commits into
masterfrom
claude/waypoints-altitude-validation-y373oh
Sep 19, 2026
Merged

pokle merged 12 commits into
masterfrom
claude/waypoints-altitude-validation-y373oh

Conversation

@pokle

@pokle pokle commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Branch preview: https://claude-waypoints-altitude-va.glidecomp.pages.dev

One thing throughout: an altitude the reader can check. What started as three
changes to the waypoints editor turned into a rebuild of the page, because the
surface that was meant to show a disagreement was the reason half of it was off
screen.

1. A zero altitude is never "missing"

WaypointFileRecord.altitude is now optional. An altitude is either known
(any number, 0 included) or absent — a waypoint on a beach is at 0 m, and a
file with no elevation column knows nothing.

Collapsing the two is what made the old workaround necessary:

  • A waypoint genuinely at sea level could not be left alone. "Fill altitudes
    from map" kept offering to fill it, and filling it wrote 0 again, so the
    button never went quiet.
  • A file with no elevation column and a file full of beaches were
    indistinguishable after parsing.
  • The waypoints editor and the route editor's turnpoint sheet disagreed about
    what a 0 in the same field meant — one treated it as a value, the other as
    a blank.

Now: parsers leave an absent altitude undefined; exporters use their format's
own way of saying nothing (OziExplorer's -777, a blank elev in .cup/CSV,
an omitted <ele> in GPX) and fall back to 0 only in the formats whose
elevation is positional and cannot express absence (FS
$FormatGEO/$FormatUTM, KML, and the packed z of an XCTrack QR); an empty
field is the only "missing" signal.

A 0 that is wrong — a 0 under a 1500 m launch — is a wrong altitude for
the check to report as a 1500 m disagreement, not something to overwrite
unasked.

The conversion from a competition waypoint to a turnpoint draft now lives in
exactly one place (draftFromRecord, with draftWithRecord delegating to it
for the "load from a waypoint" case). TurnpointSheet had kept its own copy of
the line, which is how picking a sea-level waypoint in the route editor went on
turning 0 into "unknown" after the function had been fixed not to.

2. OziExplorer .wpt states elevation in FEET

Field 14 is feet per the format; we read it as metres, inflating every imported
altitude by 3.28.

The bundled HG Worlds 2026 set proves it — the organiser published the same
points in four formats (web/samples/reference/hg-worlds-2026/waypoints/):

Waypoint FS $FormatGEO CompeGPS OziExplorer
A01 BORDANO LANDING 225 225.000000 738 (= 224.9 m)
A06 ENEMONZO LANDING 373 1224 (= 373.1 m)
A11 SAURIS LANDING 1392 1392.000000 4567 (= 1392.0 m)

LIENZ LANDING reads 2234, which is 681 m, and Lienz sits at about 673 m.
-777 (the format's "no altitude recorded") now reads as unknown rather than
237 m below sea level.

A cross-format altitude agreement test holds this down the way the
coordinate one already did: the same point must decode to the same altitude
whichever file the organiser uploads.

Nothing already scored moves. Both bundled sample comps are the
CompeGPS/PCX5 dialect and state metres, so no seeded comp's altitudes changed;
and a task freezes its own xctsk when it is built, so no existing task or
score is touched. No scoring source changed either (waypoint-files.ts is
deliberately outside the fingerprint closure), so check:scoring-note reports
"not required" and there is no scoring-change note.

3. Never read a Terrain-RGB pixel through a canvas

Found while reviewing the preview: a coastal hill that cannot be more than about
200 m read 13273 m from the map.

The first hypothesis was wrong and worth recording. I thought it was Display-P3
colour management; a Chromium --force-color-profile=display-p3 probe disproved
it, so I kept digging rather than shipping the guess.

The real cause: Mapbox's terrain tiles are RGBA, and Mapbox marks no-data —
the sea past a coastline included — with partial alpha. A canvas stores
premultiplied bytes, so the 24-bit elevation those three bytes carry does not
survive the round trip, and the red byte is 6553.6 m a step. premultiplyAlpha: 'none' does not fix it.

analysis/elevation.ts now inflates and unfilters the PNG itself — exact by
construction, and unit-testable without a browser — rejects anything outside
−500..9000 m, and takes the 3×3 median of the plausible pixels. A test fixture
standing in for a terrain tile must now be RGBA, because mine was colour type 2
and could not have caught this.

Altitudes already saved from the broken read are recognisable: out by almost
exactly 6553.6 m, or a multiple of it. The review names that as "not your file"
and offers the map's value as the correction.

4. "Check altitudes" — the review, and the page around it

The other half of the fill. The fill answers blanks, has no competing value,
and applies itself in one press. The check questions values that are already
there, so it changes nothing on its own and reports nothing until it is
pressed
.

It reviews in a sheet, and this is where the PR changed shape. The review
began as three extra columns in the Tabulator grid, which is the right shape on
a desktop and a poor one on a phone: the new columns sat to the RIGHT of the
frozen Code column, so reaching them scrolled the waypoint's own Alt (m) out
of sight behind Code. A disagreement was reported with half of itself off
screen
— "+6580", with the map's 80 m visible and the file's 6660 m hidden,
which reads as the map being broken when the file was. Every row in the sheet
states BOTH altitudes and the unit on each.

  • Under 50 m is not a finding — a ~10 m DEM pixel against a file rounded to
    10 m (the Corryong set encodes the altitude in the code: 4C-080 is 800 m).
    Agreeing rows are counted in the summary and left out of the list: the
    difference between "12 to look at" and "145 differ".
  • Past 300 m, doubt the coordinates first, said in words rather than in a
    colour, and the detail view offers the coordinates for editing right there.
  • A whole file can be wrong the same way. A constant ratio (feet read as
    metres) or offset (datum/reference) is named before the per-row work starts,
    with one bulk conversion for the feet case rather than the same decision 187
    times.
  • The list holds still, in membership AND order. Biggest disagreement first,
    frozen when the sheet opens. A live predicate makes a row vanish the instant
    it is accepted, which reads as having deleted it; a live sort keeps every row
    but sends the one under the reader's thumb to the bottom, because its
    disagreement has just become zero. Both were caught by the e2e.
  • A difference of exactly zero prints nothing. The absence of a finding is
    not a finding, and "0 m" beside a row the reader has just fixed is noise.
  • The coordinates field is a draft, applied on the way out. The page forgets
    a waypoint's terrain reading whenever its coordinates change — the reading
    belonged to where the waypoint was. Written per keystroke, the first character
    typed took mapAlt away, so "From the map" fell to "—", the difference line
    vanished and the accept button unmounted, mid-paste, destroying the very
    comparison the view exists to show. The commit hangs off unmount rather than a
    button, so it covers Back, Done, the accept button, browser Back, Escape and
    the sheet closing. The altitude field stays live on purpose: it is one of the
    two numbers being compared, so the difference recounting as you type is the
    feedback.
  • Accepting is an ordinary unsaved edit, so Save and the existing
    discard-on-leave guard are the commit and the undo. The sheet says outright
    that tasks already built keep their own copy of a waypoint, so corrections
    here do not change them.

Deselection therefore costs nothing to build: you don't press the row's accept.

5. Mobile first, and the grid is gone

The waypoints page is now a list whose rows open full-screen sheets, at every
width
(comp/WaypointList.tsx, comp/WaypointSheet.tsx). The 145-row,
eight-column Tabulator grid is gone, not hidden behind a breakpoint: two
editors would mean two code paths and a reviewer on a desktop seeing something
the author never tested on a phone.

The reasoning is the owner's, and is now a standing rule in CLAUDE.md: a
competition is run from a hill, nobody wants to carry a laptop up it, so a phone
is the shape a surface is designed FOR rather than a width it has to survive.

The read-only view got the same treatment, one pass later. It was a
six-column RAC Table in an overflow-x-auto scroll region — so the
sideways-scrolling shape the editor had just stopped using survived for the
anonymous PILOT, the one actually standing on the hill, while the organiser more
likely to be at a desk got the list. WaypointList with no onOpen is the
read-only mode: no chevron, no row action, the locate pin kept. That deletes
sortRows, numField, the sort state and the SortDescriptor import; the
table's sortable columns are the cost, and a sort control over the list can
bring them back. The filter box serves both and did not move.

The one thing the two modes do not share is the altitude's unit: the editor is
the waypoint FILE edited in place and stays metric with labels saying so, while
a read-only row is an altitude printed to a reader and honours the preference
like every other altitude in the app (#662).

One way to do a thing. The same pass deleted a "Fill altitudes from map"
button that duplicated what Check altitudes does better, a "Show on the map"
button in the waypoint sheet that duplicated the list row's pin, and a status
line that pre-judged a set nobody had asked about.

6. Back closes one layer, not the page

A sheet lives in React state rather than a route — the page behind it is unsaved
work, so a sibling route would unmount it and use-unsaved-changes-guard would
prompt on the way in. That left sheets invisible to the history stack, and one
Back from a waypoint's details left the whole editor.

lib/use-back-dismiss.ts gives each overlay a history entry while it is open
and consumes it again on the way out, so Back walks a detail view → its list →
the page, one press per layer. Read its note before touching it: a popstate
reaches every listener, popping our own entry looks like a user Back to the
layer underneath, and StrictMode runs the effect twice. All three broke it, and
all three were caught by the e2e rather than by review.

It was wired into FullScreenSheet and nowhere else, but twelve files import
rac/dialog — so Back closed a sheet and left the PAGE from a dialog. The gap
had teeth beyond the inconsistency: the Add-waypoint dialog opens elevated
over the maximised map sheet, and because a dialog registered no layer, a Back
aimed at the dialog reached the SHEET's listener as topmost, closing the map
underneath and leaving the dialog floating. Both kinds now share the one layer
stack. Back behaves as Escape does, so a dialog that has turned keyboard
dismissal off opts out of both.

7. Kit components for what four sheets and three lists had each written out

rac/full-screen-sheet.tsx now carries SheetHeader / SheetBody /
SheetFooter; rac/grid-list.tsx carries RowContent. Two findings came out of
doing it, both the opposite way round from how they looked in review:

  • The safe-area padding was backwards. A FullScreenSheet already spends
    the inset on its own p-safe, so px-gutter-safe INSIDE one counts the notch
    twice (max(1rem, inset) + inset) and pb-gutter-safe gives
    1rem + 2·inset. The older sheets' plain px-4 was right; the two I added
    last week were the ones over-padding in landscape. Gotcha Bun #25 now says so.
    What the older pair really lacked is min-h-0 — without it a flex item's
    implicit min-height: auto lets the body push the header off screen — and the
    max-w-2xl cap the newer pair dropped, which is the whole reason waypoint
    fields stretched across a tablet while turnpoint ones did not.
  • The row container was already shared. variant="rows" is navRowClass,
    which is flex items-center gap-3; two of the three lists were merely
    restating it, which is part of why three different row bodies looked like one
    shape. What was genuinely duplicated is the min-w-0 flex-1 middle block, the
    truncating detail line and the chevron.

RowContent truncates its detail line, which the review's rows did not, so it
takes untruncated children underneath for a sentence the reader has to be able
to finish — "Too far apart for terrain — check the coordinates" clipped at "Too
far apart for te…" would tell them less than nothing. chevron is off by
default for the same kind of reason: a read-only waypoint row and a turnpoint row
in reorder mode both open nothing and must not claim otherwise.

The autoFocus accessibility note that stood verbatim above five buttons is one
copy now, on FullScreenSheet, with pointers to it.

Not in scope

The route editor keeps its own fill-for-blanks and does not get the full
review: it is a 10–20 turnpoint list with no list to review against. A task
built with a wrong goal altitude still has to be corrected there — that is the
one scored quantity a waypoint altitude feeds (§13.4.6 stopped-task altitude
bonus, resolveGoalAltitude).

Left undone deliberately: use-unsaved-changes-guard is beforeunload plus
an anchor-click interceptor, neither of which sees a popstate — so on the page
this PR taught to treat Back as first-class navigation, Back with unsaved
waypoints still discards them without a word. The gap predates this PR, but it
deserves fixing. It is an app-wide navigation change affecting every dirty form,
and it has two traps a quick implementation would hit: the confirm dialog now
pushes a history entry of its own, and its promise resolves in a microtask that
beats use-back-dismiss's deferred release, so anything navigating in the
.then orphans that entry. Worth its own change rather than a rider on this
one.

Verification

  • bun run test:all — 1641 engine/scripts, 836 frontend, 773 competition-api,
    108 auth-api, every typecheck. Green.
  • bun run test:e2e197 passed, 7 skipped, both Playwright projects
    (chromium desktop and mobile Pixel 7). comp-waypoints.spec.ts runs in
    both and asserts the page never scrolls sideways, at either width, for an
    admin and for a visitor.
  • bun run test:e2e:ssr — 42/42, including "the waypoints page lists the comp's
    shared waypoints" with JS off and no hydration mismatch on :waypoints, which
    is the risk in giving the read-only list a useUnits() call.
  • bun run check:scoring-note — not required, no scoring source changed.

New tests worth naming: the feet conversion and -777; cross-format altitude
agreement; "a missing elevation reads as undefined, never 0" per format; the
terrain decoder against a partially transparent pixel at every alpha, against
the four recorded Mapbox tiles, and against an image whose five rows each use a
different PNG row filter (so a fault in Paeth fails as Paeth); the full
verdict/threshold/sort/pattern logic in altitude-check.test.ts; both entry
points to the waypoint→turnpoint conversion keeping a sea-level 0; and e2e for
the frozen review order, Back walking one layer at a time through a sheet and
through a dialog, and typing a coordinate a character at a time without losing
the comparison.

Docs

docs/waypoint-altitudes.md (units per format, the zero rule, how the review
works, the terrain decode fault, and what it deliberately does not reach),
linked from the CLAUDE.md "Where things live" table. CLAUDE.md gains the
mobile-first rule with its reasoning, the zero-altitude rule, the Terrain-RGB
canvas prohibition, and the kit-components rule. The RAC adoption guide gains
gotchas #25 (safe-area double count) and #26 (variant="rows" already lays a
row out).

Two things to confirm

The review's copy is new UI wording: the button is "Check altitudes"; the
summary reads "N of M waypoints to look at" (or "Every altitude that could
be checked agrees with the map to within 50 m"); the bulk actions are "Use the
map's altitude for all N"
, "Show all N waypoints" / "Show only the N to
look at"
, "Convert all from feet" and "Done"; a row's verdict lines are
"Not your file: an old terrain-read fault", "Too far apart for terrain —
check the coordinates"
, "The file has no altitude for this waypoint" and
"No terrain reading here".

One copy change made without proposing it first, because it came along with
the reuse rather than as a rewording: a read-only waypoint row with no altitude
now reads "no altitude" where the old table printed "—". I kept the
list's vocabulary because the zero/absent distinction is the point of the whole
PR, but it is public-facing copy and easy to change back.

Also worth an eye on the preview: standardising the sheet chrome brought the
waypoint and altitude-review sheet titles DOWN from text-lg font-bold to the
text-base font-semibold the turnpoint sheets use, and gave every sheet body the
max-w-2xl cap — so on a wide screen the waypoint fields are narrower than they
were. Deliberate, and easy to flip back.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W

…in the grid

Three related changes to the waypoints editor, all about one thing: an
altitude the reader can check.

**A zero altitude is never "missing".** `WaypointFileRecord.altitude` is now
optional: an altitude is either KNOWN (any number, 0 included) or ABSENT,
because a waypoint on a beach is at 0 m and a file with no elevation column
knows nothing. Collapsing the two meant a sea-level waypoint could never be
left alone — "Fill altitudes from map" kept offering to fill it and filling it
wrote 0 again — and it left the waypoints grid and the route editor's
turnpoint sheet disagreeing about what a 0 in the same column meant. Parsers
now leave an absent altitude undefined, exporters use their format's own way
of saying nothing (OziExplorer's -777, a blank `elev`, an omitted `<ele>`),
and only a blank cell is "missing". A 0 that is WRONG is a wrong altitude for
the new check to report, not something to overwrite unasked.

**OziExplorer .wpt states elevation in FEET** (field 14), and we read it as
metres, inflating every imported altitude by 3.28. The bundled HG Worlds 2026
set proves it: the organiser published the same points in four formats, and
BORDANO LANDING is 225 in the FS file, 225.000000 in the CompeGPS one and 738
here — 738 ft is 224.9 m. LIENZ LANDING reads 2234, which is 681 m, and Lienz
sits at about 673 m. A cross-format altitude agreement test now holds this
down the way the coordinate one already did. Both bundled sample comps are the
CompeGPS dialect and state metres, so no seeded comp's altitudes changed, and
no already-scored task moves: a task freezes its own xctsk when it is built.

**"Check altitudes" reviews the whole set against the map's terrain.** It is
the other half of the fill: the fill answers blanks and cannot be wrong, so it
applies itself; the check questions values already there, so it changes
nothing on its own. The review happens IN THE GRID, which already has the map,
the locate pin, the filter box and an editable altitude cell — because a large
disagreement usually means a wrong COORDINATE, and accepting the terrain there
would make the waypoint look fixed while leaving it in the wrong valley.

  - Two derived columns beside Alt (m): the map's reading, and a signed Δ
    computed on every redraw rather than stored, sorted by how big the
    disagreement is.
  - Under 50 m is not a finding — a ~10 m DEM pixel against a file rounded to
    10 m (Corryong encodes the altitude in the code: 4C-080 is 800 m). The
    agreeing rows are counted in the banner and left out of the list: the
    difference between "12 to look at" and "145 differ".
  - Past 300 m the coordinates are the likelier fault, marked with a visible
    "!" so the warning never lives in the colour alone.
  - A whole-file ratio or offset is named before the per-row work starts, with
    one bulk conversion for a set of feet — not the same decision 187 times.
  - The list is a snapshot, so a row does not vanish under the cursor when its
    tick is pressed; the banner's counts do the counting.
  - Accepting is an ordinary unsaved grid edit, so Save and the existing
    discard-on-leave guard are the commit and the undo. The banner says that
    tasks already built keep their own copy of a waypoint.

Also fixed in passing: the grid's display order could rewrite the SAVED
waypoint order. Tabulator's getData() returns rows in display order, so any
header sort would reorder the stored set on the next cell edit; syncFromGrid
now sorts by row id, which ascends in file order.

Docs: docs/waypoint-altitudes.md, plus the standing rules in CLAUDE.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
Big_Hill, a Great Ocean Road waypoint a few metres from the water, came back
from the altitude check as 13273 m — higher than Everest — and the editor
offered to write it into the waypoint file.

Root cause: a Terrain-RGB pixel is a 24-bit NUMBER spread over three bytes
(-10000 + (R*65536 + G*256 + B) * 0.1), and the tiles are RGBA — Mapbox marks
no-data, the sea past a coastline included, with partial alpha. A canvas stores
premultiplied 8-bit RGBA, so drawing such a pixel and reading it back does not
round-trip: the payload is multiplied by alpha going in and divided by it
coming out, and only the nearest byte survives. The red byte carries 6553.6 m
per step, so one step of that error is kilometres. Measured in a browser, for a
166 m coastal hill:

  alpha 255 -> 166 m      alpha 100 -> -6413 m
  alpha 200 -> 192 m      alpha  85 -> -6388 m
  alpha 128 -> 6720 m     alpha   0 -> -10000 m

`premultiplyAlpha: 'none'` does not help: the loss is in the canvas, not the
decoder. So elevation.ts now inflates and unfilters the PNG itself — exact by
construction, with no canvas, no premultiplication and no colour management in
the path. It also drops the OffscreenCanvas requirement, and makes this code
unit-testable for the first time.

Two guards behind it, because a wrong elevation must never be presentable as a
right one:

  - A plausibility range, -500 m (the Dead Sea shore, which is flown) to
    9000 m. Outside it the point reads as "could not be read from the map"
    rather than as a number. A backstop, not the fix: 6720 m is also an
    ordinary Himalayan summit, so no range check could have rejected that one.
  - A 3x3 median of the plausible pixels. One bad pixel cannot become the
    answer, and a waypoint on a cliff edge — where a ~10 m grid is least
    stable, and where every coastal waypoint sits — is read from its own ground
    rather than from whichever side of the escarpment the nearest pixel landed
    on. Clamped to the tile rather than fetching a second one for two samples.

This predates the altitude check: "Fill altitudes from map" has been writing
these values into waypoint files all along. The check is what made it visible.

Why the suite was green while the real site was wrong: the e2e fixture standing
in for a terrain tile wrote colour type 2, with no alpha channel at all, so it
could not reproduce the failure — and the four recorded Mapbox tiles are all
inland and fully opaque. The fixture now writes RGBA with a settable alpha, and
the waypoints spec drives the review through a partially transparent tile.

Tests: the PNG decode at every alpha (the case a canvas destroys), the range
and its limit, the median against a corrupt pixel and against a cliff edge, and
the four real recorded Mapbox tiles decoded and range-checked against an
independent node:zlib decode of the same files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W

pokle commented Sep 18, 2026

Copy link
Copy Markdown
Owner Author

A third fix: terrain tiles were being read through a canvas

Testing on the preview turned up Big_Hill — a Great Ocean Road waypoint a few metres from the water — reading 13273 m, higher than Everest, with the editor offering to write it into the waypoint file.

Root cause. A Terrain-RGB pixel is a 24-bit number spread over three bytes (-10000 + (R*65536 + G*256 + B) * 0.1), and the tiles are RGBA — Mapbox marks no-data, the sea past a coastline included, with partial alpha. A canvas stores premultiplied 8-bit RGBA, so drawing such a pixel and reading it back does not round-trip: the payload is multiplied by alpha going in and divided by it coming out, and only the nearest byte survives. The red byte carries 6553.6 m per step, so one step of that error is kilometres. Measured in a browser for a 166 m coastal hill:

alpha read back off a canvas
255 166 m
200 192 m
128 6720 m
100 −6413 m
0 −10000 m

premultiplyAlpha: 'none' does not help — the loss is in the canvas, not the decoder.

Fix. analysis/elevation.ts now inflates and unfilters the PNG itself: exact by construction, with no canvas, no premultiplication and no colour management in the path. It also drops the OffscreenCanvas requirement, and makes this code unit-testable for the first time.

Two guards behind it, because a wrong elevation must never be presentable as a right one:

  • A plausibility range, −500 m (the Dead Sea shore, which is flown) to 9000 m; outside it the point reads as "could not be read from the map" rather than as a number. A backstop, not the fix — 6720 m is also an ordinary Himalayan summit, so no range check could have rejected that one.
  • A 3×3 median of the plausible pixels. One bad pixel cannot become the answer, and a waypoint on a cliff edge — where a ~10 m grid is least stable, and where every coastal waypoint sits — is read from its own ground rather than from whichever side of the escarpment the nearest pixel landed on.

This predates the check. "Fill altitudes from map" has been writing these values into waypoint files all along; the check is only what made it visible. Worth knowing if any comp's altitudes were filled that way.

Why the suite was green while the site was wrong — and this is the part I'd want a reviewer to look hardest at: the e2e fixture standing in for a terrain tile wrote colour type 2, with no alpha channel at all, so it could not reproduce the failure; and the four recorded Mapbox tiles are all inland and fully opaque. The fixture now writes RGBA with a settable alpha, and the waypoints spec drives the review through a partially transparent tile — a test that fails against the old canvas code.

New tests: the decode at every alpha, the range and its limit, the median against a corrupt pixel and against a cliff edge, and the four real recorded Mapbox tiles decoded and range-checked against an independent node:zlib decode of the same files. Full battery green, plus analysis-map.spec.ts (which decodes real recorded tiles through the browser) and the SSR suite.


Generated by Claude Code

The decoder fix stopped the bad reads; it did not retract the ones already
written. "Fill altitudes from map" and any accepted map altitude saved those
numbers into waypoint files, so a comp can still hold them.

On the Great Ocean Road comp, Point_Addis_Hill holds 6660 m over terrain that
now reads 80 m. That is not a mystery: 80 m read through the old canvas path at
partial alpha comes out at 6659.2 m (measured in a browser), so the stored
value is exactly what the bug produced there. The review was already flagging
it correctly — the Map column reads 80 and the delta reads +6580 — but it was
blaming the coordinates, which in this case are fine.

Such an altitude is recognisable rather than merely wrong, so the review now
says which of the two numbers is at fault. looksLikeCorruptedTerrainRead()
asks whether the disagreement is a whole number of red-byte steps (6553.6 m,
because a Terrain-RGB pixel is -10000 + (R*65536 + G*256 + B) * 0.1) to within
60 m — the residual being only the difference between the single pixel the old
code read and the 3x3 median read now. Nothing anyone types is out by 6554 m to
within 60 m, and a file in feet is out by a FACTOR rather than a step, so the
two findings cannot collide.

Where it fires the banner says the number is not theirs and that taking the
map's altitude corrects it, and the row's tooltip stops pointing at the
coordinates.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W

pokle commented Sep 18, 2026

Copy link
Copy Markdown
Owner Author

The decoder fix worked; this is the data it already wrote

A second impossible number turned up on the preview — Point_Addis_Hill — but it is not the same bug, and the direction matters: Map (m) reads 80 m, which is right for Point Addis, and the delta reads +6580. So the impossible value (≈6660 m) is the waypoint file's own stored altitude, not the map's.

Where it came from: 80 m read through the old canvas path at partial alpha comes out at 6659.2 m (measured in a browser, sweeping true elevation × alpha). The bug wrote that into the file via "Fill altitudes from map" before the fix, and fixing the decoder does not retract it.

So the review was already doing its job — it just blamed the wrong side. A coordinate error was the stated suspicion, and here the coordinates are fine.

What this adds. Such an altitude is recognisable rather than merely wrong. A Terrain-RGB pixel is -10000 + (R*65536 + G*256 + B) * 0.1, so the red byte is 6553.6 m a step; looksLikeCorruptedTerrainRead() asks whether the disagreement is a whole number of those steps to within 60 m — the residual being only the difference between the single pixel the old code read and the 3×3 median read now. Both real cases fit: Point_Addis one step (6580), Big_Hill two (13273 − 166 = 13107).

It cannot be confused with the other findings: nothing anyone types is out by 6554 m to within 60 m, and a file in feet is out by a factor rather than a step. Where it fires, the banner says the number isn't theirs and that taking the map's altitude corrects it, and the row's Δ tooltip stops pointing at the coordinates.

Worth knowing for production: any comp whose altitudes were filled from the map before this PR may hold these values, and coastal comps are the likely ones — partial alpha is how Mapbox marks the sea. "Check altitudes" now finds and names them, and "Use the map's altitude for all shown" corrects them.

New copy for review, since it's user-facing:

N altitudes are out by almost exactly 6553.6 m, or a multiple of it. That is not anything in your waypoint file: it is the signature of a terrain reading taken before a decoding fault was fixed, and taking the map's altitude now corrects it.

Full battery green (824 frontend tests, 1640 engine/scripts, 773 competition-api), comp-waypoints e2e 10/10.


Generated by Claude Code

The page had one editor — a 145-row, eight-column Tabulator grid — and a
review bolted into it as three more columns. On a phone that grid scrolls
sideways inside a page that scrolls down, under a map pane that sticks, and its
frozen Code column hides whichever column is next to it. The review's columns
sat right there, so reaching them scrolled the waypoint's own Alt (m) out of
sight: a disagreement read "+6580" with the map's 80 m visible and the file's
6660 m hidden, which is exactly backwards from what the reader needed. Add a
banner, three buttons and a filter box above a 420 px grid and there was room
for three rows of the twelve.

**The altitude review is now a sheet, at every width** — one FullScreenSheet
with a list view and a per-waypoint view, swapped in place rather than stacked,
so there is one focus trap, one Escape and one back gesture. Every row states
BOTH altitudes and names their unit. No map in the per-waypoint view by
decision: the page owns a single Mapbox instance and hands it between the
inline pane and the full-screen map, and a third claimant would be a second
instance; the coordinates are editable as text instead, which is what a pasted
correction needs. Back is the way out, and back IS "leave it as it is", so
there is no third button saying so.

That retired the three grid columns and their machinery: hidden columns that
showColumn does not re-render, a Δ derived from two other cells that Tabulator
never redraws, setSort racing the effect that showed the column, and the
suspect filter sharing one predicate with the search box. The terrain readings
moved off the row into a map keyed by row id — they are not part of a waypoint
and are never saved, so the grid needs no column for them and syncFromGrid
cannot lose them.

**The waypoint editor is now a list of sheets below 64rem.** A row carries
everything a waypoint has on two lines, with the altitude ON the row; tapping
it opens every field, a locate-on-map, the radius chips and Remove. Draft
applied on the way out, like the route editor's turnpoint sheet, because the
page repaints its map markers from `rows` on every change. The Tabulator grid
stays as the wide-screen editor (editable tables are Tabulator by policy, and
for bulk cell editing it is the right tool).

Found by the new e2e, and worth calling out: freezing the review list's
MEMBERSHIP was not enough. The order was still live, so accepting a row sent it
to the bottom the instant its disagreement became zero — the same "vanishing
under the thumb" problem in a different guise. The order is snapshotted too.

e2e: comp-waypoints.spec.ts joins the mobile project, and its tests go through
a driver that speaks to whichever editor is on screen, so they mean the same
thing in both. Tests that are ABOUT one editor skip in the other project and
say why. 21 passed, 3 skipped across both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
The review is a sheet now, and the page has two editors chosen by width, so
docs/waypoint-altitudes.md and the CLAUDE.md rule both described something that
no longer exists.

Records the two findings worth keeping: both altitudes have to be on the row
(the grid columns put the file's behind the frozen Code column, so a
disagreement was reported with half of itself off screen), and the review list
is a snapshot in ORDER as well as membership — freezing only the membership
still moved the row under the reader's thumb the moment they accepted it,
because the sort is by the size of the disagreement.

Also notes that number fields commit on blur rather than per keystroke, and
that the e2e asserts the type-then-tap-Done path specifically, because losing
that value would be silent.

SSR suite re-run: 42/42.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
Mobile first is now the stance, and CLAUDE.md says so: a competition is run
from a hill and nobody wants to carry a laptop up it, so a phone is the shape a
surface is designed FOR rather than a width it has to survive. Where the two
conflict the phone wins. That revises the Tabulator policy rather than deleting
it — Tabulator is still right for a genuinely spreadsheet-shaped grid like the
pilots one, but it is no longer the default answer for "an editable
collection", and being the default is how this page ended up with a grid
nobody could use from a hill.

**The Tabulator grid is gone, not hidden behind a breakpoint.** Two editors
meant two code paths and a reviewer on a desktop seeing something the author
never tested on a phone. `/comp/:id/waypoints` is now a list of waypoints whose
rows open full-screen sheets at every width, and React `rows` state is the only
copy of the set — no mirror to keep in step, no display order that could
rewrite the saved order, and no `innerHTML` left anywhere under src/react/
(the html-sinks baseline drops its last entry).

**One way to do a thing.** Removed "Fill altitudes from map", which answered
blanks without asking and which `Check altitudes` does better by showing what
it would write first; the status line beside it, which passed a verdict on a
set nobody had asked about yet; and "Show on the map" in the waypoint sheet,
which duplicated the pin on the row that opened it.

**Back closes one sheet, not the page.** The sheets are React state rather than
routes — the page behind them is unsaved work, so a sibling route would unmount
it and the unsaved-changes guard would prompt on the way in — which left them
invisible to the history stack. `lib/use-back-dismiss.ts` gives every
FullScreenSheet a history entry while it is open and pops it again on the way
out, so Back walks a detail view → its list → the page, and closing from the UI
leaves the stack as it was found.

Three things broke that hook, and the e2e caught all three rather than review:
a popstate reaches EVERY listener, so nesting needs a layer stack; popping our
own entry looks like a user Back to the layer underneath, so a programmatic pop
is announced and every listener sits it out (cleared in a microtask, because a
timer raced the browser's delivery of the event); and StrictMode runs the
effect twice, so the first run's cleanup was releasing the entry the second run
needed — every sheet closed itself the instant it opened.

Smaller, all as asked: a chevron on the editor rows (the review's rows already
had one), a difference of exactly zero prints nothing rather than "0 m", and
inside a waypoint in the review BOTH Back and Done return to the list. Done
used to close the whole sheet from there, which dropped the reader out to the
waypoints page half way down a list of twelve.

e2e: 23 passed across both projects, including Back walking the layers and an
assertion that the page never scrolls sideways. The flake this turned up is
worth knowing: the list is keyed by row id and the page rebuilds those ids when
the fetch lands, so a test whose first act is a CLICK rather than an
auto-retrying assertion spent its timeout on "element was detached from the
DOM" — beforeEach now waits for the whole set to be listed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
@pokle
pokle marked this pull request as ready for review September 19, 2026 01:05
The altitude review's detail view wrote every keystroke straight into the
page, and the page forgets a waypoint's terrain reading whenever its
coordinates change — the reading belonged to where the waypoint was. So
the first character typed took `mapAlt` away: "From the map" fell to "—",
the difference line vanished and the accept button unmounted, mid-paste,
destroying the very comparison the view exists to show.

The field is a draft now, committed once on the way out. The commit hangs
off unmount rather than off a button, so it covers every exit there is:
Back, Done, the accept button, a browser Back, Escape, and the sheet
closing. The altitude field stays live on purpose — it is one of the two
numbers being compared, so the difference recounting as you type is the
feedback, and it blanks the moment the two agree.

Three headers still described the editor the mobile-first rule replaced:
WaypointList called itself the phone stand-in for a wide-screen Tabulator
grid, e2e/fixtures/mobile.ts said the page had two editors chosen by
width, and the spec's own header described the grid's virtual rows. A
reader hitting any of them first would go looking for a grid that is not
there.

Also adds turnpoint-draft.test.ts, which holds both entry points to the
one waypoint→turnpoint conversion to carrying a sea-level 0 across and
blanking only an absent altitude, and an e2e that types a coordinate a
character at a time and asserts the comparison stays put.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
A list was reaching into a sheet for a pure formatter: WaypointList
imported radiusLabel from WaypointSheet. It belongs beside the
formatCylinderRadius it wraps.

The terrain decoder chose its row filter inside the byte loop, which
re-decided it a million times a tile and read as though the filter could
change mid-row. It is hoisted, and an unknown one is now rejected when
the row starts rather than at its first byte.

Filters 1-4 were only ever covered in aggregate, by the recorded Mapbox
tiles whose decoded range would move. Two tests name them directly: one
image whose five rows each use a different filter type over a pattern
that varies both ways, so a fault in Paeth fails as Paeth; and a row
claiming a filter that does not exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
The editor stopped being a sideways-scrolling table on 2026-09-19. The
read-only view did not: it was a six-column RAC Table inside an
overflow-x-auto scroll region, so the shape the editor had just given up
survived for the anonymous PILOT — the one actually standing on the hill
holding a phone — while the organiser more likely to be at a desk got the
list. Fixing the admin view was half the job.

WaypointList with no `onOpen` IS the read-only mode: no chevron, no row
action, the locate pin kept, because a visitor gets the map as well. The
one thing the two modes do not share is the altitude's unit — the editor
is the waypoint FILE edited in place and stays metric, while a read-only
row is an altitude printed to a reader and honours the preference like
every other altitude in the app (issue #662).

That deletes sortRows, numField, the sort state and the SortDescriptor
import: the table's sortable columns are the cost, and a sort control
over the list can bring them back if anyone misses them. The filter box
above it is unchanged and serves both.

The sideways-scroll assertion is now a helper used by the visitor's test
as well as the admin's, since a visitor's table was the one still in a
scroll region.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
use-back-dismiss was wired into FullScreenSheet and nowhere else, but
twelve files import rac/dialog. So on a phone Back closed a sheet and
left the PAGE from a dialog — and a back gesture is how people close
things.

The gap had teeth beyond the inconsistency. The waypoints page's
Add-waypoint dialog opens `elevated` over the maximised map sheet,
which is a pairing the `elevated` prop exists for; because a dialog
registered no layer, a Back press aimed at the dialog reached the
SHEET's listener as the topmost one, closing the map underneath and
leaving the dialog floating over the page.

A dialog now takes a history entry for as long as it is open, from the
same module-level stack the sheets use, so layers interleave in the
order they opened whichever kind they are. It is a component inside the
overlay rather than a hook call in Modal, because Modal itself stays
mounted across open and closed while RAC renders overlay children only
while open — sitting inside the overlay is what makes "while it is open"
true. It closes through OverlayTriggerStateContext, which resolves the
same whether the dialog is controlled by isOpen/onOpenChange or opened
by a DialogTrigger, so no caller passes anything.

Back behaves as Escape does, so a dialog that has turned keyboard
dismissal off opts out of both.

The e2e also asserts the stack is left as it was found when the dialog
closes from the UI: an entry pushed and not consumed makes the next Back
appear to do nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
Four sheets had each grown their own title bar and body, in two
vocabularies, and three lists had each written out the same row. None of
the differences was decided; all of it was copied.

rac/full-screen-sheet.tsx now carries SheetHeader / SheetBody /
SheetFooter, and rac/grid-list.tsx carries RowContent. Two findings came
out of doing it, both the opposite way round from how they looked.

The safe-area padding was backwards. A FullScreenSheet already spends
the inset on its own `p-safe` — the RAC guide says so — so
`px-gutter-safe` INSIDE one counts the notch twice
(`max(1rem, inset) + inset`), and `pb-gutter-safe` gives
`1rem + 2·inset`. The older pair's plain `px-4` was right and the newer
pair, added last week, was the one over-padding in landscape. The shared
chrome is plain Tailwind, and gotcha #25 says why. What the older pair
really lacked is `min-h-0`, without which a flex item's implicit
`min-height: auto` lets the body push the header off screen, and the
`max-w-2xl` cap the newer pair dropped — which is the whole reason
waypoint fields stretched across a tablet while turnpoint ones did not.

And the row container was already shared: `variant="rows"` is
`navRowClass`, which is `flex items-center gap-3` — two of the three
lists were merely restating it, which is part of why three different row
bodies looked like one shape. What was genuinely duplicated is the
`min-w-0 flex-1` middle block, the truncating detail line and the
chevron, so that is what RowContent owns. It is the row's CONTENT rather
than the row, because GridListItem carries the id, the text value and
what pressing the row does — and one list deliberately has rows that do
nothing.

RowContent truncates its detail line, which the altitude review's rows
did not, so it takes untruncated children underneath for a sentence the
reader has to be able to finish: "Too far apart for terrain — check the
coordinates" clipped at "Too far apart for te…" would tell them less
than nothing. `chevron` is off by default for the same kind of reason —
a read-only waypoint row and a turnpoint row in reorder mode both open
nothing, and must not claim otherwise.

The `autoFocus` note that stood verbatim above five buttons is one copy
now, on FullScreenSheet, with four pointers to it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
@pokle pokle changed the title Waypoint altitudes: zero is never missing, OziExplorer is feet, and a review in the grid Waypoint altitudes: zero is never missing, OziExplorer is feet, and a waypoints page you can use from a hill Sep 19, 2026
The pin was a 28x28 px icon with 300 px of inert row beside it, which is
a poor thing to ask a thumb to find on a hill.

It is now the row's whole left-hand strip — 48x44 px, flush to the
card's edge — from `self-stretch` plus `-my-2.5 -ml-4` cancelling
navRowClass's own padding, with its own `px-4` putting the icon back
where it was, so the row's text moves right by 4 px and no more. In the
editor the row opens the waypoint's sheet, so the pin is a genuinely
separate action and needs a target of its own; RAC does not fire a row's
action when a focusable child of it is pressed, which is what keeps the
two apart.

Read-only there is no sheet behind a row, so the WHOLE ROW flies the map
instead, guarded on the coordinates parsing for the same reason the pin
is disabled without them. The pin stays there because it is what NAMES
the action for a reader who cannot see the map move; pressing it and
pressing the row do the same one thing, so this is one action with a big
target rather than two ways to do it.

The first attempt at the strip was silently broken and is worth
recording. It kept `size="icon-sm"` and overrode it with `h-auto w-12`,
but tailwind-merge does not treat `size-*` as conflicting with
`h-*`/`w-*`: `size-7` survived alongside `w-12`, leaving two competing
rules in the stylesheet with CSS source order deciding the winner. The
default size's `h-9` does merge away, so the button passes no `size` at
all — and the e2e measures the RENDERED box against 44 px rather than
asserting anything about classes, because a `size` prop added later in
good faith would put the target back to 28 px with every class-level
check still green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HWo1S395Amwxxc5npG4E3W
@github-actions

Copy link
Copy Markdown

Preview Deployment
https://ad7e4790.glidecomp.pages.dev
Commit: 9c2188e

@pokle
pokle merged commit 859ca0f into master Sep 19, 2026
6 checks passed
@pokle
pokle deleted the claude/waypoints-altitude-validation-y373oh branch September 19, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants