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

Keep the pivot's pf_ columns in the slice, and log the panel's expected change

Browse Source 
The ledger filtered on every column of the loaded table, while the request
kept only pf_segment and pf_bucket and stripped pf_iter, pf_note, pf_op,
pf_user and pf_logid. Under the default layout, grouped by pf_iter, a click
on a customer's `scale` row totalled the scale rows and wrote to every row
the customer has -- on GRIFFIN, $4.5M shown against $9.7M written. They now
travel: pf_iter / pf_user / pf_logid compared directly, pf_note / pf_op
resolved to log ids as pf_segment / pf_bucket are, a null note as IS NULL.
They narrow a slice but do not make one selective on their own.

Each scale also carries `expected`, the Result line as a change, and
`plug`, both kept in the log's params beside `resolved` so a panel/server
disagreement can be found by reading it. Nothing compares them yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Paul Trowbridge 2026-09-28 22:40:00 -04:00
parent 770e22d865
commit 5feb28eb76
5 changed files with 85 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CLAUDE.md
View File

@ -680,7 +680,9 @@ Turning a region back into slices re-derives, per cell, the same filters Perspec
**Selection highlight.** The datagrid highlights whatever sits in its own `model._selection_state.selected_areas`, and wipes that list on every mousedown — so a multi-slice selection built up over several ctrl-clicks would only ever show the last cell. `Forecast.jsx` keeps `areasRef`, a `sliceKey -> rectangles` map parallel to `slices`, and an effect pushes the full set back and redraws after every change. Deselecting anywhere (ctrl-click, the panel's ×, Clear selection) prunes the map by live slice key, so the grid and the panel can't disagree.
`pf_iter` is not a col_meta column, so it is stripped when a slice is built: two cells differing only by iter band produce the same effective slice. Duplicates are collapsed before the request — without that, `apply_mode: each` would apply the same change twice.
**The `pf_` columns the pivot shows are slice columns too.** `pf_segment`, `pf_bucket`, `pf_note` and `pf_op` resolve to log ids (`COMPUTED_SLICE_COLS`); `pf_iter`, `pf_user` and `pf_logid` are forecast-table columns compared directly (`TABLE_SLICE_COLS`). They used to be stripped — `pf_iter` on the theory that two cells differing only by iter band are the same slice — while the ledger filtered on them, so under the default layout (grouped by `pf_iter`) a click on a customer's `scale` row totalled the scale rows and wrote to every row the customer has. They narrow a slice but do not make one selective on their own: `pf_iter = scale` alone is every adjustment in the version, and `assertSelective` still wants a dimension, date, `pf_segment` or `pf_bucket`. Duplicates are collapsed before the request — without that, `apply_mode: each` would apply the same change twice.
A scale also carries `expected` — the panel's Result line as a change (`value_incr`, `units_incr`, `price`, `basis`) — which the log keeps beside `resolved` so a disagreement between panel and server can be found by reading it. Nothing compares them yet.
**A blank dimension value is a value.** Both blanks occur in this data and they
are different rows — on `fc_osm_skinny_34`, `partgroup` is null on 51,964 rows

43
lib/sql_generator.js
View File

@ -541,7 +541,25 @@ function applyTokens(sql, tokens) {
// Without this they were dropped from the slice, and clicking a single bucket's
// cell scaled every bucket at that dimension intersection while the panel showed
// only the one clicked.
const COMPUTED_SLICE_COLS = { pf_segment: SEGMENT_EXPR, pf_bucket: BUCKET_EXPR };
//
// pf_note and pf_op are the same kind of column -- the pivot shows them, from
// the log entry -- and used to be refused, which the client pre-empted by
// stripping them. The ledger did not strip them, so a pivot grouped by one
// totalled a narrower set of rows than the operation then wrote.
const COMPUTED_SLICE_COLS = {
pf_segment: SEGMENT_EXPR,
pf_bucket: BUCKET_EXPR,
pf_note: NOTE_EXPR,
pf_op: 'l.operation',
};
// pf_ columns that are real columns of the forecast table, compared directly.
// pf_iter was stripped by the client and skipped here, on the reasoning that
// two cells differing only by iter band are the same slice. They are not to the
// ledger, which filtered on it: with the default layout grouped by pf_iter, a
// click on a customer's `scale` row showed the scale rows' total and wrote to
// every row the customer has.
const TABLE_SLICE_COLS = new Set(['pf_iter', 'pf_user', 'pf_logid']);
function computedSlicePredicate(col, val, versionId) {
if (versionId == null) {
@ -549,12 +567,21 @@ function computedSlicePredicate(col, val, versionId) {
err.status = 500;
throw err;
}
const vals = (Array.isArray(val) ? val : [val]).map(v => `'${esc(v)}'`).join(', ');
// a blank is a value here too -- a load has no note -- and `IN (NULL)` is
// never true, so the null arm is ORed on as buildWhere does
const list = Array.isArray(val) ? val : [val];
const nulls = list.some(v => v === null);
const vals = list.filter(v => v !== null).map(v => `'${esc(v)}'`);
const expr = COMPUTED_SLICE_COLS[col];
const inList = vals.length ? `(${expr}) IN (${vals.join(', ')})` : null;
const test = nulls && inList ? `((${expr}) IS NULL OR ${inList})`
: nulls ? `(${expr}) IS NULL`
: inList;
return `pf_logid IN (
SELECT l.id
FROM pf.log l
WHERE l.version_id = ${parseInt(versionId)}
AND ${COMPUTED_SLICE_COLS[col]} IN (${vals})
AND ${test}
)`;
}
@ -571,15 +598,13 @@ function buildWhere(slice, dimCols, versionId) {
}
// A pf_ key this does not understand is refused rather than skipped.
// Skipping is how a selection silently widened: the operation ran against
// everything the dropped key would have excluded. pf_iter is the one
// exception -- the client strips it deliberately, since two cells that
// differ only by iter band are the same slice.
if (col.startsWith('pf_') && col !== 'pf_iter') {
// everything the dropped key would have excluded.
if (col.startsWith('pf_') && !TABLE_SLICE_COLS.has(col)) {
const err = new Error(`Slice names ${col}, which cannot be filtered on`);
err.status = 400;
throw err;
}
if (!allowed.has(col)) continue;
if (!allowed.has(col) && !TABLE_SLICE_COLS.has(col)) continue;
// A blank dimension value is a value. Both blanks exist in this data and
// they are different rows -- on fc_osm_skinny_34, partgroup is null on
// 51,964 and '' on 111,696 -- so a null has to become IS NULL. It used
@ -821,6 +846,6 @@ function esc(val) {
return String(val).replace(/'/g, "''");
}
module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, buildScopeClause, buildTerritoryClause,
module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, TABLE_SLICE_COLS, buildScopeClause, buildTerritoryClause,
SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS,
ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc };

13
routes/operations.js
View File

@ -1,6 +1,6 @@
const express = require('express');
const { tableFromArrays, tableToIPC } = require('apache-arrow');
const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, buildScopeClause, buildTerritoryClause, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc,
const { applyTokens, buildWhere, buildWhereAny, COMPUTED_SLICE_COLS, TABLE_SLICE_COLS, buildScopeClause, buildTerritoryClause, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc,
SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR,
ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator');
const { sessionUser, sessionTerritory, requireAdmin } = require('../lib/auth');
@ -133,7 +133,10 @@ module.exports = function(pool) {
// An empty slice passes only under a scope (see normalizeSlices), which
// buildScopeClause refuses to let reduce to nothing.
function assertSelective(slices, ctx, scope) {
const allowed = new Set([...ctx.filterCols, ...Object.keys(COMPUTED_SLICE_COLS)]);
// pf_iter, pf_note, pf_op, pf_user and pf_logid narrow a slice once it
// is selective, but do not make it so: they were only just accepted at
// all, and `pf_iter = scale` alone is every adjustment in the version
const allowed = new Set([...ctx.filterCols, 'pf_segment', 'pf_bucket']);
const scoped = Array.isArray(scope) && scope.length > 0;
slices.forEach((sl, i) => {
if (scoped && Object.keys(sl).length === 0) return;
@ -218,7 +221,11 @@ 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', 'scope'];
'target_value', 'target_units', 'target_price', 'scope', 'plug',
// What the panel's Result line promised, kept beside
// `resolved` so a disagreement between the two can be
// found by reading the log. Not checked -- yet.
'expected'];
const out = {};
for (const k of keys) if (body[k] !== undefined && body[k] !== null && body[k] !== '') out[k] = body[k];
return out;

22
ui/src/components/OperationPanel.jsx
View File

@ -583,6 +583,8 @@ function TagInput({ value, onChange, tags = [], className }) {
// here -- so flipping the plug brings it straight back. Forecast.jsx builds
// the request from the same function, so the panel and the write agree.
// Returns the inputs that count, and whether the plug is the one choosing.
const round2 = (x) => Math.round(x * 100) / 100
const hasNumber = (edit) => edit?.raw != null && edit.raw !== '' && isFinite(parseFloat(edit.raw))
export function effectiveScaleInputs(inputs = {}, plug = 'volume') {
@ -601,7 +603,7 @@ export function effectiveScaleInputs(inputs = {}, plug = 'volume') {
let editSeq = 0
const stamp = () => ++editSeq
function ScaleLedger({ currentTotals, scaleInputs: rawInputs, setScaleInputs, scalePlug, setScalePlug, targetBasis, setTargetBasis, logMeta = {}, multi, applyMode, settingsExtra }) {
function ScaleLedger({ currentTotals, scaleInputs: rawInputs, setScaleInputs, scaleExpectedRef, scalePlug, setScalePlug, targetBasis, setTargetBasis, logMeta = {}, multi, applyMode, settingsExtra }) {
// everything below reads the inputs that count; only the setters touch the raw ones
const { inputs: scaleInputs, choosing: plugChoosing } = effectiveScaleInputs(rawInputs, scalePlug)
const valueCol = currentTotals?.valueCol
@ -822,6 +824,20 @@ function ScaleLedger({ currentTotals, scaleInputs: rawInputs, setScaleInputs, sc
}
})()
// The change the Result line promises, handed to the request as `expected`.
// Measured on the same basis as the ledger, so it compares directly with
// `resolved` in the log entry. Under 'each' it is the combined figure across
// the slices, which is what the panel shows -- not a per-slice amount.
if (scaleExpectedRef) {
scaleExpectedRef.current = outcome ? {
value_incr: round2(outcome.value - (basisOf('value') ?? 0)),
units_incr: round2(outcome.units - (basisOf('units') ?? 0)),
price: outcome.price == null ? null : Math.round(outcome.price * 1e6) / 1e6,
basis: onTotal ? 'selected' : 'adjustable',
...(perSlice ? { combined_across_slices: true } : {}),
} : null
}
// The same three readings for a measure nobody typed into, taken from where
// the edit actually lands it. Scaling dollars with price holding moves qty,
// and leaving that column blank until the Result row meant the number you had
@ -1464,7 +1480,7 @@ export default function OperationPanel({
applyMode, setApplyMode,
currentTotals,
activeOp, setActiveOp,
scaleInputs, setScaleInputs,
scaleInputs, setScaleInputs, scaleExpectedRef,
scalePlug, setScalePlug,
targetBasis, setTargetBasis,
opTag, setOpTag, knownTags = [], logMeta = {},
@ -1520,7 +1536,7 @@ export default function OperationPanel({
{activeOp === 'scale' && (
<ScaleLedger
currentTotals={currentTotals}
scaleInputs={scaleInputs} setScaleInputs={setScaleInputs}
scaleInputs={scaleInputs} setScaleInputs={setScaleInputs} scaleExpectedRef={scaleExpectedRef}
scalePlug={scalePlug} setScalePlug={setScalePlug}
targetBasis={targetBasis} setTargetBasis={setTargetBasis}
logMeta={logMeta}

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

@ -15,10 +15,17 @@ import '@perspective-dev/viewer/inline'
import '@perspective-dev/viewer-datagrid'
import '@perspective-dev/viewer/themes'
// Slice keys that are not col_meta columns: computed from pf.log when the rows are
// served, so they are real columns in the loaded table but have to be resolved back
// to log ids server-side. Mirrors COMPUTED_SLICE_COLS in lib/sql_generator.js.
const COMPUTED_SLICE_COLS = new Set(['pf_segment', 'pf_bucket'])
// Slice keys that are not col_meta columns but are columns of the loaded table,
// so a click on them narrows the ledger -- and therefore has to narrow the write.
// pf_segment / pf_bucket / pf_note / pf_op come from pf.log and are resolved back
// to log ids server-side; pf_iter / pf_user / pf_logid are forecast-table columns.
// Mirrors COMPUTED_SLICE_COLS and TABLE_SLICE_COLS in lib/sql_generator.js.
//
// Only pf_segment and pf_bucket used to be kept. The rest were stripped, while
// the ledger still filtered on them: a click on a customer's `scale` row under
// the default layout (grouped by pf_iter) totalled the scale rows and wrote to
// every row the customer has.
const COMPUTED_SLICE_COLS = new Set(['pf_segment', 'pf_bucket', 'pf_note', 'pf_op', 'pf_iter', 'pf_user', 'pf_logid'])
// The unnamed last-used config stays in localStorage: it is per-browser session
// continuity, not a thing anyone names or shares. LAYOUTS_KEY is the old named
@ -156,6 +163,11 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
// you type in decides how the number is interpreted, and the other two rows of
// that measure are derived from it.
const [scaleInputs, setScaleInputs] = useState({})
// What the panel's Result line says the scale will change, written there on
// each render and sent as `expected` so the log keeps it next to what the
// server resolved. Nothing checks the two against each other yet; it is
// there so a disagreement can be found by reading the log.
const scaleExpectedRef = useRef(null)
// which side of price x volume absorbs a dollar-only change: 'price' keeps
// volume flat, 'volume' keeps price flat. Mirrors the Excel form's
// Plug Price / Plug Volume choice.
@ -1965,7 +1977,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
const viewFilter = viewFilterRef.current || []
if (!slices.length) return null
// 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
// (a computed split_by column, say, which is dropped here). Those become
// the same effective slice, and sending it twice would apply the change twice
// under apply_mode 'each'. Collapse duplicates before they reach the API.
const seen = new Set()
@ -2029,6 +2041,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
// sent when dollars are the sole input; naming units or price has already
// answered the question.
if (vn != null && un == null && pn == null) body.plug = scalePlug
if (scaleExpectedRef.current) body.expected = scaleExpectedRef.current
} else if (op === 'recode') {
const set = Object.fromEntries(Object.entries(recodeSet).filter(([, v]) => v.trim()))
body = { ...body, note: recodeNote || undefined, set }
@ -2154,7 +2167,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
applyMode, setApplyMode,
currentTotals,
activeOp, setActiveOp,
scaleInputs, setScaleInputs,
scaleInputs, setScaleInputs, scaleExpectedRef,
scalePlug, setScalePlug,
targetBasis, setTargetBasis,
opTag, setOpTag, knownTags, logMeta,