From 03278eb091c4f0cccf469c1807601aa56c834e0e Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Thu, 17 Sep 2026 22:45:11 -0400 Subject: [PATCH] Drop the bucket reorder buttons, and document the new mechanism bucket_order fed the ordering expressions and nothing else, so its up/down list is gone. The datalist it shared state with stays, now offering the buckets actually in use plus the four conventional names carried with their prefixes -- a near-miss spelling silently splits a column in two, so the options are worth more than they were. saveBucket was a duplicate of saveLogField left behind by af9e6de's refactor; the bucket cell goes through saveLogField like the label does. Both confirmations now say to reload the Forecast view, which is true of a label for the same reason it was true of a bucket: it is part of the aggregated row. pf.log.seq and pf.version.bucket_order are no longer read anywhere. The columns stay -- dropping them is not worth the migration, and nothing costs anything by their being there. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 40 ++++++++++++++- ui/src/views/Baseline.jsx | 104 +++++++------------------------------- 2 files changed, 57 insertions(+), 87 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 443e1b7..9eea694 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,8 +117,8 @@ Aggregating to the grain the pivot actually displays is the load-time fix — me `/data` and `/agg` both LEFT JOIN `pf.log` and emit two columns the forecast table does not itself carry: -- **`pf_segment`** — for a baseline or reference row, that load's label (`tag`, else - `note`); `'(adjustment)'` for everything else +- **`pf_segment`** — `pf.log.label`, else `tag`, else `note`, else `Unlabeled`; + `'99 - Adjustments'` for an adjustment that has no label of its own - **`pf_note`** — the free text on a scale/recode/clone; null on loads They are deliberately separate: commingling a segment name with an adjustment note @@ -127,6 +127,42 @@ adds no rows. The operation routes stamp the same two fields onto the rows they back incrementally, since those come from `RETURNING *` and would otherwise arrive without them. +### Column order is stored text + +Perspective orders column groups by the value string, and `SortDir`'s `col asc` / +`col desc` only reverses that — so Prior Year → Plan → Actual → Forecast is +alphabetical in neither direction and expressible as neither. A `"01 - "` prefix +is the only lever, and it lives in **`pf.log.label`** (and `pf.log.bucket`), +typed by whoever names the segment. Nothing derives it. + +`SEGMENT_EXPR` / `BUCKET_EXPR` / `NOTE_EXPR` in `lib/sql_generator.js` are the +single definition, shared with the `/data` cursor in `routes/operations.js` — +`/agg` is generated, `/data` is not, and the two have to agree. + +The one hardcoded ordinal is `ADJUSTMENT_SEGMENT` = `'99 - Adjustments'`, which +keeps unlabelled adjustments last. Labelling an adjustment's own log row +overrides it, which is how one kind of adjustment is split out from the rest. +Unlabelled loads read plain `Unlabeled` and need no ordinal, since letters follow +digits in ASCII — unlike the old `'(adjustment)'`, where `(` is `0x28` against +digits from `0x30` and so sorted *first*. + +**What this replaced.** The prefix used to be computed client-side, as +Perspective expression columns (`pf_bucket_ord`, `pf_segment_ord`) built from +`pf.log.seq` and `pf.version.bucket_order`. It ordered the pivot and nothing +else, so every other reader disagreed with it; `restore()` replaces +`expressions` wholesale, so it had to be re-applied after every layout load; and +ExprTK's string scanner tests each *byte* with `isprint()`, so a label +containing anything outside printable ASCII could not be ordered at all (`·` is +two bytes, of which `isprint(0xC2)` is false). `DEAD_ORDER_EXPRS` in +`Forecast.jsx` strips the expression names out of layouts saved under that +scheme. `pf.log.seq` and `pf.version.bucket_order` are no longer read; the +columns remain. + +Reordering now needs a page reload to show, because the label is part of the +aggregated row rather than something the pivot can re-derive. Changing labels on +an existing source also needs **Generate SQL** re-run — the load templates that +write `label` and `bucket` onto the log row are stored in `pf.sql`. + ### Forecast operations POST to `/api/versions/:id/{scale|recode|clone}` → SQL executed with `RETURNING *` → new rows returned as JSON → `pspTable.update(rows)` — no full reload. In grain mode the operation's final CTE aggregates its own new rows to grain first; since `pf_logid` is part of `pf_gkey` those keys are always new, so `update()` **appends** and the view re-sums. diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index 783c6f0..c58b441 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -137,16 +137,19 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio // locally while typing so the field does not fight the fetched value, and // written on blur. const [buckets, setBuckets] = useState({}) - // seq orders the segment columns; bucket_order orders the bucket columns. Both - // are carried into the pivot as a "01 · " prefix, because Perspective orders - // column groups by the value string and no sort setting can express an - // arbitrary order. - const [bucketOrder, setBucketOrder] = useState([]) + // Buckets already in use on this version, for the datalist. There is no stored + // bucket order any more: the order is whatever the typed text sorts as, so a + // bucket is named "02 - Forecast" and that is the whole mechanism. + const [bucketsInUse, setBucketsInUse] = useState([]) // Label and bucket are presentation, not definition: they change what the pivot // shows and what the segment counts toward, never which rows were loaded. So // they stay editable after adjustments exist, unlike the filters and offset, // where an edit would silently recalibrate scales sized against the old rows. + // + // Both are only read at load time, hence the reload in the confirmation: the + // label is part of the aggregated row the pivot holds, not something it can + // re-derive in place. async function saveLogField(entry, field, value) { const next = value.trim() if (next === (entry[field] || '')) return @@ -158,38 +161,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio }) if (!res.ok) { const d = await res.json(); flash(d.error, 'error'); return } loadLog() - flash('Saved') - } catch (err) { flash(err.message, 'error') } - } - - async function saveBucket(entry, value) { - const next = value.trim() - if (next === (entry.bucket || '')) return - try { - const res = await fetch(`/api/log/${entry.id}`, { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ bucket: next }), - }) - if (!res.ok) { const d = await res.json(); flash(d.error, 'error'); return } - loadLog() - flash(next ? `Counts toward ${next} — reload the Forecast view to see it` : 'Banner cleared') - } catch (err) { - flash(err.message, 'error') - } - } - - - async function saveBucketOrder(next) { - setBucketOrder(next) - try { - const res = await fetch(`/api/versions/${versionId}`, { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ bucket_order: next }), - }) - if (!res.ok) { const d = await res.json(); flash(d.error, 'error'); return } - flash('Bucket order saved') + flash(next ? `Saved — reload the Forecast view to see it` : 'Cleared') } catch (err) { flash(err.message, 'error') } } @@ -198,13 +170,10 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio setLog(data.filter(e => e.operation === 'baseline' || e.operation === 'reference')) setHasForecastOps(data.some(e => ['scale', 'recode', 'clone'].includes(e.operation))) - // The stored order first, then any bucket actually in use that it does not - // name — so labelling a new segment makes its bucket appear at the end, - // ready to be moved, rather than silently missing from the list. - const inUse = [...new Set(data.map(e => (e.bucket || '').trim()).filter(Boolean))] - const stored = versions.find(v => String(v.id) === String(versionId))?.bucket_order - const ordered = (Array.isArray(stored) ? stored : []).filter(b => b) - setBucketOrder([...ordered, ...inUse.filter(b => !ordered.includes(b))]) + // Every bucket in use, adjustments included: typing one on a new segment + // should offer the ones already there rather than inviting a near-miss + // spelling, which would silently split the column in two. + setBucketsInUse([...new Set(data.map(e => (e.bucket || '').trim()).filter(Boolean))].sort()) }) } @@ -426,47 +395,12 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio Segments loaded - {/* Bucket column order. Up/down rather than drag: the list is four or - five items that change once a quarter, and a keyboard-reachable - pair of buttons beats a drag target nobody can hit on a laptop - trackpad. */} - {bucketOrder.length > 1 && ( -
- column order - {bucketOrder.map((b, i) => ( - - {String(i + 1).padStart(2, '0')} - - {b} - - - - ))} -
- )} - - @@ -538,7 +472,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio value={buckets[entry.id] ?? entry.bucket ?? ''} list="pf-bucket-options" onChange={e => setBuckets(b => ({ ...b, [entry.id]: e.target.value }))} - onBlur={e => saveBucket(entry, e.target.value)} + onBlur={e => saveLogField(entry, 'bucket', e.target.value)} placeholder="—" className="w-full border border-transparent hover:border-gray-200 focus:border-blue-400 rounded px-1 py-0.5 text-xs focus:outline-none bg-transparent" />