diff --git a/CLAUDE.md b/CLAUDE.md index 846e2e3..474fc3d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -150,6 +150,37 @@ 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. +### Hardcoded display names + +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. + +| constant | value | applies to | +|---|---|---| +| `ADJUSTMENT_SEGMENT` | `99 - Adjustments` | `pf_segment` for a scale/recode/clone with no `label` | +| `ADJUSTMENT_BUCKET` | `04 - Forecast` | `pf_bucket` for a scale/recode/clone with no `bucket` | +| `UNLABELED_LOAD` | `Unlabeled` | `pf_segment` and `pf_bucket` for a load with no `label`, `tag` or `note` | + +Anything typed on the log row overrides its fallback, so none of these appears +once a segment is named. + +**They should be per-version and are not.** `ADJUSTMENT_BUCKET` in particular +carries a `04 - ` that only suits one naming convention: unprefixed it read +`Forecast` while the loads read `04 - Forecast`, which split the column in two +and sat the adjustments apart from the rows they adjust. Changing it changes +every version on every source. + +The fix is two columns on `pf.version` — it already holds per-scenario config in +`exclude_iters` — read through a join in `/data` and `/agg` rather than +substituted at generation time. That last part is the constraint worth +remembering: `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 cannot +vary by version, and regenerating for one version would silently change the +others. + **What this replaced.** The prefix used to be computed client-side, as Perspective expression columns (`pf_bucket_ord`, `pf_segment_ord`) built from `pf.log.seq` and `pf.version.bucket_order`. It ordered the pivot and nothing diff --git a/lib/sql_generator.js b/lib/sql_generator.js index efc9b96..44882bd 100644 --- a/lib/sql_generator.js +++ b/lib/sql_generator.js @@ -28,14 +28,43 @@ // 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. +// +// 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". +// +// They belong on pf.version, so a scenario can name its own. Until then they +// are global, and changing one changes it for every version on every source -- +// which is why ADJUSTMENT_BUCKET carries a prefix that only suits the current +// naming convention. Read at query time, not baked in: pf.sql templates are +// per source, so a per-version value cannot be substituted at generation. +// // Exported because /data builds its own statement in routes/operations.js while // /agg is generated here, and the two have to agree. +// --------------------------------------------------------------------------- + +// 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. const ADJUSTMENT_SEGMENT = '99 - Adjustments'; -// An unlabelled load reads 'Unlabeled' and carries 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 LOAD_SEGMENT = `COALESCE(NULLIF(l.label, ''), NULLIF(l.tag, ''), NULLIF(l.note, ''), 'Unlabeled')`; +// 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. +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, ''), '${UNLABELED_LOAD}')`; const SEGMENT_EXPR = `CASE WHEN l.operation IN ('baseline','reference') THEN ${LOAD_SEGMENT} @@ -43,13 +72,13 @@ const SEGMENT_EXPR = `CASE WHEN l.operation IN ('baseline','reference') END`; // What the row counts towards. A load falls back to its own name until it is -// bucketed; an adjustment falls back to 'Forecast', 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. +// 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 'Forecast' + ELSE '${ADJUSTMENT_BUCKET}' END)`; const NOTE_EXPR = `CASE WHEN l.operation IN ('baseline','reference') @@ -588,4 +617,5 @@ function esc(val) { } module.exports = { generateSQL, grainOf, - SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, ADJUSTMENT_SEGMENT, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; + SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, LABEL_GROUP_COLS, + ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET, UNLABELED_LOAD, dateGroupsOf, dimPeriodMapOf, dimPeriodJoins, applyTokens, buildWhere, buildWhereAny, buildExcludeClause, buildExcludePredicate, buildSetClause, buildFilterClause, esc }; diff --git a/routes/operations.js b/routes/operations.js index 47f11f0..166e07e 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, buildExcludeClause, buildExcludePredicate, buildSetClause, dateGroupsOf, dimPeriodMapOf, esc, - SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, ADJUSTMENT_SEGMENT } = require('../lib/sql_generator'); + SEGMENT_EXPR, BUCKET_EXPR, NOTE_EXPR, ADJUSTMENT_SEGMENT, ADJUSTMENT_BUCKET } = require('../lib/sql_generator'); const { sessionUser } = require('../lib/auth'); const { fcTable } = require('../lib/utils'); @@ -671,7 +671,7 @@ module.exports = function(pool) { await client.query('COMMIT'); committed = true; const opLabel = (req.body.tag || '').trim() || note || null; - const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'scale' })); + const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: ADJUSTMENT_BUCKET, pf_note: opLabel, pf_op: 'scale' })); res.json({ rows, rows_affected: rows.length, @@ -731,7 +731,7 @@ module.exports = function(pool) { await client.query('COMMIT'); committed = true; const opLabel = (req.body.tag || '').trim() || note || null; - const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'recode' })); + const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: ADJUSTMENT_BUCKET, pf_note: opLabel, pf_op: 'recode' })); res.json({ rows, rows_affected: rows.length, slices_applied: units.length }); } finally { if (!committed) try { await client.query('ROLLBACK'); } catch {} @@ -826,7 +826,7 @@ module.exports = function(pool) { await client.query('COMMIT'); committed = true; const opLabel = (req.body.tag || '').trim() || note || null; - const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: 'Forecast', pf_note: opLabel, pf_op: 'clone' })); + const rows = allRows.map(r => ({ ...r, pf_segment: ADJUSTMENT_SEGMENT, pf_bucket: ADJUSTMENT_BUCKET, pf_note: opLabel, pf_op: 'clone' })); res.json({ rows, rows_affected: rows.length, slices_applied: units.length }); } finally { if (!committed) try { await client.query('ROLLBACK'); } catch {}