feat(dashboard): decide the monthly comparison when the reporting accounts change

Aperta
  • Qualcosa non va?
#371 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
5/5
Tempo stimato
Più di una settimana
Idoneità per principianti
22/100
Tipo di issue
Funzionalità
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
nextjs, playwright, react, typescript
Ambito
data-visualization, frontend

Direzione di ricerca

Start by reading the "never a partial sum" rule at the top of src/lib/dashboard/trends.ts, then compareToPrevious/compareValues (:138-150), buildTrends expected-account logic (:203), and comparisonText (:326). Do not implement: the acceptance criteria require Eddie to pick option A-D in a comment here first, so the first task is reading the four options and the linked #308 cost note. Once chosen,

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

area: dashboard blocked decision enhancement priority: P3 size: S

Context

The monthly trend's comparison line ("Up 12 percent from last month") compares this month's total with last month's. That is the last point against the one before it: compareToPrevious and compareValues in src/lib/dashboard/trends.ts:138-150, worded by comparisonText (:326). The chart shows it under the title (src/components/dashboard/trend-chart.tsx:100), and trendSummary (:346) reads it out as part of the chart's accessible name.

The two totals can be summed over different accounts, and then the percentage measures the accounts changing, not the spend:

  • An account stops (since #370, the fix for #308). An account with no row since snapshotWindowStart(now) (45 days) is expected only up to the month of its last row (buildTrends, :203). Example: Production reports $1.00 and Staging $0.40 in November, then Staging stops. December reads "Down 29 percent from last month" although Production did not change.
    • The sentence shows this only when the last row falls early in its month. It appears from day 46 after that row until the end of the next month, so for at most about 15 days.
    • Before day 46, the current month shows as unavailable, because the account still counts as reporting. After that month, both months being compared leave the account out.
    • The plotted points step down for good either way, because that is the real sum of what reported.
    • The daily view is not affected: a stopped account's last day is more than 45 days back, outside the 30 day chart.
  • An account starts (behavior from before #370). A new account is expected from its first row (:203), so for its whole first month the comparison sets a total that includes it against one that does not, and reads "Up" by the new account's amount. This happens every time an account is added, and lasts a full month.

Raised by the Fable review of #370. The rule's cost is recorded on #308 (cost and its correction below it). Eddie asked for the choice to be put to him before anything is implemented.

Options

Option Pros Cons Effort Impact
A. Keep it (status quo) Nothing to build. The sentence always matches the two plotted points, and the sum is honest about what reported. Consistent with the dashboard cards, which also drop an account with no row in 45 days. A join reads "Up" for a whole month, and a leave can read "Down" for up to about 15 days, when spend did not change. A reader can only tell by opening the table and knowing which accounts reported. none Every member who reads the monthly sentence or the chart's accessible name in a workspace with more than one account per series. Misleading for the first month after each new account and, rarely, up to about 15 days after one stops. No money or data at risk; it corrects itself the next month.
B. No comparison when the accounts differ One rule for joins and leaves. Matches the trend's existing stance that a total over the wrong set of accounts is not shown (the "never a partial sum" rule at the top of trends.ts). Small change: TrendPoint carries which accounts it summed, and compareToPrevious compares the sets. The comparison is gone for a new account's whole first month, which is exactly when someone wants to see the effect of adding it. New copy needed, for example "No comparison: the accounts reporting changed since last month". S Same readers. They lose a percentage they could misread, and get the reason in its place. Lasts a month after a join, up to about 15 days after a leave.
C. Compare only the accounts in both months Keeps a percentage that measures real change in spend. The number no longer matches the two plotted points, which confuses anyone checking it against the chart or the table. The copy has to say what it compared, which lengthens the sentence and the accessible name. compareToPrevious would need per-account values, which only buildTrends holds in cells (:131). S, M if the copy needs a tooltip or a second line Same readers. Accurate when read carefully, harder to read at a glance. Screen reader users get a longer chart name.
D. Keep the comparison and name the change The number still matches the chart, and the reader learns why, for example "Down 29 percent from last month. Staging stopped reporting." buildTrends has no account names: TrendConnection is only id, provider, and granted_scopes. The page would have to pass the workspace's accounts in, and naming several accounts needs list copy. M Same readers, best informed. The longest sentence and accessible name of all the options.

Files each option touches:

File A B C D
src/lib/dashboard/trends.ts none TrendPoint, TrendComparison, buildTrends, compareToPrevious, comparisonText buildTrends, compareToPrevious, comparisonText buildTrends input, comparisonText
src/lib/dashboard/trends.test.ts none yes yes yes
src/app/w/[workspace]/dashboard/page.tsx none none none passes account names
src/components/dashboard/trend-chart.tsx none none maybe, for a second line maybe, for a second line
docs/architecture/schema.md (Monthly trend) none yes yes yes
e2e/trends.spec.ts none yes yes yes

Recommendation: B. The argument that decides it: the trend already refuses to show a sum over the wrong set of accounts, and B applies the same rule to the comparison, for joins and leaves alike, at size S. Second: A. The artifact is bounded and corrects itself, and the plotted points are honest.

Acceptance criteria

  • Eddie picks an option, and the choice and its reasons are commented here.
  • If it is B, C, or D: the monthly comparison follows the chosen rule for an account that starts and for one that stops, with unit tests that name each boundary in src/lib/dashboard/trends.test.ts.
  • Playwright e2e coverage in e2e/trends.spec.ts for the chosen wording, or a note saying why none is needed.

Files and areas

  • See the table above.
  • Non-goals: the plotted values, the daily view, the expected-account rule from #308, the headline strip, and public metrics.

Verification

  • npx vitest run src/lib/dashboard/trends.test.ts
  • safe-e2e.sh on e2e/trends.spec.ts
  • npm run typecheck, npm run lint

Model: anthropic/claude-opus-5-5 | Reasoning effort: high

Lingua principale
TypeScript
Stelle
1
Fork
1
Merge medio
6h 56m
PR unite (30g)
136

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di eddiejaoude/StackDash

Tutte le issue di eddiejaoude/StackDash

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.