diff --git a/CLAUDE.md b/CLAUDE.md index 96e4d74..839fcc3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -124,10 +124,12 @@ 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`** — `pf.log.label`, else `tag`, else `note`, else `Unlabeled`; - `'99 - Adjustments'` for an adjustment that has no label of its own +- **`pf_segment`** — `pf.log.label` +- **`pf_bucket`** — `pf.log.bucket` - **`pf_note`** — the free text on a scale/recode/clone; null on loads +Both names are **stored**, not derived — see §Every log entry is named. + They are deliberately separate: commingling a segment name with an adjustment note makes neither pivotable. In grain mode `pf_logid` is part of the grain, so the join adds no rows. The operation routes stamp the same two fields onto the rows they push @@ -146,45 +148,51 @@ typed by whoever names the segment. Nothing derives it. 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 after every *numbered* segment. That proviso is the +The one hardcoded ordinal is `ADJUSTMENT_SEGMENT` = `'99 - Adjustments'`, the +name an adjustment is stamped with when its version sets no default, which keeps +those adjustments after every *numbered* segment. That proviso is the whole scheme, not a caveat on it: ordering is string ordering, so `99` only lands last once the loads carry `01`–`0n`, and an unnumbered segment sorts after it (digits precede letters — `9` is `0x39`, `A` is `0x41`). The old `'(adjustment)'` sorted *first* for the same reason read the other way, `(` being `0x28`. -Unlabelled loads read plain `Unlabeled` and so land at the very end, which is -where a segment nobody has named belongs. Labelling an adjustment's own log row -overrides the fallback, which is how one kind of adjustment is split out from the -rest. +Relabelling an adjustment's own log row afterwards is how one kind of adjustment +is split out from the rest. -### Hardcoded display names +### Every log entry is named -Every name the pivot can show that does not come from `pf.log`. If a segment or -bucket appears under a name nobody typed, it is one of these. All three are in -the `DISPLAY DEFAULTS` block at the top of `lib/sql_generator.js`, exported so -the `/data` cursor and the operation routes' incremental row stamps use the same -values the generated `/agg` does. +`pf.log.label` is a row's segment and `pf.log.bucket` what it counts toward, and +**every entry carries both** — the `log_named` CHECK refuses one without. So a +report gets the pivot's segment and bucket by joining the forecast table to +`pf.log` on `pf_logid` and selecting two columns; there is no rule to copy. -| constant | `pf.version` column | built-in | applies to | -|---|---|---|---| -| `ADJUSTMENT_SEGMENT` | `adjustment_segment` | `99 - Adjustments` | `pf_segment` for a scale/recode/clone with no `label` | -| `ADJUSTMENT_BUCKET` | `adjustment_bucket` | `04 - Forecast` | `pf_bucket` for a scale/recode/clone with no `bucket` | -| `UNLABELED_LOAD` | `unlabeled_load` | `Unlabeled` | `pf_segment` and `pf_bucket` for a load with no `label`, `tag` or `note` | +They are stamped when the entry is written: -Each is set per scenario on the Baseline page, under **Fallback names**; blank -falls back to the built-in. Anything typed on the log row overrides both, so -none of these appears once a segment is named. +- **A load** — `label` is required (the routes answer 400 without one); `bucket` + defaults to the label. `loadNames()` in `routes/operations.js`. +- **An adjustment** — scale, recode and clone have no name inputs, so they are + stamped with the version's `adjustment_segment` / `adjustment_bucket`, else the + built-ins `ADJUSTMENT_SEGMENT` = `99 - Adjustments` and `ADJUSTMENT_BUCKET` = + `04 - Forecast` (`ADJUSTMENT DEFAULTS` in `lib/sql_generator.js`). + `adjustmentNames()` resolves them once per request; they go into the template's + INSERT as `{{label}}` / `{{bucket}}` and onto the rows pushed back to the pivot. -**Why the join rather than a token.** `pf.sql` is keyed on -`(source_id, operation)` — one template shared by every version of a source — so -a value baked in at Generate SQL time could not vary by version, and -regenerating for one version would silently change the others. The names are -therefore read through `VERSION_JOIN` at query time, which also means changing -one takes effect on the next load with nothing regenerated. +The two version defaults are set on the Baseline page under **Defaults for new +adjustments**. Changing one names the *next* adjustment; existing entries keep +what they were written with. Renaming past entries is `PATCH /api/log/:logid`, +one at a time, which renames but refuses to clear. -The built-ins are still a convention guess: `ADJUSTMENT_BUCKET`'s `04 - ` only -suits one numbering. A version that numbers its buckets differently sets its -own rather than inheriting that. +**What this replaced.** The names used to be derived at read time: a load's +`label`, else `tag`, else `note`, else the version's `unlabeled_load`, else +`Unlabeled`; an adjustment's `label`, else the version default, else the +constant — read through a join on `pf.version`. That made the pivot the only +reader that knew what bucket a row was in. The same CASE was copied into +`/agg`, the `/data` cursor and `rlarp.load_osm_pf` in `/opt/forecast`, and any +direct query had to copy it again. It also meant editing a version's fallback +name silently renamed every past adjustment. The 2026-09-24 backfill in +`setup_sql/01_schema.sql` stamped the existing entries with that rule; before it +ran, the pre-backfill `label`/`bucket` values were saved to +`pf.bak_log_names_20260924`. `pf.version.unlabeled_load` is read by nothing now; +the column remains. **What this replaced.** The prefix used to be computed client-side, as Perspective expression columns (`pf_bucket_ord`, `pf_segment_ord`) built from diff --git a/lib/sql_generator.js b/lib/sql_generator.js index e227759..39e812f 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -14,93 +14,58 @@ // What the pivot shows for a row's segment and its bucket. // +// Both are STORED on the row's pf.log entry -- l.label and l.bucket -- and read +// back as plain columns. Every entry carries both: pf.log's log_named CHECK +// refuses one without, so a report can join pf.log and select them with no rule +// of its own. +// +// They used to be derived at read time, through a chain of fallbacks (a load's +// label, else its tag, else its note, else the version's unlabeled_load; an +// adjustment's label, else the version's adjustment_segment, else a constant). +// That made the pivot the only reader that knew what bucket a row was in: the +// same CASE was repeated in /agg, the /data cursor and the decorate proc in +// /opt/forecast, and anything querying pf directly had to copy it again. It also +// meant editing a version's fallback name silently renamed every past adjustment. +// // The ordering prefix is part of the stored text, not computed here. Perspective // orders column groups by the value string, so "01 - Actual" is the only way an // arbitrary order can be expressed -- and l.label is where a person types it. -// Nothing derives it, which is deliberate: an earlier design built the prefix from -// a separate seq column, as Perspective expressions on the client, and the prefix -// then existed only inside the pivot -- so every other reader disagreed with it, -// and a label that could not be expressed in ExprTK's printable-ASCII-per-byte -// string scanner could not be ordered at all. Stored text has neither problem. -// -// The single exception is the adjustment fallback, whose 99 keeps unlabelled -// adjustments last. Labelling an adjustment's log row overrides it, which is how -// one kind of adjustment is split out from the rest -- l.label rather than tag or -// note, so a segment name stays separable from adjustment commentary (pf_note). // // --------------------------------------------------------------------------- -// DISPLAY DEFAULTS -- every hardcoded name the pivot can show. +// ADJUSTMENT DEFAULTS -- what a scale/recode/clone is stamped with. // -// These are the values a row falls back to when nobody has named it. They are -// the complete list: if a segment or bucket appears in the pivot under a name -// that is not in pf.log, it came from here. CLAUDE.md has the same list under -// "Hardcoded display names". +// An adjustment has no label or bucket input, so the route stamps these onto its +// log entry when it is written: the version's own adjustment_segment / +// adjustment_bucket when set, else the constants below. Resolved once, at write +// time -- changing a version's default afterwards names new adjustments, not old +// ones. Exported so the operation routes stamp the rows they push back to the +// pivot with the same values they wrote. // -// They live on pf.version -- adjustment_segment, adjustment_bucket, -// unlabeled_load -- and the constants below are only the fallback for a version -// that has not set one. Read through a join at query time rather than -// substituted at generation: pf.sql templates are keyed on (source_id, -// operation) and shared by every version of a source, so a value baked in could -// not vary by version and regenerating for one would change the others. -// -// Exported because /data builds its own statement in routes/operations.js while -// /agg is generated here, and the two have to agree. +// A load has no default: its label is required, and its bucket is its label +// unless one is given (the routes enforce both). // --------------------------------------------------------------------------- // An adjustment with no label of its own. The 99 keeps it after every numbered // segment -- ordering is string ordering, so this only works while the loads -// carry 01-0n. Labelling an adjustment's log row overrides it, which is how one -// kind of adjustment is split out from the rest. +// carry 01-0n. const ADJUSTMENT_SEGMENT = '99 - Adjustments'; -// What an adjustment counts toward. Prefixed to match the segments it adjusts: -// unprefixed it read 'Forecast' while the loads read '04 - Forecast', and the -// column split in two -- the adjustments sitting apart from the rows they -// adjust. The number is a guess at the convention in use, which is the clearest -// argument for making this per-version. +// What an adjustment counts toward. Prefixed to match the segments it adjusts, +// so the adjustments sit in the same column as the rows they adjust. const ADJUSTMENT_BUCKET = '04 - Forecast'; -// A load nobody named. No prefix, so it sorts after everything numbered -- -// letters follow digits in ASCII. The old '(unlabeled load)' sorted *first*, -// since '(' is 0x28 and digits begin at 0x30. -const UNLABELED_LOAD = 'Unlabeled'; - -const LOAD_SEGMENT = `COALESCE(NULLIF(l.label, ''), NULLIF(l.tag, ''), NULLIF(l.note, ''), - NULLIF(v.unlabeled_load, ''), '${UNLABELED_LOAD}')`; - -const SEGMENT_EXPR = `CASE WHEN l.operation IN ('baseline','reference') - THEN ${LOAD_SEGMENT} - ELSE COALESCE(NULLIF(l.label, ''), NULLIF(v.adjustment_segment, ''), '${ADJUSTMENT_SEGMENT}') - END`; - -// What the row counts towards. A load falls back to its own name until it is -// bucketed; an adjustment falls back to the forecast bucket, because that is -// what an adjustment is -- exclude_iters keeps operations off the reference -// segments, so there is no adjustment that is not part of the forecast. -const BUCKET_EXPR = `COALESCE(NULLIF(l.bucket, ''), - CASE WHEN l.operation IN ('baseline','reference') - THEN ${LOAD_SEGMENT} - ELSE COALESCE(NULLIF(v.adjustment_bucket, ''), '${ADJUSTMENT_BUCKET}') - END)`; +const SEGMENT_EXPR = `l.label`; +const BUCKET_EXPR = `l.bucket`; const NOTE_EXPR = `CASE WHEN l.operation IN ('baseline','reference') THEN NULL ELSE COALESCE(NULLIF(l.tag, ''), NULLIF(l.note, '')) END`; -// Every pf.log column the two expressions above read, for /agg's GROUP BY: they +// Every pf.log column the expressions above read, for /agg's GROUP BY: they // are functionally dependent on pf_logid, which is in the grain, but Postgres // will not infer that. -const LABEL_GROUP_COLS = ['l.operation', 'l.label', 'l.tag', 'l.note', 'l.bucket', - 'v.adjustment_segment', 'v.adjustment_bucket', 'v.unlabeled_load']; - -// The version carries the fallback names, so every statement that reads the -// expressions above needs it in scope as `v`. LEFT, not inner: a forecast row -// whose log entry somehow has no version should still come back, named by the -// constants. -const VERSION_JOIN = ` -LEFT JOIN pf.version v - ON v.id = l.version_id`; +const LABEL_GROUP_COLS = ['l.operation', 'l.label', 'l.tag', 'l.note', 'l.bucket']; // wrap a column name in double quotes for safe use in SQL function q(name) { return `"${name}"`; } @@ -319,7 +284,7 @@ SELECT ,l.operation AS pf_op FROM {{fc_table}} t LEFT JOIN pf.log l - ON l.id = t.pf_logid${VERSION_JOIN} + ON l.id = t.pf_logid WHERE {{territory_clause}} GROUP BY ${grain.groupCols('t.').join('\n ,')} @@ -431,8 +396,9 @@ SELECT count(*) AS rows_affected, (SELECT id FROM ilog) AS log_id FROM ins`.trim return ` WITH ilog AS ( - INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note) - VALUES ({{version_id}}, '{{pf_user}}', 'scale', '{{slice}}'::jsonb, '{{params}}'::jsonb, '{{note}}') + INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note, label, bucket) + VALUES ({{version_id}}, '{{pf_user}}', 'scale', '{{slice}}'::jsonb, '{{params}}'::jsonb, '{{note}}', + NULLIF('{{label}}', ''), NULLIF('{{bucket}}', '')) RETURNING id ) ,base AS ( @@ -459,8 +425,9 @@ ${opTail('ins')}`.trim(); return ` WITH ilog AS ( - INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note) - VALUES ({{version_id}}, '{{pf_user}}', 'recode', '{{slice}}'::jsonb, '{{params}}'::jsonb, '{{note}}') + INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note, label, bucket) + VALUES ({{version_id}}, '{{pf_user}}', 'recode', '{{slice}}'::jsonb, '{{params}}'::jsonb, '{{note}}', + NULLIF('{{label}}', ''), NULLIF('{{bucket}}', '')) RETURNING id ) ,src AS ( @@ -509,8 +476,9 @@ ${opTail('allrows')}` : 'SELECT * FROM neg UNION ALL SELECT * FROM ins'}`.trim() return ` WITH ilog AS ( - INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note) - VALUES ({{version_id}}, '{{pf_user}}', 'clone', '{{slice}}'::jsonb, '{{params}}'::jsonb, '{{note}}') + INSERT INTO pf.log (version_id, pf_user, operation, slice, params, note, label, bucket) + VALUES ({{version_id}}, '{{pf_user}}', 'clone', '{{slice}}'::jsonb, '{{params}}'::jsonb, '{{note}}', + NULLIF('{{label}}', ''), NULLIF('{{bucket}}', '')) RETURNING id ) ,ins AS ( @@ -557,8 +525,8 @@ function applyTokens(sql, tokens) { // build a SQL WHERE clause string from a slice object // only dimension columns are included; unrecognised keys are silently skipped // pf_segment and pf_bucket are not columns on the forecast table -- they are -// computed at read time from the row's pf.log entry -- so a slice naming one -// cannot be compared directly. It resolves to a set of log ids instead, which is +// stored on the row's pf.log entry -- so a slice naming one cannot be compared +// directly. It resolves to a set of log ids instead, which is // exact: the name lives on the log row, and every forecast row carries the // pf_logid that points at it. // @@ -576,7 +544,7 @@ function computedSlicePredicate(col, val, versionId) { const vals = (Array.isArray(val) ? val : [val]).map(v => `'${esc(v)}'`).join(', '); return `pf_logid IN ( SELECT l.id - FROM pf.log l${VERSION_JOIN} + FROM pf.log l WHERE l.version_id = ${parseInt(versionId)} AND ${COMPUTED_SLICE_COLS[col]} IN (${vals}) )`; @@ -843,5 +811,5 @@ function esc(val) { } module.exports = { generateSQL, grainOf, COMPUTED_SLICE_COLS, buildScopeClause, buildTerritoryClause, - SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, VERSION_JOIN, - ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, UNLABELED_LOAD, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; + SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, + ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; diff --git a/routes/log.js b/routes/log.js index bd657c5..d50c479 100644 --- a/routes/log.js +++ b/routes/log.js @@ -217,6 +217,14 @@ module.exports = function(pool) { error: 'Nothing to update — send note, tag, bucket and/or label' }); } + // Every entry carries a label and a bucket (pf.log's log_named CHECK), so + // they can be renamed but not cleared. Refused here for the message; the + // constraint would refuse it anyway. + for (const [field, value] of [['label', label], ['bucket', bucket]]) { + if (value !== undefined && !String(value ?? '').trim()) { + return res.status(400).json({ error: `The ${field} can be renamed but not cleared` }); + } + } try { // Same rule as undo: your own entries, or an admin's. These are // annotations, but label and bucket name the pivot's columns for diff --git a/routes/operations.js b/routes/operations.js index 13ffe9f..826da16 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -1,7 +1,7 @@ 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, - SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, VERSION_JOIN, + SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator'); const { sessionUser, sessionTerritory, requireAdmin } = require('../lib/auth'); const { fcTable } = require('../lib/utils'); @@ -9,10 +9,11 @@ const { fcTable } = require('../lib/utils'); module.exports = function(pool) { const router = express.Router(); - // the names an adjustment's rows carry until the next load re-reads them - // through VERSION_JOIN. The per-version overrides live on pf.version, so - // stamping the built-in constants here made every fresh adjustment show as - // '99 - Adjustments' and jump to the version's own name on refresh. + // The segment and bucket an adjustment is stamped with: written onto its + // pf.log entry by the template, and onto the rows pushed back to the pivot, + // so the two agree. The version's own defaults when set, else the + // built-ins. Resolved at write time, so changing a version's default + // afterwards names new adjustments and leaves existing ones as they are. function adjustmentNames(version) { const pick = (v, dflt) => (v || '').trim() || dflt; return { @@ -21,6 +22,21 @@ module.exports = function(pool) { }; } + // The segment and bucket a load is stamped with. A load's label is its + // segment name and is required -- it used to fall back through tag, note and + // the version's unlabeled_load at read time, which left pf.log unable to say + // what a row was called. The bucket is the label unless one is given, which + // is how an unbucketed load has always read. Sends the 400 and returns null + // when there is no label. + function loadNames(label, bucket, res) { + const segment = String(label ?? '').trim(); + if (!segment) { + res.status(400).json({ error: 'Give the segment a label — it is what the pivot shows and what reports group on' }); + return null; + } + return { label: segment, bucket: String(bucket ?? '').trim() || segment }; + } + async function runSQL(sql, client) { console.log('--- SQL ---\n', sql, '\n--- END SQL ---'); return (client || pool).query(sql); @@ -486,7 +502,7 @@ module.exports = function(pool) { ,${NOTE_EXPR} AS pf_note FROM ${tbl} t LEFT JOIN pf.log l - ON l.id = t.pf_logid${VERSION_JOIN} + ON l.id = t.pf_logid ${terrAlias ? `WHERE ${terrAlias}` : ''} `); @@ -590,6 +606,8 @@ module.exports = function(pool) { const dateOffset = date_offset || '0 days'; if (!await assertInterval(dateOffset, res)) return; const filterClause = (raw_where || where_clause || '').trim() || 'TRUE'; + const names = loadNames(label, bucket, res); + if (!names) return; try { const ctx = await getContext(parseInt(req.params.id), 'baseline'); if (!guardOpen(ctx.version, res)) return; @@ -603,8 +621,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), - label: esc(label || ''), - bucket: esc(bucket || ''), + label: esc(names.label), + bucket: esc(names.bucket), tag: esc(tag || ''), params: esc(paramsJson), filter_clause: filterClause, @@ -672,13 +690,15 @@ module.exports = function(pool) { // so tag is always the latter and must survive an edit made for any // other reason. const keep = (sent, prior) => esc(sent ?? prior ?? ''); + const names = loadNames(label ?? oldLog.label, bucket ?? oldLog.bucket, res); + if (!names) return; const sql = applyTokens(ctx.sql, { fc_table: ctx.table, version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), - label: keep(label, oldLog.label), - bucket: keep(bucket, oldLog.bucket), + label: esc(names.label), + bucket: esc(names.bucket), tag: keep(tag, oldLog.tag), params: esc(paramsJson), filter_clause: filterClause, @@ -749,6 +769,8 @@ module.exports = function(pool) { const pf_user = sessionUser(req); const dateOffset = date_offset || '0 days'; const filterClause = (raw_where || where_clause || '').trim() || 'TRUE'; + const names = loadNames(label, bucket, res); + if (!names) return; try { const ctx = await getContext(parseInt(req.params.id), 'reference'); if (!guardOpen(ctx.version, res)) return; @@ -762,8 +784,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), - label: esc(label || ''), - bucket: esc(bucket || ''), + label: esc(names.label), + bucket: esc(names.bucket), tag: esc(tag || ''), params: esc(paramsJson), filter_clause: filterClause, @@ -803,6 +825,7 @@ module.exports = function(pool) { // reaches the target on its own and gets its own log entry. const units = sliceUnits(slices, ctx, applyMode, req.body.scope, req); + const names = adjustmentNames(ctx.version); const client = await pool.connect(); let committed = false; try { @@ -825,6 +848,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), + label: esc(names.pf_segment), + bucket: esc(names.pf_bucket), params: esc(JSON.stringify({ slices: unit.slices, apply_mode: applyMode, @@ -862,7 +887,6 @@ module.exports = function(pool) { }); } const opLabel = (req.body.tag || '').trim() || note || null; - const names = adjustmentNames(ctx.version); const rows = allRows.map(r => ({ ...r, ...names, pf_note: opLabel, pf_op: 'scale' })); res.json({ rows, @@ -899,6 +923,7 @@ module.exports = function(pool) { const setClause = buildSetClause(ctx.dimCols, set); const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope, req); + const names = adjustmentNames(ctx.version); const client = await pool.connect(); let committed = false; try { @@ -913,6 +938,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), + label: esc(names.pf_segment), + bucket: esc(names.pf_bucket), params: esc(JSON.stringify({ slices: unit.slices, set, apply_mode: unit.mode })), slice: esc(JSON.stringify(loggedSlice)), where_clause: unit.where, @@ -936,7 +963,6 @@ module.exports = function(pool) { }); } const opLabel = (req.body.tag || '').trim() || note || null; - const names = adjustmentNames(ctx.version); const rows = allRows.map(r => ({ ...r, ...names, pf_note: opLabel, pf_op: 'recode' })); res.json({ rows, rows_affected: rows.length, slices_applied: units.length }); } finally { @@ -1001,6 +1027,7 @@ module.exports = function(pool) { const setClause = buildSetClause(ctx.dimCols, set, { derivedExprs, alias: 's' }); const units = sliceUnits(slices, ctx, apply_mode === 'each' ? 'each' : 'prorate', req.body.scope, req); + const names = adjustmentNames(ctx.version); const client = await pool.connect(); let committed = false; try { @@ -1015,6 +1042,8 @@ module.exports = function(pool) { version_id: ctx.version.id, pf_user: esc(pf_user || ''), note: esc(note || ''), + label: esc(names.pf_segment), + bucket: esc(names.pf_bucket), params: esc(JSON.stringify({ slices: unit.slices, set, scale: scaleFactor, apply_mode: unit.mode, date_offset: dateOffset, @@ -1044,7 +1073,6 @@ module.exports = function(pool) { }); } const opLabel = (req.body.tag || '').trim() || note || null; - const names = adjustmentNames(ctx.version); const rows = allRows.map(r => ({ ...r, ...names, pf_note: opLabel, pf_op: 'clone' })); res.json({ rows, rows_affected: rows.length, slices_applied: units.length }); } finally { diff --git a/setup_sql/01_schema.sql b/setup_sql/01_schema.sql index 611b3b9..fc4b1e8 100644 --- a/setup_sql/01_schema.sql +++ b/setup_sql/01_schema.sql @@ -122,6 +122,54 @@ ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS adjustment_segment text; ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS adjustment_bucket text; ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS unlabeled_load text; +-- Every log entry is named. label is the row's segment and bucket what it counts +-- toward, both stored rather than derived, so a report reads them off pf.log with +-- no rule of its own. The routes stamp them on write: a load's label is required +-- and its bucket defaults to the label; an adjustment gets the version's +-- adjustment_segment / adjustment_bucket, else the built-ins in sql_generator. +-- +-- The backfill names entries written before that, with the rule the pivot used to +-- apply at read time, so nothing already on screen changes name. It is the last +-- reader of pf.version.unlabeled_load; the column remains. +WITH +-- the read-time rule, verbatim +named AS ( + SELECT + l.id + ,CASE + WHEN l.operation IN ('baseline', 'reference') + THEN COALESCE(NULLIF(l.label, ''), NULLIF(l.tag, ''), NULLIF(l.note, ''), NULLIF(v.unlabeled_load, ''), 'Unlabeled') + ELSE COALESCE(NULLIF(l.label, ''), NULLIF(v.adjustment_segment, ''), '99 - Adjustments') + END AS label + ,l.operation + ,l.bucket + ,v.adjustment_bucket + FROM + pf.log l + LEFT JOIN pf.version v + ON v.id = l.version_id + WHERE TRUE + AND (NULLIF(l.label, '') IS NULL OR NULLIF(l.bucket, '') IS NULL) +) +UPDATE pf.log l +SET + label = n.label + ,bucket = COALESCE( + NULLIF(n.bucket, '') + ,CASE + WHEN n.operation IN ('baseline', 'reference') THEN n.label + ELSE COALESCE(NULLIF(n.adjustment_bucket, ''), '04 - Forecast') + END + ) +FROM + named n +WHERE TRUE + AND l.id = n.id; + +ALTER TABLE pf.log DROP CONSTRAINT IF EXISTS log_named; +ALTER TABLE pf.log ADD CONSTRAINT log_named + CHECK (NULLIF(label, '') IS NOT NULL AND NULLIF(bucket, '') IS NOT NULL); + -- Where an account lands when it has no saved selection of its own. -- -- Without it the app fell back to the newest version, which is whatever diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index 4677e2c..149be5c 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -176,6 +176,12 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio async function saveLogField(entry, field, value) { const next = value.trim() if (next === (entry[field] || '')) return + // every log entry carries both; the server refuses a blank one + if (!next) { + flash(`A segment's ${field} can be renamed but not cleared`, 'error') + setBuckets(b => { const { [entry.id]: _, ...rest } = b; return rest }) + return + } try { const res = await fetch(`/api/log/${entry.id}`, { method: 'PATCH', @@ -184,7 +190,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio }) if (!res.ok) { const d = await res.json(); flash(d.error, 'error'); return } loadLog() - flash(next ? `Saved — reload the Forecast view to see it` : 'Cleared') + flash(`Saved — reload the Forecast view to see it`) } catch (err) { flash(err.message, 'error') } } @@ -421,6 +427,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio async function loadSegment() { const clause = useRaw ? rawSql.trim() : buildFilterClause(filters) if (!clause) { flash(useRaw ? 'Enter a WHERE clause' : 'Add at least one filter', 'error'); return } + if (!segLabel.trim()) { flash('Give the segment a label', 'error'); return } const isRef = segType === 'reference' const offsetStr = offset.trim() || '0 days' const endpoint = isRef ? 'reference' : 'baseline' @@ -676,17 +683,19 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio )} - {/* 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 - DISPLAY DEFAULTS block. */} + {/* Adjustment defaults. A scale, recode or clone has no label or bucket + input, so it is stamped with these when it is written. Written onto + the log entry, not read back through it: changing one names the next + adjustment and leaves the existing ones as they are. Blank falls back + to the built-in in sql_generator's ADJUSTMENT DEFAULTS block. */} {versionId && (
- Fallback names + + Defaults for new adjustments — existing adjustments keep the names they were written with + {[ ['adjustment_segment', 'Adjustment segment', '99 - Adjustments'], ['adjustment_bucket', 'Adjustment bucket', '04 - Forecast'], - ['unlabeled_load', 'Unlabeled load', 'Unlabeled'], ].map(([field, label, builtin]) => (
@@ -961,10 +970,14 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio saveLogField(entry, 'label', e.target.value)} + onBlur={e => { + const typed = e.target.value + if (!typed.trim()) e.target.value = entry.label || '' + saveLogField(entry, 'label', typed) + }} readOnly={!canEdit(entry)} title={canEdit(entry) ? '' : `${entry.pf_user || 'Another account'} made this segment`} - placeholder={entry.tag || entry.note || '—'} + 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" /> @@ -1320,12 +1333,12 @@ function SegmentForm({
setSegLabel(e.target.value)} - placeholder="defaults to the note" className={`${baseInp} text-sm py-1.5`} /> + placeholder="required" className={`${baseInp} text-sm py-1.5`} />
setSegBucket(e.target.value)} - list="pf-bucket-options" placeholder="e.g. 02 - Forecast" + list="pf-bucket-options" placeholder="defaults to the label" className={`${baseInp} text-sm py-1.5`} />