From ee4a60475e33b8c23fe2ab245fe26ff879c5f2ee Mon Sep 17 00:00:00 2001 From: Paul Trowbridge Date: Thu, 17 Sep 2026 02:36:50 -0400 Subject: [PATCH] Type the segment date offset as an interval, not year and month spinners The Date offset on a baseline or reference segment was a pair of type="number" inputs, years and months, both clamped at min 0. So no characters could be typed, days could not be expressed at all, and a segment could only ever be shifted forward in whole months -- while the stored value is a Postgres interval that happily accepts "4 months", "1 year" or "-90 days". parseOffset only read year and month, so anything else would not have survived a round trip through the edit form either. One text field now, with suggestions, matching what clone already offers. parseInterval parses it far enough to draw the timeline preview; Postgres stays the authority, and assertInterval -- now shared by the baseline, reference and clone routes -- rejects what it will not accept, with a message naming the field rather than a parse error from inside a CTE. Timeline takes months and days separately rather than years and months, because the two are not interchangeable: a month shift lands on the same day of another month, a day shift can cross a month boundary. It adds months then days, as Postgres does, and its label handles negatives. Co-Authored-By: Claude Opus 5 (1M context) --- routes/operations.js | 35 +++++++++++------- ui/src/components/Timeline.jsx | 27 ++++++++++---- ui/src/views/Baseline.jsx | 67 ++++++++++++++++++++++------------ 3 files changed, 84 insertions(+), 45 deletions(-) diff --git a/routes/operations.js b/routes/operations.js index a91f6a4..067e5be 100644 --- a/routes/operations.js +++ b/routes/operations.js @@ -48,6 +48,24 @@ module.exports = function(pool) { : [{ slices, where: buildWhereAny(slices, ctx.filterCols) }]; } + // The offset is interpolated into the statement as an interval literal, so a + // typo would surface as a Postgres parse error from the middle of a CTE. Ask + // Postgres to parse it alone first, where the failure is cheap and can name the + // field it came from. Negative intervals are valid and useful -- '-90 days' + // pulls a plan back a quarter -- so this checks validity, not sign. + async function assertInterval(value, res) { + try { + await pool.query(`SELECT $1::interval`, [value]); + return true; + } catch { + res.status(400).json({ + error: `"${value}" is not a valid interval. Try something like ` + + `"4 months", "1 year", "-90 days" or "0 days".` + }); + return false; + } + } + // A slice is only meaningful if at least one of its keys is a filterable // column. buildWhere silently drops unknown keys, so {"typo": "x"} would // otherwise reduce to TRUE and apply the operation to the whole version. @@ -405,6 +423,7 @@ module.exports = function(pool) { const { where_clause, date_offset, note, filters, raw_where } = req.body; const pf_user = sessionUser(req); const dateOffset = date_offset || '0 days'; + if (!await assertInterval(dateOffset, res)) return; const filterClause = (raw_where || where_clause || '').trim() || 'TRUE'; try { const ctx = await getContext(parseInt(req.params.id), 'baseline'); @@ -441,6 +460,7 @@ module.exports = function(pool) { const { where_clause, date_offset, note, filters, raw_where } = req.body; const pf_user = sessionUser(req); const dateOffset = date_offset || '0 days'; + if (!await assertInterval(dateOffset, res)) return; const filterClause = (raw_where || where_clause || '').trim() || 'TRUE'; const client = await pool.connect(); @@ -734,20 +754,7 @@ module.exports = function(pool) { const scaleFactor = (scale != null) ? parseFloat(scale) : 1.0; const dateOffset = (date_offset || '0 days').trim() || '0 days'; - // The offset is interpolated into the SQL as an interval literal, so a - // typo would surface as a Postgres parse error mid-statement. Ask - // Postgres to parse it on its own first, where the failure is cheap and - // can be reported against the field the user typed it into. Negative - // intervals are fine and useful -- '-90 days' pulls a plan back a - // quarter -- so this checks validity, not sign. - try { - await pool.query(`SELECT $1::interval`, [dateOffset]); - } catch { - return res.status(400).json({ - error: `"${dateOffset}" is not a valid interval. Try something like ` - + `"12 months", "-90 days" or "0 days".` - }); - } + if (!await assertInterval(dateOffset, res)) return; // exclude_iters deliberately does not apply here. It exists to stop // operations *modifying* reference rows: scale would attribute forecast diff --git a/ui/src/components/Timeline.jsx b/ui/src/components/Timeline.jsx index 3218e27..a9ab284 100644 --- a/ui/src/components/Timeline.jsx +++ b/ui/src/components/Timeline.jsx @@ -32,11 +32,16 @@ function roundRect(ctx, x, y, w, h, r, fill, stroke) { if (stroke) ctx.stroke() } -export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type = 'baseline' }) { +export default function Timeline({ dateFrom, dateTo, offsetMonths = 0, offsetDays = 0, type = 'baseline' }) { const canvasRef = useRef(null) - const offsetMoTotal = (offsetYr || 0) * 12 + (offsetMo || 0) - const twoBands = type === 'baseline' && offsetMoTotal > 0 + // Months and days are kept apart because they are not interchangeable: a month + // shift lands on the same day of a different month, a day shift can cross a + // month boundary. The preview adds months first, then days, as Postgres does. + const offsetMoTotal = offsetMonths || 0 + const offsetDayTotal = offsetDays || 0 + const shifted = offsetMoTotal !== 0 || offsetDayTotal !== 0 + const twoBands = type === 'baseline' && shifted const canvasH = twoBands ? 90 : 52 useEffect(() => { @@ -61,8 +66,9 @@ export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type = const srcEnd = parseDate(dateTo) if (!srcStart || !srcEnd || isNaN(srcStart) || isNaN(srcEnd)) return - const projStart = addMonths(srcStart, offsetMoTotal) - const projEnd = addMonths(srcEnd, offsetMoTotal) + const addDays = (d, n) => { const x = new Date(d); x.setDate(x.getDate() + n); return x } + const projStart = addDays(addMonths(srcStart, offsetMoTotal), offsetDayTotal) + const projEnd = addDays(addMonths(srcEnd, offsetMoTotal), offsetDayTotal) const winStart = addMonths(srcStart, -1) const winEnd = addMonths(twoBands ? projEnd : srcEnd, 1) @@ -147,7 +153,14 @@ export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type = ctx.lineTo(px1 - 4, arrowY + 4) ctx.closePath() ctx.fill() - const offsetLabel = '+' + (offsetYr ? offsetYr + 'yr ' : '') + (offsetMo ? offsetMo + 'mo' : '') + const yrs = Math.trunc(offsetMoTotal / 12) + const mos = offsetMoTotal % 12 + const sign = (offsetMoTotal + offsetDayTotal) < 0 ? '' : '+' + const offsetLabel = sign + [ + yrs ? `${yrs}yr` : '', + mos ? `${mos}mo` : '', + offsetDayTotal ? `${offsetDayTotal}d` : '', + ].filter(Boolean).join(' ') ctx.fillStyle = '#64748b' ctx.font = '9px system-ui' ctx.textAlign = 'center' @@ -156,7 +169,7 @@ export default function Timeline({ dateFrom, dateTo, offsetYr, offsetMo, type = } raf = requestAnimationFrame(draw) return () => cancelAnimationFrame(raf) - }, [dateFrom, dateTo, offsetYr, offsetMo, type, twoBands, canvasH]) + }, [dateFrom, dateTo, offsetMoTotal, offsetDayTotal, type, twoBands, canvasH]) return } diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index cab5835..08551e8 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -48,11 +48,28 @@ function getDateRange(groups) { return null } -function parseOffset(offsetStr) { - if (!offsetStr || offsetStr === '0 days') return { yr: 0, mo: 0 } - const yr = parseInt(offsetStr.match(/(\d+)\s+year/)?.[1] || 0) - const mo = parseInt(offsetStr.match(/(\d+)\s+month/)?.[1] || 0) - return { yr, mo } +// The offset is stored and sent as a Postgres interval, so it is typed as one -- +// "4 months", "1 year", "-90 days". This only parses it far enough to draw the +// timeline preview; Postgres remains the authority on what is valid, and the +// server rejects anything it will not accept. +// +// It replaced a pair of year/month number spinners, which could not express days +// at all and were clamped at zero, so a segment could only ever be shifted +// forward in whole months. +export function parseInterval(str) { + let months = 0, days = 0 + if (!str) return { months, days } + const re = /([+-]?\d+(?:\.\d+)?)\s*(years?|yrs?|y|months?|mons?|mo|weeks?|wks?|w|days?|d)\b/gi + for (const [, n, unit] of str.matchAll(re)) { + const v = parseFloat(n) + const u = unit.toLowerCase() + if (u.startsWith('y')) months += v * 12 + else if (u.startsWith('mo') || u === 'mons' || u === 'mon') months += v + else if (u.startsWith('w')) days += v * 7 + else if (u.startsWith('d')) days += v + else months += v // 'm' alone reads as months here + } + return { months, days } } function emptyCondition(cols) { @@ -84,8 +101,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio const [filters, setFilters] = useState([]) // [[cond,...], [cond,...]] const [useRaw, setUseRaw] = useState(false) const [rawSql, setRawSql] = useState('') - const [offsetYr, setOffsetYr] = useState(0) - const [offsetMo, setOffsetMo] = useState(0) + const [offset, setOffset] = useState('0 days') const [segNote, setSegNote] = useState('') const [submitting, setSubmitting] = useState(false) const [editingLogId, setEditingLogId] = useState(null) @@ -170,7 +186,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio const clause = useRaw ? rawSql.trim() : buildFilterClause(filters) if (!clause) { flash(useRaw ? 'Enter a WHERE clause' : 'Add at least one filter', 'error'); return } const isRef = segType === 'reference' - const offsetStr = [offsetYr > 0 ? `${offsetYr} year` : '', offsetMo > 0 ? `${offsetMo} month` : ''].filter(Boolean).join(' ') || '0 days' + const offsetStr = offset.trim() || '0 days' const endpoint = isRef ? 'reference' : 'baseline' const body = { where_clause: clause, @@ -212,9 +228,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio setSegType(entry.operation) setSegNote(entry.note || '') setDescription('') - const off = parseOffset(params.date_offset) - setOffsetYr(off.yr) - setOffsetMo(off.mo) + setOffset(params.date_offset || '0 days') const groups = normalizeFilters(params.filters) if (groups) { setUseRaw(false) @@ -478,8 +492,7 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio rawSql={rawSql} setRawSql={setRawSql} description={description} setDescription={setDescription} segNote={segNote} setSegNote={setSegNote} - offsetYr={offsetYr} setOffsetYr={setOffsetYr} - offsetMo={offsetMo} setOffsetMo={setOffsetMo} + offset={offset} setOffset={setOffset} filterCols={filterCols} onSubmit={loadSegment} submitting={submitting} @@ -498,7 +511,6 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio // derive view-mode props for a saved segment function segmentValuesFor(entry, filterCols) { const params = entry.params || {} - const off = parseOffset(params.date_offset) const groups = normalizeFilters(params.filters) return { segType: entry.operation === 'reference' ? 'reference' : 'baseline', @@ -507,8 +519,7 @@ function segmentValuesFor(entry, filterCols) { rawSql: params.where_clause || '', description: '', segNote: entry.note || '', - offsetYr: off.yr, - offsetMo: off.mo, + offset: params.date_offset || '0 days', } } @@ -520,8 +531,7 @@ function SegmentForm({ rawSql, setRawSql, description, setDescription, segNote, setSegNote, - offsetYr, setOffsetYr, - offsetMo, setOffsetMo, + offset, setOffset, filterCols, onSubmit, submitting, @@ -698,10 +708,19 @@ function SegmentForm({
- setOffsetYr(parseInt(e.target.value) || 0)} className={`${baseInp} text-sm w-16 text-center`} /> - yr - setOffsetMo(parseInt(e.target.value) || 0)} className={`${baseInp} text-sm w-16 text-center`} /> - mo + setOffset(e.target.value)} + placeholder="0 days" + className={`${baseInp} text-sm w-32`} /> + + + any Postgres interval
@@ -712,8 +731,8 @@ function SegmentForm({