Hive virtual store: moczarr-backed /gridlook/hive/ (phase 6d of issue #1)#6
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
| "local-path hive stores are disabled — set " | ||
| "GridlookProxy.allow_local_hive_stores = True (development only)", | ||
| ) | ||
| root = store_url.rstrip("/") |
There was a problem hiding this comment.
🤖 from Claude (review)
Unconstrained local-path oracle behind the dev flag. With allow_local_hive_stores=True, store= reaches any local path — probed /etc, /etc/passwd, ~/.jupyter all get past the gate and into open_hive(). Two problems on a multi-user hub:
- The 400/404 bodies echo moczarr's messages verbatim (
{e}at L#L245/L250-ish), which distinguish'/etc/passwd' is not a directory(file exists) vsno morton_hive.json at /etc(dir exists, no manifest) vs a missing path → a filesystem existence/type oracle for an authenticated user. - Any real hive store anywhere on the box (another user's
$HOME, a shared scratch dir) is openable and servable — cross-user read, not just a dev convenience.
Default-off + documented-dev-only mitigates, but it's a full local-file reach the moment the flag flips. Suggest a rooted-directory trait (an allowlist of permitted roots + os.path.realpath containment check) so "dev" can't silently become whole-filesystem access. Probed with the test client; not blocking, but worth a rooted variant.
There was a problem hiding this comment.
🤖 from Claude
Replaced the boolean with a rooted-directory trait local_hive_store_roots (empty = disabled): a local path is now accepted only when its os.path.realpath is contained in a configured root, and anything outside gets a single uniform 403 ("not within an allowed root") with no existence/type-distinguishing body, closing the filesystem oracle — d6dd5f2.
| level = dggs.get("refinement_level") | ||
| if level is None: | ||
| level = ds.attrs["morton_hive"]["cell_order"] | ||
| dggs.update({"name": "healpix", "refinement_level": int(level), "coordinate": "cell_ids"}) |
There was a problem hiding this comment.
🤖 from Claude (review)
refinement_level is taken from cell_order with no float64-exact / point-kind guard. The frontend computes nside = 2**refinement_level (Healpix.vue L185) and does all cell math in JS doubles. But moczarr's fabricate_cell_ids clips POINT-kind words to order 24 (FLOAT64_EXACT_MAX_ORDER) and only warns for AREA cells above 24 — and that warning is emitted on the executor thread, so it's swallowed, never reaching the operator or the 200 response. Consequences the fixture (order 8) never exercises:
- cell_order > 24 area store: served
cell_idsare exact u64 the browser reads as lossy Numbers, and12*nside*nsideoverflows 2^53 → silently wrong render, HTTP 200, no signal. - point-kind store where cell_order ≠ 24: shim advertises
refinement_level = cell_order(e.g. 29) while the fabricated ids are clipped to order 24 → declared nside ≠ served ids' order → cells placed at the wrong scale.
Since this store exists only to feed a float64 browser consumer, recommend guarding cell_order > 24 at open (pointed 400/413) rather than serving an unrenderable/mismatched view. At minimum, document the ceiling here.
There was a problem hiding this comment.
🤖 from Claude
The shim now derives refinement_level from the SERVED ids (point words clip to order 24, so a point store declares 24 and renders correctly instead of at a mismatched nside=2**29), and area stores whose ids exceed the float64-exact range (order > 24, or any id ≥ 12·4²⁴) are rejected at /open with a pointed 422 rather than served — 0ad2923.
| store_url: str, product: str | None, window: str | None, aoi: tuple[str, ...] | None | ||
| ) -> str: | ||
| payload = json.dumps( | ||
| [store_url, product, window, list(aoi) if aoi else None], separators=(",", ":") |
There was a problem hiding this comment.
🤖 from Claude (review)
View id isn't normalized → trivial LRU blowup / redundant materialization. Probed against the SERC fixture:
aoi=4331421,4331422vsaoi=4331422,4331421→ different view ids, identical 32 cellsaoi=4331421,4331421,4331422(dup) → a third id, same 32 cellsstore=…/serc_hivevs…/serc_hive/(trailing slash) → different ids, same store
Every distinct id is a full re-open (S3 GETs + concat) and consumes an LRU slot, so a client varying token order / duplicates / trailing slash multiplies work and evicts legitimately-open views — an easy way to thrash the cache out from under other users. build_view already rstrips the root for the data path, but the hash keys on the raw store_url + raw aoi tuple. Canonicalize before hashing: rstrip store_url, and sorted(set(int(t) for t in aoi)) so order/dupes collapse to one id.
There was a problem hiding this comment.
🤖 from Claude
view_id now canonicalizes before hashing — rstrip the store, normalize empty params to absent, and sorted(set(int(t) ...)) the aoi — so reordered/duplicated tokens and a trailing slash all collapse to one id (unit-tested) — 89d9fa7.
| max_cells=proxy.hive_max_cells, | ||
| ) | ||
| try: | ||
| view = await IOLoop.current().run_in_executor(None, build) |
There was a problem hiding this comment.
🤖 from Claude (review)
The advertised memory bound covers resident views only; concurrent in-flight builds are uncapped. Real footprint on the fixture is 52 B/cell → 26 MB per 500k-cell view, ~208 MB across 8 (so the config's ~100 B/cell is a safe 2× over-estimate — good). But build_view is dispatched to the default ThreadPoolExecutor (run_in_executor(None, …), up to ~min(32, cpu+4) threads) with no concurrency limit, and each in-flight build holds the xarray ds plus the MemoryStore copy transiently. N concurrent large-view opens ⇒ N×(ds+store) on top of the 208 MB resident cache — so "hive_max_views × hive_max_cells bounds the cache footprint" (config help + PR body) understates peak on a 2–4 GB pod under a burst of opens. Consider a small bounded executor or a semaphore around opens, or at least document that the bound is resident-only.
There was a problem hiding this comment.
🤖 from Claude
Added a hive_max_concurrent_builds trait (default 2) and a per-loop module asyncio.Semaphore around the executor offload, so a burst of opens queues rather than holding N×(ds+store) transiently on top of the resident cache; the config help now says the cell bound is resident-only, and a test opens 4 at once and all succeed — 27862be.
| dggs.update({"name": "healpix", "refinement_level": int(level), "coordinate": "cell_ids"}) | ||
| dggs.setdefault("spatial_dimension", "cells") | ||
| ds.attrs["dggs"] = dggs | ||
| ds.attrs.setdefault("zarr_conventions", [_DGGS_CONVENTION_ENTRY]) |
There was a problem hiding this comment.
🤖 from Claude (review)
Shim rewrites the dggs block to healpix but leaves the stored morton-dggs entry in zarr_conventions. Probed the served root zarr.json: zarr_conventions carries both the generic dggs entry and the stored morton-dggs ("Packed-u64 morton (HEALPix) DGGS convention") entry, while the dggs block now says name: healpix, coordinate: cell_ids. Today's detector only checks that the zarr_conventions key exists then reads the dggs block (gridTypeDetector.ts determineGridTypeFromZarrConvention), so this is harmless now — but the served store is internally contradictory: it claims conformance to the morton-dggs convention while serving a healpix block against a cell_ids coordinate. A stricter/future validator that keys on the convention entries would trip. Since the shim already owns the block rewrite, consider dropping (or replacing) the morton-dggs entry here too so the envelope and the block agree.
There was a problem hiding this comment.
🤖 from Claude
The shim now drops the stored morton-dggs entry from zarr_conventions (keeping the generic dggs registry entry) so the served envelope and the healpix-flavored block agree, and preserves the store’s own morton block verbatim under a private _gridlook_source_dggs attr for provenance — 1077815.
|
🤖 from Claude (review) Fresh-context adversarial review of phase 6d (the moczarr-backed Inline findings (severity-ordered)
Minor (not posted inline)
Verified sound
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
…(issue #2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
|
🤖 from Claude Acknowledging the two minor (non-inline) notes from the review summary:
|
this would be nice, and is easier now that moczarr's on pypi... |
Refs #1 (phase 6d)
The reserved
/gridlook/hive/namespace becomes real: a moczarr-backed virtual store that serves anopen_hive()product/AOI/window selection as ONE flat zarr v3 store the browser's unmodified zarrita reads. This is the cryocloud-MVP slice from the MVP map: post-englacial/zagg#314 stores are morton-only, so moczarr's fabricated NESTEDcell_ids(fabricate_cell_ids="auto") is what makes them renderable at all — it rides every view served here.What this adds
GET /gridlook/hive/open?store=<url>[&product=…][&aoi=<decimal,csv>][&window=…]— creates (or LRU-refreshes) a session-scoped view, returns{view, url, cells, cell_order, cached}; theurlis the zarr source to paste into gridlook.GET /gridlook/hive/<view-id>/<zarr-key>— serves the view's zarr objects (metadata documents + whole small chunks; no Range needed).@web.authenticatedeverywhere,s3://stores gated by the sameGridlookProxy.allowed_bucketsallowlist (403 otherwise), local paths only behind the newallow_local_hive_storesdev flag.hive_max_views(LRU bound, default 8),hive_max_cells(per-view bound, default 500 000 — a pointed 413 tells you to narrow the AOI),allow_local_hive_stores.gridlook-jupyter[hive]extra carryingmoczarr @ git+https://github.com/espg/moczarr(until PyPI;tool.hatch.metadata.allow-direct-referencesgates the direct reference). The base install stays moczarr-free;hive.pyimports it lazily and the open endpoint 500s with a pointed install hint when it's absent.Materialize-on-open (the implementation lean, justified)
open_hive(...)→ds.to_zarr(zarr.storage.MemoryStore(), zarr_format=3, consolidated=False)per view, LRU-bounded. Chosen because: (a) views are AOI-scale and bounded byhive_max_cells, so the memory cost is capped and known (hive_max_views × hive_max_cells); (b) xarray writes the store, so the serve path contains zero zarr chunk/codec arithmetic — the handler serves opaque objects via the store's asyncget; (c) every request after open is a dict lookup. Chunks are written uncompressed (compressors: None): objects go whole over hub-local HTTP, and codec-free bytes are trivially predictable (the tests compare them raw). A streaming/virtual encoding — computing zarr objects on demand from the open dataset — is documented inhive.pyas the future optimization if views outgrow memory; the URL contract wouldn't change.View ids are deterministic (hash of the request tuple), so re-opening the same selection refreshes the existing view instead of duplicating it; serving objects LRU-bumps the view, so actively-rendered views survive eviction pressure.
Pre-6c compatibility shim (explicit, temporary)
The served root attrs do not advertise the stored morton convention block (
{name: "morton", coordinate: "morton"}, mortie spec §5) — gridlook'sgridTypeDetector.tsrejects anydggs.name != "healpix"today. Instead the view serves the healpix-compatible block the detector andHealpix.vueread now:name: "healpix",refinement_level(→ nside),coordinate: "cell_ids". That is honest at the data level — the fabricated NESTED ids are plain HEALPix NESTED indices at that refinement level, bit-for-bit what the existing sparse limited-area path consumes — so the MVP renders without phase 6c. The shim is marked inhive.py(_shim_dggs_attrs) and goes away when 6c lands the morton detector branch.Tests (50 pass locally; hive suite skips without moczarr)
pytest-jupyter, mirroring the phase-4 suite, against moczarr's committed SERC fixture (zagg-written, morton-only) as a local-path store:
zarr.jsoncarries the shimmed healpix attrs blockcell_idschunk byte-equalsserc_cell_ids_golden.npy(the last dual-written store's stored NESTED ids — fabricate-at-serve loses nothing)fabricate_cell_idsof the served morton words)hive_max_cells, LRU eviction incl. refresh-bumps-recencyGRIDLOOK_MOCZARR_TESTDATA); plain[test]envs skip the module wholesale — existing CI is untouched and stays greenGates run:
pytest -q→ 50 passed (env:uv venv && uv pip install -e '.[test]' -e /Users/espg/software/moczarr); plain[test]-only env → 35 passed, 1 module skipped;ruff check+ruff format --checkclean.MVP recipe (documented in jupyter/README.md)
pip install <wheel> "gridlook-jupyter[hive]"on the hub image/user envc.GridlookProxy.allowed_buckets = ["my-zagg-outputs"](orGRIDLOOK_ALLOWED_BUCKETS), restart the serverGET …/gridlook/hive/open?store=s3://my-zagg-outputs/store-root&aoi=4331422…/gridlook/, paste the returned viewurlas the dataset source — renders through the existing sparse-HEALPix pathQuestions for review
.github/workflows/jupyter.ymlinstalls only[test], so the hive suite skips there (left untouched per the no-CI-edits convention). If you want it exercised in CI, the job needs a moczarr checkout + editable install step — say the word and it lands as a follow-up.hive_max_cellsdefault (500 000) is sized as ~40–50 MB/view at SERC-like variable counts; happy to retune.[hive]extra's git pin is unversioned (@ git+…HEAD); once moczarr tags/publishes, it becomes a normalmoczarr>=Xpin andallow-direct-referencesdrops.🤖 Generated with Claude Code
https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax