conform to draft-02: altitude in metres, checksum rewrite, strict decode #1

Open

hugooconnor wants to merge 0 commits from checksum-and-altitude-conformance into main

hugooconnor commented

2026-07-10 03:46:55 +00:00

(Migrated from codeberg.org)

Brings this crate into conformance with draft-saywhere-geocoding-02. Companion to anuna/saywhere#21, which contains the spec change.

Three divergences, in descending severity

Altitude was silently wrong

This crate still implemented draft-01's altitude = units × 3.0 m step model, so grape.column.hip.toad:60 decoded to 180 metres rather than the 60 the spec means. No error, no warning — just a wrong altitude.

Draft-02 fixes altitude to signed metres above the WGS-84 ellipsoid per RFC 5870, and defines no alternative units.

The decoder failed open

On an invalid checksum it fell through and re-read the checksum word as a location word, instead of raising the error the spec requires — even though its own doc comment already claimed it errored. The comment justifying the fall-through ("many CHECKSUM-32 words also appear in BIP-39") was never true; zero of them do.

It also validated phrase length before stripping the checksum, so a valid 6-location-word phrase plus checksum (7 tokens) was rejected as too long — exactly the case the spec added a warning for.

The checksum vocabulary was stale

It disagreed with the spec and the Guile reference at 13 of 32 indices. The two implementations emitted different phrases for identical coordinates and rejected each other's checksummed phrases. Existing tests missed it because NYC lands on index 30, which happened to be toad in both lists.

Adopted design

Disjoint CHECKSUM_CRC_32 and CHECKSUM_ADD_31 vocabularies — the trailing word selects the method, so exactly one checksum is computed rather than either being accepted — and a position-weighted additive checksum reduced modulo the prime 31, so detection is uniform (~3.2%) across error classes and word positions.

Breaking API change

The crate is 0.1.0.

encode_altitude / decode_altitude removed — they existed only to implement the step model
encode_full loses its altitude_step parameter
DecodeResult.altitude_units: Option<i32> → altitude_metres: Option<f64>
AltitudeUnitsOutOfRange → InvalidAltitudeFormat
new: parse_altitude_metres, format_altitude_metres, get_additive_checksum_word, expected_checksum_word

Migration: any stored :N altitude changes meaning. An old :20 meant 60 m; it now means 20 m.

Tests

151 pass. New tests pin:

the spec's worked CRC-8 and additive vectors
the prefix-disjointness rule that replaces BIP-39's four-letter rule
coprimality of every weight and weight difference with the modulus — this assertion fails for any power-of-two modulus, so the gcd(4,32)=4 bug cannot return silently
the ~1-in-31 residual false negative the spec documents (column.grape.hip collides with grape.column.hip)
6 location words + checksum (7 tokens) decoding correctly
invalid checksum raising TerminalChecksumFailed rather than falling through

Verified byte-identical to the Guile reference across 144 phrases (24 locations × 6 precisions) under both checksum methods.

Clippy: no new warnings (12 pre-existing style lints in test code, untouched).

🤖 Generated with Claude Code

https://claude.ai/code/session_017R8AwqxwAq8Av8GMZvLEPT

Brings this crate into conformance with `draft-saywhere-geocoding-02`. Companion to `anuna/saywhere#21`, which contains the spec change. ## Three divergences, in descending severity ### Altitude was silently wrong This crate still implemented draft-01's `altitude = units × 3.0 m` step model, so `grape.column.hip.toad:60` decoded to **180 metres** rather than the 60 the spec means. No error, no warning — just a wrong altitude. Draft-02 fixes altitude to signed metres above the WGS-84 ellipsoid per RFC 5870, and defines no alternative units. ### The decoder failed open On an invalid checksum it fell through and re-read the checksum word as a *location* word, instead of raising the error the spec requires — even though its own doc comment already claimed it errored. The comment justifying the fall-through ("many CHECKSUM-32 words also appear in BIP-39") was never true; **zero** of them do. It also validated phrase length *before* stripping the checksum, so a valid 6-location-word phrase plus checksum (7 tokens) was rejected as too long — exactly the case the spec added a warning for. ### The checksum vocabulary was stale It disagreed with the spec and the Guile reference at **13 of 32 indices**. The two implementations emitted different phrases for identical coordinates and rejected each other's checksummed phrases. Existing tests missed it because NYC lands on index 30, which happened to be `toad` in *both* lists. ## Adopted design Disjoint `CHECKSUM_CRC_32` and `CHECKSUM_ADD_31` vocabularies — the trailing word selects the method, so exactly one checksum is computed rather than either being accepted — and a position-weighted additive checksum reduced modulo the prime **31**, so detection is uniform (~3.2%) across error classes and word positions. ## Breaking API change The crate is `0.1.0`. - `encode_altitude` / `decode_altitude` removed — they existed only to implement the step model - `encode_full` loses its `altitude_step` parameter - `DecodeResult.altitude_units: Option<i32>` → `altitude_metres: Option<f64>` - `AltitudeUnitsOutOfRange` → `InvalidAltitudeFormat` - new: `parse_altitude_metres`, `format_altitude_metres`, `get_additive_checksum_word`, `expected_checksum_word` **Migration:** any stored `:N` altitude changes meaning. An old `:20` meant 60 m; it now means 20 m. ## Tests **151 pass.** New tests pin: - the spec's worked CRC-8 and additive vectors - the prefix-disjointness rule that replaces BIP-39's four-letter rule - **coprimality of every weight and weight difference with the modulus** — this assertion fails for any power-of-two modulus, so the `gcd(4,32)=4` bug cannot return silently - the ~1-in-31 residual false negative the spec documents (`column.grape.hip` collides with `grape.column.hip`) - 6 location words + checksum (7 tokens) decoding correctly - invalid checksum raising `TerminalChecksumFailed` rather than falling through Verified **byte-identical to the Guile reference** across 144 phrases (24 locations × 6 precisions) under both checksum methods. Clippy: no new warnings (12 pre-existing style lints in test code, untouched). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_017R8AwqxwAq8Av8GMZvLEPT

This branch is already included in the target branch. There is nothing to merge.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin checksum-and-altitude-conformance:checksum-and-altitude-conformance

git switch checksum-and-altitude-conformance

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main

git merge --no-ff checksum-and-altitude-conformance

git switch checksum-and-altitude-conformance

git rebase main

git switch main

git merge --ff-only checksum-and-altitude-conformance

git switch checksum-and-altitude-conformance

git rebase main

git switch main

git merge --no-ff checksum-and-altitude-conformance

git switch main

git merge --squash checksum-and-altitude-conformance

git switch main

git merge --ff-only checksum-and-altitude-conformance

git switch main

git merge checksum-and-altitude-conformance

git push origin main

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

anuna-research/saywhere-rs!1

No description provided.

Rows
Columns