pt/pf_app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete the client-side ordering expressions

Browse Source 
With the prefix stored in pf.log.label there is nothing left to compute, so
all of it goes: ORDER_EXPR_NAMES, buildOrderExpression and its ExprTK
printable-ASCII-per-byte guard, SYNTHETIC_SEGMENTS and its 98/99 ordinals,
syncOrderExpressions with its two self-issued fetches, the dbgOrder tracing,
and the three places it had to be re-applied because restore() replaces
`expressions` wholesale.

What remains is a list of the names it used to manage, stripped by
cleanLayout so a layout saved under the old scheme does not keep ordering by
a rule nothing updates. The strip goes before the axis filter: dropping them
from `expressions` is what makes the existing ok() reject them on every axis,
which restore() requires -- an expression the pivot is using cannot vanish
from underneath it.

Net 250 lines out. The Baseline page's reorder buttons no longer feed
anything and go next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-17 22:44:07 -04:00
parent 885c9abe83
commit 7738b904bf
2 changed files with 26 additions and 218 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ui/src/components/BridgeView.jsx
View File

@ -241,10 +241,9 @@ export default function BridgeView({
} }
} else { } else {
let filter = [] let filter = []
// Expression columns have to come with the filter that uses them. The // Expression columns have to come with the filter that uses them: a
// pivot's filter can name pf_bucket_ord or pf_segment_ord, which exist // filter can name a column that exists only as an expression, and a view
// only as expressions — a view built without them cannot resolve the // built without it cannot resolve the column and fails outright.
// column and fails outright.
let expressions = {} let expressions = {}
if (scope === 'filtered' && viewerRef?.current) { if (scope === 'filtered' && viewerRef?.current) {
const cfg = await viewerRef.current.save() const cfg = await viewerRef.current.save()

237
ui/src/views/Forecast.jsx
View File

@ -18,7 +18,13 @@ const LAYOUTS_KEY = (vid) => `pf_layouts_v${vid}` // named layout list
function cleanLayout(cfg, validCols) { function cleanLayout(cfg, validCols) {
if (!cfg) return cfg if (!cfg) return cfg
const c = { ...cfg } const c = { ...cfg }
const exprNames = new Set(Object.keys(cfg.expressions || {})) // Dead expressions go before the axis filter, not after: dropping them from
// `expressions` is what makes `ok()` reject them everywhere else.
if (DEAD_ORDER_EXPRS.some(n => c.expressions?.[n] !== undefined)) {
c.expressions = { ...c.expressions }
for (const name of DEAD_ORDER_EXPRS) delete c.expressions[name]
}
const exprNames = new Set(Object.keys(c.expressions || {}))
const ok = (col) => validCols.has(col) || exprNames.has(col) const ok = (col) => validCols.has(col) || exprNames.has(col)
if (c.columns) c.columns = c.columns.filter(col => col == null || ok(col)) if (c.columns) c.columns = c.columns.filter(col => col == null || ok(col))
if (c.group_by) c.group_by = c.group_by.filter(ok) if (c.group_by) c.group_by = c.group_by.filter(ok)
@ -30,105 +36,16 @@ function cleanLayout(cfg, validCols) {
return c return c
} }
// Ordering for the pf_bucket and pf_segment column groups, as Perspective // Expression columns this view used to manage, back when the pf_bucket and
// expression columns. // pf_segment ordering prefix was computed in the pivot rather than stored in
// pf.log.label.
// //
// Perspective orders column groups by the value string, and SortDir's `col asc` / // Stripped by cleanLayout, never created: a layout saved under that scheme still
// `col desc` only reverses that — so Prior Year → Plan → Actual → Forecast is // names them, and without this they would sit there forever, ordering by a rule
// expressible as neither, being alphabetical in neither direction. The order has // nothing updates. They have to leave the axes at the same time as the
// to be part of the value, as a "01 · " prefix. // expressions themselves -- restore() rejects a config whose group_by or sort
// // names a column that no longer exists.
// Done as expressions rather than in SQL. The prefix is a pivot-ordering concern, const DEAD_ORDER_EXPRS = ['pf_bucket_ord', 'pf_segment_ord', 'Bucket', 'Segment']
// and computing it server-side put it in every other reader too: the change log
// and the bridge's basis list would both show "04 · Forecast". As expressions it
// stays in the pivot, lives in ViewConfig so it travels with saved layouts, and
// reordering takes effect immediately instead of needing a reload.
//
// Syntax per rust/perspective-client/src/rust/config/expressions.rs: columns in
// double quotes, string literals in single, if/else with braces. The ordered
// label is emitted as a literal rather than built with concat() — fewer moving
// parts, and the mapping is known here anyway.
// pf_ prefixed like every other column the server synthesises, so they sort
// beside pf_bucket and pf_segment in the column list and cannot be mistaken for
// source data. "Bucket" and "Segment" were the first choice and are too close to
// the real thing -- segment_new is an actual column on this source -- and an
// expression named after an existing column would shadow or reject rather than
// merely confuse.
const ORDER_EXPR_NAMES = { bucket: 'pf_bucket_ord', segment: 'pf_segment_ord' }
// Removed from any config that still carries them. They are no longer managed
// names, so without this they would sit in saved layouts forever, ordering by a
// rule nothing updates.
const LEGACY_ORDER_EXPR_NAMES = ['Bucket', 'Segment']
// ASCII only, and " - " specifically because the source data already reads
// "07 - Dec". A middle dot was the first choice and ExprTK rejects it: its string
// scanner tests each *byte* with
//
// is_valid_string_char(c) = isprint((unsigned char) c) || is_whitespace(c)
//
// and "·" is U+00B7, two bytes 0xC2 0xB7 in UTF-8, of which isprint(0xC2) is
// false in the C locale. The literal fails at that byte and the parser reports
// from the start of it — "Invalid string token: 01".
const ORDER_SEP = ' - '
function orderedLabel(ord, label) {
return `${String(ord).padStart(2, '0')}${ORDER_SEP}${label}`
}
// Same byte rule applies to the label being matched, so a bucket named with
// anything outside printable ASCII cannot appear in an expression at all. Such a
// label is left unordered rather than emitted into an expression that will not
// parse and take the whole column with it.
function isExprSafe(v) {
return /^[\x20-\x7e]*$/.test(String(v))
}
function sqlSafe(v) {
return String(v).replace(/'/g, "''")
}
// Anything unlisted falls through to the raw column.\n// pairs: [[rawValue, ordinal], ...] or [[rawValue, ordinal, displayAs], ...] where
// the third element renames the value as well as ordering it.
function buildOrderExpression(sourceCol, pairs, extra = []) {
const cases = [...pairs, ...extra]
.filter(([label, ord]) => label && ord > 0 && isExprSafe(label))
.sort((a, b) => a[1] - b[1])
.map(([label, ord, displayAs]) =>
`if ("${sourceCol}" == '${sqlSafe(label)}') { '${sqlSafe(orderedLabel(ord, displayAs || label))}' }`)
if (cases.length === 0) return null
return `${cases.join('\nelse ')}\nelse { "${sourceCol}" }`
}
// The server's synthetic segment labels. Unordered they sort *first*, not last:
// '(' is 0x28 and digits begin at 0x30, so '(adjustment)' precedes '01 - ...'.
// They get high ordinals instead, and lose the parentheses — those marked the
// value as not-a-real-segment, which the ordinal now does.
const SYNTHETIC_SEGMENTS = [
['(unlabeled load)', 98, 'Unlabeled'],
['(adjustment)', 99, 'Adjustments'],
]
// Build both expressions from the version's bucket_order and the log's seq values.
function buildOrderExpressions(bucketOrder, logMeta) {
const out = {}
const buckets = (Array.isArray(bucketOrder) ? bucketOrder : [])
.map((label, i) => [label, i + 1])
const bucketExpr = buildOrderExpression('pf_bucket', buckets)
if (bucketExpr) out[ORDER_EXPR_NAMES.bucket] = bucketExpr
// Segments are ordered by pf.log.seq, keyed on the label the pivot shows —
// which is the tag if there is one, else the note, the same precedence the
// server uses to build pf_segment.
const segs = Object.values(logMeta || {})
.filter(m => m.seq != null && ['baseline', 'reference'].includes(m.operation))
.map(m => [(m.tag || m.note || '').trim(), m.seq])
const segExpr = buildOrderExpression('pf_segment', segs, SYNTHETIC_SEGMENTS)
if (segExpr) out[ORDER_EXPR_NAMES.segment] = segExpr
return out
}
export default function Forecast({ sources = [], sourceId, versions = [], versionId, refreshSources }) { export default function Forecast({ sources = [], sourceId, versions = [], versionId, refreshSources }) {
const { dark } = useTheme() const { dark } = useTheme()
@ -653,11 +570,8 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
}, [sourceId, versionId]) }, [sourceId, versionId])
useEffect(() => { refreshLogMeta(versionId) }, [versionId]) useEffect(() => { refreshLogMeta(versionId) }, [versionId])
// Reordering on the Baseline page takes effect here without a reload: the // Relabelling on the Baseline page needs a reload to show here: the label is
// expressions are rebuilt and the pivot re-renders. // part of the aggregated row, so the pivot cannot re-derive it in place.
// Re-run when the ordering could have changed on the Baseline page. logMeta is
// the signal, not the source: the function reads the ordering itself.
useEffect(() => { if (tableRef.current) syncOrderExpressions() }, [logMeta, versionId])
useEffect(() => { refreshTags(sourceId) }, [sourceId]) useEffect(() => { refreshTags(sourceId) }, [sourceId])
// Stream an Arrow IPC endpoint into one buffer, reporting download progress. // Stream an Arrow IPC endpoint into one buffer, reporting download progress.
@ -804,19 +718,12 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
adoptSplit(cfg.split_full?.length ? cfg.split_full : cfg.split_by, (cfg.split_by || []).length) adoptSplit(cfg.split_full?.length ? cfg.split_full : cfg.split_by, (cfg.split_by || []).length)
} }
// auto-persist viewer state (formatting, columns, etc.) to the last-used cache // Its own try: the restore above has one, but this sits outside it, and a
// After the layout, not before: restoring a saved config replaces // throw here would abort the rest of initViewer silently.
// `expressions` wholesale, so expressions applied earlier were being wiped
// by the very next line and the ordering columns never appeared in the
// column list.
try { try {
await syncOrderExpressions()
await ensureRowLabelWidth() await ensureRowLabelWidth()
} catch (err) { } catch (err) {
// Its own try covers only restore(); save() and the builders sit outside console.error('[pf-layout] threw', err)
// it, and a throw there would abort the rest of initViewer silently.
console.error('[pf-order] threw', err)
flash(`Column ordering failed: ${err.message || err}`, 'error')
} }
if (viewer._pspUpdate) viewer.removeEventListener('perspective-config-update', viewer._pspUpdate) if (viewer._pspUpdate) viewer.removeEventListener('perspective-config-update', viewer._pspUpdate)
@ -938,100 +845,6 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
// log's seq values. Merged into the live config rather than replacing // log's seq values. Merged into the live config rather than replacing
// expressions, so anything the user defined themselves survives. // expressions, so anything the user defined themselves survives.
// //
// Removing an ordering removes its column: leaving a stale Bucket expression
// behind would keep ordering by an order that no longer exists.
// Silent unless localStorage.pf_debug is set, like the rest of the tracing.
// Kept rather than removed: it took several rounds to work out that this ran
// before its inputs existed, and it will be wanted again.
function dbgOrder(msg, extra) {
try { if (!localStorage.getItem('pf_debug')) return } catch { return }
if (extra === undefined) console.log(`[pf-order] ${msg}`)
else console.log(`[pf-order] ${msg}`, extra)
}
// Fetches its own inputs rather than reading the `versions` prop and the
// `logMeta` state.
//
// Those are populated asynchronously, and this runs from initViewer — which
// finishes well before them on a large load. It was therefore called with
// versionFound: false and logMetaCount: 0 every time, could never build
// anything, and the effect meant to re-run it once the data arrived never
// fired. Two small queries are worth more than the right timing.
async function syncOrderExpressions() {
const viewer = viewerRef.current
if (!viewer) return
let bucketOrder = null
let entries = []
try {
const [vers, log] = await Promise.all([
fetch(`/api/sources/${sourceId}/versions`).then(r => r.ok ? r.json() : []),
fetch(`/api/versions/${versionId}/log`).then(r => r.ok ? r.json() : []),
])
bucketOrder = (Array.isArray(vers) ? vers : [])
.find(v => String(v.id) === String(versionId))?.bucket_order ?? null
entries = Array.isArray(log) ? log : []
} catch (err) {
console.error('[pf-order] could not read the ordering', err)
return
}
const byId = Object.fromEntries(entries.map(e => [e.id, e]))
const wanted = buildOrderExpressions(bucketOrder, byId)
dbgOrder('inputs', {
bucket_order: bucketOrder,
seqs: entries.filter(e => e.seq != null).map(e => `${e.tag || e.note}=${e.seq}`),
wanted: Object.keys(wanted),
})
const { table: _t, ...cfg } = await viewer.save()
dbgOrder('saved config read', { existing: Object.keys(cfg.expressions || {}) })
const current = cfg.expressions || {}
const managed = [...Object.values(ORDER_EXPR_NAMES), ...LEGACY_ORDER_EXPR_NAMES]
const next = { ...current }
for (const name of managed) delete next[name]
Object.assign(next, wanted)
const same = JSON.stringify(next) === JSON.stringify(current)
if (same) {
// Nothing to do is a legitimate outcome, but "nothing was ordered" and
// "the ordering is already applied" look identical from the outside.
if (Object.keys(wanted).length === 0) {
dbgOrder('no ordering to apply', {
bucket_order: version?.bucket_order,
seqs: Object.entries(logMeta || {})
.filter(([, m]) => m.seq != null)
.map(([id, m]) => `${id}:${m.tag || m.note}=${m.seq}`),
})
}
else dbgOrder('exit: already applied', Object.keys(next))
return
}
dbgOrder('applying', Object.keys(next))
// An expression the pivot is using cannot simply vanish; drop it from the
// axes first or restore() rejects the config.
const stillValid = (col) => next[col] !== undefined || !managed.includes(col)
try {
await viewer.restore({
...cfg,
expressions: next,
columns: (cfg.columns || []).filter(c => c == null || stillValid(c)),
group_by: (cfg.group_by || []).filter(stillValid),
split_by: (cfg.split_by || []).filter(stillValid),
sort: (cfg.sort || []).filter(([c]) => stillValid(c)),
})
dbgOrder('applied', Object.keys(next))
} catch (err) {
// Surfaced rather than logged: this failing silently is what made the
// missing Bucket and Segment columns so hard to pin down -- the columns
// simply were not there, with nothing to say why.
console.error('[syncOrderExpressions]', err, { wanted, next })
flash(`Column ordering failed: ${err.message || err}`, 'error')
}
}
// Size every column to its contents. // Size every column to its contents.
// //
// Values fit on their own: draw() calls regular_table.resetAutoSize(), which // Values fit on their own: draw() calls regular_table.resetAutoSize(), which
@ -1378,15 +1191,11 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
if (cfg.group_by_depth != null) setExpandDepth(cfg.group_by_depth - 1) if (cfg.group_by_depth != null) setExpandDepth(cfg.group_by_depth - 1)
else if (cfg.expand_depth != null) await applyDepth(cfg.expand_depth) else if (cfg.expand_depth != null) await applyDepth(cfg.expand_depth)
// restore() replaces `expressions` wholesale, so loading a layout drops the
// ordering columns — and a layout saved before they existed has none to put
// back. Re-applied after every restore that comes from a saved config, which
// is the general form of the fix initViewer already needed.
await syncOrderExpressions()
await ensureRowLabelWidth() await ensureRowLabelWidth()
setActiveLayoutId(layout.id) setActiveLayoutId(layout.id)
// After the sync, so the persisted copy carries the expressions too. // The persisted copy is taken after the restore, so it carries whatever
// cleanLayout dropped on the way in rather than the stale original.
const merged = await captureConfig() const merged = await captureConfig()
await persistLayout(versionId, merged || cfg) await persistLayout(versionId, merged || cfg)
} }