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

オープン
  • 何か問題がありますか？
#371 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
22/100
issue の種類
機能追加
明瞭さ
おおむね明確
活発さ
活発
技術スタック
nextjs, playwright, react, typescript
領域
data-visualization, frontend

調査の方向性

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,

索引モデルが issue の本文から書いたものです。

説明

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

主要言語
TypeScript
スター
1
フォーク
1
平均マージ
6時間 56分
マージ済み PR（30日）
136

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

eddiejaoude/StackDash のほかの issue

eddiejaoude/StackDash の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。