From d983e2b1df7e6810c6bd158897779691944353e3 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Fri, 18 Sep 2026 08:46:23 -0400 Subject: [PATCH] Carry the pivot filter as a scope, operators and all Refusing anything but == was safe and useless: a view bounded to sseas_e <= 2027 is an ordinary way to scope a forecast, and it has no slice form at all, a slice being {col: value}. The filter now travels beside the slices as [col, op, value] triples and is ANDed onto every unit -- not folded into the slices, since it applies to all of them equally and under apply_mode 'each' would just repeat itself in every statement. Operators are Perspective's, since that is where they come from, and the list is a whitelist: anything outside it is refused rather than ignored, because a scope silently dropped is a write wider than the panel that authorised it. The scope goes into the log's params too, so the audit trail records what bounded the write and not only what was clicked. The panel prints it above the selection as "within sseas_e <= 2027". It scopes every figure below it and every row the operation writes while appearing in none of the slices, so without it the panel showed a selection wider than the one it was acting on -- which is exactly what made the ledger's 956,485.13 look plausible against a cell of 921,225.71. Co-Authored-By: Claude Opus 5 (1M context) --- lib/sql_generator.js | 58 +++++++++++++++++++++++++++- routes/operations.js | 23 ++++++----- ui/src/components/OperationPanel.jsx | 21 +++++++++- ui/src/views/Forecast.jsx | 37 ++++++++---------- 4 files changed, 108 insertions(+), 31 deletions(-) diff --git a/lib/sql_generator.js b/lib/sql_generator.js index 445f699..3d655b9 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -571,6 +571,62 @@ function buildWhere(slice, dimCols, versionId) { return parts.length ? parts.join('\nAND ') : 'TRUE'; } +// The pivot's own filter, carried alongside the slices so an operation writes +// exactly the rows the ledger counted. +// +// A slice is {col: value} and can only ever mean equality, so a view filtered to +// sseas_e <= 2027 could not be expressed as one. Refusing it was safe but +// useless -- a bounded season is an ordinary way to scope a forecast -- so the +// operators travel as [col, op, value] triples instead. +// +// Perspective's operator names, not SQL's, since that is where these come from. +// Anything outside this list is refused rather than ignored: a scope silently +// dropped is a write that is wider than the panel that authorised it. +const SCOPE_OPS = { + '==': (c, v) => `${c} = ${v[0]}`, + '!=': (c, v) => `${c} != ${v[0]}`, + '>': (c, v) => `${c} > ${v[0]}`, + '>=': (c, v) => `${c} >= ${v[0]}`, + '<': (c, v) => `${c} < ${v[0]}`, + '<=': (c, v) => `${c} <= ${v[0]}`, + 'in': (c, v) => `${c} IN (${v.join(', ')})`, + 'not in': (c, v) => `${c} NOT IN (${v.join(', ')})`, + 'is null': (c) => `${c} IS NULL`, + 'is not null': (c) => `${c} IS NOT NULL`, +}; + +function buildScopeClause(scope, dimCols, versionId) { + if (!Array.isArray(scope) || scope.length === 0) return ''; + const allowed = new Set(dimCols); + const parts = scope.map((entry) => { + if (!Array.isArray(entry) || entry.length < 2) { + const err = new Error(`Malformed scope entry ${JSON.stringify(entry)}`); + err.status = 400; throw err; + } + const [col, op, ...rest] = entry; + const vals = (Array.isArray(rest[0]) ? rest[0] : rest).filter(v => v !== undefined); + + if (COMPUTED_SLICE_COLS[col]) { + if (op !== '==' && op !== 'in') { + const err = new Error(`${col} can only be scoped with == or in, not ${op}`); + err.status = 400; throw err; + } + return computedSlicePredicate(col, vals, versionId); + } + if (!allowed.has(col)) { + const err = new Error(`Column "${col}" is not available for filtering`); + err.status = 400; throw err; + } + const fn = SCOPE_OPS[op]; + if (!fn) { + const err = new Error(`Unsupported filter operator "${op}" on ${col}`); + err.status = 400; throw err; + } + return fn(`"${col}"`, vals.map(v => `'${esc(String(v))}'`)); + }); + return parts.join('\nAND '); +} + // build a WHERE clause spanning several slices — an OR of AND-groups. // A union of slices cannot be flattened into one IN list per column: slices // {Region:East, State:NY} and {Region:West, State:CA} would become @@ -667,6 +723,6 @@ function esc(val) { return String(val).replace(/'/g, "''"); } -module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, +module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, buildScopeClause, SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, VERSION_JOIN, ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, UNLABELED_LOAD, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; diff --git a/routes/operations.js b/routes/operations.js index 28cac9b..d4a7b48 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -1,6 +1,6 @@ const express = require('express'); const { tableFromArrays, tableToIPC } = require('apache-arrow'); -const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, +const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, buildScopeClause, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, VERSION_JOIN, ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator'); const { sessionUser } = require('../lib/auth'); @@ -44,11 +44,16 @@ module.exports = function(pool) { // Only scale has a target to prorate towards; recode and clone rewrite rows // rather than distribute an amount, so for them this only decides whether the // work lands as one log entry or several. - function sliceUnits(slices, ctx, applyMode) { - const vid = ctx.version.id; + // The scope is ANDed onto every unit rather than folded into the slices: it + // applies to all of them equally, and under apply_mode 'each' folding it in + // would repeat the same predicate in every statement for no gain. + function sliceUnits(slices, ctx, applyMode, scope) { + const vid = ctx.version.id; + const scl = buildScopeClause(scope, ctx.filterCols, vid); + const and = (w) => (scl ? (w === 'TRUE' ? scl : `${w}\nAND ${scl}`) : w); return applyMode === 'each' - ? slices.map(sl => ({ slices: [sl], where: buildWhere(sl, ctx.filterCols, vid) })) - : [{ slices, where: buildWhereAny(slices, ctx.filterCols, vid) }]; + ? slices.map(sl => ({ slices: [sl], where: and(buildWhere(sl, ctx.filterCols, vid)) })) + : [{ slices, where: and(buildWhereAny(slices, ctx.filterCols, vid)) }]; } // The offset is interpolated into the statement as an interval literal, so a @@ -91,7 +96,7 @@ module.exports = function(pool) { // echo back what the caller asked for, for the audit log function pickIntent(body) { const keys = ['mode', 'target_basis', 'value_incr', 'units_incr', 'value_pct', 'units_pct', 'pct', - 'target_value', 'target_units', 'target_price']; + 'target_value', 'target_units', 'target_price', 'scope']; const out = {}; for (const k of keys) if (body[k] !== undefined && body[k] !== null && body[k] !== '') out[k] = body[k]; return out; @@ -624,7 +629,7 @@ module.exports = function(pool) { // sum() OVER () distribute the increment across the whole pool. // 'each' runs the same statement once per slice, so every slice // reaches the target on its own and gets its own log entry. - const units = sliceUnits(slices, ctx, applyMode); + const units = sliceUnits(slices, ctx, applyMode, req.body.scope); const client = await pool.connect(); let committed = false; @@ -706,7 +711,7 @@ module.exports = function(pool) { const excludeClause = buildExcludeClause(ctx.version.exclude_iters); const setClause = buildSetClause(ctx.dimCols, set); - const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate'); + const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope); const client = await pool.connect(); let committed = false; @@ -795,7 +800,7 @@ module.exports = function(pool) { [cname, `${alias}."${periodCol}"`]) ); const setClause = buildSetClause(ctx.dimCols, set, { derivedExprs, alias: 's' }); - const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate'); + const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope); const client = await pool.connect(); let committed = false; diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 3803b6c..df16b0b 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -110,11 +110,22 @@ function Submit({ onClick, children, disabled }) { } // ── 1. Selection ──────────────────────────────────────────────────────────── -function SelectionList({ slices, currentTotals, onRemove, onClear }) { +function SelectionList({ slices, viewScope = [], currentTotals, onRemove, onClear }) { const multi = slices.length > 1 const perSlice = currentTotals?.perSlice || [] const valueCol = currentTotals?.valueCol + // The pivot's own filter. Shown because it scopes every figure below and + // every row the operation writes, while appearing in none of the slices -- + // perspective-click reports only the cell's own dimensions, so without this + // the panel prints a selection wider than the one it is acting on. + const scopeLine = viewScope + .map(([col, op, ...rest]) => { + const vals = (Array.isArray(rest[0]) ? rest[0] : rest).filter(v => v !== undefined) + return `${col} ${op}${vals.length ? ' ' + vals.join(', ') : ''}` + }) + .join(' · ') + if (!slices.length) { return (

@@ -126,6 +137,12 @@ function SelectionList({ slices, currentTotals, onRemove, onClear }) { return (

+ {scopeLine && ( +
+ within + {scopeLine} +
+ )}
@@ -763,6 +780,7 @@ function RequestPreview({ payload }) { export default function OperationPanel({ dock, slices, setSlices, distinctSlices, + viewScope = [], applyMode, setApplyMode, currentTotals, activeOp, setActiveOp, @@ -805,6 +823,7 @@ export default function OperationPanel({ )} setSlices(prev => prev.filter((_, x) => x !== i))} onClear={() => setSlices([])} diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 88c97fc..f25f293 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -56,6 +56,11 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const { dark } = useTheme() const [loading, setLoading] = useState(false) const [largeDataset, setLargeDataset] = useState(false) + // The pivot's own filter, refreshed whenever the ledger recomputes. A ref + // rather than state: it is read at dispatch and at totals time, never + // rendered from directly, and making it state would re-run the totals effect + // that sets it. + const viewFilterRef = useRef([]) const [loadProgress, setLoadProgress] = useState(null) // { received, total } // Rows the load is waiting on, so the wait can say what it is waiting for -- // on this data the row count is the wait (see CLAUDE.md, "Load time is @@ -67,6 +72,9 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // first, from the same table-info the status bar reads, and the exact figure // replaces it when the headers land. const [loadRows, setLoadRows] = useState(null) + // the same filter, for display: the panel has to show the scope it is acting + // inside or the slice it prints is not the slice that gets written + const [viewScope, setViewScope] = useState([]) const [msg, setMsg] = useState(null) // layouts @@ -450,6 +458,8 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio return (cfg?.filter || []).filter(f => Array.isArray(f) && f.length >= 2) } catch { return [] } })() + viewFilterRef.current = viewFilter + setViewScope(viewFilter) const schema = await tableRef.current.schema() const typed = (col, val) => { @@ -1294,22 +1304,10 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio // The pivot's filter scopes what the ledger counted, so it has to scope what // gets written too -- otherwise the panel shows one number and the operation - // changes a larger set. Only equalities can travel in a slice; anything else - // is refused rather than dropped, because dropping it is exactly the silent - // widening this is here to prevent. - let viewFilter = [] - try { - const cfg = await viewerRef.current?.save() - viewFilter = (cfg?.filter || []).filter(f => Array.isArray(f) && f.length >= 2) - } catch { viewFilter = [] } - const unsendable = viewFilter.filter(f => f[1] !== '==') - if (unsendable.length) { - flash(`The pivot filter ${unsendable.map(f => f.join(' ')).join(', ')} cannot be ` - + `applied to an operation. Narrow the selection instead, or use "==".`, 'error') - return - } - - const body = buildPayload(op, viewFilter) + // changes a larger set. It travels as [col, op, value] rather than folded + // into the slices, because a slice is {col: value} and can only mean + // equality: a view filtered to sseas_e <= 2027 has no slice form at all. + const body = buildPayload(op, viewFilterRef.current) if (!body) return if (body.slices.some(sl => !Object.keys(sl).length)) { flash('No dimension or date columns in slice — check col_meta', 'error'); return @@ -1440,7 +1438,6 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio function buildPayload(op, viewFilter = []) { if (!slices.length) return null - const scope = Object.fromEntries(viewFilter.map(([col, , val]) => [col, val])) // Two clicked cells can differ only by a column the operation cannot filter on // (pf_iter, say, which is not in col_meta and so is dropped here). Those become // the same effective slice, and sending it twice would apply the change twice @@ -1448,9 +1445,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio const seen = new Set() const effectiveSlices = [] for (const sl of slices) { - // the view's scope first, so a cell's own value wins if they name the - // same column -- it cannot contradict the filter it was drawn inside - const eff = buildEffectiveSlice({ ...scope, ...sl }) + const eff = buildEffectiveSlice(sl) const key = JSON.stringify(Object.keys(eff).sort().map(k => [k, eff[k]])) if (seen.has(key)) continue seen.add(key) @@ -1460,6 +1455,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio let body = { tag: opTag.trim() || undefined, slices: effectiveSlices, + ...(viewFilter.length ? { scope: viewFilter } : {}), ...(effectiveSlices.length > 1 ? { apply_mode: applyMode } : {}), } if (op === 'scale') { @@ -1609,6 +1605,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio distinctSlices, dock, slices, setSlices, + viewScope, applyMode, setApplyMode, currentTotals, activeOp, setActiveOp,