|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * An unrecognised `compareTo.kind` is REFUSED, not answered with a |
| 5 | + * previous-period window under a 200. |
| 6 | + * |
| 7 | + * ## What was wrong |
| 8 | + * |
| 9 | + * `shiftRange` had one branch and a fall-through: `previousYear` was named, and |
| 10 | + * EVERYTHING else — including a value the declared type says is impossible — |
| 11 | + * landed in the `previousPeriod` arm. There was no `default` and no |
| 12 | + * exhaustiveness check, so `compareTo: { kind: 'previousQuarter' }` came back as |
| 13 | + * a previous-period comparison under an ordinary **200**. Nothing in the |
| 14 | + * response distinguished it from a real answer, and the wrong answer is a |
| 15 | + * comparison WINDOW — a number a dashboard renders and a person reads as fact. |
| 16 | + * |
| 17 | + * ## Why the declared type did not save it |
| 18 | + * |
| 19 | + * `DatasetCompareTo.kind` is declared as the closed pair |
| 20 | + * `'previousPeriod' | 'previousYear'` (`spec/contracts/analytics-service.ts`), |
| 21 | + * but `DatasetSelection` is a TypeScript interface with **no Zod schema anywhere |
| 22 | + * in the repo**, and `/analytics/dataset/query`'s door parses only the seven |
| 23 | + * members `DatasetSelection` shares with `AnalyticsQuery` — `compareTo` is one of |
| 24 | + * the four it projects away before the parse, and the route forwards the |
| 25 | + * caller's selection to the service untouched. So `kind` is checked by `tsc` |
| 26 | + * inside this repo and by nothing at all on the wire. The dashboard AUTHORING |
| 27 | + * path is doored (`dashboard.zod.ts` parses the widget's `kind` as a |
| 28 | + * `z.enum`), which is why both halves are pinned here: the compile-time one that |
| 29 | + * already held, and the runtime one that did not. |
| 30 | + * |
| 31 | + * ## What is pinned |
| 32 | + * |
| 33 | + * - the refusal itself, in the ADR-0112 envelope the route classifies on |
| 34 | + * (`DATASET_INVALID` / 400) — the fourth member of `resolveCompareDimension`'s |
| 35 | + * family, not a new error vocabulary; |
| 36 | + * - its message discipline: what was received, the two legal values, what to do; |
| 37 | + * - the CONTROL that the refusal did not widen — both declared kinds return |
| 38 | + * exactly the windows they returned before; |
| 39 | + * - that the refusal reaches the executor seam, and reaches it BEFORE the |
| 40 | + * comparison pass is issued; |
| 41 | + * - ⭐ the surface asymmetry that makes ONE refusal site sufficient: |
| 42 | + * `alignedCompareBucketKey` branches on the same two-valued `kind` with the |
| 43 | + * same shape, and needs no refusal of its own because it is not on the |
| 44 | + * package's public surface and its only caller runs `shiftRange` first. That |
| 45 | + * is a claim about `src/index.ts`, so it is pinned against `src/index.ts` |
| 46 | + * rather than asserted in prose — export that function and this file goes red. |
| 47 | + * |
| 48 | + * Every runtime assertion here was run RED first against the pre-fix |
| 49 | + * `shiftRange` (the fall-through returned a window, so each refusal case came |
| 50 | + * back as a successful previous-period answer). |
| 51 | + */ |
| 52 | + |
| 53 | +import { describe, it, expect, vi } from 'vitest'; |
| 54 | +import { DatasetSchema } from '@objectstack/spec/ui'; |
| 55 | +import type { AnalyticsQuery, AnalyticsResult, IAnalyticsService } from '@objectstack/spec/contracts'; |
| 56 | +import { compileDataset } from '../dataset-compiler.js'; |
| 57 | +import { DatasetExecutor, shiftRange } from '../dataset-executor.js'; |
| 58 | + |
| 59 | +/** The ADR-0112 fields `rest-server.ts`'s catch classifies a thrown error on. */ |
| 60 | +interface Refusal extends Error { |
| 61 | + code?: unknown; |
| 62 | + status?: unknown; |
| 63 | +} |
| 64 | + |
| 65 | +/** Run `thunk` and hand back the error it threw, if any. */ |
| 66 | +async function refusalFrom(thunk: () => unknown | Promise<unknown>): Promise<Refusal | undefined> { |
| 67 | + try { |
| 68 | + await thunk(); |
| 69 | + return undefined; |
| 70 | + } catch (e) { |
| 71 | + return e as Refusal; |
| 72 | + } |
| 73 | +} |
| 74 | + |
| 75 | +const WINDOW: [string, string] = ['2026-01-01', '2026-01-31']; |
| 76 | + |
| 77 | +/** The window `previousPeriod` made of {@link WINDOW} before this change, and still does. */ |
| 78 | +const PREVIOUS_PERIOD: [string, string] = ['2025-12-01', '2025-12-31']; |
| 79 | +/** And what `previousYear` made of it. */ |
| 80 | +const PREVIOUS_YEAR: [string, string] = ['2025-01-01', '2025-01-31']; |
| 81 | + |
| 82 | +/** |
| 83 | + * Three spellings an unparsed wire body can carry in `kind`: a plausible fourth |
| 84 | + * window name, the retired string form, and a non-string. The declared type |
| 85 | + * permits none of them; nothing on the wire stops any of them. |
| 86 | + */ |
| 87 | +const UNRECOGNISED: unknown[] = ['previousQuarter', 'previous_period', 7]; |
| 88 | + |
| 89 | +/** Calls the published export the way an unparsed body reaches it. */ |
| 90 | +const shiftWith = (kind: unknown) => shiftRange(WINDOW, kind as never); |
| 91 | + |
| 92 | +/** |
| 93 | + * A trend dataset: `close_date` is a GRID dimension AND bucketed, which is the |
| 94 | + * #6007 shape — the only shape that reaches `alignedCompareBucketKey` at all. |
| 95 | + */ |
| 96 | +const dataset = DatasetSchema.parse({ |
| 97 | + name: 'sales_trend', |
| 98 | + label: 'Sales Trend', |
| 99 | + object: 'opportunity', |
| 100 | + dimensions: [ |
| 101 | + { name: 'close_date', field: 'close_date', type: 'date', label: 'Close Date', dateGranularity: 'month' }, |
| 102 | + ], |
| 103 | + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount', label: 'Revenue' }], |
| 104 | +}); |
| 105 | + |
| 106 | +/** An analytics service that records every query the executor issues. */ |
| 107 | +function recordingService(): { svc: IAnalyticsService; seen: AnalyticsQuery[] } { |
| 108 | + const seen: AnalyticsQuery[] = []; |
| 109 | + const svc: IAnalyticsService = { |
| 110 | + query: vi.fn(async (q: AnalyticsQuery): Promise<AnalyticsResult> => { |
| 111 | + seen.push(q); |
| 112 | + return { rows: [{ close_date: '2026-01', revenue: 100 }], fields: [] }; |
| 113 | + }), |
| 114 | + getMeta: async () => [], |
| 115 | + }; |
| 116 | + return { svc, seen }; |
| 117 | +} |
| 118 | + |
| 119 | +/** The trend selection, with whatever `kind` the case is about. */ |
| 120 | +const selectionWith = (kind: unknown) => ({ |
| 121 | + dimensions: ['close_date'], |
| 122 | + measures: ['revenue'], |
| 123 | + timeDimensions: [{ dimension: 'close_date', dateRange: WINDOW, granularity: 'month' }], |
| 124 | + compareTo: { kind }, |
| 125 | +}); |
| 126 | + |
| 127 | +/** Did the executor ask the service for the SHIFTED window? */ |
| 128 | +const sawShiftedPass = (seen: AnalyticsQuery[]) => |
| 129 | + seen.some((q) => JSON.stringify(q.timeDimensions ?? []).includes('2025-12')); |
| 130 | + |
| 131 | +// ───────────────────────────────────────────────────────────────────────────── |
| 132 | + |
| 133 | +describe('shiftRange refuses an unrecognised compareTo.kind', () => { |
| 134 | + for (const kind of UNRECOGNISED) { |
| 135 | + it(`${JSON.stringify(kind)} → DATASET_INVALID / 400, the envelope the route reads`, async () => { |
| 136 | + const err = await refusalFrom(() => shiftWith(kind)); |
| 137 | + expect(err, 'an unrecognised kind was ANSWERED — the fall-through is back').toBeInstanceOf(Error); |
| 138 | + // Read exactly as `rest-server.ts`'s catch reads them: code + 4xx status. |
| 139 | + expect(err?.code).toBe('DATASET_INVALID'); |
| 140 | + expect(err?.status).toBe(400); |
| 141 | + }); |
| 142 | + } |
| 143 | + |
| 144 | + it('says what it received, what the two legal windows are, and what to do', async () => { |
| 145 | + const err = await refusalFrom(() => shiftWith('previousQuarter')); |
| 146 | + const msg = String(err?.message); |
| 147 | + // ① what arrived — so the caller can find it in the body they sent. |
| 148 | + expect(msg).toContain('"previousQuarter"'); |
| 149 | + // ② the whole legal set, both members, spelled as an author would write them. |
| 150 | + expect(msg).toContain("'previousPeriod'"); |
| 151 | + expect(msg).toContain("'previousYear'"); |
| 152 | + // ③ the fix, and the fact that it IS this key's value that is wrong. |
| 153 | + expect(msg).toContain('compareTo.kind'); |
| 154 | + expect(msg).toContain('drop compareTo'); |
| 155 | + }); |
| 156 | + |
| 157 | + it('CONTROL — both declared kinds still return the windows they always did', () => { |
| 158 | + expect(shiftRange(WINDOW, 'previousPeriod')).toEqual(PREVIOUS_PERIOD); |
| 159 | + expect(shiftRange(WINDOW, 'previousYear')).toEqual(PREVIOUS_YEAR); |
| 160 | + // A window whose length is not a month, so the previousPeriod arithmetic is |
| 161 | + // exercised rather than only the month-aligned happy case. |
| 162 | + expect(shiftRange(['2026-03-10', '2026-03-12'], 'previousPeriod')).toEqual(['2026-03-07', '2026-03-09']); |
| 163 | + }); |
| 164 | + |
| 165 | + it('and `tsc` still owns the compile-time half', () => { |
| 166 | + // @ts-expect-error — 'previousQuarter' is not a `DatasetCompareTo['kind']`. |
| 167 | + // This assertion is about the EXPECTED ERROR, not the throw: the runtime |
| 168 | + // refusal above exists for the wire, where no schema parses `kind` at all. |
| 169 | + expect(() => shiftRange(WINDOW, 'previousQuarter')).toThrow(); |
| 170 | + }); |
| 171 | +}); |
| 172 | + |
| 173 | +describe('the refusal reaches the executor seam, BEFORE the comparison pass', () => { |
| 174 | + it('DatasetExecutor.execute() refuses the selection in the same envelope', async () => { |
| 175 | + const { svc } = recordingService(); |
| 176 | + const err = await refusalFrom(() => |
| 177 | + new DatasetExecutor(svc).execute(compileDataset(dataset), selectionWith('previousQuarter') as never), |
| 178 | + ); |
| 179 | + expect(err).toBeInstanceOf(Error); |
| 180 | + expect(err?.code).toBe('DATASET_INVALID'); |
| 181 | + expect(err?.status).toBe(400); |
| 182 | + }); |
| 183 | + |
| 184 | + it('the SHIFTED pass is never issued — so the second `kind` reader is never reached', async () => { |
| 185 | + const { svc, seen } = recordingService(); |
| 186 | + await refusalFrom(() => |
| 187 | + new DatasetExecutor(svc).execute(compileDataset(dataset), selectionWith('previousQuarter') as never), |
| 188 | + ); |
| 189 | + // `runCompare` reads `kind` twice: into `shiftRange`, and (for this exact |
| 190 | + // grid-and-bucketed shape) into `alignedCompareBucketKey` over the shifted |
| 191 | + // pass's ROWS. The shift is what produces the second call's own argument, so |
| 192 | + // refusing in `shiftRange` means no shifted query, no shifted rows, and |
| 193 | + // nothing for the second reader to mis-align. |
| 194 | + expect(sawShiftedPass(seen), 'the comparison window was queried despite the refusal').toBe(false); |
| 195 | + }); |
| 196 | + |
| 197 | + it('CONTROL — the same selection with a legal kind DOES issue the shifted pass', async () => { |
| 198 | + const { svc, seen } = recordingService(); |
| 199 | + const res = await new DatasetExecutor(svc).execute( |
| 200 | + compileDataset(dataset), |
| 201 | + selectionWith('previousPeriod') as never, |
| 202 | + ); |
| 203 | + expect(sawShiftedPass(seen), 'the control never reached the comparison pass either').toBe(true); |
| 204 | + expect(res.rows.length).toBeGreaterThan(0); |
| 205 | + }); |
| 206 | +}); |
| 207 | + |
| 208 | +describe('one refusal site is enough — the surface asymmetry that makes it so', () => { |
| 209 | + /** |
| 210 | + * `shiftRange` owes a refusal because an external caller can reach it without |
| 211 | + * passing through `runCompare`. `alignedCompareBucketKey` does not, because no |
| 212 | + * external caller can reach it at all: the barrel names its exports one by one |
| 213 | + * (no `export *`) and the manifest maps only `.`, so the function's sole caller |
| 214 | + * is `runCompare`, which runs `shiftRange` first. |
| 215 | + * |
| 216 | + * ⚠️ Adding it to the barrel makes that reasoning false. This case is what |
| 217 | + * turns that into a red test instead of a silent reopening of the defect. |
| 218 | + */ |
| 219 | + it('publishes shiftRange and NOT alignedCompareBucketKey', async () => { |
| 220 | + const barrel = await import('../index.js'); |
| 221 | + const published = Object.keys(barrel); |
| 222 | + // The firing control: the same probe, on the export that IS published. |
| 223 | + expect(published, 'the barrel probe itself is broken').toContain('shiftRange'); |
| 224 | + expect( |
| 225 | + published, |
| 226 | + 'alignedCompareBucketKey is now published — it needs its own `kind` verdict, see its docblock', |
| 227 | + ).not.toContain('alignedCompareBucketKey'); |
| 228 | + }); |
| 229 | +}); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?v=4&size=40){kind=avatar}
0 commit comments