Sync measures.yaml after tidy review apply#68
Merged
Conversation
Wide-form measure columns are pivoted into a single value column on the long form, so any pre-existing measure overrides that target those source columns become stale (the columns no longer exist on the relevant table). The apply pipeline previously only synced schema.yaml, leaving measures.yaml pointing at columns that no longer exist. Adds update_measures_yaml_for_apply, mirroring the existing schema sync. Per disposition mode: - keep: existing entries on source stay valid; append a stub for the long form's value column. - rename: rewrite `table` field on entries from the source name to the rename_to name (the columns themselves are unchanged); append value stub on the new long-form table. - replace / drop: drop entries whose columns no longer exist on the source table after the reshape; append value stub on whichever table ends up holding the long form. Seeded value entries are intentionally minimal (table + column only) so missing fields fall back to inference at read time. Re-applying the same proposal is idempotent. Wired into both the CLI apply path (cli_commands/tidy.py) and the web Apply endpoint (web/app.py), matching the create_if_absent semantics already in place for schema.yaml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new update_measures_yaml_for_apply is at complexity 11, and the existing _apply_review_proposals went from 11 to 12 once the new measures.yaml sync block was added. Both follow the established convention from the C901 enforcement PR: existing complex functions get tagged so the rule applies to new code only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
daniel-thom
force-pushed
the
fix/tidy-measures-sync
branch
from
aecb86a to
de6f8d9
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the tidy review apply pipeline to keep measures.yaml consistent with wide-to-long reshapes, mirroring the existing schema.yaml sync so measure overrides don’t keep pointing at pivoted-away wide columns.
Changes:
- Added
update_measures_yaml_for_applyinsrc/datasight/tidy_review.pyto prune stale measure override entries and seed a stub for the new long-form value column. - Wired measures syncing into both the CLI apply path and the web
/api/tidy/applyendpoint (withcreate_if_absent=Truefor the web path). - Added 7 unit tests covering disposition modes, idempotency, and create-if-absent behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/datasight/tidy_review.py |
Adds the measures.yaml sync helper and exports it. |
src/datasight/cli_commands/tidy.py |
Calls the new helper after successful applies (CLI semantics: no-op if missing). |
src/datasight/web/app.py |
Calls the new helper in the web Apply endpoint (web semantics: create file if absent). |
tests/test_tidy_review.py |
Adds unit tests for update_measures_yaml_for_apply. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #68 +/- ##
==========================================
- Coverage 86.90% 86.87% -0.04%
==========================================
Files 61 61
Lines 11462 11539 +77
==========================================
+ Hits 9961 10024 +63
- Misses 1501 1515 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Mirrors what `datasight generate` writes: run `_infer_measure_semantics` against the value column with the long form's id + dimension columns as siblings, then emit all fields (role, default_aggregation, allowed_aggregations, etc.). Functionally equivalent at read time but gives the user visible fields to edit inline. The dtype assumption (DOUBLE) is safe because DuckDB UNPIVOT widens the value column to the broadest numeric type of the mapped columns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace mode now also rewrites pre-existing entries that point at the long form's intermediate target_object_name, since that name disappears when the long form is renamed to take over the source's name. Drop mode now removes every entry referencing the dropped source table, not just entries on mapped pivot columns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions Bot
pushed a commit
that referenced
this pull request
May 10, 2026
Sync measures.yaml after tidy review apply
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
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.
Summary
The
tidy reviewapply pipeline previously only syncedschema.yaml, leavingmeasures.yamlpointing at columns that no longer exist after a wide-to-long reshape. Reproed againstdsgrid-test-data/.../Commercial_End_Uses/load_data.csv: after--replace-source,measures.yamlstill referencedelec_heating,elec_cooling, etc. which had been pivoted into a single value column.Adds
update_measures_yaml_for_applyinsrc/datasight/tidy_review.py, mirroring the existing schema sync. Wired into both the CLI apply path (cli_commands/tidy.py:865) and the web Apply endpoint (web/app.py:3387).Behavior per disposition mode
tablefield on entries from the source name torename_to(columns themselves unchanged); append value stub on the new long form.Seeded value entries are intentionally minimal (
table+columnonly) so missing fields fall back to inference defaults at read time. Re-applying the same proposal is idempotent.CLI behavior matches schema.yaml: no-op if the file is absent. Web Apply uses
create_if_absent=Trueso an explicit user click persists across restarts.Test plan
pytest -m "not integration"passes (1488 tests, +7 new)prek run --all-filespassesload_datareshape to verify stale entries clear🤖 Generated with Claude Code