Filter aggregates with the rest of the layout
cleanLayout's job is to let a layout outlive the columns it names, and it walked every config field that carries a column name except aggregates -- which carries one as its key and, in the multi-arg form, a second as the weight. Empty in practice today, so nothing was breaking; the first explicit aggregate would have made a later column change abort the whole restore rather than lose one entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d2ba944d41
commit
9e9782419a
@ -361,10 +361,9 @@ anyone else, gone on another machine. The one server-side layout was
|
|||||||
published layout with no owner. That route is gone; the column remains, migrated
|
published layout with no owner. That route is gone; the column remains, migrated
|
||||||
and read by nothing.
|
and read by nothing.
|
||||||
|
|
||||||
**Known gap:** `cleanLayout()` still does not guard `aggregates`
|
`cleanLayout()` guards `aggregates` along with the axes — including the weight
|
||||||
(PERSPECTIVE.md §5). Harmless while `save()` emits `aggregates: {}`, but a
|
column of the multi-arg form — and drops the offending *entry* rather than the
|
||||||
published layout that adopts the weighted-mean pattern and then loses a column
|
layout, since `restore()` is all-or-nothing.
|
||||||
would break the restore for everyone rather than for one browser.
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@ -257,15 +257,15 @@ it would retire the `if(...)`-expression workaround. It costs the d3fc charts, t
|
|||||||
exist in the current dataset (plus any `expressions`). dataflow's `cleanLayout()` is
|
exist in the current dataset (plus any `expressions`). dataflow's `cleanLayout()` is
|
||||||
the reference implementation; a stale layout referencing a dropped column otherwise
|
the reference implementation; a stale layout referencing a dropped column otherwise
|
||||||
throws on restore.
|
throws on restore.
|
||||||
- **`aggregates` needs the same guard, and doesn't currently have it** (verified 2026-08,
|
- **`aggregates` needs the same guard.** pf_app's `cleanLayout()` has it as of 2026-09;
|
||||||
pf_app). Both existing `cleanLayout()` implementations filter
|
**dataflow's does not** and should adopt it. Filtering
|
||||||
`columns`/`group_by`/`split_by`/`sort`/`filter` but leave `aggregates` untouched. That
|
`columns`/`group_by`/`split_by`/`sort`/`filter` and stopping there is harmless only
|
||||||
is harmless *today* only because `viewer.save()` emits `aggregates: {}` until someone
|
while `viewer.save()` emits `aggregates: {}`, which it does until someone sets an
|
||||||
sets one explicitly. The moment a layout adopts the weighted-mean pattern (§3a), a
|
aggregate explicitly. The moment a layout adopts the weighted-mean pattern (§3a), a
|
||||||
dropped column aborts the entire restore — both `table.view()` and `viewer.restore()`
|
dropped column aborts the entire restore — both `table.view()` and `viewer.restore()`
|
||||||
throw `Could not get dtype for column 'X' as it does not exist in the schema`. An
|
throw `Could not get dtype for column 'X' as it does not exist in the schema`. An
|
||||||
aggregate entry references a *target* column and, in the multi-arg form, a *weight*
|
aggregate entry references a *target* column and, in the multi-arg form, a *weight*
|
||||||
column; both need validating, and the entry should be dropped rather than the layout.
|
column; both need validating, and the entry is what gets dropped, never the layout.
|
||||||
Add this guard as part of adopting §3a, not after.
|
Add this guard as part of adopting §3a, not after.
|
||||||
|
|
||||||
### Auto-pause deletes the view — turn it off for a static pivot
|
### Auto-pause deletes the view — turn it off for a static pivot
|
||||||
|
|||||||
@ -43,6 +43,17 @@ function cleanLayout(cfg, validCols) {
|
|||||||
if (c.split_full) c.split_full = c.split_full.filter(ok)
|
if (c.split_full) c.split_full = c.split_full.filter(ok)
|
||||||
if (c.sort) c.sort = c.sort.filter(([col]) => ok(col))
|
if (c.sort) c.sort = c.sort.filter(([col]) => ok(col))
|
||||||
if (c.filter) c.filter = c.filter.filter(([col]) => ok(col))
|
if (c.filter) c.filter = c.filter.filter(([col]) => ok(col))
|
||||||
|
// aggregates is keyed by column and, in the multi-arg form, names a second
|
||||||
|
// one: ["weighted mean", "units"] depends on the weight column as much as on
|
||||||
|
// the column it is set against, so both have to survive or the entry goes.
|
||||||
|
// Drop the entry, never the layout -- restore() is all-or-nothing, so one
|
||||||
|
// stale aggregate would otherwise take the whole config down with it.
|
||||||
|
if (c.aggregates) {
|
||||||
|
c.aggregates = Object.fromEntries(
|
||||||
|
Object.entries(c.aggregates).filter(([col, agg]) =>
|
||||||
|
ok(col) && (!Array.isArray(agg) || agg.slice(1).every(a => typeof a !== 'string' || ok(a))))
|
||||||
|
)
|
||||||
|
}
|
||||||
return c
|
return c
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user