feat: set-dimension wildcard as in-band sentinel (frontier-dedup fix) - #52
Merged
Merged
Conversation
…spec Supersedes the null-list wildcard A1 approach. Represents set wildcards as [unknown_sentinel_for(dtype)] across input/filter/build; frontier fix is free (normalization makes co_ columns null-free + canonical, _frontier_filter untouched). Honors the null-is-not-a-portable-sentinel principle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ressed - reject bool set dims (no typed bool sentinel) — F1 - enforce sentinel reservation + reject mixed/embedded sentinel + element-nulls at ingestion — F2/F6/F7 - narwhals scoped honestly under #89; no unified-backend claim — F3 - typed sentinel via element coercion (float), NOT native list cast (purity) — F4 - normalization a named mandatory build stage before empty/partition/anchor + pre-frontier non-null assert — F5 - co_<field>_na = wildcard predicate of FINAL coalesced value; 3-case test — F8 - cross-backend idempotence test — F9 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6 TDD tasks: bool-set rejection; core/set_wildcard.py helpers+validation; filter compiler short-circuit+engine validation; accumulator compiler set branches; engine normalization stage+seed NA+pre-frontier assertion; apply round-trip+float+docs. Frontier untouched; backend-pure throughout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two confirmed bugs fixed + minors:
- split validation: portable reservation check (contains+len, both engines) vs
null-element check (drop_nulls, Ibis-unsupported -> accumulator build only)
- filter validation via shared _validate_set_rules_once() called in BOTH
_scored_relation AND _evaluate_batch_frame (batch path was bypassed)
- F8 spec permits proven-equivalent co_wild AND rhs_wild input formula
- fix fail-first expected output; dedup test asserts exact {6}; empty-path
assertion vacuous note; docs backend scope (not 'polars-native fallback')
- cross-dtype (float/date) idempotence test; pin tests/filter/test_engine.py
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Includes cross-assertions pinning each validator to its own violation class (reservation vs null-element) and a direct sentinel_list_expr typing test, per sol task review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… rule lists Wire the in-band sentinel into the filter compiler (normalize rule list, short-circuit wildcard to ternary 0) and validate set columns once on both scoring entry points (single via _scored_relation, batch via _evaluate_batch_frame). Also fix set_wildcard validators to count rows with the relation's portable count_rows() instead of len(collect()) — the latter raises on the Ibis backend (ExpressionError: Use .count() instead), which broke the ibis-duckdb mixed-strategy integration tests once the validator was first wired in. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, dedupes correctly
Normalize every set rule column to the non-null canonical in-band-sentinel form
at ingestion (before the empty-frame branch and anchor), plus a seed-NA set
branch and a pre-frontier non-null safety assertion. The frontier self-join is
unchanged — its inputs are now null-free and canonical, so wildcard-set
combinations dedupe (3 wildcard rules collapse to prime-product {30}).
The pre-frontier assertion counts nulls via all_combos.filter(...).count_rows()
(all_combos is already a Relation; do not re-wrap, and count_rows is portable).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…document in-band sentinel Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
❌ The last analysis has failed. |
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.
A1: set-dimension wildcard as an in-band sentinel (frontier-dedup fix)
Represents a
SET_MEMBERSHIP/SET_EXCLUSIONwildcard as the in-band list[unknown_sentinel_for(dtype)]— never a null — across the filter engine andthe accumulator build. This fixes the frontier self-join bug where null-list set
wildcards were silently dropped by polars'
join_nulls=False, so wildcard-setcombinations were never pruned (
applyreturned too many survivors, wrongaggregates).
Governing principle:
null-is-not-a-portable-sentinel— nulls diverge across backends in joins/grouping/membership; use in-band typed sentinels.Design
Set wildcard = the single-element list
[unknown_sentinel_for(dim.data_type)],element-coerced to the dim's Python type (float sentinel →
-999999999.0).Rule columns are validated and normalized at ingestion in both engines, so
_frontier_filteris untouched — its inputs are simply null-free andcanonical. Bool set dims, sentinel-embedding lists, and element-nulls are
rejected fail-loud.
Spec:
docs/superpowers/specs/2026-07-20-accumulator-set-sentinel-wildcard-design.mdPlan:
docs/superpowers/plans/2026-07-20-accumulator-set-sentinel-wildcard.md(both adversarially reviewed with Codex).
What changed (6 commits)
feat(dimension): reject bool set dimensions (no typed wildcard sentinel).feat(core): sharedcore/set_wildcard.py— sentinel/normalize/detect helpers + portable reservation validation + accumulator-only null-element validation.feat(filter): set ternary short-circuits the wildcard rule to0on the normalized column; set rule lists validated once on both_scored_relationand_evaluate_batch_frame.feat(accumulator): set coalescing (membership → list intersection, exclusion → list union), compatibility, and NA-flag (co_wild AND rhs_wild).feat(accumulator): the frontier fix — set-column normalization stage at ingestion + seed-NA set branch + pre-frontier non-null assertion. Frontier self-join unchanged.test+docs: apply round-trip, float-dim typing,CLAUDE.md.The bug, verified fixed
Three wildcard-set rules now dedupe to the single maximal combination
(prime-product
{30}) instead of surviving as 7 combinations. Covered by theanchor regression
test_three_wildcard_rules_collapse_to_single_maximal.Two portability defects found and fixed during execution
len(collect())raisesExpressionError: Use .count() insteadon the Ibisbackend → both validators now use the relation's portable
count_rows().(Restored 3 previously-passing
ibis-duckdbmixed-strategy integration tests.)Relationall_combos→rewritten to
all_combos.filter(is_null).count_rows().Testing
conftest.pyuntouched — no new xfail groups).# allow:tags, zero new native imports.🤖 Generated with Claude Code