feat/metrics-agg

Off-by-one: depth guard allows MAX_DERIVATION_DEPTH + 1 nesting levels at read time

domains/metrics/src/metric/metric.queries.ts:211

`ctx.depth > MAX_DERIVATION_DEPTH` fires only at depth 11 (with MAX=10), so depth 10 is accepted — producing 11 levels (depths 0–10) rather than the intended 10. The write-time guard uses the same expression so both are consistently one level more permissive than the constant name implies. Change to `ctx.depth >= MAX_DERIVATION_DEPTH` to enforce exactly MAX_DERIVATION_DEPTH levels.

Unary minus/plus in parseFactor recurses without incrementing depth, bypassing MAX_FORMULA_NESTING guard

domains/metrics/src/derivation/derivation.decisions.ts:310

Both the unary-minus and unary-plus branches call `parseFactor(depth)` — not `parseFactor(depth + 1)`. MAX_FORMULA_NESTING=32 is only incremented in the parenthesis branch. A formula of 512 consecutive `-` characters (within the 1024-char limit) creates 512 recursive frames all at depth=0. V8's stack limit prevents a crash in practice, but the guard offers no protection. Should be `parseFactor(depth + 1)` on both unary branches.

Leading-dot number literal (.5) accepted at read time but grammar documents integer-required prefix

domains/metrics/src/derivation/derivation.decisions.ts:208

The tokenizer accepts `.5` (no leading digit) since the digit branch checks `c === '.'`. The grammar comment documents `NUMBER := [0-9]+ ('.' [0-9]+)?` (leading digit required). A future write-time validator that strictly follows the documented grammar would reject formulas with `.5` that the read evaluator already stored as valid. Document the accepted extension or align the grammar comment.

Handler switch on ResolveMetricStreamWindowError lacks a `never`-typed default for static exhaustiveness

apps/platform/src/api/handlers/metric-stream-window.handler.ts:94

The switch covers all 8 current `_tag` values correctly. However, without a `default: { const _e: never = result.error; serverError('unhandled error') }` branch, TypeScript cannot enforce exhaustiveness — a future error variant added to the union will compile silently without a handler. Consider adding the never-typed default to make this statically safe.

Input variable count amplifies Tinybird fetches — no cap on number of distinct formula variables

domains/metrics/src/metric/metric.queries.ts:227

resolveDerivationWindow resolves inputs sequentially, one Tinybird pipe call per input variable. With MAX_FORMULA_LENGTH=1024, a formula like `a+b+c+...` with single-char names can reference up to ~512 distinct variables. Any authenticated user who can trigger a window read on a derived stream (via admin-created source_config) can force 512 serial Tinybird reads per API call. Consider adding a per-formula variable count cap (e.g. 20) enforced in parseDerivationConfig.

Formula string and stream public-IDs surfaced in HTTP 500 body

apps/platform/src/api/handlers/metric-stream-window.handler.ts:111

All derivation error messages are propagated verbatim to serverError(...), including the formula (up to 1024 chars) for MalformedFormula, and stream public-IDs for InputNotFound. RLS ensures the caller owns the stream, so this is not a cross-org leak. But surfacing source_config internals in HTTP 500 responses is more disclosure than necessary — consider a generic 500 message and log the detail server-side.

No eval / SQL injection risk — formula evaluates to AST only

domains/metrics/src/derivation/derivation.decisions.ts

The formula string is parsed into an in-memory AST via the recursive-descent parser. No eval(), new Function(), vm.runInContext(), or string interpolation into SQL on this path. The grammar restricts tokens to +/-/*//, parens, numeric literals, and identifier names. Confirmed clean.

Cycle detection uses correct immutable-copy semantics per recursive branch

domains/metrics/src/metric/metric.queries.ts

childCtx.ancestors is created as `new Set([...ctx.ancestors, self])` — a fresh copy per recursion level. Sibling derivation branches do not share ancestor state, preventing false-positive cycle detection. Self-referential and multi-hop cycles are both detected correctly.

NaN/Infinity correctly filtered before series output

domains/metrics/src/derivation/derivation.decisions.ts

Runtime non-finite evaluation results (e.g. variable/0 at a slot where denominator is zero at runtime) are dropped by `Number.isFinite(value)` before being pushed to the output series. Static literal div-by-zero (formula: 'a / 0') is caught at parse time by hasLiteralDivByZero. Neither NaN nor Infinity can propagate into billing-grade series output.

Decision function names don't follow `decide{Operation}` convention

domains/metrics/src/derivation/derivation.decisions.ts:89

The three exported functions are `parseDerivationConfig`, `parseFormula`, and `evaluateDerivation`. The canonical form prescribes `decide{Operation}` for functions in `.decisions.ts`. These are parser/evaluator utilities rather than classic domain decision gates, so the deviation may be intentional — but it creates an inconsistency and breaks the pattern other FCIS modules rely on for discoverability.

Error mapping moved from mapper to handler, violating the canonical api-patterns split

apps/platform/src/api/handlers/metric-stream-window.handler.ts:94

The previous `mapResolveMetricStreamWindowError` in the mapper performed the exhaustive `_tag` → HTTP response translation. This commit deletes that function and inlines the switch in the handler. The canonical pattern (handlers → mappers for error translation) keeps handlers focused on routing and mappers on translation. The inline switch makes the handler harder to test in isolation and means the new derivation error tags have no mapper coverage.

`resolveWindowWithReader` exported from `.queries.ts` violates the thin-wrapper contract

domains/metrics/src/metric/metric.queries.ts:126

`.queries.ts` is defined as 'thin DB wrappers returning Entity | null'. `resolveWindowWithReader` is a recursive async coordinator that routes between physical and derivation paths, manages depth/cycle context, and returns `Result<SeriesPoint[], ...>`. It is a coordinator, not a query. Exporting it here (even for testability) blurs the layer and will mislead future contributors about what belongs in `.queries.ts`.

Coordinator logic (`resolveDerivationWindow`) lives in `.queries.ts`

domains/metrics/src/metric/metric.queries.ts:198

`resolveDerivationWindow` orchestrates: validates config, guards depth/cycle, recursively resolves N input streams, calls the pure evaluator. This is coordinator/shell logic, not a thin DB wrapper. Its placement in `.queries.ts` means the file now has two distinct responsibilities: PG adapter AND recursive orchestration. A dedicated `metric.coordinator.ts` (or renaming the file) would restore the canonical contract.

Derivation errors carry `statusCode: 422` but map to HTTP 500 — documented mismatch

domains/metrics/src/derivation/derivation.errors.ts

DerivationMalformedFormula, DerivationInputNotFound, and DerivationMalformedConfig all have statusCode:422, but the handler maps them to serverError (HTTP 500). The handler comment acknowledges this as intentional until formula authoring is user-facing. The mismatch is low-risk now but should be tracked as a cleanup item to prevent future contributors from assuming the statusCode field is authoritative for HTTP response selection.

`DerivationFCIS` namespace wraps a pure utility with no entity lifecycle

domains/metrics/src/index.ts:27

The *FCIS convention signals a domain entity module (type + queries + shells + decisions). DerivationFCIS has no DB table, no shell, no mapper — it is a pure library. The FCIS suffix implies entity-lifecycle semantics it does not have. A direct named export or a suffix like `DerivationEngine` would be more accurate.

Tinybird read failure for a derivation INPUT stream is untested

domains/metrics/src/metric/__tests__/metric.derivation.queries.test.ts:47

The in-memory reader mock always returns ok:true for readWindow. There is no test where the top stream is a derivation but one of its input (physical) streams fails with MetricTinybirdReadFailedError and that error propagates back through the coordinator. The described 'Tinybird read failure propagates' test covers the top stream failing, not an input mid-derivation.

makePgTbReader (real PG+Tinybird adapter) has zero automated test coverage

domains/metrics/src/metric/metric.queries.ts:249

All derivation tests use an in-memory mock reader. The makePgTbReader factory — including its integrations JOIN, coverage subquery, and Tinybird pipe call — is fully uncovered by automated tests. SG-4 e2e (live Tinybird + real Postgres) is noted as deferred. A DB integration test seeding a real MetricStream + MetricSource row should cover this adapter before shipping.

3-input strict policy with partial slot overlap (not all feeders aligned) is untested

domains/metrics/src/derivation/__tests__/derivation.decisions.test.ts:50

The 3-feeder sum test uses fully-aligned grids. The critical billing case — three inputs where one device loses data mid-window (e.g. m1:[0,300,600], m2:[0,300], m3:[0,600]) — is untested under strict. Expected: only slot [0] survives. This is the property that prevents over-counting demand on a site where a feeder goes offline.

Partial policy with fully-disjoint inputs (union behavior) is untested

domains/metrics/src/derivation/__tests__/derivation.decisions.test.ts:209

The 'strict: fully-disjoint inputs yield empty series' test exists. Under partial policy the same disjoint inputs should return the UNION of all slots with each missing input reading as 0 — a non-trivial and distinct behavior. This is untested. A billing context where one feeder has zero historical data must not collapse to an empty total.

MAX_FORMULA_LENGTH=1024 boundary is entirely untested

domains/metrics/src/derivation/__tests__/derivation.decisions.test.ts:305

The 'oversized string' test exercises MAX_FORMULA_NESTING (100 nested parens), not the character-length guard. There is no test for a formula of exactly 1024 chars (should reject), 1023 chars (should accept), or 1025 chars (should reject). The length guard in tokenize() is completely unexercised.

parseFormula standalone export has no error/rejection test cases

domains/metrics/src/derivation/__tests__/derivation.decisions.test.ts:299

The `parseFormula` standalone test suite tests only valid formulas (returns ParsedFormula with ast + variables). No test exercises parseFormula returning Err for malformed input. The export contract for the future write-time validator (noted in PR description) is not verified directly — a regression on the error path would be hidden by the evaluateDerivation wrapper tests.

Sequential input resolution serializes independent Tinybird round-trips

domains/metrics/src/metric/metric.queries.ts:227

The `for...of Object.entries(inputs)` loop in `resolveDerivationWindow` resolves each input stream sequentially. Inputs are fully independent (same window, same interval, same childCtx) and could be parallelized with `Promise.all(Object.entries(inputs).map(...))`. At 2-5 feeders per site, this adds ~100-300ms of avoidable serial latency per derivation read.

`resolveStream` makes 2 sequential DB queries that could be 1 JOIN

domains/metrics/src/metric/metric.queries.ts:251

The adapter first queries metric_streams JOIN metric_types (for aggregator + internal ID), then a separate query for sources. These could be combined into a single query. For a 3-input derivation this means 6 DB queries where 3 would suffice — one per stream node visited in the walk.

`MAX_DERIVATION_DEPTH` defined in metric-source.errors — wrong module for a read-path policy constant

domains/metrics/src/metric/metric.queries.ts:44

The constant governs both write-time (decideCreateMetricSource) and read-time (resolveDerivationWindow) depth enforcement. Its home in metric-source.errors creates a cross-concern import: the read coordinator depends on a write-side error types file for a non-error value. A cleaner home: `derivation/derivation.decisions.ts` or `derivation/derivation.constants.ts`, imported by both sides.