From 8bc3fcc3be0def45d5b2b0bc81cdf3835d628d7c Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Sat, 31 Oct 2026 09:36:32 +0100 Subject: [PATCH 1/6] feat(registry): map every Presentation route and state where a frame refusal lands --- python/spec-kit/CHANGELOG.md | 26 +++- .../src/dfo_spec_kit/registry/codes.json | 118 +++++++++++++++++- registries/codes.json | 118 +++++++++++++++++- schemas/registry/README.md | 2 +- schemas/registry/code-registry.schema.json | 6 + tools/tests/test_validate_registry.py | 3 + 6 files changed, 259 insertions(+), 14 deletions(-) diff --git a/python/spec-kit/CHANGELOG.md b/python/spec-kit/CHANGELOG.md index 6c5e2eb..45f1745 100644 --- a/python/spec-kit/CHANGELOG.md +++ b/python/spec-kit/CHANGELOG.md @@ -98,6 +98,15 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `edit_conflict` (409), `internal_error` (500), `release_mismatch` and `service_unavailable` (503), and `timeout` (504), at the statuses the components routes give them. +- Endpoints `presentation.frames`, `presentation.component_read` and + `presentation.component_data` in the registry schema, with existing codes + in `registries/codes.json` and its bundled mirror at the statuses the + routes serve. All three carry `request_invalid` (400), `internal_error` + (500), `service_unavailable` (503) and `timeout` (504). + `presentation.frames` adds `binding_invalid` (400) and `frame_unavailable` + (404). `presentation.component_read` adds `not_found` (404), `edit_conflict` + (409) and `release_mismatch` (503). `presentation.component_data` adds + `not_found` and `frame_unavailable` (404) and `edit_conflict` (409). ### Changed @@ -113,11 +122,18 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `release_mismatch` at 503, the statuses the Presentation write routes serve. `presentation.render` keeps 422 for the first two and 409 for the rest. `frame_unavailable` covers a frame that is not held in a state - Presentation can load, and its definition names each case. A frame whose - stored artifact the store fails to read is also `frame_unavailable`, served - at 503 and retryable; the registry holds one status for each endpoint and - channel, so it records the 404 the write routes serve for every other - case. + Presentation can load, and its definition names each case. A pin of a + derived frame with no recipe yet is `frame_unavailable` at 404 with + `cause.code` `recipe_pending` and is retryable, because a recipe write lets + a retry pin. A store that fails to answer is `service_unavailable` at 503, + and the definition of `service_unavailable` names it. A pin of a derived + frame whose recipe cannot be read is `internal_error` at 500 and is not + retryable, since recipes are write-once and a retry cannot clear it. The + definition of `release_mismatch` adds a read of a stored payload whose + shape the release does not recognise. +- Code `unsupported_schema_version` maps to `presentation.render` and + `presentation.shares` at 422, for a request body that carries + `schema_version`. - Code `edit_conflict` maps to `presentation.render` at 409, for a render of a component that has no current revision, and its definition names that case. diff --git a/python/spec-kit/src/dfo_spec_kit/registry/codes.json b/python/spec-kit/src/dfo_spec_kit/registry/codes.json index b35f039..e700f69 100644 --- a/python/spec-kit/src/dfo_spec_kit/registry/codes.json +++ b/python/spec-kit/src/dfo_spec_kit/registry/codes.json @@ -236,6 +236,11 @@ "endpoint": "presentation.shares", "channel": "problem_details", "http_status": 400 + }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 400 } ], "lifecycle": "active" @@ -1262,6 +1267,16 @@ "endpoint": "presentation.shares", "channel": "problem_details", "http_status": 409 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 409 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 409 } ], "lifecycle": "active" @@ -2008,7 +2023,7 @@ { "code": "frame_unavailable", "classification": "refusal", - "definition": "The frame a request names is not held in a state Presentation can load: it was never stored or published, it has expired or been revoked, its spec is quarantined, or its stored artifact is missing, fails its digest check, does not decode, or holds more rows than Presentation loads.", + "definition": "The frame a request names is not held in a state Presentation can load: it was never stored or published, it has expired or been revoked, its spec is quarantined, or its stored artifact is missing, fails its digest check, does not decode, or holds more rows than Presentation loads. A pin of a derived frame with no recipe yet is refused with cause code recipe_pending and is retryable, because a recipe write lets a retry pin. A store that fails to answer is service_unavailable.", "owning_contract": "presentation", "owner": "platform", "introduced_in": "v1", @@ -2028,6 +2043,16 @@ "channel": "problem_details", "http_status": 404 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 404 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 404 + }, { "endpoint": "mcp.tools", "channel": "mcp_refusal" @@ -2190,7 +2215,7 @@ { "code": "internal_error", "classification": "refusal", - "definition": "An unclassified, non-retryable failure occurred while handling the request.", + "definition": "An unclassified, non-retryable failure occurred while handling the request. This includes a pin of a derived frame whose recipe cannot be read, since recipes are write-once and a retry cannot clear it.", "owning_contract": "platform", "owner": "platform", "introduced_in": "v1", @@ -2275,6 +2300,21 @@ "channel": "problem_details", "http_status": 500 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 500 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 500 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 500 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -2823,6 +2863,16 @@ "channel": "problem_details", "http_status": 404 }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 404 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 404 + }, { "endpoint": "datasets.export_link", "channel": "problem_details", @@ -4145,7 +4195,7 @@ { "code": "release_mismatch", "classification": "refusal", - "definition": "The request names a catalogue or grammar release this deployment does not serve.", + "definition": "The request names a catalogue or grammar release this deployment does not serve, or reads a stored payload whose shape this release does not recognise.", "owning_contract": "presentation", "owner": "platform", "introduced_in": "v1", @@ -4164,6 +4214,11 @@ "endpoint": "presentation.shares", "channel": "problem_details", "http_status": 503 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 503 } ], "lifecycle": "active" @@ -4350,6 +4405,21 @@ "channel": "problem_details", "http_status": 400 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 400 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 400 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 400 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -4878,7 +4948,7 @@ { "code": "service_unavailable", "classification": "refusal", - "definition": "The service cannot handle the request right now, typically because a dedicated capacity or connection pool has no room left; a retry is expected to succeed.", + "definition": "The service cannot handle the request right now, typically because a dedicated capacity or connection pool has no room left or a store it reads fails to answer; a retry is expected to succeed.", "owning_contract": "platform", "owner": "platform", "introduced_in": "v1", @@ -5031,6 +5101,21 @@ "channel": "problem_details", "http_status": 503 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 503 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 503 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 503 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -6212,6 +6297,21 @@ "channel": "problem_details", "http_status": 504 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 504 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 504 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 504 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -6443,6 +6543,16 @@ "channel": "problem_details", "http_status": 422 }, + { + "endpoint": "presentation.render", + "channel": "problem_details", + "http_status": 422 + }, + { + "endpoint": "presentation.shares", + "channel": "problem_details", + "http_status": 422 + }, { "endpoint": "graph.nodes", "channel": "problem_details", diff --git a/registries/codes.json b/registries/codes.json index b35f039..e700f69 100644 --- a/registries/codes.json +++ b/registries/codes.json @@ -236,6 +236,11 @@ "endpoint": "presentation.shares", "channel": "problem_details", "http_status": 400 + }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 400 } ], "lifecycle": "active" @@ -1262,6 +1267,16 @@ "endpoint": "presentation.shares", "channel": "problem_details", "http_status": 409 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 409 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 409 } ], "lifecycle": "active" @@ -2008,7 +2023,7 @@ { "code": "frame_unavailable", "classification": "refusal", - "definition": "The frame a request names is not held in a state Presentation can load: it was never stored or published, it has expired or been revoked, its spec is quarantined, or its stored artifact is missing, fails its digest check, does not decode, or holds more rows than Presentation loads.", + "definition": "The frame a request names is not held in a state Presentation can load: it was never stored or published, it has expired or been revoked, its spec is quarantined, or its stored artifact is missing, fails its digest check, does not decode, or holds more rows than Presentation loads. A pin of a derived frame with no recipe yet is refused with cause code recipe_pending and is retryable, because a recipe write lets a retry pin. A store that fails to answer is service_unavailable.", "owning_contract": "presentation", "owner": "platform", "introduced_in": "v1", @@ -2028,6 +2043,16 @@ "channel": "problem_details", "http_status": 404 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 404 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 404 + }, { "endpoint": "mcp.tools", "channel": "mcp_refusal" @@ -2190,7 +2215,7 @@ { "code": "internal_error", "classification": "refusal", - "definition": "An unclassified, non-retryable failure occurred while handling the request.", + "definition": "An unclassified, non-retryable failure occurred while handling the request. This includes a pin of a derived frame whose recipe cannot be read, since recipes are write-once and a retry cannot clear it.", "owning_contract": "platform", "owner": "platform", "introduced_in": "v1", @@ -2275,6 +2300,21 @@ "channel": "problem_details", "http_status": 500 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 500 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 500 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 500 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -2823,6 +2863,16 @@ "channel": "problem_details", "http_status": 404 }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 404 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 404 + }, { "endpoint": "datasets.export_link", "channel": "problem_details", @@ -4145,7 +4195,7 @@ { "code": "release_mismatch", "classification": "refusal", - "definition": "The request names a catalogue or grammar release this deployment does not serve.", + "definition": "The request names a catalogue or grammar release this deployment does not serve, or reads a stored payload whose shape this release does not recognise.", "owning_contract": "presentation", "owner": "platform", "introduced_in": "v1", @@ -4164,6 +4214,11 @@ "endpoint": "presentation.shares", "channel": "problem_details", "http_status": 503 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 503 } ], "lifecycle": "active" @@ -4350,6 +4405,21 @@ "channel": "problem_details", "http_status": 400 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 400 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 400 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 400 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -4878,7 +4948,7 @@ { "code": "service_unavailable", "classification": "refusal", - "definition": "The service cannot handle the request right now, typically because a dedicated capacity or connection pool has no room left; a retry is expected to succeed.", + "definition": "The service cannot handle the request right now, typically because a dedicated capacity or connection pool has no room left or a store it reads fails to answer; a retry is expected to succeed.", "owning_contract": "platform", "owner": "platform", "introduced_in": "v1", @@ -5031,6 +5101,21 @@ "channel": "problem_details", "http_status": 503 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 503 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 503 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 503 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -6212,6 +6297,21 @@ "channel": "problem_details", "http_status": 504 }, + { + "endpoint": "presentation.frames", + "channel": "problem_details", + "http_status": 504 + }, + { + "endpoint": "presentation.component_read", + "channel": "problem_details", + "http_status": 504 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 504 + }, { "endpoint": "datasets.exports", "channel": "problem_details", @@ -6443,6 +6543,16 @@ "channel": "problem_details", "http_status": 422 }, + { + "endpoint": "presentation.render", + "channel": "problem_details", + "http_status": 422 + }, + { + "endpoint": "presentation.shares", + "channel": "problem_details", + "http_status": 422 + }, { "endpoint": "graph.nodes", "channel": "problem_details", diff --git a/schemas/registry/README.md b/schemas/registry/README.md index ec64821..10c9c0e 100644 --- a/schemas/registry/README.md +++ b/schemas/registry/README.md @@ -61,7 +61,7 @@ Retiring a door is a schema change, reviewed as one: dropping a value from `code The Datasets endpoints are `datasets.register` (`POST /api/datasets/register`), `datasets.mint` (`POST /api/datasets`), `datasets.definition` (`GET /api/datasets/{id}`), `datasets.options` (`GET /api/datasets/{id}/options`, the write-free preview door), `datasets.members` (`POST /api/datasets/{id}/members`, the write-free member read), `datasets.snapshot` (`GET /api/datasets/{id}/{content_version}`, and `GET /api/datasets/{id}/versions`, which pages a dataset's servable versions), `datasets.files` (`GET /api/datasets/{id}/{content_version}/files/{digest}`, which answers a stored artifact with a 302 redirect to its download link, and `GET /api/datasets/{id}/{content_version}/file`, which selects a stored version's file by format, bundle and layout, renders it when it is missing, and answers the same redirect), `datasets.description` (`GET /api/datasets/{id}/{content_version}/description`, a stored version's description, for a `spec_` id with a `cv_` version or an `ops_` id with an `ocv_` version, and `GET .../description/values`, which pages one dimension's values), `datasets.observations` (`GET /api/datasets/{id}/{content_version}/observations`, a stored version's observations), `datasets.pin` (`POST /api/datasets/{id}/{content_version}/pin`), `datasets.publish` (`POST /api/datasets/frames/publish`, the door an analysis service uses to publish an already-computed frame under its own identity pair), `datasets.recipe` (`PUT` and `GET /api/datasets/frames/{spec_id}/{content_version}/recipe`, the recorded computation behind a published frame, and `GET /api/datasets/recipes/replays/{replay_key}`, the frame stored under a replay key), `datasets.refresh` (`POST /api/datasets/frames/refresh`, which rebuilds a dataset frame's rendering on current data), `datasets.operations` (`GET /api/datasets/operations/{operation_id}`), `datasets.analysis` (`POST /api/datasets/analysis`, which runs an analysis recipe tree and answers its derived frame, and `POST /api/datasets/analysis/anchor`, which resolves a recipe's latest period and stores nothing), `datasets.exports` (`POST /api/datasets/exports`, which prepares an export file of a stored frame and answers its facts), and `datasets.export_link` (`GET /api/datasets/exports/{export_id}`, which answers a 302 redirect to the export file's download link). - The Presentation endpoints are `presentation.write` (`POST /api/presentation/components`, `PATCH /api/presentation/components/{component_id}`) `presentation.render` (`POST /api/presentation/components/{component_id}/revisions/{revision}/render` and `GET .../render/data`, which draw a stored revision on a named target frame and mint no revision), and `presentation.shares` (the routes under `/api/presentation/shares` that create, replace and delete a share). Search's and Datasets' `schema`, `capabilities`, and `healthz` routes and Presentation's read routes carry no permanent codes. + The Presentation endpoints are `presentation.write` (`POST /api/presentation/components`, `PATCH /api/presentation/components/{component_id}`), `presentation.component_read` (`GET /api/presentation/components/{component_id}`), `presentation.component_data` (`GET /api/presentation/components/{component_id}/revisions/{revision}/data`, the rows behind one revision), `presentation.render` (`POST /api/presentation/components/{component_id}/revisions/{revision}/render` and `GET .../render/data`, which draw a stored revision on a named target frame and mint no revision), `presentation.frames` (`GET /api/presentation/frames/{spec_id}/{content_version}` and `GET /api/presentation/frames/{spec_id}`), and `presentation.shares` (every route under `/api/presentation/shares`). Search's and Datasets' `schema`, `capabilities`, and `healthz` routes and Presentation's `capabilities`, `healthz`, and `release` routes carry no permanent codes. The Graph endpoints are `graph.node` (`GET /api/graph/node/{dcid}`), `graph.nodes` (`POST /api/graph/nodes`, a batch of node reads), `graph.members` (`GET /api/graph/node/{dcid}/members`, one page of a node's list), `graph.roots` (`GET /api/graph/roots`), `graph.documentation` (`GET /api/graph/documentation/{dcid}`, a source's document), `graph.sources` (`GET /api/graph/sources`), and `graph.resolve` (`POST /api/graph/resolve`). `graph.node` and `graph.documentation` carry `node_not_held`, `request_invalid`, `service_unavailable`, and `upstream_error`; `graph.nodes` and `graph.resolve` carry `request_invalid`, `unsupported_schema_version`, `service_unavailable`, and `upstream_error`; `graph.members` carries `node_not_held`, `request_invalid`, `service_unavailable`, and `upstream_error`, the last when the read that checks whether the graph holds the node fails upstream; and `graph.roots` carries `service_unavailable`. A node the graph does not hold answers inside a `graph.nodes` batch as a `not_held` entry, so that endpoint carries no `node_not_held`. Graph's `sources` and `capabilities` routes carry no permanent codes. diff --git a/schemas/registry/code-registry.schema.json b/schemas/registry/code-registry.schema.json index 22217f2..efb93a3 100644 --- a/schemas/registry/code-registry.schema.json +++ b/schemas/registry/code-registry.schema.json @@ -102,6 +102,9 @@ "presentation.write", "presentation.render", "presentation.shares", + "presentation.frames", + "presentation.component_read", + "presentation.component_data", "datasets.register", "datasets.mint", "datasets.definition", @@ -288,6 +291,9 @@ "presentation.write", "presentation.render", "presentation.shares", + "presentation.frames", + "presentation.component_read", + "presentation.component_data", "datasets.register", "datasets.mint", "datasets.definition", diff --git a/tools/tests/test_validate_registry.py b/tools/tests/test_validate_registry.py index 6328f57..c65291e 100644 --- a/tools/tests/test_validate_registry.py +++ b/tools/tests/test_validate_registry.py @@ -334,6 +334,9 @@ def schema_validator() -> Draft202012Validator: "endpoint", [ "presentation.write", + "presentation.frames", + "presentation.component_read", + "presentation.component_data", "datasets.mint", "datasets.options", "platform.any", From 32c47517128d6c4311c09bd5072203edd859fc48 Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Sat, 31 Oct 2026 09:52:48 +0100 Subject: [PATCH 2/6] fix(registry): map binding_invalid on presentation.component_data --- python/spec-kit/CHANGELOG.md | 3 ++- python/spec-kit/src/dfo_spec_kit/registry/codes.json | 5 +++++ registries/codes.json | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/python/spec-kit/CHANGELOG.md b/python/spec-kit/CHANGELOG.md index 45f1745..d7b2250 100644 --- a/python/spec-kit/CHANGELOG.md +++ b/python/spec-kit/CHANGELOG.md @@ -106,7 +106,8 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `presentation.frames` adds `binding_invalid` (400) and `frame_unavailable` (404). `presentation.component_read` adds `not_found` (404), `edit_conflict` (409) and `release_mismatch` (503). `presentation.component_data` adds - `not_found` and `frame_unavailable` (404) and `edit_conflict` (409). + `binding_invalid` (400), `not_found` and `frame_unavailable` (404) and + `edit_conflict` (409). ### Changed diff --git a/python/spec-kit/src/dfo_spec_kit/registry/codes.json b/python/spec-kit/src/dfo_spec_kit/registry/codes.json index e700f69..fa20b64 100644 --- a/python/spec-kit/src/dfo_spec_kit/registry/codes.json +++ b/python/spec-kit/src/dfo_spec_kit/registry/codes.json @@ -241,6 +241,11 @@ "endpoint": "presentation.frames", "channel": "problem_details", "http_status": 400 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 400 } ], "lifecycle": "active" diff --git a/registries/codes.json b/registries/codes.json index e700f69..fa20b64 100644 --- a/registries/codes.json +++ b/registries/codes.json @@ -241,6 +241,11 @@ "endpoint": "presentation.frames", "channel": "problem_details", "http_status": 400 + }, + { + "endpoint": "presentation.component_data", + "channel": "problem_details", + "http_status": 400 } ], "lifecycle": "active" From fa6264ff70e552d07cd34e332f8017b41c431b8a Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Sat, 31 Oct 2026 13:00:54 +0100 Subject: [PATCH 3/6] fix(frame-grammar): validate stored unit cells where unit_cells and Frame.rows read them --- python/frame-grammar/CHANGELOG.md | 6 + python/frame-grammar/README.md | 2 +- .../src/dfo_frame_grammar/_unit_codec.py | 119 ++++++++++++++---- .../src/dfo_frame_grammar/arrow.py | 79 +----------- .../src/dfo_frame_grammar/columnar.py | 4 +- .../src/dfo_frame_grammar/frame.py | 6 +- python/frame-grammar/tests/test_columnar.py | 73 ++++++++++- 7 files changed, 179 insertions(+), 110 deletions(-) diff --git a/python/frame-grammar/CHANGELOG.md b/python/frame-grammar/CHANGELOG.md index 42815a5..aa10b0c 100644 --- a/python/frame-grammar/CHANGELOG.md +++ b/python/frame-grammar/CHANGELOG.md @@ -153,6 +153,12 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 refuses a `composed` member whose Arrow type is neither a string type nor null with `schema_violation`. Canonical bytes, `frame_sha256` and `content_version` are unchanged for every frame. +- `unit_cells` and `Frame.rows` read each stored `unit` struct value as the + Arrow build reads it, and raise `FrameError` for a struct with no member or + more than one (`schema_violation`) and for a member outside its form: an + empty ref, a request outside its closed shape, or `composed` text that is not + JSON, repeats a member name, or holds no composed unit (`invalid_unit`). + `unit_cells` reads each distinct value once. - `unit_cells` returns `ComposedUnit` cells, and `collation_keys` ranks a composed unit 3, after a unit request at 2. - `canonical_frame_bytes` and `frame_sha256` stream through diff --git a/python/frame-grammar/README.md b/python/frame-grammar/README.md index c4fa07e..46c79cd 100644 --- a/python/frame-grammar/README.md +++ b/python/frame-grammar/README.md @@ -59,7 +59,7 @@ A consumer that keeps frames in Arrow uses the package root exports. - `Frame.from_table(columns=..., table=...)` wraps a table this package built, such as `frame.table` written to Parquet and read back. It checks `columns` and the schema against `canonical_frame_schema(columns)`, accepts the `unit` column in every layout a release stored, and trusts the cells and row order. Untrusted tables go through `build_frame_from_arrow`. - `canonical_frame_bytes` and `frame_sha256` serialise through `stream_canonical_frame_bytes`, 8,192 rows at a time. Hashing a 250,000-row frame raises peak RSS by about 4 MiB, against about 197 MiB for one whole-table document (`benchmarks/arrow_frame_rss.py`). A table whose rows are out of canonical order refuses with `FrameError` `row_key_not_increasing`; only `Frame.from_table` or the `Frame` constructor can produce one. -- `unit_codes(unit)` gives each row an integer code, equal exactly where the `unit` values are equal, read from the struct's leaf dictionary codes. PyArrow cannot `dictionary_encode`, `unique` or `group_by` the `unit` struct itself. `UnitCodes.count` is the number of distinct units, `unit_cells` their values, and `map_units` applies a function once per distinct unit. +- `unit_codes(unit)` gives each row an integer code, equal exactly where the `unit` values are equal, read from the struct's leaf dictionary codes. PyArrow cannot `dictionary_encode`, `unique` or `group_by` the `unit` struct itself. `UnitCodes.count` is the number of distinct units, `unit_cells` their values, validated as the Arrow build validates a `unit` column and raising `FrameError` for one outside the grammar's forms, and `map_units` applies a function once per distinct unit. - A non-null `unit` cell is a `UnitCell`: a ref string, a `UnitRequest` (`currency`, `prices`, `base_year` and an optional `recipe`), or a `ComposedUnit` whose `numerator` and `denominator` are each a ref string, a `UnitRequest`, or a `StatVarPart`. `unit_cell_from_json(value)` validates the JSON form and returns the cell, `unit_cell_to_json(cell)` gives the JSON form back, and `unit_cell_array(cell, length)` builds `length` rows of the cell in the canonical `unit` column type. Both take and return `None` for a null unit. `unit_cell_array` refuses a hand-built cell that validation would not return with `FrameError` `invalid_unit`. A reader on frame grammar `1.0.0` refuses a cell that carries `recipe`, so every reader takes `1.1.0` before any producer writes one. - The `unit` column is `struct`. Its `request` struct gains a `recipe` child only in a frame where some cell carries a recipe, so a frame without one stores as frame grammar `1.0.0` stored it and a `1.0.0` reader reads it. `canonical_frame_schema(columns)` is that layout and `canonical_frame_schema(columns, recipe=True)` the recipe layout. `composed` is a string holding the composed unit's JCS text, so `unit_codes` and `collation_keys` read it as one leaf, and a column with no composed unit spends about 4 bytes a row on it (`benchmarks/unit_column_bytes.py`). - `widen_frame_table(table, recipe=False)` takes a canonical table whose `unit` column is in any layout a release stored, `struct` or `struct` with or without `recipe`, and returns it in the recipe layout when `recipe` is true or some cell carries a recipe, and in the layout without `recipe` otherwise. `Frame.from_table` applies it, so a reader never calls it first. A table already in the result's layout comes back unchanged, and any other schema, a table with no `unit` column included, refuses with `FrameError` `schema_violation`. diff --git a/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py b/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py index b1bc91c..34edcfa 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py +++ b/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py @@ -9,12 +9,13 @@ import json from collections.abc import Mapping -from typing import cast +from typing import Any, cast import pyarrow as pa from dfo_spec_kit.identity import canonicalize from dfo_frame_grammar import grammar +from dfo_frame_grammar.grammar import FrameError def _unit_request_type(*, recipe: bool) -> pa.StructType: @@ -130,33 +131,97 @@ def _unit_to_arrow(cell: grammar.UnitCell | None) -> dict[str, object] | None: return {"composed": canonicalize(cell.as_json()).decode("utf-8")} -def _arrow_part_to_cell(value: object) -> grammar.UnitPart: - """A ref string, a `request` struct, or one side of a parsed composed - unit as its cell, trusted as canonical.""" - if isinstance(value, str): - return value - part = cast("Mapping[str, object]", value) - if grammar.STAT_VAR_COLUMN in part: - return grammar.StatVarPart(stat_var=cast(str, part[grammar.STAT_VAR_COLUMN])) - return grammar.UnitRequest( - currency=cast(str, part["currency"]), - prices=cast(str, part["prices"]), - base_year=cast("int | None", part.get("base_year")), - recipe=cast("str | None", part.get("recipe")), +def _without_nulls(struct: Mapping[Any, object]) -> dict[str, object]: + """The non-null members of a struct value as `to_pylist` reads it.""" + return {key: value for key, value in struct.items() if value is not None} + + +def _wrong_unit_type(value: object) -> FrameError: + return FrameError( + "schema_violation", + f"unit must be a string, mapping, or null, got {type(value)!r}", ) -def _arrow_unit_to_cell(value: object) -> grammar.UnitCell | None: - if value is None: +def _unit_request_from_mapping(raw: object) -> grammar.UnitRequest | None: + if raw is None: return None - assert isinstance(value, Mapping) - tagged = cast("Mapping[str, object]", value) - composed = tagged["composed"] - if composed is None: - member = tagged["ref"] if tagged["ref"] is not None else tagged["request"] - return cast("str | grammar.UnitRequest", _arrow_part_to_cell(member)) - sides = json.loads(cast(str, composed)) - return grammar.ComposedUnit( - numerator=_arrow_part_to_cell(sides["numerator"]), - denominator=_arrow_part_to_cell(sides["denominator"]), - ) + if not isinstance(raw, Mapping): + raise _wrong_unit_type(raw) + return grammar.unit_request_from_json(_without_nulls(raw)) + + +def _unique_members(pairs: list[tuple[str, object]]) -> dict[str, object]: + members = dict(pairs) + if len(members) != len(pairs): + raise FrameError("invalid_unit", "composed unit text repeats a member name") + return members + + +def _composed_unit_from_text(text: str) -> grammar.ComposedUnit: + """Parse and validate the JSON text of a composed unit, refusing text + that is not JSON, repeats a member name, or holds anything but a + composed unit.""" + try: + parsed = json.loads(text, object_pairs_hook=_unique_members) + # `ValueError` covers `JSONDecodeError` and an integer past Python's digit + # limit; `RecursionError` covers nesting past the interpreter's depth. + except (ValueError, RecursionError) as exc: + raise FrameError( + "invalid_unit", f"composed unit text is not JSON: {exc}" + ) from exc + if not isinstance(parsed, dict): + raise FrameError( + "invalid_unit", + f"composed unit text must hold a JSON object, got {type(parsed)!r}", + ) + return grammar._composed_unit_from_json(parsed) + + +def _canonical_unit_from_mapping(raw: object) -> grammar.UnitCell | None: + """The validated cell of one tagged struct value, read from its single + non-null member: a `ref` string, a `request` struct with its null + members dropped, or the JSON text of a `composed` unit. A null struct + is a null unit. `unit_cells`, `Frame.rows` and the Arrow build read a + `unit` column through it, so each refuses a stored cell outside these + forms. + + Raises: + FrameError: `schema_violation` for a struct with no member or more + than one, or a member of the wrong type; `invalid_unit` for a + member outside its form. + """ + if raw is None: + return None + if not isinstance(raw, Mapping): + raise _wrong_unit_type(raw) + variants = _without_nulls(raw) + if len(variants) != 1: + raise FrameError( + "schema_violation", "unit tagged struct must carry one variant" + ) + ((member, value),) = variants.items() + if member == "request": + return _unit_request_from_mapping(value) + if member == "composed": + if not isinstance(value, str): + raise _wrong_unit_type(value) + return _composed_unit_from_text(value) + if not isinstance(value, str): + raise _wrong_unit_type(value) + return grammar.unit_cell_from_json(value) + + +def _arrow_part_to_cell(value: object) -> grammar.UnitPart: + """A ref string or a `request` struct as its validated cell, refusing + what `grammar.unit_cell_from_json` would refuse.""" + if isinstance(value, Mapping): + return grammar.unit_request_from_json(_without_nulls(value)) + return _unit_ref(value) + + +def _unit_ref(value: object) -> str: + cell = grammar.unit_cell_from_json(value) + if not isinstance(cell, str): + raise _wrong_unit_type(value) + return cell diff --git a/python/frame-grammar/src/dfo_frame_grammar/arrow.py b/python/frame-grammar/src/dfo_frame_grammar/arrow.py index 1d828a9..af62a45 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/arrow.py +++ b/python/frame-grammar/src/dfo_frame_grammar/arrow.py @@ -3,17 +3,17 @@ from __future__ import annotations import io -import json import unicodedata -from collections.abc import Callable, Iterable, Mapping -from typing import Any, cast +from collections.abc import Callable, Iterable import pyarrow as pa import pyarrow.csv as pacsv from dfo_frame_grammar import grammar from dfo_frame_grammar._unit_codec import ( + _canonical_unit_from_mapping, _conform_unit, + _unit_request_from_mapping, _unit_to_arrow, _unit_type, ) @@ -158,79 +158,6 @@ def _normalise_value(array: pa.ChunkedArray) -> pa.Array: ) -def _without_nulls(struct: Mapping[Any, object]) -> dict[str, object]: - """The non-null members of a struct value as `to_pylist` reads it.""" - return {key: value for key, value in struct.items() if value is not None} - - -def _wrong_unit_type(value: object) -> FrameError: - return FrameError( - "schema_violation", - f"unit must be a string, mapping, or null, got {type(value)!r}", - ) - - -def _unit_request_from_mapping(raw: object) -> grammar.UnitRequest | None: - if raw is None: - return None - if not isinstance(raw, Mapping): - raise _wrong_unit_type(raw) - return grammar.unit_request_from_json(_without_nulls(raw)) - - -def _unique_members(pairs: list[tuple[str, object]]) -> dict[str, object]: - members = dict(pairs) - if len(members) != len(pairs): - raise FrameError("invalid_unit", "composed unit text repeats a member name") - return members - - -def _composed_unit_from_text(text: str) -> grammar.ComposedUnit: - """Parse and validate the JSON text of a composed unit, refusing text - that is not JSON, repeats a member name, or holds anything but a - composed unit.""" - try: - parsed = json.loads(text, object_pairs_hook=_unique_members) - # `ValueError` covers `JSONDecodeError` and an integer past Python's digit - # limit; `RecursionError` covers nesting past the interpreter's depth. - except (ValueError, RecursionError) as exc: - raise FrameError( - "invalid_unit", f"composed unit text is not JSON: {exc}" - ) from exc - if not isinstance(parsed, dict): - raise FrameError( - "invalid_unit", - f"composed unit text must hold a JSON object, got {type(parsed)!r}", - ) - return grammar._composed_unit_from_json(parsed) - - -def _canonical_unit_from_mapping(raw: object) -> grammar.UnitCell | None: - """The validated cell of one tagged struct value, read from its single - non-null member: a `ref` string, a `request` struct with its null - members dropped, or the JSON text of a `composed` unit. A null struct - is a null unit. `_refuse_untyped_composed` has already refused a - `composed` member that is not text. - """ - if raw is None: - return None - if not isinstance(raw, Mapping): - raise _wrong_unit_type(raw) - variants = _without_nulls(raw) - if len(variants) != 1: - raise FrameError( - "schema_violation", "unit tagged struct must carry one variant" - ) - ((member, value),) = variants.items() - if member == "request": - return _unit_request_from_mapping(value) - if member == "composed": - return _composed_unit_from_text(cast(str, value)) - if not isinstance(value, str): - raise _wrong_unit_type(value) - return grammar.unit_cell_from_json(value) - - # The tagged spellings of the `unit` struct: the canonical three-member # layout and the two-member layout, which holds no composed unit. A struct # with any other member names is a bare unit request. diff --git a/python/frame-grammar/src/dfo_frame_grammar/columnar.py b/python/frame-grammar/src/dfo_frame_grammar/columnar.py index b7810ef..4a464ce 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/columnar.py +++ b/python/frame-grammar/src/dfo_frame_grammar/columnar.py @@ -19,7 +19,7 @@ from dfo_frame_grammar import grammar from dfo_frame_grammar._unit_codec import ( _arrow_part_to_cell, - _arrow_unit_to_cell, + _canonical_unit_from_mapping, _unit_type, ) from dfo_frame_grammar.grammar import FrameError @@ -226,7 +226,7 @@ def unit_cells( `grammar.UnitRequest`, a `grammar.ComposedUnit`, or `None`. """ distinct = array_kernel("take", _combined(unit), codes.first_rows).to_pylist() - return [_arrow_unit_to_cell(value) for value in distinct] + return [_canonical_unit_from_mapping(value) for value in distinct] def _request_payload_of(value: object) -> bytes | None: diff --git a/python/frame-grammar/src/dfo_frame_grammar/frame.py b/python/frame-grammar/src/dfo_frame_grammar/frame.py index 417c8b8..bd58446 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/frame.py +++ b/python/frame-grammar/src/dfo_frame_grammar/frame.py @@ -15,7 +15,7 @@ from dfo_frame_grammar import grammar from dfo_frame_grammar._unit_codec import ( _STORED_UNIT_TYPES, - _arrow_unit_to_cell, + _canonical_unit_from_mapping, _conform_unit, _holds_recipe, _unit_array, @@ -233,7 +233,7 @@ def _python_rows( each `unit` struct converted back to its hashable cell.""" return tuple( tuple( - _arrow_unit_to_cell(row[column]) + _canonical_unit_from_mapping(row[column]) if column == grammar.UNIT_COLUMN else row[column] for column in columns @@ -433,7 +433,7 @@ def build( def _unit_to_json(value: object) -> object: - return grammar.unit_cell_to_json(_arrow_unit_to_cell(value)) + return grammar.unit_cell_to_json(_canonical_unit_from_mapping(value)) def _row_cells(frame: Frame, table_row: Mapping[str, object]) -> list[object]: diff --git a/python/frame-grammar/tests/test_columnar.py b/python/frame-grammar/tests/test_columnar.py index 64389ba..2410106 100644 --- a/python/frame-grammar/tests/test_columnar.py +++ b/python/frame-grammar/tests/test_columnar.py @@ -11,7 +11,7 @@ import pytest from dfo_frame_grammar import arrow as arrow_module -from dfo_frame_grammar import grammar +from dfo_frame_grammar import columnar, grammar from dfo_frame_grammar.arrow import build_frame_from_arrow from dfo_frame_grammar.columnar import ( array_kernel, @@ -235,6 +235,77 @@ def test_composed_units_code_by_value(self) -> None: assert codes.codes.to_pylist() == [0, 1, 0, 2, 3, 4, 0] assert unit_cells(unit, codes) == list(dict.fromkeys(cells)) + @pytest.mark.parametrize( + ("stored", "category"), + [ + pytest.param( + {"ref": None, "request": None, "composed": None}, + "schema_violation", + id="no-member", + ), + pytest.param( + {"ref": "USDollar", "composed": '{"numerator":"a","denominator":"b"}'}, + "schema_violation", + id="two-members", + ), + pytest.param({"ref": ""}, "schema_violation", id="empty-ref"), + pytest.param( + {"request": {"currency": "usd", "prices": "current"}}, + "invalid_unit", + id="request-outside-its-form", + ), + pytest.param( + {"request": {"currency": "USD", "prices": "constant"}}, + "invalid_unit", + id="constant-without-base-year", + ), + pytest.param({"composed": "not json"}, "invalid_unit", id="not-json"), + pytest.param({"composed": "[" * 100_000}, "invalid_unit", id="too-deep"), + pytest.param( + {"composed": '{"numerator":' + "9" * 5000 + ',"denominator":"b"}'}, + "invalid_unit", + id="integer-past-the-digit-limit", + ), + pytest.param( + {"composed": '{"numerator":"a","numerator":"c","denominator":"b"}'}, + "invalid_unit", + id="repeated-member", + ), + pytest.param({"composed": '"USDollar"'}, "invalid_unit", id="a-ref"), + pytest.param( + {"composed": '{"numerator":"a"}'}, "invalid_unit", id="one-side" + ), + pytest.param( + {"composed": '{"numerator":"a","denominator":{"stat_var":3}}'}, + "invalid_unit", + id="stat-var-not-text", + ), + ], + ) + def test_unit_cells_refuses_a_stored_cell_outside_its_forms( + self, stored: dict[str, object], category: str + ) -> None: + unit_type = canonical_frame_schema(("unit",)).field("unit").type + unit = pa.array([stored, {"ref": "USDollar"}], unit_type) + with pytest.raises(FrameError) as excinfo: + unit_cells(unit, unit_codes(unit)) + assert excinfo.value.category == category + + def test_unit_cells_reads_each_distinct_cell_once( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + read: list[object] = [] + original = columnar._canonical_unit_from_mapping + + def counting(value: object) -> object: + read.append(value) + return original(value) + + monkeypatch.setattr(columnar, "_canonical_unit_from_mapping", counting) + unit = unit_cell_array("USDollar", 1000) + unit_cells(unit, unit_codes(unit)) + assert len(read) == 1 + def test_an_all_null_child_keeps_null_structs_distinct(self) -> None: inner = pa.struct([("x", pa.string())]) values = pa.array( From 4a250e88adea258752ecf319445e179156afe0d0 Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Sat, 31 Oct 2026 13:17:59 +0100 Subject: [PATCH 4/6] perf(frame-grammar): read the unit column by code in Frame.rows and the canonical byte stream --- python/frame-grammar/CHANGELOG.md | 4 +- .../benchmarks/arrow_frame_rss.py | 9 ++- .../src/dfo_frame_grammar/frame.py | 63 ++++++++++--------- python/frame-grammar/tests/test_streaming.py | 12 +++- 4 files changed, 51 insertions(+), 37 deletions(-) diff --git a/python/frame-grammar/CHANGELOG.md b/python/frame-grammar/CHANGELOG.md index aa10b0c..4001eff 100644 --- a/python/frame-grammar/CHANGELOG.md +++ b/python/frame-grammar/CHANGELOG.md @@ -158,7 +158,9 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 more than one (`schema_violation`) and for a member outside its form: an empty ref, a request outside its closed shape, or `composed` text that is not JSON, repeats a member name, or holds no composed unit (`invalid_unit`). - `unit_cells` reads each distinct value once. + `unit_cells` reads each distinct value once, and `Frame.rows` and + `canonical_frame_bytes` read the `unit` column through it, so neither + validates per row. - `unit_cells` returns `ComposedUnit` cells, and `collation_keys` ranks a composed unit 3, after a unit request at 2. - `canonical_frame_bytes` and `frame_sha256` stream through diff --git a/python/frame-grammar/benchmarks/arrow_frame_rss.py b/python/frame-grammar/benchmarks/arrow_frame_rss.py index e6860d4..6177b52 100644 --- a/python/frame-grammar/benchmarks/arrow_frame_rss.py +++ b/python/frame-grammar/benchmarks/arrow_frame_rss.py @@ -19,7 +19,7 @@ from dfo_frame_grammar import grammar from dfo_frame_grammar.arrow import build_frame_from_arrow from dfo_frame_grammar.content_version import frame_sha256 -from dfo_frame_grammar.frame import Frame, _row_cells +from dfo_frame_grammar.frame import Frame, _column_cells ROW_COUNT = 250_000 CHUNK_COUNT = 44 @@ -62,7 +62,12 @@ def _rss_mib() -> float: def _whole_table_sha256(frame: Frame) -> str: - rows = [_row_cells(frame, row) for row in frame.table.to_pylist()] + rows = [ + list(row) + for row in zip( + *_column_cells(frame.columns, frame.table, json_form=True), strict=True + ) + ] document = { "frame_version": grammar.FRAME_VERSION, "columns": list(frame.columns), diff --git a/python/frame-grammar/src/dfo_frame_grammar/frame.py b/python/frame-grammar/src/dfo_frame_grammar/frame.py index bd58446..a95d4ed 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/frame.py +++ b/python/frame-grammar/src/dfo_frame_grammar/frame.py @@ -15,7 +15,6 @@ from dfo_frame_grammar import grammar from dfo_frame_grammar._unit_codec import ( _STORED_UNIT_TYPES, - _canonical_unit_from_mapping, _conform_unit, _holds_recipe, _unit_array, @@ -26,6 +25,8 @@ _grain_columns, _sort_and_check, collation_sort_keys, + unit_cells, + unit_codes, ) from dfo_frame_grammar.grammar import FrameError @@ -229,17 +230,32 @@ def _table_from_rows( def _python_rows( columns: tuple[str, ...], table: pa.Table ) -> tuple[tuple[object, ...], ...]: - """`table`'s rows as tuples of Python cells in `columns` order, with - each `unit` struct converted back to its hashable cell.""" - return tuple( - tuple( - _canonical_unit_from_mapping(row[column]) - if column == grammar.UNIT_COLUMN - else row[column] - for column in columns + """`table`'s rows as tuples of Python cells in `columns` order.""" + return tuple(zip(*_column_cells(columns, table, json_form=False), strict=True)) + + +def _column_cells( + columns: tuple[str, ...], table: pa.Table, *, json_form: bool +) -> list[list[object]]: + """Each column's Python cells. The `unit` column is read through + `unit_cells`, which validates each distinct struct value once, and each + row takes its cell by unit code: the hashable cell, or with `json_form` + the JCS-ready form `grammar.unit_cell_to_json` gives it.""" + cells: list[list[object]] = [] + for column in columns: + values = table.column(column) + if column != grammar.UNIT_COLUMN: + cells.append(values.to_pylist()) + continue + codes = unit_codes(values) + units = unit_cells(values, codes) + distinct: list[object] = ( + [grammar.unit_cell_to_json(cell) for cell in units] + if json_form + else list(units) ) - for row in table.to_pylist() - ) + cells.append([distinct[code] for code in codes.codes.to_pylist()]) + return cells @dataclass(frozen=True, eq=False) @@ -432,24 +448,6 @@ def build( ) -def _unit_to_json(value: object) -> object: - return grammar.unit_cell_to_json(_canonical_unit_from_mapping(value)) - - -def _row_cells(frame: Frame, table_row: Mapping[str, object]) -> list[object]: - """One row's cells in canonical column order, with the ``unit`` column - resolved to its JCS-ready form (`_unit_to_json`) and every other column - passed through unchanged -- the per-row projection - `stream_canonical_frame_bytes` serialises. - """ - return [ - _unit_to_json(table_row[column]) - if column == grammar.UNIT_COLUMN - else table_row[column] - for column in frame.columns - ] - - def canonical_frame_bytes(frame: Frame) -> bytes: """Return the frame's canonical JCS document bytes. @@ -593,7 +591,10 @@ def _stream_rows( # sorts the key columns. keys = collation_sort_keys(row_slice, grain) row_keys = zip(*(column.to_pylist() for column in keys.columns), strict=True) - for table_row, key in zip(row_slice.to_pylist(), row_keys, strict=True): + rows = zip( + *_column_cells(frame.columns, row_slice, json_form=True), strict=True + ) + for row, key in zip(rows, row_keys, strict=True): if last_key is not None and key <= last_key: raise FrameError( "row_key_not_increasing", @@ -602,6 +603,6 @@ def _stream_rows( ) if last_key is not None: yield b"," - yield rfc8785.dumps(_row_cells(frame, table_row)) + yield rfc8785.dumps(list(row)) last_key = key return last_key diff --git a/python/frame-grammar/tests/test_streaming.py b/python/frame-grammar/tests/test_streaming.py index 2e5b18a..46db765 100644 --- a/python/frame-grammar/tests/test_streaming.py +++ b/python/frame-grammar/tests/test_streaming.py @@ -40,9 +40,15 @@ def _whole_table_canonical_bytes(frame: Frame) -> bytes: - """The oracle: the whole table read with one `to_pylist` and serialised - as one JCS document.""" - rows = [frame_module._row_cells(frame, row) for row in frame.table.to_pylist()] + """The oracle: the whole table read at once and serialised as one JCS + document.""" + rows = [ + list(row) + for row in zip( + *frame_module._column_cells(frame.columns, frame.table, json_form=True), + strict=True, + ) + ] return canonicalize( { "frame_version": grammar.FRAME_VERSION, From eb6afef5debf03813d51837f28d6a42aff25d2ed Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Sat, 31 Oct 2026 13:36:34 +0100 Subject: [PATCH 5/6] refactor(frame-grammar): expose unit_column_cells and read unit columns through it --- python/frame-grammar/CHANGELOG.md | 11 ++--- .../benchmarks/arrow_frame_rss.py | 11 ++--- .../src/dfo_frame_grammar/__init__.py | 2 + .../src/dfo_frame_grammar/_unit_codec.py | 20 ++------- .../src/dfo_frame_grammar/columnar.py | 44 +++++++++++++++++-- .../src/dfo_frame_grammar/frame.py | 42 +++++++----------- python/frame-grammar/tests/test_streaming.py | 37 +++++++++++++--- 7 files changed, 103 insertions(+), 64 deletions(-) diff --git a/python/frame-grammar/CHANGELOG.md b/python/frame-grammar/CHANGELOG.md index 4001eff..daa0b08 100644 --- a/python/frame-grammar/CHANGELOG.md +++ b/python/frame-grammar/CHANGELOG.md @@ -155,12 +155,13 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `content_version` are unchanged for every frame. - `unit_cells` and `Frame.rows` read each stored `unit` struct value as the Arrow build reads it, and raise `FrameError` for a struct with no member or - more than one (`schema_violation`) and for a member outside its form: an - empty ref, a request outside its closed shape, or `composed` text that is not + more than one or an empty ref (`schema_violation`) and for a member outside + its form: a request outside its closed shape, or `composed` text that is not JSON, repeats a member name, or holds no composed unit (`invalid_unit`). - `unit_cells` reads each distinct value once, and `Frame.rows` and - `canonical_frame_bytes` read the `unit` column through it, so neither - validates per row. + `unit_cells` reads each distinct value once. `unit_column_cells` gives a + `unit` column's per-row cells, plain or in JSON form, through it, and + `Frame.rows` and `canonical_frame_bytes` read the column that way, so + neither validates per row. - `unit_cells` returns `ComposedUnit` cells, and `collation_keys` ranks a composed unit 3, after a unit request at 2. - `canonical_frame_bytes` and `frame_sha256` stream through diff --git a/python/frame-grammar/benchmarks/arrow_frame_rss.py b/python/frame-grammar/benchmarks/arrow_frame_rss.py index 6177b52..7c99c02 100644 --- a/python/frame-grammar/benchmarks/arrow_frame_rss.py +++ b/python/frame-grammar/benchmarks/arrow_frame_rss.py @@ -19,7 +19,7 @@ from dfo_frame_grammar import grammar from dfo_frame_grammar.arrow import build_frame_from_arrow from dfo_frame_grammar.content_version import frame_sha256 -from dfo_frame_grammar.frame import Frame, _column_cells +from dfo_frame_grammar.frame import Frame, _python_rows ROW_COUNT = 250_000 CHUNK_COUNT = 44 @@ -63,10 +63,11 @@ def _rss_mib() -> float: def _whole_table_sha256(frame: Frame) -> str: rows = [ - list(row) - for row in zip( - *_column_cells(frame.columns, frame.table, json_form=True), strict=True - ) + [ + grammar.unit_cell_to_json(cell) if column == grammar.UNIT_COLUMN else cell + for column, cell in zip(frame.columns, row, strict=True) + ] + for row in _python_rows(frame.columns, frame.table) ] document = { "frame_version": grammar.FRAME_VERSION, diff --git a/python/frame-grammar/src/dfo_frame_grammar/__init__.py b/python/frame-grammar/src/dfo_frame_grammar/__init__.py index d8f1333..733b37b 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/__init__.py +++ b/python/frame-grammar/src/dfo_frame_grammar/__init__.py @@ -37,6 +37,7 @@ scalar_kernel, unit_cells, unit_codes, + unit_column_cells, ) from dfo_frame_grammar.content_version import ( ContentVersionError, @@ -138,5 +139,6 @@ "unit_cell_to_json", "unit_cells", "unit_codes", + "unit_column_cells", "widen_frame_table", ] diff --git a/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py b/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py index 34edcfa..b5eb207 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py +++ b/python/frame-grammar/src/dfo_frame_grammar/_unit_codec.py @@ -188,8 +188,9 @@ def _canonical_unit_from_mapping(raw: object) -> grammar.UnitCell | None: Raises: FrameError: `schema_violation` for a struct with no member or more - than one, or a member of the wrong type; `invalid_unit` for a - member outside its form. + than one, a member of the wrong type, or an empty ref; + `invalid_unit` for a request or composed member outside its + form. """ if raw is None: return None @@ -210,18 +211,3 @@ def _canonical_unit_from_mapping(raw: object) -> grammar.UnitCell | None: if not isinstance(value, str): raise _wrong_unit_type(value) return grammar.unit_cell_from_json(value) - - -def _arrow_part_to_cell(value: object) -> grammar.UnitPart: - """A ref string or a `request` struct as its validated cell, refusing - what `grammar.unit_cell_from_json` would refuse.""" - if isinstance(value, Mapping): - return grammar.unit_request_from_json(_without_nulls(value)) - return _unit_ref(value) - - -def _unit_ref(value: object) -> str: - cell = grammar.unit_cell_from_json(value) - if not isinstance(cell, str): - raise _wrong_unit_type(value) - return cell diff --git a/python/frame-grammar/src/dfo_frame_grammar/columnar.py b/python/frame-grammar/src/dfo_frame_grammar/columnar.py index 4a464ce..e29c656 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/columnar.py +++ b/python/frame-grammar/src/dfo_frame_grammar/columnar.py @@ -8,7 +8,8 @@ from __future__ import annotations -from collections.abc import Callable, Sequence +import json +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from typing import cast @@ -18,9 +19,9 @@ from dfo_frame_grammar import grammar from dfo_frame_grammar._unit_codec import ( - _arrow_part_to_cell, _canonical_unit_from_mapping, _unit_type, + _without_nulls, ) from dfo_frame_grammar.grammar import FrameError @@ -34,6 +35,7 @@ "scalar_kernel", "unit_cells", "unit_codes", + "unit_column_cells", ] _UNIT_TYPES = (_unit_type(recipe=False), _unit_type(recipe=True)) @@ -229,11 +231,45 @@ def unit_cells( return [_canonical_unit_from_mapping(value) for value in distinct] +def unit_column_cells( + unit: pa.Array | pa.ChunkedArray, *, json_form: bool = False +) -> list[object]: + """Each row's cell of a canonical `unit` column, read through + `unit_cells`, so each distinct struct value is validated once and the + rows that share it share one object. + + A cell has the form `Frame.rows` gives it, or with `json_form` its JSON + form as `json.loads` parses its JCS text: a ref string, a mapping with + sorted keys, or `None`. + + Raises: + FrameError: For a stored value outside the `unit` cell's forms + (`unit_cells`). + """ + codes = unit_codes(unit) + distinct: list[object] = list(unit_cells(unit, codes)) + if json_form: + distinct = [ + _parsed_json_form(cast("grammar.UnitCell | None", cell)) + for cell in distinct + ] + return [distinct[code] for code in codes.codes.to_pylist()] + + +def _parsed_json_form(cell: grammar.UnitCell | None) -> object: + """`cell`'s JSON form as `json.loads` parses its JCS text.""" + if cell is None or isinstance(cell, str): + return cell + return json.loads(canonicalize(cell.as_json())) + + def _request_payload_of(value: object) -> bytes | None: if value is None: return None - request = cast("grammar.UnitRequest", _arrow_part_to_cell(value)) - return canonicalize(request.as_json()) + request = cast("Mapping[str, object]", value) + return canonicalize( + grammar.unit_request_from_json(_without_nulls(request)).as_json() + ) def _unit_payload(unit: pa.Array) -> pa.Array: diff --git a/python/frame-grammar/src/dfo_frame_grammar/frame.py b/python/frame-grammar/src/dfo_frame_grammar/frame.py index a95d4ed..5bc27d8 100644 --- a/python/frame-grammar/src/dfo_frame_grammar/frame.py +++ b/python/frame-grammar/src/dfo_frame_grammar/frame.py @@ -25,8 +25,7 @@ _grain_columns, _sort_and_check, collation_sort_keys, - unit_cells, - unit_codes, + unit_column_cells, ) from dfo_frame_grammar.grammar import FrameError @@ -237,25 +236,14 @@ def _python_rows( def _column_cells( columns: tuple[str, ...], table: pa.Table, *, json_form: bool ) -> list[list[object]]: - """Each column's Python cells. The `unit` column is read through - `unit_cells`, which validates each distinct struct value once, and each - row takes its cell by unit code: the hashable cell, or with `json_form` - the JCS-ready form `grammar.unit_cell_to_json` gives it.""" - cells: list[list[object]] = [] - for column in columns: - values = table.column(column) - if column != grammar.UNIT_COLUMN: - cells.append(values.to_pylist()) - continue - codes = unit_codes(values) - units = unit_cells(values, codes) - distinct: list[object] = ( - [grammar.unit_cell_to_json(cell) for cell in units] - if json_form - else list(units) - ) - cells.append([distinct[code] for code in codes.codes.to_pylist()]) - return cells + """Each column's Python cells, the `unit` column's from + `unit_column_cells`.""" + return [ + unit_column_cells(table.column(column), json_form=json_form) + if column == grammar.UNIT_COLUMN + else table.column(column).to_pylist() + for column in columns + ] @dataclass(frozen=True, eq=False) @@ -317,11 +305,13 @@ def from_table(cls, *, columns: tuple[str, ...], table: pa.Table) -> Frame: derived column tuple; nothing else is checked. The frame holds the table as `widen_frame_table` gives it: the recipe layout exactly when some cell carries a recipe. - Cell contents are trusted: a null grain cell, a non-NFC string, a - NaN or `-0.0` value, a unit struct with no variant or more than - one, or a `composed` member that is not the canonical JCS text of a - composed unit passes here and serialises wrongly or fails later - with an error other than `FrameError`. Rows out of the order + Cell contents are trusted: a null grain cell, a non-NFC string, or + a NaN or `-0.0` value passes here and serialises wrongly. A `unit` + cell outside its forms, such as a struct with no variant or more + than one, or a `composed` member that is not a composed unit's JSON + text, passes here and raises `FrameError` where the cells are read + (`unit_cells`, `Frame.rows`, `canonical_frame_bytes`). Rows out of + the order `columnar.collation_sort_keys` gives, or with equal grains, make `canonical_frame_bytes` and `frame_sha256` raise `row_key_not_increasing`. Every other table, including any diff --git a/python/frame-grammar/tests/test_streaming.py b/python/frame-grammar/tests/test_streaming.py index 46db765..3a22db5 100644 --- a/python/frame-grammar/tests/test_streaming.py +++ b/python/frame-grammar/tests/test_streaming.py @@ -27,6 +27,7 @@ from dfo_frame_grammar import frame as frame_module from dfo_frame_grammar import grammar +from dfo_frame_grammar._unit_codec import _canonical_unit_from_mapping from dfo_frame_grammar.arrow import build_frame_from_arrow from dfo_frame_grammar.content_version import frame_sha256, stream_frame_sha256 from dfo_frame_grammar.frame import ( @@ -40,14 +41,17 @@ def _whole_table_canonical_bytes(frame: Frame) -> bytes: - """The oracle: the whole table read at once and serialised as one JCS - document.""" + """The oracle: the whole table read row by row with `to_pylist`, each + `unit` struct through the grammar's reader and JSON form, and serialised + as one JCS document.""" rows = [ - list(row) - for row in zip( - *frame_module._column_cells(frame.columns, frame.table, json_form=True), - strict=True, - ) + [ + grammar.unit_cell_to_json(_canonical_unit_from_mapping(row[column])) + if column == grammar.UNIT_COLUMN + else row[column] + for column in frame.columns + ] + for row in frame.table.to_pylist() ] return canonicalize( { @@ -440,6 +444,25 @@ def test_bytes_are_identical_across_slice_sizes( streamed_bytes = b"".join(stream_canonical_frame_bytes(chunks)) assert streamed_bytes == _whole_table_canonical_bytes(built_frame) + def test_a_frame_past_one_shipped_slice_streams_as_the_oracle(self) -> None: + units = [ + None, + "USDollar", + {"currency": "USD", "prices": "current"}, + {"currency": "EUR", "prices": "constant", "base_year": 2020}, + {"numerator": "USDollar", "denominator": {"stat_var": "dcid:Count_Person"}}, + ] + rows = [ + _row( + recipient=f"country/C{index}", + value=float(index), + unit=units[index % len(units)], + ) + for index in range(frame_module._STREAM_SLICE_ROWS * 2 + 5) + ] + frame = Frame.build(role_keys=_ROLE_KEYS, rows=rows) + assert canonical_frame_bytes(frame) == _whole_table_canonical_bytes(frame) + _ALPHABET = ["a", "b", "Z", "é", "e\u0301", "å", "ß", "\U0001f600"] From afdae027301f17efe7210d6d0095189043b89bab Mon Sep 17 00:00:00 2001 From: Jorge Rivera Date: Sat, 31 Oct 2026 19:54:52 +0100 Subject: [PATCH 6/6] fix(registry): register over_ceiling on datasets.observations and datasets.files --- python/spec-kit/CHANGELOG.md | 4 ++++ python/spec-kit/src/dfo_spec_kit/registry/codes.json | 10 ++++++++++ registries/codes.json | 10 ++++++++++ 3 files changed, 24 insertions(+) diff --git a/python/spec-kit/CHANGELOG.md b/python/spec-kit/CHANGELOG.md index d7b2250..b740881 100644 --- a/python/spec-kit/CHANGELOG.md +++ b/python/spec-kit/CHANGELOG.md @@ -179,6 +179,10 @@ and this package adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Code `identity_quarantined` (500) maps to `datasets.snapshot`, `datasets.files`, `datasets.pin`, `datasets.observations` and `datasets.refresh`, the same status it has on the other Datasets doors. +- Code `over_ceiling` maps to `datasets.observations` and `datasets.files` + at 422, for a stored frame whose recorded size passes the decode bound or a + file render past the fold ceiling, the status it has on the other Datasets + doors. ### Added diff --git a/python/spec-kit/src/dfo_spec_kit/registry/codes.json b/python/spec-kit/src/dfo_spec_kit/registry/codes.json index fa20b64..054e61d 100644 --- a/python/spec-kit/src/dfo_spec_kit/registry/codes.json +++ b/python/spec-kit/src/dfo_spec_kit/registry/codes.json @@ -3485,6 +3485,16 @@ "channel": "problem_details", "http_status": 422 }, + { + "endpoint": "datasets.observations", + "channel": "problem_details", + "http_status": 422 + }, + { + "endpoint": "datasets.files", + "channel": "problem_details", + "http_status": 422 + }, { "endpoint": "mcp.tools", "channel": "mcp_refusal" diff --git a/registries/codes.json b/registries/codes.json index fa20b64..054e61d 100644 --- a/registries/codes.json +++ b/registries/codes.json @@ -3485,6 +3485,16 @@ "channel": "problem_details", "http_status": 422 }, + { + "endpoint": "datasets.observations", + "channel": "problem_details", + "http_status": 422 + }, + { + "endpoint": "datasets.files", + "channel": "problem_details", + "http_status": 422 + }, { "endpoint": "mcp.tools", "channel": "mcp_refusal"