Strict input validation family-wide (issue #194) - #213
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
==========================================
+ Coverage 96.53% 96.64% +0.10%
==========================================
Files 20 21 +1
Lines 2365 2382 +17
==========================================
+ Hits 2283 2302 +19
+ Misses 82 80 -2
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 degrade performance by 15.42%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_norm2mort_batch |
1.5 ms | 1.8 ms | -15.42% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/194-strict-validation (8c15e86) with main (4900a7e)
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. ↩
| from .orders import _rust_mort2nested | ||
|
|
||
| morton = np.atleast_1d(np.asarray(morton, dtype=np.uint64)) | ||
| morton = _as_u64(morton, "morton") |
There was a problem hiding this comment.
🤖 from Claude (review)
blocking — to_geometry / to_wkb / to_wkt are only strict on the non-default path. _per_cell_polygons got the validator, but to_geometry routes to it only when dissolve=False (geometry.py:775-776); the default dissolve=True goes to _dissolved_polygons, and dissolve.py still carries the old silent coercion at lines 118, 831 and 986 (np.atleast_1d(np.asarray(morton, dtype=np.uint64))). So the default spelling of three public entry points listed as "strict (phase 3)" in the inventory still truncates floats and wraps negatives.
Repro on 0c8a1a8:
>>> import numpy as np, mortie
>>> w = np.asarray(mortie.norm2mort([0],[0],4), dtype=np.uint64) # order-4 word
>>> mortie.orders_of(w)
array([4], dtype=uint8)
>>> f = w.astype(np.float64) # float64 cannot hold the word exactly
>>> mortie.to_geometry(f, dissolve=False)
ValueError: morton must be integer-typed, got dtype float64
>>> mortie.to_geometry(f) # DEFAULT dissolve=True: accepted
<MULTIPOLYGON (((90 90, 0 90, ... # the whole base cell, not the order-4 cell
>>> np.uint64(np.float64(w[0])), mortie.orders_of(np.array([np.uint64(np.float64(w[0]))]))
(1152921504606846976, array([0], dtype=uint8)) # suffix zeroed -> order 0This is the exact corruption the PR body cites as "the posture caught a live in-repo bug while landing" (test_array_input), still live on the default path of to_geometry.
Negatives are worse — they reach the kernel and produce the #185 panic-arc surface instead of a named refusal:
>>> mortie.to_wkt(np.asarray([3, -7], dtype=np.int64))
thread '<unnamed>' panicked at mortie-core/src/morton.rs:29:17: Morton index cannot be zero
ValueError: Morton index cannot be zerovs dissolve=False, which gives morton must be non-negative, got -7.
The test set has the same hole: WORD_CALLS_P3 (test_strict_validation.py:249) pins only to_geometry(w, dissolve=False), and to_wkb / to_wkt are not tested at all — so CI is green with the default path unvalidated. Suggest validating once at the top of to_geometry (and to_wkb / to_wkt, which each call to_geometry) rather than inside _per_cell_polygons, plus dissolve=True rows in WORD_CALLS_P3.
There was a problem hiding this comment.
🤖 from Claude
Folded in 8f11d8d — the seam moved up to the shared public intake, exactly as suggested.
to_geometry now validates before the dissolve branch, so both arms (and to_wkb / to_wkt, which route here) refuse alike; dissolve.py is untouched, since _dissolved_polygons and _per_cell_polygons have no caller but this one:
_check_latitude(latitude)
# One validation seam for both arms -- and so for to_wkb/to_wkt, which
# route here. The dissolved arm's own coercion (dissolve.py) would
# otherwise truncate floats and wrap negatives on the *default* spelling.
morton = _as_u64(morton, "morton")
if dissolve:_per_cell_polygons keeps its own _as_u64 as a private backstop (second pass sees uint64, no rescan).
Your repro, on 8f11d8d — all twelve combinations now refuse:
to_geometry True float -> morton must be integer-typed, got dtype float64
to_geometry True neg -> morton must be non-negative, got -7
to_wkt True neg -> morton must be non-negative, got -7 # was the Rust panic arc
...
and valid covers are unchanged (to_geometry(w).area == 13.504372296124904, per-cell …917).
Tests: WORD_CALLS_P3 gained five rows — to_geometry_dissolved, to_wkb, to_wkb_per_cell, to_wkt, to_wkt_per_cell — so each runs through both the float and the negative refusal. They are ungated like the existing to_geometry row, since shapely is in the test extra.
Docstrings: to_geometry's Raises now names the morton refusal for both arms, and to_wkb / to_wkt gained the ValueError entry they were missing entirely.
| |---|---| | ||
| | `_moc` | `compress_moc`, `moc_to_order`, `moc_or`, `moc_and`, `moc_intersects`, `moc_minus`, `moc_xor`, `moc_min`, `moc_not`, `common_ancestor`, `split_base_cells` (both arms where polymorphic) | | ||
| | `batch` | `polygons_to_morton_mocs` and every ragged kernel behind the `offsets=` forms | | ||
| | `geometry` | `from_wkb(offsets=)`; the `to_geometry` / `to_wkb` / `to_wkt` word intake | |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — this row is not accurate as written. Only the dissolve=False arm of to_geometry / to_wkb / to_wkt gained validation (the validator sits in _per_cell_polygons); the default dissolve=True arm goes through dissolve.py's untouched np.asarray(..., dtype=np.uint64) and still truncates/wraps — see the repro on mortie/geometry.py:700. Either fix the seam (preferred) or narrow this row and the corresponding inventory row in the PR body so the breaking-register does not over-promise.
There was a problem hiding this comment.
🤖 from Claude
Fixed the seam (preferred option) in 8f11d8d, then tightened this row in b3a727e rather than narrowing it.
The row now pins the property by name instead of leaving "word intake" to be read charitably:
|
geometry|from_wkb(offsets=); theto_geometry/to_wkb/to_wktword intake — validated at the shared seam, so bothdissolvearms refuse alike (the defaultdissolve=Trueroute throughdissolveincluded) |
So the breaking-register no longer over-promises, and it now says the thing that was actually at issue — the default spelling — out loud, which is the part a future reader would otherwise have to re-derive.
| """ | ||
| values = np.ascontiguousarray(np.asarray(values, dtype=np.uint64).ravel()) | ||
| offsets = np.ascontiguousarray(np.asarray(offsets, dtype=np.int64).ravel()) | ||
| values = np.ascontiguousarray(_as_u64(values, "values").ravel()) |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the refusal blames the kernel's parameter name, not the public one, so the same argument gets two different names depending on whether offsets= was passed. The CHANGELOG promises "Every refusal names the parameter" (CHANGELOG.md:20); today it names whichever private kernel it landed in.
Measured on 0c8a1a8 with F = np.asarray([1.5, 2.0]):
moc_to_order(morton=F) -> morton must be integer-typed, got dtype float64
moc_to_order(morton=F, offsets=..) -> values must be integer-typed, got dtype float64 <- same arg
moc_and(a=W, b=F) -> b must be integer-typed, got dtype float64
moc_and(a=W, b=F, offsets=..) -> values must be integer-typed, got dtype float64 <- same arg
common_ancestor(morton=F, offsets=..) -> values must be integer-typed, got dtype float64
generate_morton_children(parent_morton=F) -> words must be integer-typed, got dtype float64
Moc(W) & F -> words must be integer-typed, got dtype float64
values and words are not parameters of any public signature. Note generate_morton_children already has the machinery for this — orders.py:517-523 rewrites children_of → generate_morton_children in the kernel's message — but _as_u64's text contains no children_of, so it sails through unrenamed while the scalar arm correctly says parent_morton.
Cheapest fix: pass the caller-facing name down (_mocs_to_orders(values, ..., name="morton") etc., or have the public wrappers validate before delegating). The parametrized tests currently encode the inconsistency (("moc_to_order_ragged", "values", ...) at test_strict_validation.py:148, ("generate_morton_children_array", "words", ...) at :162), so they would need updating too — worth doing, since as-is the suite locks in the confusing register.
There was a problem hiding this comment.
🤖 from Claude
Folded in b072530. I took the public wrappers validate before delegating option rather than threading a name= parameter into the kernels.
Why that one: the kernels are private and their own parameter names (values, words) are correct for them — the bug is that a private name was reaching a public message, not that the private name is wrong. Validating in the public arm fixes it where the public name actually lives, keeps the kernels' signatures untouched, and leaves their _as_u64 in place as a backstop that costs nothing (the second pass sees uint64 and returns without a rescan). Threading a name through would have put the public vocabulary inside five private signatures and made every future kernel caller responsible for passing it.
Four sites in _moc.py and one in orders.py:
if offsets is not None:
# Name the caller-facing parameter before delegating -- the kernel's
# own pass stays as the backstop and sees uint64 (no rescan).
return _mocs_and(_as_u64(a, "a"), _as_u64(b, "b"), offsets)Your measured table, re-run on b072530:
moc_to_order(morton=F) -> morton must be integer-typed, got dtype float64
moc_to_order(morton=F, offsets=..) -> morton ... <- was `values`
moc_and(a=W, b=F) -> b ...
moc_and(a=W, b=F, offsets=..) -> b ... <- was `values`
moc_intersects(a=W, b=F, offsets=..) -> b ... <- was `values`
common_ancestor(morton=F, offsets=..) -> morton ... <- was `values`
generate_morton_children(parent_morton=F) -> parent_morton ... <- was `words`
Moc(W) & F -> operand ... <- was `words`
split_base_cells(words=F) -> words ... <- correct already: `words` *is* its public parameter
One addition beyond your list: moc_object._words(operand) was also blaming "words", which is not a parameter of any Moc operator either — it is now "operand", the name the docstrings use for the right-hand side.
Note this also makes the orders.py:517-523 children_of → generate_morton_children rename redundant for the dtype refusal (the public validator now fires first); it stays for the kernel's other messages, e.g. the max_cells refusal.
Tests: the six parametrized rows that encoded the inconsistency now assert the public names — moc_to_order_ragged → morton, moc_and_ragged / moc_intersects_ragged → b, common_ancestor_ragged → morton, generate_morton_children_array → parent_morton, Moc_operand → operand.
| # above; re-try the exact cast so an oversized *integer* is refused as | ||
| # out of range rather than as a float. | ||
| try: | ||
| np.asarray(offsets, dtype=np.int64) |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — this probe cast runs for every non-integer dtype, not just the Python-int-past-int64 case it is commented for, and it leaks numpy's own failure out of the family's message register.
>>> from mortie._validate import _as_offsets
>>> _as_offsets(['a'])
ValueError: invalid literal for int() with base 10: 'a' # was, pre-hoist:
# offsets must be integer-typed, got dtype <U1
>>> _as_offsets([None, 1])
TypeError: int() argument must be a string, ... not 'NoneType' # not even a ValueError
>>> _as_offsets(np.array([0.0, np.nan]))
RuntimeWarning: invalid value encountered in cast # spurious warning on a refusal path
ValueError: offsets must be integer-typed, got dtype float64The old _toc._as_offsets returned the named message for all three (it never attempted the cast). The NaN case is the sharp one: under -W error (or a downstream filterwarnings = error pytest config) the probe's RuntimeWarning becomes the raised exception and the caller never sees the named ValueError:
>>> warnings.simplefilter("error"); _as_offsets(np.array([0.0, np.nan]))
RuntimeWarning: invalid value encountered in castSuggest gating the probe to the case it exists for — if arr.dtype.kind in "fO" and the input is not already an ndarray, or just scan offsets for a Python int outside int64 directly — and add a <U/None/NaN row to TestValidators. _as_u64 has no equivalent hole (_as_u64(np.array(['a']), 'morton') still says morton must be integer-typed, got dtype <U1), which is itself evidence the two validators have drifted apart.
There was a problem hiding this comment.
🤖 from Claude
Folded in 8f38df9. I went further than gating the probe: the trial cast is gone, replaced by a direct scan, so no numpy exception or warning can reach the caller from this path at all.
if arr.dtype.kind not in "iu":
# A Python int past int64 lands as float64 (or, further out, object)
# in the untyped asarray above, so an oversized *integer* would
# otherwise be blamed on its promoted dtype. Look for one directly
# rather than probe-casting: a trial cast would leak numpy's own
# message for strings and None, and would raise a RuntimeWarning on
# NaN that becomes the exception under warnings-as-errors -- in every
# case burying this family's named refusal (issue #194 review).
if arr.dtype.kind in "fO":
flat = np.atleast_1d(np.asarray(offsets, dtype=object)).ravel()
bad = next((v for v in flat.tolist() if isinstance(v, int)
and not -2**63 <= v < 2**63), None)
if bad is not None:
raise ValueError(f"offsets must fit in int64, got {bad}")
raise ValueError(
f"offsets must be integer-typed, got dtype {arr.dtype}")A scan beats a suppressed probe here because it needs no errstate/catch_warnings scaffolding to be correct, and it is exact — there is no cast whose failure mode has to be classified. It only runs on the refusal path, which was already O(n).
All three of your cases, under -W error:
["a"] -> ValueError: offsets must be integer-typed, got dtype <U1
[None, 1] -> ValueError: offsets must be integer-typed, got dtype object
np.array([0.0, np.nan])-> ValueError: offsets must be integer-typed, got dtype float64 # no RuntimeWarning
Including "O" alongside "f" also picked up a case neither of us listed: a Python int large enough to land as object rather than float64 ([0, 10**40]) is now named too — offsets must fit in int64, got 10000… — where the old probe's OverflowError branch happened to catch it and the kind == "f"-only gate would have lost it.
Tests in TestValidators: a parametrized <U1 / object(None) / object(mixed) row asserting the family message, a NaN row wrapped in warnings.simplefilter("error"), and an object-dtype oversized-int row. The existing [0, 10**19] and uint64 ≥ 2**63 pins are unchanged and still pass.
On the drift you noted: _as_u64 never grew a probe, which is why it had no equivalent hole — the two validators now agree again, both refusing on dtype alone and reaching for values only to name one.
| If *morton_array* is empty or not 1-D. | ||
| """ | ||
| morton_array = np.ascontiguousarray(np.asarray(morton_array, dtype=np.uint64)) | ||
| arr = np.atleast_1d(np.asarray(morton_array)) |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the new np.atleast_1d silently retires the 0-D refusal. Previously np.asarray(scalar, dtype=np.uint64) stayed 0-D and the very next check (morton_array.ndim != 1) raised; now the scalar is promoted to 1-D before that check ever runs, so a scalar is accepted:
# origin/main
>>> split_children(np.uint64(1152921504606846980))
ValueError: morton_array must be a non-empty 1-D integer array
# this branch (0c8a1a8)
>>> split_children(np.uint64(1152921504606846980))
[MortonChild(characteristic='11111', len=1, nchildren=0)]
>>> split_children(1152921504606846980) # plain int, same
[MortonChild(characteristic='11111', len=1, nchildren=0)]Same for morton_polygon_from_array, which delegates here. That is a loosening in the opposite direction from the PR's stated posture, it is untested, and the Raises section two lines below still says "If morton_array is empty or not 1-D". Either drop the atleast_1d (do the dtype check on np.asarray(morton_array) directly, whose .ndim is then still 0 for a scalar) or make the loosening deliberate and document it. While you are here: the Raises block also does not mention the new float refusal — the Parameters entry does, but Raises is what numpydoc readers scan.
There was a problem hiding this comment.
🤖 from Claude
Folded in ad570df — dropped the atleast_1d, which was the loosening, and reordered so the rank check runs before the cast.
# Not atleast_1d: a 0-D scalar must keep its rank so the 1-D check below
# still refuses it, as it did before this seam was validated.
arr = np.asarray(morton_array)
if arr.size and arr.dtype.kind not in "iu":
...
if arr.ndim != 1 or arr.size == 0:
raise ValueError("morton_array must be a non-empty 1-D integer array")
morton_array = np.ascontiguousarray(arr.astype(np.uint64, copy=False))The ascontiguousarray had to move below the rank check too — it has atleast_1d semantics of its own, so leaving it above would have re-promoted the scalar past the very check it feeds.
Back to main's behavior on ad570df:
split_children(np.uint64(1152921504606846980)) -> ValueError: morton_array must be a non-empty 1-D integer array
split_children(1152921504606846980) -> ValueError: ... (same)
morton_polygon_from_array(<either>, 1) -> ValueError: ... (same)
Tests: test_split_children_still_refuses_scalars, parametrized over np.uint64 and plain int, asserting both split_children and the delegating morton_polygon_from_array.
Raises is corrected as well — it now reads "If morton_array is float-typed (issue #194), or is empty, a scalar, or not 1-D", so both the new float refusal and the scalar case are where numpydoc readers look, not only in Parameters.
There was a problem hiding this comment.
🤖 from Claude
Ruling recorded (espg, in-session 2026-08-24): the signed int64 bit-view carve-out on split_children stays as documented — the trie branches on the decimal characteristic, whose first column is the sign, and the golden fixtures pin the form. PR-body Question 1 is marked RESOLVED; no code change.
| # squeeze on one side alone made the round trip lose its shape. | ||
| is_scalar = np.ndim(morton) == 0 | ||
| morton = np.atleast_1d(np.asarray(morton, dtype=np.uint64)) | ||
| morton = _as_u64(morton, "morton") |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix (inventory accuracy, not this line) — the PR-body inventory row
|
unique2parent,orders_of_uniq(UNIQ ints) | strict | — | strict (float + wrap guarded, #185 arc) | unchanged |
is wrong for unique2parent, and uniq2geo is missing from the inventory entirely. Both do np.atleast_1d(np.asarray(x, dtype=np.int64)) (convert.py:170 and convert.py:631) before handing off to orders_of_uniq, so the float truncates first and the strict check downstream never sees it:
>>> mortie.orders_of_uniq(np.asarray([16.5, 20.9]))
ValueError: Not a valid UNIQ cell number for orders 0-29: np.float64(16.5) is not ...
>>> mortie.unique2parent(np.asarray([16.5, 20.9])) # truncates to 16, 20 -- accepted
array([0, 1])
>>> mortie.uniq2geo(np.asarray([16.5, 20.9])) # 16.5 and 20.9 decode as different cells
(array([19.47122063, 19.47122063]), array([ 45., 135.]))Same class for norm2mort (convert.py:220-221), also absent from the inventory: norm2mort(np.asarray([16.5, 20.9]), [0, 0], 4) is accepted and truncates.
UNIQ ids and normalized addresses are arguably out of the issue's "packed word / arrow offset" scope — that is a fine ruling to make — but then the inventory should say so rather than claim unique2parent is already strict. Since the inventory is the review artifact, an inaccurate row is the thing most likely to be trusted later.
There was a problem hiding this comment.
🤖 from Claude
Confirmed and folded into the PR body (no code change — see the ruling below). Your three repros reproduce exactly on 0c8a1a8:
orders_of_uniq([16.5, 20.9]) -> RAISED (Not a valid UNIQ cell number ...)
unique2parent([16.5, 20.9]) -> array([0, 1]) # truncated
uniq2geo([16.5, 20.9]) -> (19.552…, 19.552…), (45., 135.) # truncated
norm2mort([16.5, 20.9],…,4) -> [1224979098644774916, …] # truncated
The row was wrong, and I agree the inventory being the review artifact is exactly why. It is now split in two, with the scoped-out half stating the posture honestly instead of claiming a strictness it does not have:
|
orders_of_uniq(UNIQ ints) | strict | — | strict (float + wrap guarded, #185 arc) | unchanged |
|unique2parent,uniq2geo(UNIQ ints),norm2mort(normed ints) | silent truncate | — | batch-style | unchanged — scoped out, see below |
plus a "Scoped out, deliberately" paragraph under the table carrying your repro, the three convert.py line numbers (170 / 631 / 220), and the ruling: UNIQ ids and normalized addresses are not packed words or arrow offsets, which is the domain issue #194 rules on, so tightening them is a separate issue rather than a silent scope expansion here. The row-count summary moved with it (the deferred count now names this row alongside pandas.py).
I took the ruling you offered rather than the code change because widening to UNIQ/normed intake mid-PR would change what #194 was ruled on — but it is a real gap, and it is now written down as one instead of hidden behind a wrong "strict".
| "moc_object_and": _ints( | ||
| (mortie.Moc(cover_a) & mortie.Moc(cover_b)).words), | ||
| } | ||
| return g |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — the golden set is described as "every touched entry point" (PR body, and the test docstring at test_strict_validation.py:107) but omits several touched surfaces, so "valid inputs are unaffected" is pinned less tightly than claimed:
mort2bbox,mort2polygon— both changed in phase 3, neither captured (onlymort2norm/mort2geoare).to_geometry/to_wkb/to_wkt— changed in phase 3, not captured (and see the seam bug onmortie/geometry.py:700).from_wkb(offsets=)— the phase-2 offsets seam, not captured.- The whole toc family. Phase 1 changed
_as_u64under_toc.pyin three ways (zero-size now accepted where it was refused, negative message text changed, andastype(..., copy=False)where the toc copy was unconditional), yet notoc_*entry appears in the capture.time2toc([])going from a refusal to an empty cover is exactly the kind of drift a golden would have surfaced; today it is covered only by the CHANGELOG prose.
Also worth noting for the record (checked, and it is fine): the generator's phase-2 edit (4d50dee) is a pure main → capture split with no value changes, so the JSON committed in phase 1 really is the pre-change record.
There was a problem hiding this comment.
🤖 from Claude
Folded in 16d43ac. I extended the capture rather than only narrowing the claim — and captured it from the 4900a7e source, not from this branch.
Method (so it is auditable): a detached worktree at 4900a7e, the branch's _rustie.abi3.so copied in — sound because git diff --name-only 4900a7e HEAD is pure Python, no Rust or Cargo.* change — and the new generator run against the old package via PYTHONPATH. Pre-change was verified in that interpreter before capturing: no mortie/_validate.py on disk, and orders_of([1.5, 2.0]) still answered [1, 2].
The check that matters: the 34 original entries came back byte-identical to the committed JSON. That is an independent confirmation of the thing you checked by reading the phase-2 diff — the committed capture really is the pre-change record — arrived at from the other direction.
34 → 48 entries. Added: mort2bbox, mort2polygon, from_wkb_ragged (the phase-2 offsets seam, via hard-coded WKB hex so the generator stays numpy-only), and the whole toc family — time2toc, span2toc, toc2time, toc_reduce, toc_reduce_ragged, toc_normalize, toc_and, toc_merge, from_gps_ns, to_gps_ns, to_datetime64. The post-change tree reproduces all 48.
Two deliberate omissions, now stated in the generator's docstring rather than papered over:
to_geometry/to_wkb/to_wktneed shapely, which is a test extra, not a runtime dep. Keeping the generator numpy-only means the golden test never turns on an optional install; those three are pinned bytest_geometry.py's behavior suite plus the five refusal rows added in8f11d8d.time2toc([])is not captured, on purpose: the untyped-empty acceptance deliberately changed that answer from a refusal to an empty cover. A golden there would pin the one valid path this PR does not claim is unchanged — it belongs in the CHANGELOG and Q2, where it is.
Docstrings corrected in both places: "every touched entry point" is gone from the generator and from test_valid_paths_byte_identical_to_pre_change_goldens, replaced by an enumeration of what is covered and why those two are not. The PR body's "all 34 entries" is updated to 48 with the same detail.
|
🤖 from Claude (review) should-fix — Questions for review, Q3: the The PR body defers
Repro on >>> import numpy as np, mortie
>>> from mortie import arrow
>>> w = np.asarray(mortie.norm2mort([0], [0], 4), dtype=np.uint64)
>>> int(w[0])
1152921504606846980
>>> a = arrow.from_morton_index(w.astype(np.float64)) # accepted
>>> a[0].value
1152921504606846976 # order-4 word silently demoted to order 0
>>> arrow.export_c_array(w.astype(np.float64)) # accepted, same corruption, then handed to Rust
(<capsule ...>, <capsule ...>)That is the same corruption the PR body reports finding in For the record on the other questions I checked and found nothing to add beyond the inline threads: Q2's empty-accept does not break |
|
🤖 from Claude Accepted — the Q3 rationale was wrong for exactly the two skins you name, and both are folded in Your repro is the deciding fact: Both intakes now take # from_morton_index (arrow.py:234)
data = _as_u64(getattr(array, "_data", array), "array")
# export_c_array (arrow.py:700)
data = np.ascontiguousarray(_as_u64(getattr(words, "_data", words), "words"))On and valid words are untouched — Tests: PR body updated — the inventory row is split ( Thanks for checking Q1/Q2 as well — the |
|
🤖 from Claude espg rulings recorded (in-session, 2026-08-24):
Questions 2 (zero-size posture) and 4 (int64-wrap interop) remain open with espg. |
|
🤖 from Claude Two more espg rulings recorded (in-session, 2026-08-24) — both standing questions now resolved, no code changes from either:
With Question 1 already ruled ( |
| normed = np.atleast_1d(np.asarray(normed, dtype=np.int64)) | ||
| parent = np.atleast_1d(np.asarray(parent, dtype=np.int64)) | ||
| normed = _as_u64(normed, "normed") | ||
| parent = _as_u64(parent, "parent") |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — phase 5 hardens norm2mort's normed/parent but leaves its exact sibling norm2uniq(normed, parent, order) (convert.py:522) on the old silent path, so the producer of UNIQ ids stays loose while the two consumers just went strict.
On 7f2c5bf:
>>> mortie.norm2uniq(11.5, 0, 4)
1035.5 # int answer is 1035 -- a float UNIQ, returned
>>> mortie.norm2uniq(-3, 0, 4)
1021
>>> mortie.orders_of_uniq(1021), mortie.unique2parent(1021)
(array([3], dtype=uint8), 11)
The negative case is precisely the hazard the new Raises block two lines up names — "silently cast into a different, possibly valid, word": normed=-3 at order 4 lands on a real order-3 cell in base 11, with no error at any point downstream.
The float case now produces a worse message than before phase 5, because the wall moved in front of the wrong parameter:
>>> mortie.uniq2geo(np.asarray([mortie.norm2uniq(11.5, 0, 4)]))
ValueError: uniq must be integer-typed, got dtype float64
The caller's mistake was normed, two calls earlier — the same misblame the fold at b072530 ("blame the public parameter name, not the kernel's") went out of its way to remove.
norm2uniq is also the documented way to obtain UNIQ ids for unique2parent / uniq2geo (USAGE.md:470-489), and it is absent from the inventory table entirely. Either extend the phase to norm2uniq (_as_u64 on both operands, same as here — its internal callers at convert.py:715/782/983/1072 all pass validated mort2norm output, so nothing in-tree would start refusing), or add an explicit scope note saying why the sibling is left out.
There was a problem hiding this comment.
🤖 from Claude
Folded in 01dbf36 — norm2uniq takes the same unsigned intake as norm2mort, on both operands.
One deliberate difference from the sibling: the validators are called for their refusal, not rebound.
_as_u64(normed, "normed")
_as_u64(parent, "parent")
bcast = np.broadcast(np.asarray(normed), np.asarray(parent))norm2uniq's scalar/array form and output dtype fall out of the caller's operands (there is no is_scalar switch — the scalar path is plain Python arithmetic returning a Python int, and an int64 column returns int64). Rebinding to the uint64 validator's atleast_1d output would have turned norm2uniq(11, 0, 4) into array([1035], dtype=uint64) and promoted every int64 column, so the valid path is left byte-identical and only the refusals are new. Pinned in test_norm2uniq_valid_path_keeps_form_and_dtype, and the four internal callers (convert.py:715/782/983/1072) pass mort2norm output, which is non-negative int64 — nothing in-tree started refusing (full suite green).
Both hazards you named are now pinned as regressions: norm2uniq(-3, 0, 4) used to answer UNIQ 1021 (a real order-3 cell in base 11) and now raises normed must be non-negative, got -3; norm2uniq(11.5, 0, 4) used to answer the float UNIQ 1035.5 whose misblamed refusal landed two calls later, and now raises normed must be integer-typed. CHANGELOG's convert row and the PR-body inventory row both name norm2uniq now.
| """ | ||
| is_scalar = np.ndim(unique) == 0 | ||
| u = np.atleast_1d(np.asarray(unique, dtype=np.int64)) | ||
| u = _as_i64(unique, "unique") |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the inventory row | orders_of_uniq (UNIQ ints) | strict | ... | **strict** (float + wrap guarded, #185 arc) | unchanged | over-claims, and phase 5 turns that into a live inconsistency: orders_of_uniq guards only non-integral floats, so it still accepts an integral float, while the two functions that call it now refuse the same value.
On 7f2c5bf:
>>> mortie.orders_of_uniq(np.asarray([16.0]))
array([1], dtype=uint8)
>>> mortie.unique2parent(np.asarray([16.0]))
ValueError: unique must be integer-typed, got dtype float64
The guard is orders.py:193 (arr.dtype.kind == "f" and not np.all(np.equal(np.mod(arr, 1), 0))) — a value-based check, not the dtype-based one the family's definition of strict uses everywhere else. orders_of_uniq is public and exported (__init__.py:124/164), and it is the same int64-domain UNIQ intake the espg ruling's wording covers, so after phase 5 it is the one UNIQ entry point where a float column still decodes.
Either bring it onto _as_i64 and move it out of the "strict already" bucket, or downgrade the row's claim (e.g. "integral floats accepted; value-based guard") so the table does not read as parity with the phase-5 seams.
There was a problem hiding this comment.
🤖 from Claude
Took option (a) — orders_of_uniq is on _as_i64 as of 73839af, and the inventory row moved out of the "strict already" bucket.
Rationale for hardening rather than downgrading the row: the PR's whole claim is one posture, and this was the last UNIQ entry point where a float column's fate depended on which decoder saw it first (orders_of_uniq([16.0]) -> 1 vs unique2parent([16.0]) refusing). Its two in-tree callers already validate the same column with _as_i64 before handing it over, so the tightening costs them nothing.
Two message changes fall out of it, both deliberate and both re-pinned in test_uniq_orders_raises_valueerror_not_overflow:
orders_of_uniq(1.5)— wasNot a valid UNIQ cell number …: 1.5 is not an integer, nowuniq must be integer-typed, got dtype float64(the family register, and the same textuniq2geogives for the same input).orders_of_uniq(2**63)— was… is out of the int64 range, nowuniq must fit in int64, got 9223372036854775808.
The bespoke float/uint64/OverflowError block is gone; the UNIQ domain refusal (Not a valid UNIQ cell number for orders 0-29) is untouched and still owns out-of-range values, including negatives. CHANGELOG's orders row records the change with the [16.0] example, and test_strict_validation.py gains a refusal + valid-path pair in the phase-5 class. Full suite green.
| # otherwise be blamed on its promoted dtype. Look for one directly | ||
| # rather than probe-casting (issue #194 review). | ||
| if arr.dtype.kind in "fO": | ||
| flat = np.atleast_1d(np.asarray(values, dtype=object)).ravel() |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — the object-dtype probe is now on the bulk UNIQ path, where it is both expensive and (for the common input) incapable of firing.
np.asarray(values, dtype=object) on an ndarray of float64 yields Python floats, so isinstance(v, int) is never true:
>>> any(isinstance(v, int) for v in np.asarray(np.array([1.0, 2.0]), dtype=object).tolist())
False
Only a list/tuple input can carry an oversized Python int into a float64 promotion — which is exactly the case the comment above describes. For an ndarray the whole tolist() materialization is dead work in front of a refusal it cannot change. Measured on 7f2c5bf:
5M-element float64 ndarray -> unique2parent: 0.333 s to the ValueError
2M-element float64 ndarray -> tracemalloc peak 96 MB
This was latent in _as_offsets (offsets arrays are n_groups+1 long), but phase 5 puts it on unique2parent / uniq2geo, whose inputs are per-observation UNIQ columns — and "a UNIQ column that came back float from a parquet/pandas read" is the single most likely way a caller trips this validator. A one-line gate keeps the message and drops the cost:
if arr.dtype.kind == "O" or not isinstance(values, np.ndarray):(or check arr.dtype.kind == "O" plus a list/tuple test — anything that skips the object round-trip when the caller already handed over a typed float ndarray).
There was a problem hiding this comment.
🤖 from Claude
Folded in 40324c8 — the probe is gated, on the input's own dtype rather than on isinstance(values, np.ndarray):
if arr.dtype.kind == "O" or getattr(values, "dtype", None) is None:Same effect for the case you measured, and it also skips the round-trip for a float pandas.Series (no ndarray instance, but a dtype that likewise cannot hide a Python int). Only an untyped container — the one shape that can carry an oversized int into a float64 promotion — and a genuine object array still get probed.
Measured on the same 5M float64 column: 0.333 s → 0.000 s to the identical unique must be integer-typed, got dtype float64. All existing refusal texts are unchanged; the big-Python-int cases ([0, 10**40] list, [16, 10**19] list, object array) still name the value.
Pinned by test_i64_skips_the_object_probe_for_typed_float_input, which spies on np.asarray(..., dtype=object) and asserts zero probes for a float ndarray and exactly one for an untyped list — a behavioural pin rather than a timing one.
| this family's own message: no numpy cast error or warning (strings, | ||
| ``None``, ``NaN``) is allowed to surface in its place. | ||
| """ | ||
| return np.ascontiguousarray(_as_i64(offsets, "offsets").ravel()) |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — the delegation quietly drops _as_offsets' guaranteed private copy. The pre-phase-5 body ended np.ascontiguousarray(arr.astype(np.int64).ravel()) — astype defaults to copy=True, so the result was always a fresh, writeable buffer the caller could not reach. _as_i64 ends arr.astype(np.int64, copy=False), so an already-int64 C-contiguous input now comes back as a view aliasing the caller's array, and a read-only input yields a read-only result:
>>> src = np.array([0, 2, 4], dtype=np.int64)
>>> np.shares_memory(src, _as_offsets(src))
True
>>> ro = np.array([0, 3, 6], dtype=np.int64); ro.flags.writeable = False
>>> _as_offsets(ro).flags.writeable
False
No current consumer breaks — I checked all seven call sites (batch.py:175/472/579/655/764, geometry.py:427, _toc.py:419); none mutate the array or retain it past the call, and the Rust kernels take it read-only (polygons_to_morton_mocs and toc_reduce both accept a non-writeable offsets array on this commit). So this is arguably a free copy elimination.
But it is an unannounced contract change on a validator whose docstring (unchanged in this diff) still reads like a normalized owned buffer — "Returns: The offsets as a contiguous 1-D int64 array" — and the batch kernels run over it with the GIL released. Either say so in the _as_offsets Returns/Notes ("may alias the input when it is already contiguous int64"), or keep copy=True at this seam and let _as_i64 stay the no-copy primitive.
There was a problem hiding this comment.
🤖 from Claude
Took the documentation option in ca710fc — the copy stays eliminated, the contract now says so.
_as_offsets' Returns section reads:
Not a private copy: like
_as_u64and_as_i64this is a no-copy validator, so an input that is already contiguousint64comes back as a view aliasing the caller's array (a read-only input therefore yields a read-only result). No consumer mutates or retains it — the ragged kernels read it under a released GIL and take it read-only — so the buffer is validated, not owned.
That keeps _as_i64 the single no-copy primitive of the family rather than giving one of its two wrappers a different ownership rule, and it records your seven-call-site audit (batch.py:175/472/579/655/764, geometry.py:427, _toc.py:419) as the reason the aliasing is safe. If a future consumer does need to own the buffer, the fix belongs at that call site (np.array(...)), not in a validator that every ragged kernel pays for.
| normed, parent, order = mortie.mort2norm(words) | ||
| assert normed.tolist() == [11, 7] | ||
| assert parent.tolist() == [0, 3] | ||
| assert np.atleast_1d(order).tolist() in ([4], [4, 4]) |
There was a problem hiding this comment.
🤖 from Claude (review)
should-fix — phase 5 changed three entry points' intakes and added zero golden entries, but the CHANGELOG's blanket claim still covers them: "Valid inputs are unaffected — byte-identity is pinned against pre-change goldens (mortie/tests/data/strict_validation_goldens.json, captured at 4900a7e)", and the PR body says the capture holds "the touched entry points' answers".
The committed goldens have 48 keys and none of them is unique2parent, uniq2geo, or norm2mort:
clip2order, common_ancestor(_ragged), compress_moc, from_gps_ns, from_wkb_ragged,
generate_morton_children_{array,scalar}, infer_order_from_morton, is_point,
moc_*, mort2bbox, mort2geo, mort2norm_{normed,parent,order}, mort2polygon,
morton_buffer(_meters), orders_of, polygons_to_morton_mocs, span2toc,
split_base_cells_values, split_children_roots, time2toc, to_datetime64,
to_gps_ns, toc2time, toc_*, validate_morton, words, words_b
norm2mort appears in generate_strict_goldens.py only as a fixture builder (lines 74-78), never as a captured entry. The precedent is right there in this PR's own history — fold 16d43ac "extend the pre-change goldens to the toc and convert seams" did exactly this for phase 3's convert surfaces, and re-captured from 4900a7e.
The in-class substitutes do not close the gap evenly. test_unique2parent_valid_ints_unchanged pins [0, 1] and test_norm2mort_valid_roundtrip_unchanged roundtrips, but uniq2geo's valid path has no numeric pin at all — see test_uniq2geo_valid_matches_unique2parent_cells above, which only asserts shape and isfinite. That is the one phase-5 function whose output is float coordinates through a group-by-order dispatch, i.e. the one where a silent numeric shift would be hardest to notice.
Either add the three entries to the capture (re-run from 4900a7e as the fold did), or narrow the CHANGELOG/PR-body claim to the phases the goldens actually cover.
There was a problem hiding this comment.
🤖 from Claude
Folded in f21eb65 — the capture is extended, re-run from the pre-change source, not narrowed.
Same technique as the 16d43ac fold: git worktree add <tmp> 4900a7e, this venv's _rustie.abi3.so dropped into that tree's mortie/, generator run with PYTHONPATH pointing at it (mortie.__file__ verified to resolve there). Four entries added, 48 → 52:
norm2mort— the three mixed-hemisphere order-4 words the fixtures already build (base cells 0/8/11, so bit-63 words are in the pin).norm2uniq— added alongside, since the fold above put it on the same seam.unique2parentanduniq2geo— over a mixed order-4/order-5 UNIQ column, souniq2geo's group-by-order dispatch (the case you flagged as hardest to notice) is what gets pinned, not a single-order path.
All 48 previously committed entries reproduced byte-identically from the 4900a7e run — the JSON diff is 35 insertions(+), no modified lines — which is the same independent confirmation the earlier fold relied on. The four new values are pre-change captures, and they happen to equal the current answers, which is the point of the pin.
Docstring claims fixed with it: the generator's coverage paragraph now names the phase-5 entry points (and says "every phase" rather than "all three phases"); the PR body's testing bullet says 52 entries, phases 2–5, and describes both fold re-captures. The CHANGELOG's blanket byte-identity claim now holds as written.
uniq2geo's valid path also gained a real numeric pin in the test file — see the reply on the nit below (a7c1154).
| def test_uniq2geo_valid_matches_unique2parent_cells(self): | ||
| lat, lon = mortie.uniq2geo(np.asarray([16, 20])) | ||
| assert lat.shape == lon.shape == (2,) | ||
| assert np.isfinite(lat).all() and np.isfinite(lon).all() |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — two of the three valid-path tests in this class pin less than their names promise.
test_uniq2geo_valid_matches_unique2parent_cells never matches anything against unique2parent: it asserts a shape and isfinite, both of which would hold for any pair of arrays the function happened to return. Cells 16 and 20 are order-1 base-cell-0/1 centres with fixed coordinates — pinning the two (lat, lon) values (or comparing the decoded base cells against unique2parent([16, 20]), as the name says) costs one line and actually detects a shift.
And on line 486, assert np.atleast_1d(order).tolist() in ([4], [4, 4]) accepts either form, so it pins nothing about mort2norm's order return — the array/scalar distinction is a documented contract elsewhere in this module (issue #187, the rule the is_scalar comment in norm2mort cites). Pick the one that is correct for a 2-element input and assert it.
There was a problem hiding this comment.
🤖 from Claude
Both strengthened in a7c1154.
test_uniq2geo_valid_matches_unique2parent_cells now does what its name says, three ways: it asserts unique2parent([16, 20]) == [0, 1] (the base cells the name claims), compares the coordinates element-wise against mort2geo(norm2mort([0, 0], [0, 1], 1)) — the same two cells through the morton decoder — and pins the literals lat == 19.552022266396 (atol 1e-11) and lon == [45.0, 135.0]. A shift through the group-by-order dispatch now fails on the literals even if both decoders shifted together.
For the order return: mort2norm's Returns is explicit — "order : int … always a python int, since the words must share one order" — so the two-element case has exactly one correct form. The assertion is now assert order == 4 and isinstance(order, int), with a comment citing that contract line. (np.atleast_1d(order).tolist() in ([4], [4, 4]) would have passed for an array return, i.e. for a regression of issue #187's rule.)
uniq2geo's valid answer is additionally pinned in the pre-change goldens as of f21eb65, over a mixed order-4/order-5 column.
| | `batch` | `polygons_to_morton_mocs` and every ragged kernel behind the `offsets=` forms | | ||
| | `geometry` | `from_wkb(offsets=)`; the `to_geometry` / `to_wkb` / `to_wkt` word intake — validated at the shared seam, so **both** `dissolve` arms refuse alike (the default `dissolve=True` route through `dissolve` included) | | ||
| | `orders` | `generate_morton_children`, `clip2order`, `orders_of`, `is_point`, `infer_order_from_morton`, `validate_morton` | | ||
| | `convert` | `mort2norm`, `mort2geo`, `mort2bbox`, `mort2polygon` (and `mort2healpix` through them); the UNIQ/normed intakes `unique2parent`, `uniq2geo`, and `norm2mort`'s `normed`/`parent` (phase 5, espg ruling 2026-08-24 — `unique2parent([16.5, 20.9])` used to truncate to UNIQ 16/20 and answer `[0, 1]`; UNIQ ids stay int64-domain, so negatives keep their own `Not a valid UNIQ` refusal downstream) | |
There was a problem hiding this comment.
🤖 from Claude (review)
nit — the row's negatives clause under-claims for norm2mort. "UNIQ ids stay int64-domain, so negatives keep their own Not a valid UNIQ refusal downstream" is true of unique2parent / uniq2geo, but norm2mort took _as_u64, so its negatives are now refused up front with a new message — a behaviour change the register does not record.
Before (4900a7e body, replayed):
>>> norm2mort(np.asarray([-3], dtype=np.int64), 0, 4)
thread '<unnamed>' panicked at mortie-core/src/decimal_morton.rs:497:5:
nested index 18446744073709551613 too large for depth 4 (base 72057594037927935 > 11)
ValueError: nested index 18446744073709551613 too large for depth 4 ...
After:
ValueError: normed must be non-negative, got -3
Strictly better (it also stops the stderr panic dump), and it is genuinely part of the #185 panic-arc story this entry opens with — worth half a clause rather than being covered by a sentence that says the opposite. Something like "…negatives keep their own Not a valid UNIQ refusal downstream, while norm2mort's unsigned normed/parent now refuse them by name instead of reaching the kernel's panic".
There was a problem hiding this comment.
🤖 from Claude
Folded in bdf42b2. The clause now reads:
UNIQ ids stay int64-domain, so negatives keep their own
Not a valid UNIQrefusal downstream, while the unsignednormed/parentoperands refuse them by name up front —normed must be non-negative, got -3wherenorm2mortused to wrap to18446744073709551613and panic inside the kernel (the #185 arc again, stderr dump included), and wherenorm2uniq(-3, 0, 4)used to answer UNIQ1021, a real order-3 cell in base 11, with no error anywhere downstream.
Split by operand rather than left as one sentence, since the two halves now genuinely differ: the int64-domain UNIQ ids keep the downstream domain message, the unsigned normed/parent operands refuse up front. The norm2uniq half comes from the fold at 01dbf36 (thread above), and both are pinned by test_norm2mort_refuses_negative_naming_value / test_norm2uniq_refuses_negative_naming_value.
|
🤖 from Claude (review) nit (PR body, not diff-anchorable) — the inventory tally no longer reconciles with the table it summarizes.
That is 4 + 13 + 1 + 1 = 19, but the table has 17 data rows. Counting by the "after" column:
Phase 5 added a row to the table; the summary line looks like it was incremented rather than recounted. The |
|
🤖 from Claude Recounted and rewritten in the PR body (no commit — body edit only), in reply to the tally nit on Your 5/10/1/1 was right against the table as it stood; the fold above moved
The |
|
🤖 from Claude CodSpeed note (not chased, on record): the Performance Analysis check on Worth a look when reading the CodSpeed report though, because this PR does add real work to hot intakes: |
1 participant
Closes #194.
Unifies the family's input-validation posture on the strict form, per the ruling recorded on the issue (option (a), confirmed 2026-08-24; trigger fired by PR #195's merge,
4900a7e): float-typed word/offset arrays are refused rather than truncated, out-of-range values are refused before any narrowing cast rather than wrapped, and every refusal names the parameter and the offending value — the toc module's existing_as_offsets/_as_u64discipline, hoisted to a shared home (mortie/_validate.py) and applied at the polymorphic choke points from #187/#195, retiring the batch family's silentnp.asarray(..., dtype=np.int64)coercions.The posture caught a live in-repo bug while landing.
test_mort_inverse.py::TestMort2Geo::test_array_inputbuilt its morton array from a mixed Python-int list (np.array([int(w1), int(w2), int(w3)])); the base-8 word tops int64, so numpy promoted the whole list to float64, and the old silent cast then zeroed every word's suffix bits — three order-6 cells silently decoded as order-0 base cells, and the test (which only checks lengths) passed anyway. Repro on4900a7e:0x3848000000000006 → 0x3848000000000000(all three words corrupted). The strict validators refuse that array outright; the test now pinsdtype=np.uint64with a comment recording the incident. That is the #185/#192 bug class, live in our own tree.Phases
598d31a).mortie/_validate.pyhoists_as_u64/_as_offsetsfrom_toc.py(which now imports them), with three deliberate deltas: the non-negative refusal names the first offending value (additive; existing message pins still match); zero-size input of any dtype passes as a typed empty — the rulingTocalready applied to itssource("it is not numeric, it is empty"), now uniform; and an oversized Python-int offset is named as out-of-int64-range rather than refused as float64 (numpy's promotion would otherwise degrade the message). Validator unit tests pin the float refusal, negative-value naming, the ≥2^63 offset wrap refusal (the PR Segmented toc reduce: tocs_reduce (issue #177 v1) #192 class, by value), and that bit-63 words (base cells 7–11, spec §1) survive unchanged.4d50dee).batch.py(polygons_to_morton_mocs,_mocs_to_orders,_mocs_and,_mocs_intersect,_common_ancestors,_children_of),_moc.py(all 11 public operators, both arms),geometry.pyfrom_wkb(offsets=),orders.py(generate_morton_childrenscalar arm,clip2order,orders_of,is_point,infer_order_from_morton,validate_morton). Per-entry-point refusal tests (29 word seams × float + negative; 7 offsets seams × float + uint64-wrap) + byte-identity against pre-change goldens.67d52fa).convert.py(mort2norm,mort2geo,mort2bbox,mort2polygon),buffer.py(both),moc_object.py(Mocword sources + operands; its docstring had documented the wrap),geometry.pyto_geometry/to_wkb/to_wktword intake,prefix_trie.pysplit_children(floats refused; signed bit-view kept — see Questions). Includes thetest_array_inputfix above.unique2parent,uniq2geo, andnorm2mort'snormed/parentadopt the shared validators — a new signed_as_i64(which_as_offsetsnow delegates to) for the int64-domain UNIQ ids,_as_u64fornormed/parent. The pinned regression:unique2parent(np.asarray([16.5, 20.9]))returnedarray([0, 1])(16.5/20.9 truncated to UNIQ 16/20); it now raisesunique must be integer-typed, got dtype float64. Negative UNIQ ids stay int64-representable, so the domain check keeps its ownNot a valid UNIQ cell number … -5message — pinned too. The phase-5 review fold extended the seam to the two UNIQ surfaces left behind:norm2uniq(the documented producer of UNIQ ids —norm2uniq(-3, 0, 4)answered1021, a real order-3 cell in base 11, andnorm2uniq(11.5, 0, 4)answered a float UNIQ whose refusal only landed two calls later, blaminguniq) andorders_of_uniq(whose guard was value-based, so an integral float column still decoded).Inventory (the review artifact)
Every public entry point accepting packed-word arrays or arrow offsets, and its posture before this PR:
toc_reduce,toc_and/…,time2toc,span2toc,toc2time,to/from_gps_ns,to_datetime64(_toc.py)_as_u64/_as_offsets)decimal_to_wordarray form (morton_index.py)rank_to_xy/xy_to_rank(rank_xy.py)orders_of_uniq(UNIQ ints) (orders.py)orders_of_uniq([16.0])still decoded to order 1 whileunique2parent([16.0])refused the same column)_as_i64)unique2parent,uniq2geo(UNIQ ints),norm2mort+norm2uniq(normed ints)norm2uniqadded in the fold)moc_to_order,moc_and,moc_intersects,common_ancestor(both arms),moc_or/minus/xor/min/not,compress_moc,split_base_cells(_moc.py)polygons_to_morton_mocs(batch.py)from_wkb(offsets=)(geometry.py)generate_morton_children,clip2order,orders_of,is_point,infer_order_from_morton,validate_morton(orders.py)mort2norm,mort2geo,mort2bbox,mort2polygon(+mort2healpixvia them) (convert.py)morton_buffer,morton_buffer_meters(buffer.py)to_geometry/to_wkb/to_wktword intake (geometry.py)Moc(...)/moc(...)word path + set-op operands (moc_object.py)split_children/morton_polygon_from_array(prefix_trie.py)arrow.pypyarrow skins (from_wkb,polygons_to_morton_mocs,to_morton_index)arrow.pyarray_likeintakes (from_morton_index,export_c_array)array_like, not typed columns)pandas.pyMortonIndexArrayEARecounted against the table as it stands (17 data rows, 4 + 11 + 1 + 1): strict already / unchanged — 4 rows (
toc_*,decimal_to_word,rank_to_xy/xy_to_rank, thearrow.pypyarrow skins). Posture changed — 11 rows (~39 coercion sites: the ~36 of phases 1–3 plus the phase-5 fold's three —norm2uniq's two operands andorders_of_uniq's float acceptance). Carve-out — 1 row (split_children). Deferred with rationale — 1 row (pandas.py's EA).Formerly scoped out — landed as phase 5 at espg's direction (in-session ruling, 2026-08-24: "if the fix is small, and it blocks us, and touches the same area, add it as a phase").
unique2parent,uniq2geoandnorm2morteach truncated a float throughnp.asarray(..., dtype=np.int64)beforeorders_of_uniq's own strict check could refuse it; phase 5 validates at those intakes with the shared validators, and the pre-existing UNIQ domain refusals (negative / out-of-range →Not a valid UNIQ cell number) keep their own message register downstream.How it was tested
mortie/tests/test_strict_validation.py(140 tests): per-entry-point refusals (float words, float offsets, negative words naming the value, ≥2^63 uint64 offsets naming the value, oversized Python-int offsets naming the value), plus byte-identity goldens:generate_strict_goldens.pycaptured the touched entry points' answers for a fixed valid input set (mixed-order words across northern and southern base cells, so bit-63 words are pinned) at4900a7e, before any validator was adopted — committed in phase 1, asserted after phases 2–5 (test_valid_paths_byte_identical_to_pre_change_goldensreplays the capture and compares all 52 entries). The capture was extended twice during the review folds — firstmort2bbox,mort2polygon,from_wkb(offsets=)and the whole toc family (whose validators moved house in phase 1), then the four phase-5 surfaces (norm2mort,norm2uniq,unique2parent,uniq2geo, the last over a mixed order-4/order-5 UNIQ column souniq2geo's group-by-order dispatch is exercised) — and each time re-captured from the4900a7esource itself, not from this branch; all 48 previously committed entries reproduced byte-identically against the pre-change capture, which independently confirms the committed JSON is the pre-change record. Two surfaces are deliberately out of the capture and say so in the generator's docstring:to_geometry/to_wkb/to_wktneed the shapely test extra (the generator stays numpy-only; they are pinned bytest_geometry.pyplus this PR's refusal rows), andtime2toc([])is the one valid answer this PR does not claim is unchanged.pytest(2008 passed, 16 skipped),flake8 --select=E9,F63,F7,F82,ruff --select E,F,W,I --ignore E501andnumpydoc linton touched files,codespellon new files.convert.py:895carries an unusedon_antimeridian(ruff F841), andtest_mort_inverse.py:4has an unsorted import block (ruff I001); both predate this PR and sit outside its diff context — flagged, not fixed, per convention.Questions for review
split_childrenkeeps the signed int64 bit-view carve-out as documented — the trie branches on the decimal characteristic, whose first column is the sign, and the golden fixtures pin the form. (Original question: whether to go strict-negative there too and move the fixtures touint64.)op([]) → []), reductions keep their empty-segment refusals. Espg flagged it for one more look before the 1.0 tag; the revisit note lives on issue Version 1.0 Sweep (Follow up to new morton datatype) #48 (the 1.0 tracking issue). No code change. (Original question: whether to refuse untyped empties family-wide, which would have broken documented moc empty paths.)pandas.pydeferred;arrow.pypicked up (corrected during the review fold). The original deferral claimed the arrow skins "receive dtype-carrying columns (strict by construction)". That is true offrom_wkbandpolygons_to_morton_mocs(pyarrow in, pyarrow out) but false offrom_morton_index(arrow.py:234) andexport_c_array(arrow.py:700), whose docstrings documentarray_like— a raw numpy array carries whatever dtype the caller gave it. Both silently demoted an order-4 word to order 0 on float input,export_c_arraydoing so on the way across an FFI boundary, so both now take_as_u64. What remains deferred is onlypandas.py's EA, whose intake is governed by the pandas casting contract (_from_sequenceetc.) where refusing floats interacts withastyperound-trips — a genuinely separate question, and a follow-up issue can pick it up.Parallel-work note: #152 (MortonIndexScalar constructor) and #176 (docs pages) are in flight; this PR does not touch
morton_index.pyand touches docs only where a docstring documented the old truncate/wrap behavior.decimal_to_wordneeded no change (already strict).