From 20ac306c2631178c71beba170b2080651bacdde2 Mon Sep 17 00:00:00 2001 From: Andrew Bryant Date: Fri, 30 Oct 2026 15:53:45 -0400 Subject: [PATCH] Decide that chart stays generic Investigated ChartController/ChartService directly rather than taking the old open question at its word: the eleven routes share an outer currency-keyed idiom in six of nine distinct bodies, but the leaf rows are raw DB::select results with no shared columns, and the remaining bodies are bespoke service outputs or a ~20-way scalar match. Typing any of it would trade dozens of hand-maintained structs for marginal benefit over the existing generic chart::. --- GOALS.md | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/GOALS.md b/GOALS.md index 49424b2..3b8fd2e 100644 --- a/GOALS.md +++ b/GOALS.md @@ -269,14 +269,34 @@ client-portal family, the provider webhooks, and a few company and user extras. Keep that file current — it names exact counts, so it goes stale quietly. +**Decided: `chart` stays generic.** The premise in the old open question — +"eleven chart endpoints share no response shape" — undersold the problem. +Reading `ChartController`/`ChartService` directly: the eleven routes are +nine distinct bodies (`totals`/`totalsV2` and `chart_summary`/ +`chart_summaryV2` are pairs of the same implementation), and six of those +nine *do* share an outer idiom — an object keyed by a company's own +currency ids, plus a sentinel `999` key for the cross-currency aggregate. +But the leaf rows inside that shape are raw `DB::select` results with no +model behind them, and they don't agree with each other: within `totals()` +alone, the `invoices` leaf carries `invoiced_amount`, `revenue` carries +`paid_to_date`, `outstanding` carries `amount` *and* `outstanding_count`, +and `chart_summary`'s four leaves differ again. That is goal 2's +"ambiguous upstream type" problem — compounded by `SUM()` often coming +back from the DB driver as a numeric string — repeated dozens of times +over. The remaining three bodies (`cashflow_forecast`, +`client_payment_analytics`, `project_analytics`/`project_burnup`) delegate +to standalone service classes with no shape in common with each other or +with the currency-keyed family, and `calculated_fields` returns whichever +scalar or array type its ~20-way `field` match picks, so there is no fixed +shape to type at all. Modeling the shared outer envelope would still leave +dozens of hand-maintained, DB-column-shaped leaf structs for marginal +benefit over a caller defining their own `T` for the one endpoint they +use — which `chart::` already lets them do. + ## Open questions These are genuinely undecided. Deciding one is a change to this file. -- **`chart` is still generic** over `T: DeserializeOwned`, because the - eleven chart endpoints share no response shape. Per-chart structs would - be eleven small models of unstable payloads; unclear whether that earns - its keep. - **Large bodies are buffered entirely into memory.** PDF downloads and CSV report exports come back as `Vec`, and an uploaded `FileUpload` holds its whole contents until the request finishes. Streaming would be better