[ENHANCEMENT](pyspark) Generalize field_path to arbitrary map/array nesting#572
Draft
Seth Fitzsimmons (sethfitz) wants to merge 16 commits into
Draft
[ENHANCEMENT](pyspark) Generalize field_path to arbitrary map/array nesting#572Seth Fitzsimmons (sethfitz) wants to merge 16 commits into
Seth Fitzsimmons (sethfitz) wants to merge 16 commits into
Conversation
…helpers Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…ction_check rationale Add test_nested_map_keys_check_flattens_inner_array_errors to close coverage gap and verify the keys helper properly flattens nested array errors. Restore docstring rationale explaining why _map_projection_check calls _null_guarded_transform directly. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…ents, dropping iter_count Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…to one iteration fold Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…-map, and map-in-array validation Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…on fold Rewrites the Check Builder section of packages/overture-schema-codegen/docs/design.md to describe the current Direct/Iterated FieldPath taxonomy, anonymous segments, and the mixed array/map nesting cases (dict[K,list], list[dict], nested maps, map-in-array), replacing the retired ScalarPath/ArrayPath/MapPath description. Net non-test line delta against the branch base (31d4b97), informational only per task brief (emission is in scope, net reduction is not a gate): git diff --stat 31d4b97 -- ':!*/tests/*' ':!*test*' reports 616 insertions(+), 654 deletions(-) across _render_common.py, check_builder.py, check_ir.py, renderer.py, column_patterns.py, and field_path.py -- a net -38 lines. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…e frame terminology Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…ation
Task 4 removed the old "Union-under-multi-list" NotImplementedError,
which was correct for `list[list[Union{scalar fields}]]` -- those render
and validate, the variant field sitting at the union element level. But
when a union variant field is itself an iterated container (e.g.
`list[list[Union{codes: list[int]}]]` or `items[]` where an arm's field
is a list), the check is reached through iteration BEYOND the
discriminated element. The renderer applies the ElementGuard at the
innermost iteration variable, which for that shape is the deeper element
-- not where the discriminator lives -- so the render silently gates on
the wrong element.
Detect the divergence in check_builder: when an ElementGuard is created,
any guarded check whose target has more iteration frames than the
union's prefix iterates past the discriminator, so raise a
capability-gate NotImplementedError naming the real gap. This restores a
loud guard for the raise->silent regression while leaving the
correctly-rendering scalar-variant shape
(test_union_under_multi_list_renders) untouched. Covers both the
anonymous-array terminal (`list[list[Union]]`) and the named-array/map
terminals (bd-au22).
Also key the render_synthetic memo cache on `(cls, name)` rather than
`name` alone, so a name reused with a different model class cannot
return a stale module.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…h docstring Give `leaf_list_depth`'s trailing-anonymous-segment scan an explicit `while leaf_index > 0` bound so the termination (guaranteed by the Iterated first-iterating-named invariant) is local rather than implicit. Behavior is unchanged: the first iterating segment is always named, so the scan never reaches index 0. Reword `promote_terminal`'s docstring to describe current behavior only, dropping the "absorbs the former ... cases" historical framing that named removed symbols. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…cated dispatch Extract terminal_run_start into field_path (the backward mirror of _array_run_length); scaffold.leaf_list_depth and mutations.mutate_unique_items call it instead of carrying identical backward-scan loops. Collapse set_at_path's anonymity dispatch into _array_slot, retiring the one-line _descend_through_array forwarder. Extract the shared _iter_kwargs prelude into _array_target_paths, thread the model-constraint condition bindings through a _condition_ref closure, fold _check_belongs_to_arm, and drop the dead _is_array_target predicate. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…helpers Narrow check.target to Iterated once at the top of _iter_kwargs_leaf and _iter_kwargs_inner, reading iter_struct_paths/leaf/outer_column directly rather than through the _ArrayTargetPaths passthrough vocabulary. Delete the _ArrayTargetPaths NamedTuple and _array_target_paths helper, whose only computed output was a single array_path= string. Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Seth Fitzsimmons (sethfitz)
temporarily deployed
to
staging
July 18, 2026 03:28 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
🗺️ Schema reference docs preview is live!
Note ♻️ This preview updates automatically with each push to this PR. |
…nt iteration (bd-swy2)
`_is_map_target` classified a target as a map target when its innermost
iterating frame was a `MapSegment`. For an array-first-map path like
`items[].tags{value}` that returned True, so the field-mutation path
emitted `mutate_map_value(row, "items", ...)` on the array column
instead of the inner map -- latent because no live schema field is
array-first-map and the execution test never drove `render_test_module`.
Part 1: `_is_map_target` reads the first iterating frame (map-first),
matching `check_builder`. A map reached only after array iteration is no
longer a map target and falls to `set_at_path`.
Part 2: `set_at_path` learned the `{value}` / `{key}` grammar,
corrupting a map's single entry in place after descending any preceding
arrays -- so `set_at_path("items[].tags{value}", "")` mutates the inner
map value. The model path and the map-first-with-leaf field path, which
have no in-place mutation and no live case, raise `NotImplementedError`
rather than misroute silently.
A synthetic `list[Model{dict[str, str]}]` runs through the full
test-generation path and the Spark harness, proving the mutation catches
the violation on the invalid row while the valid row stays clean.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…rmance mutation (bd-swy2)
`_map_field_mutate_expr` and `_map_kwargs` gated a map-first target on
`len(iter_frames) > 1 or leaf`, which caught `subs{value}.label` but
missed `subs{value}[]` (`dict[K, list[...]]`): the trailing anonymous
array folds into the map's single named frame, so `iter_frames` stays
length 1 and `leaf` stays empty. The field path silently rendered
`mutate_map_value(row, 'subs', '')`, corrupting a list-valued map entry
with a scalar; the model path silently emitted `map_path="subs"`,
addressing the list as a model instead of descending into it.
Both guards now also check `iter_struct_paths`, which is non-empty
exactly when a map-first target has trailing iteration folded into its
outer frame. `names.common{key}` and `sources.license_priority{value}`
-- the only live map-first fields -- have a single, first iterating
segment and empty `iter_struct_paths`, so they are unaffected.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
Seth Fitzsimmons (sethfitz)
temporarily deployed
to
staging
July 18, 2026 04:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
…ks with trailing iteration
Two map-first field-check shapes reach the constrained scalar through a
map value that is itself an iterated container: `subs{value}[]` (dict[K,
list[X]]) and `subs{value}{value}` (dict[K, dict[K2, X]]). Both were
loudly gated in the conformance-test renderer because
`mutate_map_key`/`mutate_map_value` corrupt a map's single entry in
place and cannot descend a container value.
Generalize `set_at_path`/`parse_path` to descend a non-terminal
`{value}` projection into the sole entry's value and keep navigating --
`[]` peels to element 0, another `{value}` peels to the next sole value.
A non-terminal `{key}` raises: a map key is an immutable scalar with
nothing to descend into.
Route iteration-only-trailing map-first field checks to `set_at_path`
with the full path (its grammar now peels each trailing container to the
scalar); keep sole map projections (`names.common{key}`) on
`mutate_map_key`/`mutate_map_value` and keep a struct leaf after a map
(`subs{value}.label`) loudly gated. Extend the scaffold generator's
anonymous-run peel to strip trailing anonymous map segments so
map-of-map targets scaffold like map-of-list.
Ground-truthed end to end: new conformance tests build the model, run
the generated `Scenario.mutate` through the real Spark harness, and
assert the invalid row trips the violation on the nested node while the
valid row stays clean.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…der mixed map/array nesting
Two model-constraint shapes reach the constrained model through a
container-after-container boundary that no scalar mutation kwarg
expresses: `subs{value}[]` (dict[K, list[Model]], map-then-array) and
`items[].configs{value}` (list[dict[K, Model]], array-then-map). Both
were loudly gated in the conformance-test renderer.
Add one composite kwarg `element_path` to the model mutation helpers
(`mutate_require_any_of`/`mutate_min_fields_set` via
`_null_all_named_fields`; `mutate_require_if`/`mutate_forbid_if` via
`_apply_to_targets`), carrying the full element-relative descent walked
generically by `_descend_to_targets`: array segments iterate every
element, map `{value}` segments take the sole value, struct segments
navigate a field, in any order. The scalar
array_path/map_path/struct_path/inner_array_path kwargs are untouched --
element_path fills exactly the gap they cannot express, no forked
descent.
Emit it from the two shared raise sites: `_map_kwargs` returns the
composite path when a map value iterates further, and
`_array_first_map_kwargs` (replacing `_reject_array_first_map`) returns
it when a map sits under array iteration. The emission chokepoint
rejects a map-key frame at codegen time, matching the map-first guard --
a model can't sit on a map key in either nesting order.
Ground-truthed end to end: new conformance tests run the generated
require_any_of mutation through the real Spark harness for both orders,
asserting the invalid row trips the constraint on the nested model while
the valid row stays clean.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
…t-map stub; cover composite-descent claims
RequireAnyTrue silently rendered a root-level mutation for a nested
ModelCheck target, ignoring the iteration entirely -- mirror
RadioGroup's existing raise so the gap fails loudly instead. Make the
map-then-array absent-map stub in _element_map_value shape-aware (list
vs dict) so subs{value}[] works when the map itself is missing, not just
when it's present. Add direct tests for the "generalizes for free"
claims on composite element_path descent (forbid_if, min_fields_set,
subs{value}[][], subs{value}[].inner) -- all held.
Signed-off-by: Seth Fitzsimmons <seth@mojodna.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #570 — generalize
field_pathto arbitrary map/array nesting.What changed
Collapses
field_path's three-class taxonomy (ScalarPath/ArrayPath/MapPath) intoDirect+Iterated, extends the grammar to interleaved map/array markers, and replaces the four array/map wrap functions with one_wrap_in_iterationfold overRenderFrames. A container nested directly inside another with no field name between them is an anonymous iterating segment, solist[list[X]],dict[K, list],list[dict], nested maps, and maps reached through arrays are now representable and render working validation end-to-end. Adds the flattening runtime pairnested_map_{keys,values}_check.How it was built
Five tasks from the implementation plan, each implemented test-first, task-reviewed, and fixed, then a whole-branch review and a convergent-simplification pass (3 rounds, converged).
make test-all(regenerate +-W errorconformance) is green at every commit; 6321 passing at HEAD.Behavior is preserved for the current schema — the two live
dict[K, scalar]maps (CommonNames, annexSources.license_priority) and every other shape validate identically. The newly-representable shapes have no schema field to exercise them, so their ground truth is a hand-built real-Spark execution test (test_iterated_emission.py) that catches a violation on an invalid row and passes a valid one for each new frame combination.Net impact
20 files, +1825/−1400. Non-test source is +32 net despite the added emission capability, because the new shapes ride on shared primitives (one fold, one enriched-frame builder, one runtime pair) rather than parallel code paths.
Notes
The whole-branch review caught that removing the old "union under multi-list" guard turned
list[list[Union{list-valued variant field}]]from a raise into a silent mis-render; it is restored as a deliberate capability guard (_guard_variant_field_past_element), closing bd-au22.The generated-conformance-test path now handles mixed map/array shapes: a field check on a map value/key reached through array iteration (
items[].tags{value}) descends the array and mutates the right map side, ground-truthed against real Spark. The remaining mixed shapes with no in-place mutation (subs{value}[],items[].configs{value}) are loud capability gates, not silent misroutes. This closes bd-swy2 (the_is_map_targetmisrouting).bd-mwy8 (open, pre-existing, orthogonal): the base-row synthesizer fills one character for a string
min_length, so a field with stringmin_length > 1would produce an invalid conformance baseline. No current schema field hits it.The shared codegen-pipeline rule doc (
dotclaude/rules/pyspark-codegen-pipeline.md, outside this repo) was updated to the new taxonomy separately.Base is
pyspark-expression-codegen— this stacks on that branch, which is not yet onmain. Design spec and plan:docs/superpowers/{specs,plans}/2026-07-17-generalize-field-path*.