perf(relational): coordinate validation rides the aggregate already running

[FBumann]

Closes #273.

check_coordinates_single_valued ran one group_by(d) per declared coordinate, and because the frame is a lazy scan over the caller's index, each collect() re-opened and re-parsed the source. Four coordinates meant four passes and four reads of the same file.

The fix is the issue's own "better still"

_explicit_dim_frame is the only caller, and it already groups by d over that same frame to build (val, ord, coordinates…). The n_unique counts now ride in that aggregate and the check reads them out of the result:

grouped = (
    frame.select(d, *names)
    .with_row_index(_ROW_POSITION)
    .group_by(d)
    .agg(pl.col(_ROW_POSITION).min(),
         *(pl.col(c).first() for c in names),
         *data_validation.nunique_exprs(names))
    ...
    .collect()
)
data_validation.check_coordinates_single_valued(d, names, grouped)

Zero extra passes, and one collect where there were N + 1.

Measured

200k-label index with four coordinates, minimum of five builds, three alternating rounds:

round	folded	before
1	0.0378 s	0.0645 s
2	0.0386 s	0.0676 s
3	0.0408 s	0.0681 s

~1.75x on that step, and repeatable rather than inside the noise — which is worth stating given how noisy this machine has been on smaller numbers. The re-read is what makes this more than the polish the issue was filed as; index sources are dim-sized, but re-parsing a parquet file per coordinate is not.

A usability bug went with it

The loop raised on the first offending coordinate, so a source with two bad ones was fixed, rebuilt, and refused again. Every offender is named now, which is free once the counts arrive in one frame:

DataError: dimension 'line' carries more than one value per label for
coordinate(s): 'from' (1 label(s)); 'to' (1 label(s)). …

test_every_multi_valued_coordinate_is_named_at_once pins it.

This landed cheaply because #355 had just isolated these checks into relational/data_validation.py as pure functions, and #354's parity table pins the verdicts from outside — so the optimisation could not quietly change what gets refused.

Verified

700 passed, 4 skipped, 1 xfailed. ruff check, ruff format --check, pyrefly check (0 errors). Built in a git worktree.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved validation of dimension coordinates during data processing.
  • Errors now report all coordinates with multiple label values in a single message, rather than stopping at the first issue.
  • Preserved the existing output structure while streamlining validation of grouped data.

• Tests

  • Added coverage for cases where multiple coordinates are invalid simultaneously.

[read-the-docs-community]

Documentation build overview

📚 lpspec | 🛠️ Build #33835981 | 📁 Comparing df4eae1 against latest (370aaf6)

  🔍 Preview build  

1 file changed

± changelog/index.html

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3dafc384-2535-4f8f-8be7-fca3c0b2c449

📥 Commits

Reviewing files that changed from the base of the PR and between 370aaf6 and df4eae1.

📒 Files selected for processing (3)

• src/lpspec/relational/data_validation.py
• src/lpspec/relational/executor.py
• tests/test_relational.py

---

📝 Walkthrough

Walkthrough

Coordinate validation now computes all coordinate uniqueness counts during the existing dimension aggregation. The validator consumes those counts and reports every multi-valued coordinate in one DataError; regression coverage verifies both offending coordinates are named.

Changes

Coordinate validation

Layer / File(s)	Summary
Uniqueness count validation src/lpspec/relational/data_validation.py	Adds standardized uniqueness-count expressions and updates validation to inspect aggregated counts and report all offending coordinates.
Grouped dimension aggregation src/lpspec/relational/executor.py, tests/test_relational.py	Combines uniqueness checks with dimension aggregation and adds a regression test covering two invalid coordinates reported together.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• #273: Folds per-coordinate validation into one grouped aggregation and reports all invalid coordinates at once.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/one-pass-coordinate-validation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}