feat/metrics-agg

Write-time derivation walk reads inputs as array — cycle/depth write-time guards are dead

domains/metrics/src/metric-source/metric-source.queries.ts:487

walkDerivationGraph does `Array.isArray(row.inputs) ? row.inputs : []` but source_config.inputs is always a JSON object, never an array. Walk never descends, so derivationDepth=0 and hasCycle=false always. Both write-time guards are non-functional. Only read-time guards in metric.queries.ts protect. Mitigated by derivation creation being FE-v2-only (not yet user-reachable).

Depth check `> MAX_DERIVATION_DEPTH` allows MAX+1 levels — off-by-one vs constant name

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

Check `ctx.depth > MAX_DERIVATION_DEPTH` (MAX=10) allows depths 0–10 inclusive — 11 levels before the error fires. The constant name implies a cap of 10. Write-time uses the same `>` so guards are internally consistent and tests match, but the constant name communicates a cap of 10 while behavior is 11.

Dead-code guard `physical.length === 0` is unreachable after derivation filtering

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

After the early-return derivation branch, sources[0] is guaranteed physical and survives the filter. The guard never fires and misleads future maintainers.

Diamond dependency causes repeated Tinybird reads — correct but redundant

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

When two derivation inputs share a sub-stream, its window is fetched from Tinybird twice. Results are identical. No correctness impact under current invariant (one stream per site/MetricType) but worth noting for future multi-device topologies.

Handler error switch missing 5 new derivation error types — TypeError/500 at runtime

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

The switch on result.error._tag handles only 3 of 8 members in ResolveMetricStreamWindowError. The five new derivation errors (MetricDerivationCycle, MetricDerivationDepthExceeded, DerivationMalformedFormula, DerivationInputNotFound, DerivationMalformedConfig) fall through to `return success(toMetricStreamWindowResponse(..., result.value))` where result.value is undefined — throws TypeError at runtime. TypeScript doesn't catch this because the return type is inferred.

No request window size limit enables unbounded Tinybird fan-out

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

No maximum window duration enforced. A single authenticated request with 6-year window at 5m interval against a 10-deep derivation graph triggers O(inputs^depth) Tinybird pipe calls. ~630k points per input per year. Realistic cost/compute amplification from one API call.

No sourceConfigSchema on internal:derivation — formula not validated at write time

packages/integration-manifests/src/manifests/internal-derivation.ts:25

validateSourceConfig() is a no-op for this integration. Malformed formula accepted verbatim into source_config and only fails at read time, causing billing reads to return errors instead of data. Add parseFormula() check to the manifest's sourceConfigSchema or decideCreateMetricSource.

RLS enforcement on recursive input reads relies on caller convention, not type system

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

As currently wired (handler wraps in createRLSDb transaction), RLS IS in effect for recursive reads. But if a future internal caller passes a non-RLS db, recursive input resolution silently reads cross-org streams. Consider a JSDoc note or typed RLS-scoped db.

No formula string length cap before tokenization

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

tokenize() iterates with no upfront length check. A multi-MB formula from JSONB allocates a proportional tokens[] array. A length cap (e.g. 4096 chars) eliminates this as a resource-exhaustion vector. Low severity: write requires authentication + RBAC.

Coordinator + I/O-port logic in metric.queries.ts violates FCIS

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

canonical-form.md defines queries files as 'thin DB wrappers (DbOrTx first param, Entity | null returns).' metric.queries.ts now exports StreamWindowReader (I/O port), resolveWindowWithReader (recursive coordinator), resolveWindowInner (full orchestrator with 5 phases), and resolveDerivationWindow. None are thin DB wrappers. Per ADR-016, the imperative shell owns I/O + orchestration. Correct home: metric.shells.ts or metric.read-coordinator.ts.

Multi-paragraph file header comments violate one-line-max rule

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

35-line JSDoc block at file top. CLAUDE.md: 'don't write multi-paragraph docstrings or multi-line comment blocks — one short line max.' Grammar BNF belongs in .claude/rules/site-metrics-seam.md.

Multi-line block comments in derivation.errors.ts and derivation/index.ts

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

14-line JSDoc block in errors.ts, 7-line block in index.ts, plus JSDoc on each error type. All violate the one-line-max commenting rule.

Multi-line styled banner comments in resolveWindowInner violate commenting rule

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

5-line banner with box-drawing characters. Should be a single inline comment, e.g. `// P7: derivation wins; never splice a computed total with raw channels.`

Decision functions not named `decide{Operation}`

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

parseDerivationConfig, parseFormula, evaluateDerivation deviate from the canonical `decide{Operation}` naming. Defensible (sub-operations, not CQRS decisions) but not aligned with the naming table.

`resolveWindowWithReader` exported from queries file but not from metric barrel

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

metric/index.ts exports resolveMetricStreamWindow but not resolveWindowWithReader or StreamWindowReader. Tests import directly from metric.queries. Either surface through the barrel (public) or remove the export from the file (internal).

`partial` policy not exercised through the coordinator

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

partial is tested at the evaluateDerivation unit level but not through resolveWindowWithReader. The full call chain (parseDerivationConfig → coordinator → evaluator with partial policy) has no integration coverage.

Aggregator non-propagation into derivation inputs not asserted

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

The same-interval test captures reads[] including aggregator but only asserts interval. Adding `reads.every(r => r.aggregator === 'mean')` when calling with `aggregator: 'sum'` would pin the aggregator non-propagation contract.

Window clamp (openStartedAt) never exercised in new tests

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

Every ActiveSourceRow sets openStartedAt: null. The window-clamp step (trim from forward to earliest open-coverage start) is never exercised by the new test suite.

Multi-source priority-ordered splice not regression-tested post-refactor

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

All test helpers create single-source streams. No test verifies that a stream with two physical sources passes both sourceIds to readWindow in priority order — a core pre-refactor correctness property.

Sequential input resolution adds O(N) latency for N-feeder derivations

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

childCtx is immutable so Promise.all is safe. For a 3-feeder site total: 3×(1 PG + 1 Tinybird call) in series vs parallel. Tradeoff: lose early-exit on first error (all requests fire). At 2–4 feeders delta is modest but grows linearly.

Redundant `.sort()` in strict-intersection — Tinybird already returns rows ordered

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

resolve_stream_window pipe emits ORDER BY bucket_ts. Map keys iterate in insertion order (ascending). The sort at line 485 is a no-op in practice — O(n log n) work already paid by Tinybird. Documenting the ordering assumption on SeriesPoint[] inputs captures the implicit contract.

`MAX_DERIVATION_DEPTH` belongs in derivation module, not metric-source.errors

domains/metrics/src/metric-source/metric-source.errors.ts

An errors file is wrong home for a configuration constant governing two layers. Moving to derivation/derivation.decisions.ts (alongside MAX_FORMULA_NESTING) removes a cross-layer dependency.

Two PG queries per `resolveStream` could be one join

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

resolveStream makes two sequential PG round-trips (metricStreams+metricTypes, then metricSources+integrations). A single LEFT JOIN reduces round-trip latency by ~50% per call. For a 3-feeder derivation: 6 PG calls today vs 3.