From b014f7815a70a94b8fb9e3bfb000ee724d42327f Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 10:23:35 -0400 Subject: [PATCH 01/13] Clone a version's definition without running it A segment only existed once it had been loaded -- a pf.log entry with forecast rows behind it -- so there was nowhere to hold "this is the cut I want, but the dates need changing first", and rebuilding a scenario meant retyping every segment by hand. pf.load_plan is the definition on its own, with run_logid naming the log entry it produced once it has been run. Not in pf.log deliberately: every row there means a write happened, and unexecuted intent would cost that table its meaning. The FK is ON DELETE SET NULL, so undoing a load returns its plan to unrun rather than taking the definition with it. POST /sources/:id/versions now takes clone_from: settings, version-scoped layouts, and the segments as plans, running none of them. Plans are read from the template's own pf.load_plan rows, or derived from its baseline/reference log entries when it has none -- every version built before this -- since params already stores the filters and the offset. Adjustments are not copied in any form. A scale's target was sized against the rows that were there. Shift years exists because cloning a definition forward a year is the reason to clone one, and doing it by hand across eight segments is where the mistakes come from. Filter values only, and only a bare 20xx: a month reads "04 - Sep" and a part number that looks like a year must not move. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 53 +++++++ routes/plans.js | 184 ++++++++++++++++++++++ routes/versions.js | 116 +++++++++++++- server.js | 1 + setup_sql/01_schema.sql | 34 +++++ ui/src/views/Baseline.jsx | 312 ++++++++++++++++++++++++++++++++++++-- 6 files changed, 682 insertions(+), 18 deletions(-) create mode 100644 routes/plans.js diff --git a/CLAUDE.md b/CLAUDE.md index 9fdb323..24f77bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,6 +31,7 @@ routes/ operations.js scale, recode, clone, undo — the core forecast ops log.js GET /api/versions/:id/log, DELETE /api/log/:logid layouts.js Named pivot layouts — list per version, create, patch, delete + plans.js Planned load segments — list, create, patch, delete, shift-years lib/ sql_generator.js buildFilterClause, token substitution helpers auth.js scrypt hash/verify, requireAuth, sessionUser; `node lib/auth.js hash` CLI @@ -65,6 +66,7 @@ ui/src/ - **`pf.fc_{tname}_{version_id}`** — one forecast table per version; contains both operational rows (`pf_iter = baseline|scale|recode|clone`) and reference rows (`pf_iter = reference`). Indexed on `pf_logid`, which is how undo, the change-log aggregate and the grain key all find their rows — without it each is a sequential scan of the whole table. Tables created before that index was added do not have it. - **`pf.log`** — audit log; every write gets one entry; `slice` + `params` stored as jsonb - **`pf.layout`** — named Perspective view configs; see §Pivot layouts +- **`pf.load_plan`** — load segment definitions, before and apart from having been run; see §Cloning a version's definition - **`pf.sql`** — generated SQL templates per source/operation; tokens substituted at request time - **`pf.app_user`** — login accounts; scrypt `pass_hash`, `is_active`, `last_login_at` - **`pf.session`** — express-session store (connect-pg-simple layout) @@ -321,6 +323,57 @@ together. `columns` selects which *measures* appear, not individual split combinations. +## Cloning a version's definition + +`POST /sources/:id/versions` takes `clone_from`. It copies the template +version's settings (`description`, `exclude_iters`, the three fallback names, +`bucket_order`), its **version-scoped** layouts, and its load segments — and +**runs nothing**. The reason to clone a definition is to change the dates before +anything loads, and a clone that fetched two million rows first would have to be +undone before it could be edited. + +Not copied: adjustments, in any form. A scale's target is a figure sized against +the rows that were there, so replaying one against a different load is +arithmetic nobody asked for. Source-wide layouts (`version_id IS NULL`) are not +copied either — they already apply to the new version, and a copy would +duplicate the name. + +### `pf.load_plan` + +A segment used to exist only once it had been run: a `pf.log` entry with +forecast rows behind it. A plan is the definition on its own — `kind`, `label`, +`bucket`, `tag`, `note`, `filters`, `raw_where`, `date_offset`, `seq` — with +`run_logid` naming the `pf.log` entry it produced, or null while it is still a +plan. + +**Deliberately not `pf.log`.** Every row there means a write happened; holding +unexecuted intent in the audit log would cost it that meaning, and the change +log would list entries that never ran. + +`run_logid` is `ON DELETE SET NULL`, so undoing a load returns its plan to unrun +rather than deleting the definition with it. The plans list reads the row count +back through that FK, which is how it tells run from unrun. + +**Derivation from an old version.** `cloneDefinition()` in `routes/versions.js` +reads `pf.load_plan` when the template has plans and falls back to deriving them +from its `baseline`/`reference` log entries when it does not — which is every +version built before plans existed. `pf.log.params` already stores `filters`, +`raw_where` and `date_offset`, so this is a read, not a reconstruction. + +**`POST /versions/:id/plans/shift-years`** adds a whole number of years to every +*unrun* plan's `date_offset` and to any bare four-digit `20xx` in its filter +**values**. Values only, and only a bare year: a season reads `2027`, a month +reads `04 - Sep`, and a part number that happens to look like a year would be +corrupted by anything looser. Cloning a definition forward a year is the reason +plans exist, and doing it by hand across eight segments is where the mistakes +come from. + +**Run all stops at the first failure.** The segments of a definition are not +independent — a baseline followed by references that date-shift off it — so +carrying on past a broken one builds something nobody described. + +--- + ## Pivot layouts A layout is a named `ViewConfig`, stored in **`pf.layout`** and owned by an diff --git a/routes/plans.js b/routes/plans.js new file mode 100644 index 0000000..24384ba --- /dev/null +++ b/routes/plans.js @@ -0,0 +1,184 @@ +const express = require('express'); +const { sessionUser } = require('../lib/auth'); + +// Planned load segments — a version's definition, before it has been run. +// +// A plan carries exactly what POST /versions/:id/{baseline|reference} takes, so +// running one is that same call with the plan's fields; the client then stamps +// run_logid. It is two round trips rather than one because the alternative is a +// second copy of the load route's token substitution, and a plan that reads +// unrun while its rows exist is visible and fixable, where a divergent copy of +// the load SQL is neither. +module.exports = function(pool) { + const router = express.Router(); + + const FIELDS = ['seq', 'kind', 'label', 'bucket', 'tag', 'note', + 'filters', 'raw_where', 'date_offset']; + + // A plan whose log entry has been undone is a plan again: the FK is + // ON DELETE SET NULL, so run_logid clears itself. The join is for the + // row count, which is what makes "already run" worth seeing. + const SELECT_PLANS = ` + SELECT + p.* + ,l.row_count AS run_rows + ,l.stamp AS run_at + FROM pf.load_plan p + LEFT JOIN pf.log l + ON l.id = p.run_logid + WHERE TRUE + AND p.version_id = $1 + ORDER BY + p.seq + ,p.id`; + + router.get('/versions/:id/plans', async (req, res) => { + try { + const { rows } = await pool.query(SELECT_PLANS, [parseInt(req.params.id)]); + res.json(rows); + } catch (err) { + res.status(500).json({ error: err.message }); + } + }); + + router.post('/versions/:id/plans', async (req, res) => { + const versionId = parseInt(req.params.id); + const b = req.body || {}; + if (!['baseline', 'reference'].includes(b.kind)) { + return res.status(400).json({ error: 'kind must be baseline or reference' }); + } + try { + // Append by default: a new plan runs after the ones already there. + const next = await pool.query( + `SELECT COALESCE(max(seq), -1) + 1 AS seq FROM pf.load_plan WHERE version_id = $1`, + [versionId] + ); + const { rows } = await pool.query( + `INSERT INTO pf.load_plan + (version_id, seq, kind, label, bucket, tag, note, filters, raw_where, + date_offset, created_by) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, COALESCE($10, '0 days'), $11) + RETURNING *`, + [versionId, b.seq ?? next.rows[0].seq, b.kind, + b.label || null, b.bucket || null, b.tag || null, b.note || null, + b.filters ? JSON.stringify(b.filters) : null, b.raw_where || null, + b.date_offset || null, sessionUser(req)] + ); + res.status(201).json(rows[0]); + } catch (err) { + res.status(500).json({ error: err.message }); + } + }); + + // Any subset of the fields, plus run_logid when a run has just happened. + // The flag-and-value pair rather than COALESCE on the value: clearing a + // label to blank has to be distinguishable from not mentioning it. + router.patch('/plans/:pid', async (req, res) => { + const pid = parseInt(req.params.pid); + const b = req.body || {}; + const keys = [...FIELDS, 'run_logid'].filter(k => b[k] !== undefined); + if (!keys.length) return res.status(400).json({ error: 'Nothing to update' }); + if (b.kind !== undefined && !['baseline', 'reference'].includes(b.kind)) { + return res.status(400).json({ error: 'kind must be baseline or reference' }); + } + try { + const sets = []; + const params = [pid]; + for (const k of keys) { + let v = k === 'filters' + ? (b[k] === null ? null : JSON.stringify(b[k])) + : (typeof b[k] === 'string' ? (b[k].trim() || null) : b[k]); + // date_offset is NOT NULL, so a blank one means the default. + // Coerced here rather than in the statement: a second assignment + // to the same column is an error, not a fallback. + if (k === 'date_offset' && !v) v = '0 days'; + params.push(v); + sets.push(`${k} = $${params.length}${k === 'filters' ? '::jsonb' : ''}`); + } + const { rows } = await pool.query( + `UPDATE pf.load_plan + SET ${sets.join(', ')}, updated_at = now() + WHERE id = $1 + RETURNING *`, + params + ); + if (!rows.length) return res.status(404).json({ error: 'Plan not found' }); + res.json(rows[0]); + } catch (err) { + res.status(500).json({ error: err.message }); + } + }); + + router.delete('/plans/:pid', async (req, res) => { + try { + const { rows } = await pool.query( + `DELETE FROM pf.load_plan WHERE id = $1 RETURNING id`, [parseInt(req.params.pid)] + ); + if (!rows.length) return res.status(404).json({ error: 'Plan not found' }); + res.json({ deleted: rows[0].id }); + } catch (err) { + res.status(500).json({ error: err.message }); + } + }); + + // Shift every unrun plan's date_offset and any four-digit year in its filter + // values by a whole number of years. Cloning a definition forward is the + // reason plans exist, and doing it by hand across eight segments is where + // the mistakes come from. + // + // Years in filter *values* only, and only a bare four-digit 20xx: a season + // reads "2027", a month reads "04 - Sep", and a part number that happens to + // look like a year would be corrupted by anything looser. + router.post('/versions/:id/plans/shift-years', async (req, res) => { + const versionId = parseInt(req.params.id); + const years = parseInt(req.body?.years); + if (!Number.isInteger(years) || years === 0) { + return res.status(400).json({ error: 'years must be a non-zero whole number' }); + } + const bumpYear = (v) => (typeof v === 'string' && /^20\d{2}$/.test(v.trim())) + ? String(parseInt(v, 10) + years) + : v; + const bumpOffset = (off) => { + // '0 days' stays put: an offset of nothing is not a date to shift. + const m = /^\s*(-?\d+)\s+year/.exec(off || ''); + if (!m) return off; + return `${parseInt(m[1], 10) + years} years`; + }; + const client = await pool.connect(); + try { + await client.query('BEGIN'); + const { rows } = await client.query( + `SELECT * FROM pf.load_plan + WHERE version_id = $1 AND run_logid IS NULL + ORDER BY seq, id`, + [versionId] + ); + const out = []; + for (const p of rows) { + const filters = Array.isArray(p.filters) + ? p.filters.map(group => group.map(f => ({ + ...f, + values: Array.isArray(f.values) ? f.values.map(bumpYear) : f.values + }))) + : p.filters; + const upd = await client.query( + `UPDATE pf.load_plan + SET filters = $2::jsonb, date_offset = $3, updated_at = now() + WHERE id = $1 + RETURNING *`, + [p.id, filters ? JSON.stringify(filters) : null, bumpOffset(p.date_offset)] + ); + out.push(upd.rows[0]); + } + await client.query('COMMIT'); + res.json({ shifted: out.length, plans: out }); + } catch (err) { + await client.query('ROLLBACK').catch(() => {}); + res.status(500).json({ error: err.message }); + } finally { + client.release(); + } + }); + + return router; +}; diff --git a/routes/versions.js b/routes/versions.js index 11e2652..e77a01d 100644 --- a/routes/versions.js +++ b/routes/versions.js @@ -3,6 +3,74 @@ const { fcTable, mapType, RELATION_COLUMNS_SQL } = require('../lib/utils'); const { sessionUser, sessionTerritory } = require('../lib/auth'); const { buildTerritoryClause } = require('../lib/sql_generator'); +// Copy a version's definition onto a freshly created one: its load segments as +// plans, and its saved layouts. +// +// The plans come from pf.load_plan when the template has them, and are derived +// from its baseline/reference log entries when it does not — which is every +// version created before plans existed, so a definition can be lifted out of +// one that was built by hand. `params` on a load entry already stores the +// filters, the raw WHERE when one was used, and the date offset, so the +// derivation is a read rather than a reconstruction. +async function cloneDefinition(client, template, version, created_by) { + const existing = await client.query( + `SELECT * FROM pf.load_plan WHERE version_id = $1 ORDER BY seq, id`, + [template.id] + ); + + let source = existing.rows.map(p => ({ + kind: p.kind, label: p.label, bucket: p.bucket, tag: p.tag, note: p.note, + filters: p.filters, raw_where: p.raw_where, date_offset: p.date_offset + })); + + if (!source.length) { + const log = await client.query( + `SELECT operation, label, bucket, tag, note, params + FROM pf.log + WHERE TRUE + AND version_id = $1 + AND operation IN ('baseline', 'reference') + ORDER BY id`, + [template.id] + ); + source = log.rows.map(e => ({ + kind: e.operation, + label: e.label, bucket: e.bucket, tag: e.tag, note: e.note, + filters: e.params?.filters || null, + raw_where: e.params?.raw_where || null, + date_offset: e.params?.date_offset || '0 days' + })); + } + + for (const [i, p] of source.entries()) { + await client.query( + `INSERT INTO pf.load_plan + (version_id, seq, kind, label, bucket, tag, note, filters, raw_where, + date_offset, created_by) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8::jsonb, $9, COALESCE($10, '0 days'), $11)`, + [version.id, i, p.kind, p.label, p.bucket, p.tag, p.note, + p.filters ? JSON.stringify(p.filters) : null, p.raw_where, + p.date_offset, created_by] + ); + } + + // Version-scoped layouts only. The source-wide ones (version_id IS NULL) + // already apply to the new version, so copying them would duplicate them + // under a name that now exists twice. + const layouts = await client.query( + `INSERT INTO pf.layout (source_id, version_id, name, config, owner, visibility, is_default) + SELECT source_id, $2, name, config, owner, visibility, is_default + FROM pf.layout + WHERE TRUE + AND version_id = $1 + RETURNING id`, + [template.id, version.id] + ); + + return { from: template.id, from_name: template.name, + plans: source.length, layouts: layouts.rowCount }; +} + module.exports = function(pool) { const router = express.Router(); @@ -24,7 +92,7 @@ module.exports = function(pool) { // inserts version row, then CREATE TABLE pf.fc_{tname}_{version_id} in one transaction router.post('/sources/:id/versions', async (req, res) => { const sourceId = parseInt(req.params.id); - const { name, description, exclude_iters } = req.body; + const { name, description, exclude_iters, clone_from } = req.body; const created_by = sessionUser(req); if (!name) return res.status(400).json({ error: 'name is required' }); @@ -62,19 +130,52 @@ module.exports = function(pool) { }); } + // clone_from copies a version's *definition* -- its settings, its + // saved layouts, and its load segments as plans -- and stops there. + // The loads are deliberately not run: the reason to clone a + // definition is to change the dates before it loads anything, and a + // clone that fetched two million rows first would have to be undone + // before it could be edited. + // + // Adjustments are not copied in any form. A scale's target is a + // figure sized against the rows that were there, so replaying one + // against a different load is arithmetic nobody asked for. + let template = null; + if (clone_from) { + const t = await client.query( + `SELECT * FROM pf.version WHERE id = $1 AND source_id = $2`, + [parseInt(clone_from), sourceId] + ); + if (!t.rows.length) { + return res.status(404).json({ + error: 'Version to clone not found, or it belongs to a different source' + }); + } + template = t.rows[0]; + } + await client.query('BEGIN'); // insert version to get id const verResult = await client.query(` - INSERT INTO pf.version (source_id, name, description, created_by, exclude_iters) - VALUES ($1, $2, $3, $4, $5) + INSERT INTO pf.version (source_id, name, description, created_by, exclude_iters, + adjustment_segment, adjustment_bucket, unlabeled_load, + bucket_order) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) RETURNING * `, [ sourceId, name, - description || null, + // An explicit description wins; otherwise the template's is + // carried, since it describes the definition being copied. + description || template?.description || null, created_by || null, - exclude_iters ? JSON.stringify(exclude_iters) : '["reference"]' + exclude_iters ? JSON.stringify(exclude_iters) + : (template ? JSON.stringify(template.exclude_iters) : '["reference"]'), + template?.adjustment_segment || null, + template?.adjustment_bucket || null, + template?.unlabeled_load || null, + template?.bucket_order ? JSON.stringify(template.bucket_order) : null ]); const version = verResult.rows[0]; @@ -109,8 +210,11 @@ ${colDefs}, `CREATE INDEX ${table.split('.').pop()}_logid_idx ON ${table} (pf_logid)` ); + let cloned = null; + if (template) cloned = await cloneDefinition(client, template, version, created_by); + await client.query('COMMIT'); - res.status(201).json({ ...version, fc_table: table }); + res.status(201).json({ ...version, fc_table: table, cloned }); } catch (err) { await client.query('ROLLBACK'); console.error(err); diff --git a/server.js b/server.js index 2c2f543..aced83b 100644 --- a/server.js +++ b/server.js @@ -81,6 +81,7 @@ app.use('/api', require('./routes/versions')(pool)); app.use('/api', require('./routes/operations')(pool)); app.use('/api', require('./routes/log')(pool)); app.use('/api', require('./routes/layouts')(pool)); +app.use('/api', require('./routes/plans')(pool)); const port = process.env.PORT || 3010; diff --git a/setup_sql/01_schema.sql b/setup_sql/01_schema.sql index 96db788..9577b39 100644 --- a/setup_sql/01_schema.sql +++ b/setup_sql/01_schema.sql @@ -261,3 +261,37 @@ WHERE TRUE AND l.version_id IS NULL AND l.is_default ); + +-- pf.load_plan: a segment's definition, before and apart from having been run. +-- +-- Until now a segment existed only once it had been loaded -- as a pf.log entry +-- with forecast rows behind it -- so cloning a version's definition meant +-- re-running every load, and there was nowhere to hold "this is the cut I want, +-- but the dates need changing first". +-- +-- Deliberately not pf.log. That table is the audit log: every row in it means a +-- write happened, and putting unexecuted intent there would cost it that +-- meaning. A plan is intent; run_logid is the pf.log entry it produced. +CREATE TABLE IF NOT EXISTS pf.load_plan ( + id serial PRIMARY KEY, + version_id integer NOT NULL REFERENCES pf.version(id) ON DELETE CASCADE, + seq integer NOT NULL DEFAULT 0, -- run order + kind text NOT NULL, -- baseline | reference + label text, + bucket text, + tag text, + note text, + filters jsonb, -- the structured filter builder's own shape + raw_where text, -- the raw-SQL escape hatch, when used instead + date_offset text NOT NULL DEFAULT '0 days', + -- the pf.log entry this plan produced, or null while it is still a plan. + -- ON DELETE SET NULL so undoing a load returns its plan to unrun rather than + -- deleting the definition with it. + run_logid bigint REFERENCES pf.log(id) ON DELETE SET NULL, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + created_by text, + CHECK (kind IN ('baseline', 'reference')) +); + +CREATE INDEX IF NOT EXISTS load_plan_version ON pf.load_plan (version_id, seq, id); diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index 6e92d01..9fd6b35 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -96,6 +96,13 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio const [newVerName, setNewVerName] = useState('') const [newVerDesc, setNewVerDesc] = useState('') const [creatingVer, setCreatingVer] = useState(false) + const [cloneFrom, setCloneFrom] = useState('') + // Planned segments: a version's definition before it has been run. Separate + // from `log`, which is what has actually happened. + const [plans, setPlans] = useState([]) + const [editingPlanId, setEditingPlanId] = useState(null) + const [runningAll, setRunningAll] = useState(false) + const [shiftYears, setShiftYears] = useState('1') // segment form const [segType, setSegType] = useState('baseline') @@ -130,8 +137,9 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio }, [sourceId]) useEffect(() => { - if (!versionId) { setLog([]); return } + if (!versionId) { setLog([]); setPlans([]); return } loadLog() + loadPlans() }, [versionId]) // A segment's banner: what it counts toward, independent of pf_iter. Held @@ -196,6 +204,160 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio }) } + function loadPlans() { + fetch(`/api/versions/${versionId}/plans`) + .then(r => r.ok ? r.json() : []) + .then(setPlans) + .catch(() => setPlans([])) + } + + // Run one plan: the same call the Add Segment form makes, then stamp the log + // id back onto the plan so the list can tell run from unrun. Returns the + // error text, or null on success, so Run all can stop at the first failure + // rather than carrying on down a definition that has gone wrong. + async function runPlan(plan) { + const body = { + where_clause: plan.raw_where || buildFilterClause(normalizeFilters(plan.filters) || []), + note: plan.note || '', + date_offset: plan.date_offset || '0 days', + label: plan.label || '', + bucket: plan.bucket || '', + ...(plan.raw_where ? { raw_where: plan.raw_where } : { filters: plan.filters }), + } + if (!body.where_clause) return 'No filters on this plan' + try { + const res = await fetch(`/api/versions/${versionId}/${plan.kind}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }) + const data = await res.json() + if (!res.ok) return data.error || 'Failed' + const logId = data.log_id ?? data.id ?? null + if (logId) { + await fetch(`/api/plans/${plan.id}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ run_logid: logId }), + }) + } + return null + } catch (err) { return err.message } + } + + async function runOnePlan(plan) { + setSubmitting(true) + const err = await runPlan(plan) + setSubmitting(false) + loadLog(); loadPlans() + flash(err || `Loaded “${plan.label || plan.tag || plan.kind}”`, err ? 'error' : 'ok') + } + + // In seq order, and stopping at the first failure: the segments of a + // definition are not independent -- a baseline followed by references that + // date-shift off it -- so carrying on past a broken one builds something + // nobody described. + async function runAllPlans() { + const pending = plans.filter(p => !p.run_logid) + if (!pending.length) return + setRunningAll(true) + let done = 0 + for (const plan of pending) { + const err = await runPlan(plan) + if (err) { + setRunningAll(false) + loadLog(); loadPlans() + flash(`Stopped after ${done} of ${pending.length}: ${err}`, 'error') + return + } + done++ + } + setRunningAll(false) + loadLog(); loadPlans() + flash(`Loaded ${done} segment${done === 1 ? '' : 's'}`) + } + + async function shiftPlanYears() { + const years = parseInt(shiftYears) + if (!Number.isInteger(years) || years === 0) { flash('Enter a whole number of years', 'error'); return } + try { + const res = await fetch(`/api/versions/${versionId}/plans/shift-years`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ years }), + }) + const data = await res.json() + if (!res.ok) { flash(data.error, 'error'); return } + loadPlans() + flash(`Shifted ${data.shifted} plan${data.shifted === 1 ? '' : 's'} by ${years > 0 ? '+' : ''}${years} year${Math.abs(years) === 1 ? '' : 's'}`) + } catch (err) { flash(err.message, 'error') } + } + + async function deletePlan(plan) { + await fetch(`/api/plans/${plan.id}`, { method: 'DELETE' }) + if (editingPlanId === plan.id) cancelEdit() + loadPlans() + } + + // A plan opens in the same form a segment does; the difference is only which + // call the submit makes. + function startEditPlan(plan) { + setSegType(plan.kind) + setSegNote(plan.note || '') + setSegLabel(plan.label || '') + setSegBucket(plan.bucket || '') + setOffset(plan.date_offset || '0 days') + const groups = normalizeFilters(plan.filters) + if (groups) { setUseRaw(false); setRawSql(''); setFilters(groups) } + else if (plan.raw_where) { setUseRaw(true); setRawSql(plan.raw_where) } + else { setUseRaw(false); setRawSql(''); setFilters(filterCols.length > 0 ? [emptyGroup(filterCols)] : []) } + setEditingLogId(null) + setEditingPlanId(plan.id) + setExpandedId(null) + setTimeout(() => { + document.getElementById('add-segment')?.scrollIntoView({ behavior: 'smooth', block: 'start' }) + }, 0) + } + + // Save the definition without running it — the whole point of a plan. + async function savePlan(opts = {}) { + const plan = plans.find(p => p.id === editingPlanId) + if (!plan) return null + setSubmitting(true) + try { + const res = await fetch(`/api/plans/${editingPlanId}`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + kind: segType, + label: segLabel.trim(), + bucket: segBucket.trim(), + note: segNote.trim(), + date_offset: offset.trim() || '0 days', + ...(useRaw ? { raw_where: rawSql.trim(), filters: null } + : { filters, raw_where: null }), + }), + }) + const data = await res.json() + if (!res.ok) { flash(data.error, 'error'); return null } + loadPlans() + cancelEdit() + if (!opts.returnRow) flash('Plan saved — not loaded yet') + return data + } catch (err) { flash(err.message, 'error'); return null } + finally { setSubmitting(false) } + } + + // Save what is in the form, then run what was saved. Reading the plan back + // from the PATCH rather than reusing the row in state: the form is what the + // user just edited, and running the stale copy would load the old definition + // while showing the new one. + async function saveAndRunPlan() { + const saved = await savePlan({ returnRow: true }) + if (!saved) return + await runOnePlan(saved) + } + async function createVersion() { if (!newVerName.trim()) return setCreatingVer(true) @@ -203,7 +365,13 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio const res = await fetch(`/api/sources/${sourceId}/versions`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ name: newVerName.trim(), description: newVerDesc, created_by: 'admin' }) + // created_by is ignored by the server, which takes the account from the + // session; sending it would let any client claim to be anyone. + body: JSON.stringify({ + name: newVerName.trim(), + description: newVerDesc, + ...(cloneFrom ? { clone_from: parseInt(cloneFrom) } : {}), + }) }) const data = await res.json() if (!res.ok) { flash(data.error, 'error'); return } @@ -212,7 +380,10 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio setShowNewVersion(false) setNewVerName('') setNewVerDesc('') - flash(`Version "${data.name}" created`) + setCloneFrom('') + flash(data.cloned + ? `Version "${data.name}" created from "${data.cloned.from_name}" — ${data.cloned.plans} segment${data.cloned.plans === 1 ? '' : 's'} planned, nothing loaded yet` + : `Version "${data.name}" created`) } catch (err) { flash(err.message, 'error') } finally { @@ -294,6 +465,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio function cancelEdit() { setEditingLogId(null) + setEditingPlanId(null) setShowAddForm(false) setSegNote('') setSegLabel('') @@ -436,6 +608,14 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio setNewVerName(e.target.value)} placeholder="e.g. FY2026 Plan" className="border border-gray-200 rounded px-2 py-1 text-sm w-48" /> +
+ + +
setNewVerDesc(e.target.value)} placeholder="optional" className="border border-gray-200 rounded px-2 py-1 text-sm" /> @@ -446,6 +626,9 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
+ {cloneFrom + ? <>Copies that version's settings, its saved layouts, and its load segments as plans — nothing is loaded, so the dates can be changed before anything runs. Adjustments are not copied: a scale's target was sized against the rows that were there. + : null} Creates a forecast table pf.fc_{sources.find(s=>String(s.id)===sourceId)?.tname}_<id> in the database from the current col meta. If col meta changes after creation the table and SQL will be out of sync — delete and recreate the version to realign.
@@ -453,6 +636,86 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio {versionId && <> + {/* Planned segments — the definition, before it has been run. Only + shown when there are any: a version built by hand has none, and an + empty panel would be one more thing to read past. */} + {plans.length > 0 && ( +
+
+ Planned segments + + {plans.filter(p => !p.run_logid).length} of {plans.length} not yet loaded + +
+
+ Shift years + setShiftYears(e.target.value)} + className="border border-gray-200 rounded px-1 py-0.5 w-12 text-center" /> + +
+ +
+
+ + + + + + + + + + + + + + + {plans.map((p, i) => { + const where = p.raw_where + || buildFilterClause(normalizeFilters(p.filters) || []) + || '—' + return ( + + + + + + + + + + + ) + })} + +
#kindlabelcounts towardoffsetwhererows
{i + 1}{p.kind}{p.label || unnamed}{p.bucket || '—'}{p.date_offset}{where} + {p.run_logid ? (p.run_rows ?? '').toLocaleString?.() || p.run_rows : ''} + + {p.run_logid ? ( + loaded + ) : ( + <> + + + + + )} +
+
+ )} + {/* Segments loaded */}
@@ -586,10 +849,17 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio
{/* Add / Edit Segment */} - {(showAddForm || editingLogId) && ( -
-
+ {(showAddForm || editingLogId || editingPlanId) && ( +
+
{(() => { + if (editingPlanId) { + const idx = plans.findIndex(p => p.id === editingPlanId) + const plan = plans[idx] + return plan + ? `Edit plan ${idx + 1} — ${plan.kind}${plan.label ? ` — ${plan.label}` : ''} — not loaded yet` + : 'Edit plan' + } if (!editingLogId) return 'Add Segment' const idx = log.findIndex(e => e.id === editingLogId) if (idx < 0) return 'Edit Segment' @@ -601,7 +871,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio : `Edit segment #${segNum} — ${label}` })()}
)} @@ -659,6 +932,12 @@ function SegmentForm({ onSubmit, submitting, editing, + // A plan saves without running, so its primary action is named for that and + // running is the secondary. Both null for an ordinary segment, where loading + // is the only thing submitting can mean. + submitLabel = null, + onSecondary = null, + secondaryLabel = null, }) { const disabled = mode === 'view' const compiled = useRaw ? rawSql : (buildFilterClause(filters) || '') @@ -879,10 +1158,19 @@ function SegmentForm({ {mode === 'edit' && ( + )} + {mode === 'edit' && onSecondary && ( + )}
From 738e177793ad362b7fb00b0f27d7f2ce89ed392f Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 10:26:37 -0400 Subject: [PATCH 02/13] Let a forecast be renamed PUT /versions/:id has taken name and description since it existed; only the fallback-name fields ever called it, so the one thing a version's name could not be was changed. A This forecast panel on the Baseline page now saves both on blur. Safe at any time, closed versions included: nothing derives from the name. The forecast table is keyed on the id, as are pf.log, pf.layout and the UI's localStorage. Two edges the route left open. (source_id, name) is unique, so a rename can collide -- it now answers 409 with the same sentence the POST has always used, rather than a raw Postgres message that reads as a server fault. And the UPDATE COALESCEs a falsy name back to the existing one, so a blank box would have looked saved and changed nothing; that is refused where it is typed. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 16 +++++++++++++++ routes/versions.js | 7 +++++++ ui/src/views/Baseline.jsx | 43 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 24f77bf..9cc8d22 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -323,6 +323,22 @@ together. `columns` selects which *measures* appear, not individual split combinations. +## Renaming a version + +Safe at any time, including on a closed one. Nothing derives from the name: the +forecast table is `fc_{tname}_{version_id}`, `pf.log` and `pf.layout` key on the +id, and the UI's `localStorage` keys are id-based. The Baseline page's **This +forecast** panel saves `name` and `description` on blur through +`PUT /api/versions/:id`, which has accepted both all along — it was only ever +called for the fallback names. + +Two edges the panel handles: `(source_id, name)` is unique, so the route +answers 409 on a collision rather than a raw Postgres error; and the `UPDATE` +COALESCEs a falsy name back to the existing one, so a blank box is refused +client-side rather than looking saved and changing nothing. + +--- + ## Cloning a version's definition `POST /sources/:id/versions` takes `clone_from`. It copies the template diff --git a/routes/versions.js b/routes/versions.js index e77a01d..aa03e22 100644 --- a/routes/versions.js +++ b/routes/versions.js @@ -463,6 +463,13 @@ ${colDefs}, res.json(result.rows[0]); } catch (err) { console.error(err); + // (source_id, name) is unique, so a rename can collide. The POST has + // said so for as long as it has existed; this said it with a raw + // Postgres message, which reads as a server fault rather than a name + // that is already taken. + if (err.code === '23505') { + return res.status(409).json({ error: 'A version with that name already exists for this source' }); + } res.status(500).json({ error: err.message }); } }); diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index 9fd6b35..bf07df8 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -529,9 +529,15 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio // The names a row falls back to when nobody has named it. Blank means "use the // built-in", so these save on blur like the segment fields and an empty box is // a meaningful value rather than a missing one. - async function saveVersionName(field, value) { + async function saveVersionName(field, value, opts = {}) { const next = value.trim() if (next === (selectedVersion?.[field] || '')) return + // The route COALESCEs a falsy name back to the existing one, so a blank box + // would look saved and change nothing. Say so instead. + if (field === 'name' && !next) { + flash('A version needs a name', 'error') + return + } try { const res = await fetch(`/api/versions/${versionId}`, { method: 'PUT', @@ -540,7 +546,10 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio }) if (!res.ok) { const d = await res.json(); flash(d.error, 'error'); return } await refreshVersions(sourceId) - flash('Saved — reload the Forecast view to see it') + // A rename shows everywhere the version is listed and needs no reload; a + // fallback name is baked into the aggregated row, so the pivot cannot + // re-derive it in place. + flash(opts.message || 'Saved — reload the Forecast view to see it') } catch (err) { flash(err.message, 'error') } } @@ -576,6 +585,36 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio )}
+ {/* Name and description. Nothing depends on the name -- the forecast table + is fc_{tname}_{version_id}, pf.log and pf.layout key on the id, and the + UI's localStorage keys are id-based -- so it is safe to change at any + time, including on a closed version, where correcting a name is still + legitimate. Saves on blur, like the fallback names below. */} + {versionId && ( +
+ This forecast +
+ + saveVersionName('name', e.target.value, { message: 'Renamed' })} + onKeyDown={e => { if (e.key === 'Enter') e.currentTarget.blur() }} + className="border border-gray-200 rounded px-2 py-1 text-sm w-64" /> +
+
+ + saveVersionName('description', e.target.value, { message: 'Saved' })} + onKeyDown={e => { if (e.key === 'Enter') e.currentTarget.blur() }} + placeholder="optional" + className="border border-gray-200 rounded px-2 py-1 text-sm w-full" /> +
+
+ )} + {/* Fallback names. Not part of any segment -- they are what the pivot shows for rows nobody has named, so they belong to the version rather than to a log entry. Blank falls back to the built-in in sql_generator's From fe456d4e90c1653ab0aaa9f50ebe9d91b7a16968 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 12:39:07 -0400 Subject: [PATCH 03/13] Drop Shift years Nobody asked for it. It was my own addition to the plans panel, guessing that cloning a definition forward a year would be the common case; the actual edit wanted was two dates moved by five days, typed by hand. A bulk rewrite of filter values is a lot of machinery to leave sitting next to Run all on the chance it is ever right. Never used, so nothing to unwind: every plan on the cloned version still carries the template's filters verbatim apart from the two dates edited in the form. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 10 +------ routes/plans.js | 59 --------------------------------------- ui/src/views/Baseline.jsx | 25 ----------------- 3 files changed, 1 insertion(+), 93 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9cc8d22..5d318bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ routes/ operations.js scale, recode, clone, undo — the core forecast ops log.js GET /api/versions/:id/log, DELETE /api/log/:logid layouts.js Named pivot layouts — list per version, create, patch, delete - plans.js Planned load segments — list, create, patch, delete, shift-years + plans.js Planned load segments — list, create, patch, delete lib/ sql_generator.js buildFilterClause, token substitution helpers auth.js scrypt hash/verify, requireAuth, sessionUser; `node lib/auth.js hash` CLI @@ -376,14 +376,6 @@ from its `baseline`/`reference` log entries when it does not — which is every version built before plans existed. `pf.log.params` already stores `filters`, `raw_where` and `date_offset`, so this is a read, not a reconstruction. -**`POST /versions/:id/plans/shift-years`** adds a whole number of years to every -*unrun* plan's `date_offset` and to any bare four-digit `20xx` in its filter -**values**. Values only, and only a bare year: a season reads `2027`, a month -reads `04 - Sep`, and a part number that happens to look like a year would be -corrupted by anything looser. Cloning a definition forward a year is the reason -plans exist, and doing it by hand across eight segments is where the mistakes -come from. - **Run all stops at the first failure.** The segments of a definition are not independent — a baseline followed by references that date-shift off it — so carrying on past a broken one builds something nobody described. diff --git a/routes/plans.js b/routes/plans.js index 24384ba..5a03614 100644 --- a/routes/plans.js +++ b/routes/plans.js @@ -121,64 +121,5 @@ module.exports = function(pool) { } }); - // Shift every unrun plan's date_offset and any four-digit year in its filter - // values by a whole number of years. Cloning a definition forward is the - // reason plans exist, and doing it by hand across eight segments is where - // the mistakes come from. - // - // Years in filter *values* only, and only a bare four-digit 20xx: a season - // reads "2027", a month reads "04 - Sep", and a part number that happens to - // look like a year would be corrupted by anything looser. - router.post('/versions/:id/plans/shift-years', async (req, res) => { - const versionId = parseInt(req.params.id); - const years = parseInt(req.body?.years); - if (!Number.isInteger(years) || years === 0) { - return res.status(400).json({ error: 'years must be a non-zero whole number' }); - } - const bumpYear = (v) => (typeof v === 'string' && /^20\d{2}$/.test(v.trim())) - ? String(parseInt(v, 10) + years) - : v; - const bumpOffset = (off) => { - // '0 days' stays put: an offset of nothing is not a date to shift. - const m = /^\s*(-?\d+)\s+year/.exec(off || ''); - if (!m) return off; - return `${parseInt(m[1], 10) + years} years`; - }; - const client = await pool.connect(); - try { - await client.query('BEGIN'); - const { rows } = await client.query( - `SELECT * FROM pf.load_plan - WHERE version_id = $1 AND run_logid IS NULL - ORDER BY seq, id`, - [versionId] - ); - const out = []; - for (const p of rows) { - const filters = Array.isArray(p.filters) - ? p.filters.map(group => group.map(f => ({ - ...f, - values: Array.isArray(f.values) ? f.values.map(bumpYear) : f.values - }))) - : p.filters; - const upd = await client.query( - `UPDATE pf.load_plan - SET filters = $2::jsonb, date_offset = $3, updated_at = now() - WHERE id = $1 - RETURNING *`, - [p.id, filters ? JSON.stringify(filters) : null, bumpOffset(p.date_offset)] - ); - out.push(upd.rows[0]); - } - await client.query('COMMIT'); - res.json({ shifted: out.length, plans: out }); - } catch (err) { - await client.query('ROLLBACK').catch(() => {}); - res.status(500).json({ error: err.message }); - } finally { - client.release(); - } - }); - return router; }; diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index bf07df8..570a51d 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -102,7 +102,6 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio const [plans, setPlans] = useState([]) const [editingPlanId, setEditingPlanId] = useState(null) const [runningAll, setRunningAll] = useState(false) - const [shiftYears, setShiftYears] = useState('1') // segment form const [segType, setSegType] = useState('baseline') @@ -277,22 +276,6 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio flash(`Loaded ${done} segment${done === 1 ? '' : 's'}`) } - async function shiftPlanYears() { - const years = parseInt(shiftYears) - if (!Number.isInteger(years) || years === 0) { flash('Enter a whole number of years', 'error'); return } - try { - const res = await fetch(`/api/versions/${versionId}/plans/shift-years`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ years }), - }) - const data = await res.json() - if (!res.ok) { flash(data.error, 'error'); return } - loadPlans() - flash(`Shifted ${data.shifted} plan${data.shifted === 1 ? '' : 's'} by ${years > 0 ? '+' : ''}${years} year${Math.abs(years) === 1 ? '' : 's'}`) - } catch (err) { flash(err.message, 'error') } - } - async function deletePlan(plan) { await fetch(`/api/plans/${plan.id}`, { method: 'DELETE' }) if (editingPlanId === plan.id) cancelEdit() @@ -687,14 +670,6 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio {plans.filter(p => !p.run_logid).length} of {plans.length} not yet loaded
-
- Shift years - setShiftYears(e.target.value)} - className="border border-gray-200 rounded px-1 py-0.5 w-12 text-center" /> - -
- ))} +
+ {label} + + + + + + + + {open && ( +
+ {Array.from({ length: max + 1 }, (_, n) => ( + + ))} +
+ )}
) } From b015b0c09bfc3d6e9bbb41f858a244ecda387dc7 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 22:05:14 -0400 Subject: [PATCH 06/13] Give the tag field back its space The six quick-pick pills under it wrapped onto two rows as soon as the tags were longer than a word, which most are, and the operations panel docked at the bottom has a fixed height to spend -- so the form ran past its own border. The same tags complete from the input's datalist, which costs no space and is not capped at six. The datalist now carries the tag alone. An option's text content renders as a second column beside the value, so the use count was widening every row of the list to show a number nobody picks on. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 9 +++++++++ ui/src/components/OperationPanel.jsx | 26 +++++++------------------- ui/src/views/Forecast.jsx | 9 +++++---- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 98dd643..15b9100 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -348,6 +348,15 @@ no adjustments. The `pf_bucket` datalist on the Baseline page was already per version (`bucketsInUse`), so the two now agree. +**Completion is the datalist and nothing else.** The operation panel used to +carry six quick-pick pills under the tag field as well. They wrapped to two rows +as soon as the tags were longer than a word, and the panel docked at the bottom +has a fixed height, so they pushed the form past its own border. The datalist +costs no space, is not capped at six, and now shows the tag alone — an option's +text content renders as a second column beside the value, so the use count was +widening every row of the pick list to carry a number nobody picks on. `uses` +and `last_used` still come back from the route; nothing reads them. + --- ## Renaming a version diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index f6e9193..e41dd66 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -898,7 +898,7 @@ export default function OperationPanel({ scaleInputs, setScaleInputs, scalePlug, setScalePlug, targetBasis, setTargetBasis, - opTag, setOpTag, knownTags = [], logMeta = {}, + opTag, setOpTag, logMeta = {}, scaleNote, setScaleNote, recodeSet, setRecodeSet, recodeNote, setRecodeNote, @@ -1066,24 +1066,12 @@ export default function OperationPanel({ )} - {/* Under the tag field and inside the same column, so they read as - values for it rather than as a row of unexplained buttons. */} - {knownTags.length > 0 && ( - -
- {knownTags.slice(0, 6).map(t => ( - - ))} -
- - )} + {/* The six quick-pick pills that used to sit here are gone. They + wrapped onto two rows once the tags were more than a word long, + which is most of them, and the panel docked at the bottom has a + fixed height to spend -- so they pushed the form past its own + border. The same tags complete from the input's datalist, which + costs no space and is not capped at six. */} setNote(e.target.value)} diff --git a/ui/src/views/Forecast.jsx b/ui/src/views/Forecast.jsx index 8d8d528..45cbb7a 100644 --- a/ui/src/views/Forecast.jsx +++ b/ui/src/views/Forecast.jsx @@ -1881,7 +1881,7 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio scaleInputs, setScaleInputs, scalePlug, setScalePlug, targetBasis, setTargetBasis, - opTag, setOpTag, knownTags, logMeta, + opTag, setOpTag, logMeta, scaleNote, setScaleNote, recodeSet, setRecodeSet, recodeNote, setRecodeNote, @@ -1898,10 +1898,11 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio {/* Tag completions, shared by the operation panel and the change log's inline editor. Lives here so it is in the DOM even when the panel is shut. */} + {/* Value only. A datalist option's text content renders as a second + column beside the value, so the use count widened every row of the + pick list to carry a number nobody picks on. */} - {knownTags.map(t => ( - - ))} + {knownTags.map(t => {/* Toolbar */} From 6c24aa8390dc5351809edb326ed2b5703410c292 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 22:15:33 -0400 Subject: [PATCH 07/13] Make the ledger's settings read as settings Absorbed by and Target applies to each hand-rolled their own label row and their own hint, at two different type sizes, with no shared label width and no rule separating them from the numbers above. Stacked, they read as a paragraph of loose words -- label, two buttons, a sentence, label, two buttons, a longer sentence -- rather than as two settings on the edit. Both now use Field, like every other setting in this panel: one aligned label column, the control, the hint below it in the same muted 11px. Per slice joins them instead of trailing off as a bare paragraph. Target applies to becomes Target on, which fits the label column without wrapping. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 90 ++++++++++++++-------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index e41dd66..dd40ae7 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -666,53 +666,53 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se {/* A dollar figure on its own does not say whether price or volume moved. Only ask once the edit is actually dollars-only -- naming units or price has already answered it, and the control would just be noise. */} - {valueCol && unitsCol && dollarsOnly && ( -
-
- Absorbed by - -
-
- {scalePlug === 'price' - ? `${unitsCol} holds; price moves to reach the number.` - : `Price holds; ${unitsCol} scales with the dollars.`} -
-
- )} + {/* Both of these are settings on the edit, so they use Field like every + other one -- an aligned label column and a hint below the control. + They used to hand-roll their own label row and hint, with two + different type sizes and no shared label width, so the two stacked + into a paragraph of loose words rather than reading as two settings. */} + {(valueCol && unitsCol && dollarsOnly || hasExcl || perSlice) && ( +
- {hasExcl && ( -
-
- Target applies to - -
-

- {onTotal - ? `The ${exclName} rows cannot change, so the adjustable rows absorb the whole difference — the pivot will show your target.` - : `${exclName} rows are ignored. The pivot will show your target plus ${fmtNum(excl.value)}.`} -

-
- )} + {valueCol && unitsCol && dollarsOnly && ( + + + + )} - {perSlice && ( -

- Applied to each slice separately — the figures above are combined totals, - so each slice's own change will differ. -

+ {hasExcl && ( + + + + )} + + {perSlice && ( + + Applied to each slice separately + + )} +
)}
) From a642c65d1dae034a7df57242e48b882fe68e5277 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 22:20:49 -0400 Subject: [PATCH 08/13] Line the percent box up with the other two The % was a sibling of the input in a flex row, so it took width of its own. In a right-aligned cell that pushed the percent row's box a suffix and a gap to the left of New value and Change -- three ways of saying the same thing, not lining up. It now sits inside the box, over the input's right padding, so every row is the same width against the same edge, and that edge is where the numbers above it end too. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index dd40ae7..7bf8172 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -305,8 +305,13 @@ function LedgerInput({ value, active, onChange, onFocus, suffix }) { try { el.setSelectionRange(pos, pos) } catch {} }, [display]) + // The suffix is positioned inside the box rather than set beside it. As a + // sibling in the flex row it took width of its own, so in a right-aligned cell + // the % row's input sat a suffix-and-a-gap to the left of the rows above it, + // and the three boxes that are meant to be three ways of saying one thing did + // not line up. Inside, every row's box is the same width and the same edge. return ( - + - {suffix && {suffix}} + {suffix && ( + {suffix} + )} ) } From 75f662d12af74e46d13ea78bbe84f7115443b268 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 22:27:31 -0400 Subject: [PATCH 09/13] Stop reading numbers back out of formatted text The Result row re-parsed the ledger's own display strings, which fmtNum grouped with toLocaleString. parseFloat stops at the first separator, so "3,456,789" came back as 3 -- a plausible small number rather than an error, which is why it read as a formatting quirk instead of a wrong answer. A 3% change on sales_usd previewed as 3. It showed only where the new value was derived rather than typed: the field you type into keeps your raw text, so entering a target directly looked right while the % and Change rows did not. derive now returns the three figures as numbers alongside the strings, and outcome reads those. Nothing has to know how the display is grouped. The write was never affected -- buildPayload reads scaleInputs[*].raw, which LedgerInput has always kept comma-free -- so this was a wrong preview of a correct write. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 7bf8172..a498961 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -258,6 +258,12 @@ function derive(current, edit, dp = 2) { new: fmtNum(next, dp), change: fmtNum(change, dp), pct: pct == null ? '—' : fmtNum(pct, 1), + // The same three as numbers, for anything that needs to compute with them. + // The strings above are grouped by toLocaleString, and parseFloat stops at + // the first separator -- "3,456,789" reads back as 3 -- so a caller that + // re-parsed the display got a plausible small number rather than an error. + // Nothing should have to know that; the numbers travel alongside. + n: { new: next, change, pct }, } out[edit.field] = edit.raw // keep what you typed exactly as typed return out @@ -442,9 +448,8 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se // left units and price blank -- exactly the two numbers you need to see to // know whether you asked for a price change or a volume change. const numOf = (key, field) => { - const raw = derived[key]?.[field] - const n = parseFloat(raw) - return isFinite(n) ? n : null + const n = derived[key]?.n?.[field] + return n != null && isFinite(n) ? n : null } const outcome = (() => { From 1035dae76ba72c69fb4f276a9d0b2d44529f30a8 Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 22:50:34 -0400 Subject: [PATCH 10/13] Draw the derived ledger rows as output, not input One box per measure is live; the other two are the same number said differently. Both states were full input boxes differing by a border colour, so three typed figures appeared where there was one typed figure and two calculations -- and with six boxes on screen there was no reading which one the adjustment would be built from. The live box keeps its blue border and ground and gains weight; the derived two lose their border and background and go muted. They are still editable, which is what the border on hover says, and the title spells out that clicking enters the number that way instead. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index a498961..f616267 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -237,6 +237,15 @@ const FIELDS = [ ['pct', '% change'], ] +// Exactly one box per measure is live -- scaleInputs is keyed by measure, so +// focusing another row hands that measure's edit over rather than adding a +// second one. The other two are the same number said differently, so they are +// drawn as output: no border, no ground, muted. They are still editable, and a +// border on hover is what says so. +// +// Both states used to be full input boxes differing by a border colour, which +// made three typed figures out of one typed figure and two calculations. +// // given the active edit for a measure, what do the three rows read? // dp is the measure's own precision, so the editable rows read the same way as // the lines above them -- whole dollars against whole dollars, five places @@ -328,11 +337,16 @@ function LedgerInput({ value, active, onChange, onFocus, suffix }) { onChange(el.value.replace(/,/g, '')) }} onFocus={onFocus} placeholder="—" + title={active ? undefined : 'Derived — click to enter the number this way instead'} className={`border rounded py-0.5 text-xs w-24 text-right font-mono tabular-nums + transition-colors ${suffix ? 'pl-2 pr-5' : 'px-2'} - ${active ? 'border-blue-400 bg-blue-50/40 text-gray-800' : 'border-gray-200 bg-white text-gray-700'}`} /> + ${active + ? 'border-blue-400 bg-blue-50/40 text-gray-900 font-medium' + : 'border-transparent bg-transparent text-gray-400 hover:border-gray-300 hover:bg-white'}`} /> {suffix && ( - {suffix} )} From 042c85d6c3acf6a94bae992a6737bdbba9d8469a Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 22:53:03 -0400 Subject: [PATCH 11/13] Back-fill the measures an edit forces Scaling dollars with price holding moves qty, but the qty column stayed blank until the Result row -- so the number you had just implied was the one thing not shown beside the number you typed. A measure with no edit of its own now reads all three ways from where the edit actually lands it. derived stays the edit-driven map, since it is what outcome is computed from and back-filling into it would feed the result into its own inputs; a separate shown map is what the rows render. Clicking into a back-filled figure pins it, which is the difference between "units may move" and "units are this". The write is the same either way when the pinned number is the implied one, so the cost of pinning by accident is nothing. Reverts the muted styling for the derived rows -- all three boxes look like boxes again. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 53 +++++++++++++++++----------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index f616267..4ae9ce6 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -237,15 +237,6 @@ const FIELDS = [ ['pct', '% change'], ] -// Exactly one box per measure is live -- scaleInputs is keyed by measure, so -// focusing another row hands that measure's edit over rather than adding a -// second one. The other two are the same number said differently, so they are -// drawn as output: no border, no ground, muted. They are still editable, and a -// border on hover is what says so. -// -// Both states used to be full input boxes differing by a border colour, which -// made three typed figures out of one typed figure and two calculations. -// // given the active edit for a measure, what do the three rows read? // dp is the measure's own precision, so the editable rows read the same way as // the lines above them -- whole dollars against whole dollars, five places @@ -337,16 +328,11 @@ function LedgerInput({ value, active, onChange, onFocus, suffix }) { onChange(el.value.replace(/,/g, '')) }} onFocus={onFocus} placeholder="—" - title={active ? undefined : 'Derived — click to enter the number this way instead'} className={`border rounded py-0.5 text-xs w-24 text-right font-mono tabular-nums - transition-colors ${suffix ? 'pl-2 pr-5' : 'px-2'} - ${active - ? 'border-blue-400 bg-blue-50/40 text-gray-900 font-medium' - : 'border-transparent bg-transparent text-gray-400 hover:border-gray-300 hover:bg-white'}`} /> + ${active ? 'border-blue-400 bg-blue-50/40 text-gray-800' : 'border-gray-200 bg-white text-gray-700'}`} /> {suffix && ( - {suffix} )} @@ -501,6 +487,30 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se } })() + // 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 + // just implied was the one thing not shown next to the one you typed. + const backfill = (current, next, dp) => { + if (next == null || !isFinite(next) || current == null) return { new: '', change: '', pct: '' } + const change = next - current + const pct = current === 0 ? null : (change / Math.abs(current)) * 100 + return { + new: fmtNum(next, dp), + change: fmtNum(change, dp), + pct: pct == null ? '—' : fmtNum(pct, 1), + n: { new: next, change, pct }, + } + } + + // What the rows render. derived stays the edit-driven map -- it is what + // outcome is computed from, so back-filling into it would feed the result + // back into its own inputs. + const shown = Object.fromEntries(measures.map(m => [ + m.key, + (filled(m.key) || !outcome) ? derived[m.key] : backfill(basisOf(m.key), outcome[m.key], m.dp), + ])) + const setEdit = (key, field, raw) => setScaleInputs(prev => ({ ...prev, [key]: { field, raw } })) @@ -510,9 +520,12 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se setScaleInputs(prev => { const cur = prev[key] if (cur && cur.field === field) return prev - const shown = derived[key]?.[field] ?? '' - const raw = shown === '—' ? '' : String(shown).replace(/,/g, '') - return { ...prev, [key]: { field, raw: cur ? raw : '' } } + // Whatever the box reads is what you get, including a back-filled + // number: clicking into an implied figure is how you pin it, which is + // the difference between "units may move" and "units are this". + const text = shown[key]?.[field] ?? '' + const raw = text === '—' ? '' : String(text).replace(/,/g, '') + return { ...prev, [key]: { field, raw } } }) // A running total across the walk, in the primary measure. The lines sum to @@ -661,7 +674,7 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se return ( setEdit(m.key, field, raw)} onFocus={() => focusRow(m.key, field)} From 46a812dd74182c041f31e15496154bc42508987a Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Mon, 21 Sep 2026 23:05:54 -0400 Subject: [PATCH 12/13] Grey the measure the plug holds Absorbed by asks which of price and volume moves, and then said so only in a sentence underneath itself. The column it holds still now says it: the header reads "holds" in place of its hint and the three boxes go flat grey, so what you are editing is legible from the numbers rather than from prose about them. The sentence goes, since the greyed column is the same statement made where it applies. The toggle stays -- it is an input, and with dollars alone nothing in the data answers it. A held box is still typeable, and doing so is how you override the toggle: naming qty or price answers the question directly, which is why the control only appears while dollars are the sole input. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 31 ++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 4ae9ce6..110b112 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -289,7 +289,7 @@ function groupDigits(raw) { return `${neg ? '-' : ''}${grouped}${dot === -1 ? '' : `.${frac}`}` } -function LedgerInput({ value, active, onChange, onFocus, suffix }) { +function LedgerInput({ value, active, held, onChange, onFocus, suffix }) { const ref = useRef(null) const caret = useRef(null) const display = groupDigits(value) @@ -328,9 +328,12 @@ function LedgerInput({ value, active, onChange, onFocus, suffix }) { onChange(el.value.replace(/,/g, '')) }} onFocus={onFocus} placeholder="—" - className={`border rounded py-0.5 text-xs w-24 text-right font-mono tabular-nums + title={held ? 'Held by the Absorbed by setting — type here to set it yourself' : undefined} + className={`border rounded py-0.5 text-xs w-24 text-right font-mono tabular-nums transition-colors ${suffix ? 'pl-2 pr-5' : 'px-2'} - ${active ? 'border-blue-400 bg-blue-50/40 text-gray-800' : 'border-gray-200 bg-white text-gray-700'}`} /> + ${active ? 'border-blue-400 bg-blue-50/40 text-gray-800' + : held ? 'border-gray-100 bg-gray-50 text-gray-400' + : 'border-gray-200 bg-white text-gray-700'}`} /> {suffix && ( {suffix} @@ -549,6 +552,14 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se })() const showRunning = !!valueCol && (lines.length + exclLines.length) > 1 + // Which measure the plug holds still. Only meaningful while dollars are the + // sole input -- that is the one case where price and volume are both unknown + // and the toggle is answering for them. 'price' means price absorbs the + // change, so qty holds; 'volume' means qty absorbs it, so price holds. + const heldKey = (valueCol && unitsCol && dollarsOnly) + ? (scalePlug === 'price' ? 'units' : 'price') + : null + const numCell = 'text-right font-mono tabular-nums whitespace-nowrap px-2' const rule =
@@ -559,9 +570,13 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se {measures.map(m => ( - + {m.label} - {m.hint && · {m.hint}} + {m.key === heldKey + ? · holds + : m.hint && · {m.hint}} ))} {showRunning && ( @@ -676,6 +691,7 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se setEdit(m.key, field, raw)} onFocus={() => focusRow(m.key, field)} suffix={field === 'pct' ? '%' : null} @@ -716,10 +732,7 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se
{valueCol && unitsCol && dollarsOnly && ( - + Date: Mon, 21 Sep 2026 23:08:01 -0400 Subject: [PATCH 13/13] Let a measure be handed back to the others Clicking a held box pins it, and the only way out was to know that emptying the field unpins it -- filled() is false on a blank either way. Clicking into a greyed column to see what is in it and then wanting out again is the ordinary case, not the awkward one. An x on the column header clears that measure's edit, shown only while it has one, and Escape in the box does the same and steps out. The panel's own Escape handler already ignores events from inputs, so the two do not collide. Co-Authored-By: Claude Opus 5 (1M context) --- ui/src/components/OperationPanel.jsx | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/ui/src/components/OperationPanel.jsx b/ui/src/components/OperationPanel.jsx index 110b112..0bfa855 100644 --- a/ui/src/components/OperationPanel.jsx +++ b/ui/src/components/OperationPanel.jsx @@ -289,7 +289,7 @@ function groupDigits(raw) { return `${neg ? '-' : ''}${grouped}${dot === -1 ? '' : `.${frac}`}` } -function LedgerInput({ value, active, held, onChange, onFocus, suffix }) { +function LedgerInput({ value, active, held, onChange, onFocus, onClear, suffix }) { const ref = useRef(null) const caret = useRef(null) const display = groupDigits(value) @@ -328,6 +328,7 @@ function LedgerInput({ value, active, held, onChange, onFocus, suffix }) { onChange(el.value.replace(/,/g, '')) }} onFocus={onFocus} placeholder="—" + onKeyDown={e => { if (e.key === 'Escape' && onClear) { e.preventDefault(); onClear(); e.target.blur() } }} title={held ? 'Held by the Absorbed by setting — type here to set it yourself' : undefined} className={`border rounded py-0.5 text-xs w-24 text-right font-mono tabular-nums transition-colors ${suffix ? 'pl-2 pr-5' : 'px-2'} @@ -517,6 +518,17 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se const setEdit = (key, field, raw) => setScaleInputs(prev => ({ ...prev, [key]: { field, raw } })) + // Hand a measure back to being derived. Emptying the box does the same thing + // -- filled() is false either way -- but that is only obvious once you know + // it, and clicking into a held column to look at it and then wanting out + // again is the ordinary case, not the awkward one. + const clearEdit = (key) => + setScaleInputs(prev => { + if (!(key in prev)) return prev + const { [key]: _drop, ...rest } = prev + return rest + }) + // focusing a different row hands that measure's edit to the focused row, // carrying across whatever it currently reads const focusRow = (key, field) => @@ -577,6 +589,11 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se {m.key === heldKey ? · holds : m.hint && · {m.hint}} + {scaleInputs[m.key] && ( + + )} ))} {showRunning && ( @@ -694,6 +711,7 @@ function ScaleLedger({ currentTotals, scaleInputs, setScaleInputs, scalePlug, se held={m.key === heldKey} onChange={(raw) => setEdit(m.key, field, raw)} onFocus={() => focusRow(m.key, field)} + onClear={() => clearEdit(m.key)} suffix={field === 'pct' ? '%' : null} />