From aa03e74b5e1dd94631d498a51e9aa4974d53588d Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Thu, 17 Sep 2026 23:42:51 -0400 Subject: [PATCH] Prefix the adjustment bucket, and list the hardcoded names Adjustments fell back to a bare 'Forecast' while the loads they adjust read '04 - Forecast', so the bucket column split in two and the adjustments sat apart from the rows they came from. The fallback now matches. The three fallback names are gathered into one DISPLAY DEFAULTS block at the top of sql_generator, exported, and tabulated in CLAUDE.md, so the answer to "where did that name come from" is one place rather than a grep. The incremental row stamps in the operation routes use the constant now instead of restating the literal, which is how they drifted apart in the first place. None of this belongs in the source. ADJUSTMENT_BUCKET carries a number that only suits one convention and changing it changes every version on every source; the note says what per-version would take. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 31 ++++++++++++++++++++++++++++ lib/sql_generator.js | 48 +++++++++++++++++++++++++++++++++++--------- routes/operations.js | 8 ++++---- 3 files changed, 74 insertions(+), 13 deletions(-) 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 {}