v1.0 specification & conventions page (issue #62)#118
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
=======================================
Coverage 94.33% 94.33%
=======================================
Files 9 9
Lines 1307 1307
=======================================
Hits 1233 1233
Misses 74 74
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 20.07%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | fast_norm2mort_orders[6] |
327.8 ns | 269.4 ns | +21.65% |
| ⚡ | fast_norm2mort_orders[10] |
373.6 ns | 315.3 ns | +18.5% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/62-spec-page (231106b) with main (41d557a)
Footnotes
-
1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩
| `[start, end)`; readers never parse semantics out of a custom label. The | ||
| charset excludes `_` by construction. | ||
| - The reserved token `all` (§6.4) is **permanently excluded from the window | ||
| label grammar** under every schedule. |
There was a problem hiding this comment.
🤖 from Claude (review)
F1 (major) — all exclusion extends a /3-scoped ratification onto the already-frozen /2 grammar.
This sentence lives in §6.3 (morton-hive/2) and asserts the reserved all token is excluded from the window-label grammar "under every schedule" — and §8 lists "the all reserved token" among the immutable 1.x guarantees. But the ratification tied all to /3 only: it is the structural name of the /3 schedule: none leaf (all.zarr). In a /2 store the schedule: none leaf is the bare {full_id}.zarr; all has no structural role there, and the /2 explicit-list grammar was frozen as opaque [0-9A-Za-z-]{1,32} in issuecomment-4986809092, which admits a label all.
zagg D23 says all is "excluded from the window grammar forever" in the context of the /3 revision — "the window grammar" there is the /3 grammar. Retroactively narrowing the separately-frozen /2 explicit-list grammar is a contract change to /2 that the record does not clearly ratify.
Recommend either (a) scope the exclusion to /3 (where all is a real token), or (b) if espg intends a cross-version reservation, state it as a deliberate cross-version tightening of the /2 grammar and confirm on the thread — don't present it as already-frozen. As written it's an overclaim placed in the frozen-contract list.
There was a problem hiding this comment.
🤖 from Claude
Fixed in c015d84 — scoped the exclusion instead of narrowing the frozen /2 grammar.
§6.3 now states the reservation as forward-going: no store written under any spec version may declare an explicit window labeled all, but this is a constraint on new writers, not a retroactive narrowing of the frozen /2 opaque [0-9A-Za-z-]{1,32} grammar. A pre-existing /2 store that declared an all label stays readable under its frozen grammar — /2 labels are opaque and manifest-resolved, and all has no structural role in /2 (the schedule: none leaf there is the bare {full_id}.zarr, not all.zarr). §6.4 marks the token as structural only in /3 and cross-references §6.3; §8 now reads "the all reserved token (structural in /3; a forward-going window-label exclusion across all spec versions)".
This keeps the espg-ratified "excluded from the window grammar forever" intent (D23, tied to /3) while making the cross-version scoping explicit rather than presenting a retroactive /2 change as already-frozen. The scoping is now on record and explicit — espg can adjust to the harder retroactive form (banning all even in a legacy /2 manifest) if that was the intent.
| exceed 2^53 and would be silently mangled by float-based JSON parsers. | ||
| - Carrier fields (`source`, `generated_at`, optional `time_range`) are | ||
| informative cache metadata; the ranges are a regenerable cache of the | ||
| leaf-stamp truth. |
There was a problem hiding this comment.
🤖 from Claude (review)
F2 (minor) — root-MOC envelope: required order (and spec) fields not enumerated.
§7.3 names the informative carrier fields (source, generated_at, time_range) and the ranges/encoding fields, but the actual serialization (zagg build_root_coverage, COVERAGE_SPEC = "morton-moc/1") emits a required order integer (the shard order) and a spec key in every envelope:
{"spec": "morton-moc/1", "encoding": "ranges", "order": <int>,
"source": ..., "generated_at": ..., "ranges": [...], "time_range"?: [...]}
The order is only implied here ("shard-order coverage", "same-order cells" — recoverable from the decimal endpoints). For a normative serialization a reader shouldn't have to infer the key set: enumerate the required keys (spec, encoding, order, ranges) explicitly, the way §7.2 enumerates the bitmap sidecar's fields. spec is given in the §7 header but not tied to this envelope.
There was a problem hiding this comment.
🤖 from Claude
Fixed in ff7f877 — §7.3 now enumerates the required key set explicitly, the way §7.2 does for the bitmap sidecar. Added a Required keys list: spec ("morton-moc/1"), encoding ("ranges"), order (integer shard order of the listed cells), and ranges; with source, generated_at, and optional time_range called out as informative carrier fields. This matches exactly what zagg build_root_coverage emits (src/zagg/hive.py, envelope dict at the COVERAGE_SPEC/"ranges"/int(order) construction). A reader now validates the set rather than inferring order from the decimal endpoints.
| Informative (the formulas are contract; the rendered values are derived). | ||
| `nside = 2^order`; the cell-scale column is `order2res(order)` = | ||
| `111 × 58.6323 × 0.5^order` km (`mortie.tools.order2res`); the area column is | ||
| the exact HEALPix cell area `4πR² / (12 · 4^order)` with `R = 6371.0088` km. |
There was a problem hiding this comment.
🤖 from Claude (review)
F3 (minor) — the resolution table's two columns use different Earth models, and the area constant has no code source of truth.
The cell scale column is order2res = 111 × 58.6323 × 0.5^order km (an implied sphere of R ≈ 6366 km, 111 km/deg), while the cell area column uses 4πR²/(12·4^order) with R = 6371.0088 km. So the two columns describing the same cells rest on different radii — sqrt(area) and cell scale diverge by ~0.2%. This is the PR's own open Question 2; flagging it as a review item so the resolution is on record.
Related test note: EARTH_RADIUS_KM = 6371.0088 is redefined locally in test_spec_page.py (not imported from mortie), and order2res's radius is a third, different value. The drift pin therefore proves the table is internally consistent with a test-local constant, not that the area column tracks any mortie source of truth — fine as a drift pin, but the area column's authority is the test file itself. If the two-radius split stays, a one-line note in §3 ("cell scale and cell area use different Earth models — see PR discussion") would pre-empt the confusion.
There was a problem hiding this comment.
🤖 from Claude
Fixed the transparency in d09b724; left the constants as the standing question. §3 now carries a note stating both models explicitly: cell scale keeps the historical order2res constant (111 km/deg × 58.6323, implied R ≈ 6366 km), cell area uses the exact HEALPix sphere area at mean radius R = 6371.0088 km, so sqrt(area) and cell scale diverge by ~0.2%. It also records that the drift pin only proves each column is internally consistent with its own constant, not that they share a radius.
Whether to unify the two radii is espg's call — this is the PR's open Question 2, so I left the constants as documented and flagged the divergence rather than picking a radius. If you want them unified, the cleanest path is a single EARTH_RADIUS_KM sourced from mortie (not test-local) feeding both columns.
There was a problem hiding this comment.
🤖 from Claude
Resolved per espg's ruling on this thread's open question: unify both columns on one Earth model — the exact HEALPix sphere R = 6371.0088 km (541f080).
- §3 now derives cell area =
4πR²/(12·4^order)and cell scale =sqrt(area)=sqrt(4πR²/(12·4^order))(the RMS cell spacing) from that single radius. HEALPix is equal-area by construction, sosqrt(area)is an exact per-cell edge scale, not an approximation — and by definition it no longer diverges from the area column. - The drift pin (
test_spec_page.py) now regenerates both columns from the sphere formula; it no longer importsorder2resfor the scale column, andtest_table_covers_full_order_rangestill pins the MAX_ORDER row count. - The code is deliberately not changed in this PR:
mortie.tools.order2reskeeps its historical111 × 58.6323constant (R ≈ 6366 km) because it is consumed behaviorally —res2displayand the coverage buffer-pad intests/test_coverage_boundary.py:78(pad_cells * order2res(order) / KM_PER_DEG), andtest_tools.pypins its values. §3 carries an explicit code-vs-page note stating the page is normative andorder2resis ~0.2% smaller at every order. - Unification of
order2resonto this sphere (a behavioral change) is tracked as a follow-up: Unify order2res Earth model with the spec page sphere (R=6371.0088) #119, linked from both the §3 note and PR Question 2 (now marked resolved).
|
🤖 from Claude Folded two espg-directed review rulings that had no dedicated inline thread, recording them here for the record. §4 §6.5 product-name length cap — 192 chars (34679c8). espg's final ruling is 192. §6.5 grammar is now |
|
🤖 from Claude In response to the discriminator questions on this review: §4 is reworked into one coherent treatment of a three-value
Folded alongside: declaration scoping (point-ness carried entirely by the declaration for Also updated: the §4 clip-rule subsection (now naming exact-29-not-clippable and mixed-inapplicable), §8's frozen line (three values + point-only clip scope), and the §5 bullet/example (still |
|
🤖 from Claude §4 superseded per the final ruling (espg, 2026-07-21) — 462ffb1, one commit replacing the declaration design (including 4a3823a's three-value form):
zagg follow-through (removing the |
|
|
||
| # Points exist only at order 29: any shorter string denotes exactly | ||
| # one word (an area cell), which round-trips bit-identically. | ||
| arr = MortonIndexArray.from_latlon(np.array([45.0]), np.array([45.0])) |
There was a problem hiding this comment.
🤖 from Claude (review)
F4 (medium) — test_sub29_strings_are_unambiguous never exercises a sub-order-29 string.
from_latlon(lat, lon) defaults to order=MAX_ORDER (= 29), so this builds an order-29 area cell, not a shorter string. I confirmed the rendered id has 29 order digits and suffix 44 (in the 28..=47 area region), so the word & 0x3F <= 47 assertion passes on an order-29 word.
That means the "points exist only at order 29 / any shorter string denotes exactly one word" property the test name and docstring promise is not actually pinned — the sub-29 case is untested. The test still asserts something true (an order-29 area word round-trips), but it duplicates coverage the order-29 half of test_order29_string_parses_to_area_word already gives.
Suggest passing an explicit sub-29 order, e.g. from_latlon(..., order=10) (verified: renders 14111411111, suffix 10, round-trips bit-identically), so the "shorter string is unambiguous" claim is genuinely golden-pinned.
There was a problem hiding this comment.
🤖 from Claude
Fixed in 31fa1e0. test_sub29_strings_are_unambiguous now builds an explicit sub-29 cell — from_latlon(..., order=10) (verified from_latlon accepts order, default MAX_ORDER) — so the test genuinely exercises a shorter string instead of the order-29 area word. It renders 14111411111 (11 digits), and the assertions now pin len(dec) < 29, suffix <= 27 (the sub-29 area region, below the order-29 28..47 band), and exact bit-identical round-trip. pytest mortie/tests/test_spec_page.py green (4 passed); flake8 --select=E9,F63,F7,F82 clean.
|
|
||
| With the kind suffix, the decimal round-trip is **lossless for both | ||
| kinds**: an area word renders unmarked and parses back to itself; a point | ||
| word renders `p`-marked and parses back to itself. The residual ambiguity |
There was a problem hiding this comment.
🤖 from Claude (review)
F5 (low-medium) — §2.1 states p-marked point rendering as present fact, but it isn't implemented and the golden test shows points render unmarked.
Here §2.1 says, unconditionally and in the present tense: "a point word renders p-marked and parses back to itself." But §4 (lines 226–229 of the same commit) explicitly hedges the other way — "the p emission/acceptance is tracked as a mortie implementation change (issue linked from the PR)" — i.e. decimal_repr does not emit p today. I confirmed this: from_latlon(45, 45, points=True).decimal_repr() returns an unmarked 29-digit string, and test_order29_string_parses_to_area_word relies on exactly that (it derives t28,t29 from the unmarked dec and expects the area word).
So within one commit §2 asserts the p round-trip as done while §4 says it's pending, and the pinned test agrees with §4. Suggest making §2.1 conditional/normative-target ("MAY render p-marked", or "once the suffix is emitted…") to match §4's status note, so a reader/implementer doesn't conclude mortie currently emits p.
There was a problem hiding this comment.
🤖 from Claude
Reconciled in 231106b. The implementation now exists — the p emit/accept is implemented in PR #121 (issue #120) with golden vectors — so the §2.1↔§4 tension is resolved by describing the lossless round-trip as the normative contract and citing the concrete implementation, rather than hedging one section as "pending". §2.1 now reads "the normative contract … The p emit/accept is implemented in PR #121 (issue #120), with golden vectors." §4's former "tracked as a mortie implementation change (issue linked from the PR)" is likewise replaced with the same PR #121 / issue #120 citation. Note this PR's branch does not itself carry the emit change (points still render unmarked here); that lands via #121, which the text now points to directly.
|
🤖 from Claude (review) Two low-severity consistency notes from the F6 (low) — §7 root-MOC endpoints are never F7 (low/nit) — "render-only" vs "render/interchange" terminology drift. §2 opening (line 82) and §8 (line 516) still call the decimal string "render-only", while the new text describes lossless round-trip interchange and labels the suffix "render/interchange-only" (lines 97, 521). No substantive conflict — "render-only" is about the string's role (display form vs |
|
🤖 from Claude Folded both notes from the
§7.3 now states it explicitly. The endpoints bullet reads: "They are always unmarked area words at the shard order, never
Harmonized on render/interchange. §2 opening (was "render-only external form") and §8 (was "its render-only status") now both say render/interchange, matching the suffix's existing "render/interchange form only" / "render/interchange-only" phrasing. No substantive change — just the two phrasings no longer sit side by side.
|
Closes #62. Refs #48.
Adds
docs/specification.md— the v1.0 specification & conventions page the 1.x stability contract points at — linked from the README, with a drift pin (mortie/tests/test_spec_page.py) that regenerates the resolution table fromorder2resand compares it literally against the page (the issue's acceptance criterion).This PR consolidates content already recorded on the #62 thread and adds the pieces ratified since on the zagg side (zagg#305 carries the decision record; zagg's
docs/design/sparse_coverage.mdis the decisions registry and now cites this page as the normative home for grammars and constants — the acceptance criterion recorded on zagg#305 is that no normative constant lives only in the design doc).What the page freezes
[4-bit prefix | 54-bit body | 6-bit suffix], sourced fromsrc_rust/src/decimal_morton.rsmodule docs (prefixbase_id + 1, 27×2-bit body, suffix regions0..=27/28..=47area /48..=63point, canonical zero-fill, unsigned storage, raw-sort Z-order).uint64storage / capped legacyi64), the base-component grammar-?[1-6], and the order-29 point/area parse non-injectivity note.order2rescell scale, exact cell area; generated fromorder2res, pinned bytest_spec_page.py.resolution: "exact" | "point"discriminator + 29→24 clip rule — espg-ratified field name/values/placement (zagg#305 decision 1): dggs attrs block, emission per data kind (writer always knows), clip rule applies topointonly.name: "morton"+coordinate: "morton"(nevername: healpix+indexing_scheme: morton— the scheme-blind silent-misread hazard from zagg D16), and the self-declared convention UUID3e22156d-ea9e-4e01-95fe-e3809a4b41e7(minted once, permanent;zarr_conventionsis a list so a future upstream registry entry coexists)./1–/3grammars consolidated from the thread (/2windowed-leaf record,/3token ratification): underscore split rule, window-label grammar table, the reservedalltoken (permanently excluded from the window grammar),/3sidecar names{window}.stats.json/all.stats.json, node invariant, commit stamp, overviewroleflagging, and the product-name grammar ([a-z0-9_-]+, base-component exclusion) per zagg D19's final named-roots form — note the earlier heads-up comment described the interim hash-root design; the page records the ratified named-root revision instead.ranges|bitmap|fulldiscriminator, the bitmap bit convention and root-MOC range ordering as contract with zstd level non-normative (zagg O8 addendum, espg-blessed 2026-07-20).Phases
Testing
mortie/tests/test_spec_page.py(2 tests) green against a localpip install -e ".[test]"build (maturin-via-pip; Rust 1.96).flake8 mortie --select=E9,F63,F7,F82clean; fullpytest mortie/tests: 672 passed, 11 skipped.Questions for review
Product-name length cap— Resolved (espg): cap at 192 chars. §6.5 now specifies[a-z0-9_-]{1,192}; §8 records the cap in the frozen list. Derivation on the page: POSIX 255-byte filename ceiling − 13 chars reserved for the immutable-provenance decoration (++ 12-hex catalog fingerprint) = 242 hard ceiling; 192 sits 50 under it and keeps total-path budgets comfortable (S3 1,024-byte key / macOSPATH_MAXat order-24 depth). Folded in 34679c8.Area column constants— Resolved (espg): unify both columns on the exact HEALPix sphereR = 6371.0088km. §3 now derives cell area (4πR²/(12·4^order)) and cell scale (sqrt(area), RMS cell spacing) from that one model; the drift pin regenerates both columns from it.mortie.tools.order2resin code keeps the historical111 × 58.6323constant for behavioral compatibility (consumed byres2displayand thetest_coverage_boundary.pypad); §3 carries an explicit code-vs-page note and the migration is tracked in Unify order2res Earth model with the spec page sphere (R=6371.0088) #119. Folded in 541f080.🤖 Generated with Claude Code
https://claude.ai/code/session_019zYV2pvzhVp8cv7e7H4JY3