fix(economy): make breakdown --since actually filter, and stop doctor certifying drift between two zeros

[andrei-hasna]

Fixes two read instruments that returned confident, wrong answers at rc=0, plus a third copy of the same arithmetic. Todos row 68346b5f; found by the finance-instrument audit (13310121).

1. breakdown --since was inert — and it is CLOUD-MODE ONLY

GET /api/breakdown read since and then applied it to exactly one dimension:

if (by === 'project') return ok(since ? queryProjectBreakdownSince(...) : ...)
if (by === 'agent')   return ok(queryAgentBreakdown(db, period, machine))   <- since DROPPED
if (by === 'account') return ok(queryAccountBreakdown(db, period, machine)) <- since DROPPED
if (by === 'cost-center') ...                                              <- since DROPPED
return ok(queryModelBreakdown(db))                                          <- since DROPPED

That last line is the CLI's default dimension, so a bare economy breakdown --since <anything> returned the all-time table at HTTP 200. Anyone quoting a period figure from this verb was quoting the all-time figure, with nothing to say so.

The *Since query functions already existed and were already correct, and the local store already branched on them — so this is a wiring fix, not new SQL, and it explains why the sibling compare verb filters correctly (different code path entirely: store.rangeStats(from, to)).

Live proof, both directions, only the server hunk varying

An ephemeral SQLite db seeded with one 2020 row and one current row, a real economy-serve, and the real CLI in cloud mode. total from breakdown --json:

read	server at HEAD (bug)	server with this PR
no filter	2	2
--since 2021-01-01	2	1
--since 2099-01-01	2	0

The negative control is the middle column: with the hunk reverted, a year-2099 since still returns every row. Local mode measured 2 / 1 / 0 both before and after, which is what scopes the defect to cloud mode.

The regression test asserts the row count changes on every dimension, because asserting that the flag was accepted is precisely what the defect already did.

2. doctor certified health from two absences

const deltaPct = actual.total_usd > 0 ? (delta / actual.total_usd) * 100 : 0

With no billing imported, actual is 0, so delta_pct is 0, so is_alert is false, so doctor printed a green billing drift month: 0.0%. 0 meant both "measured, and they agree" and "not measurable" — the check could not fail on the absence it exists to detect, on the surface an operator consults to decide whether to trust everything else.

BillingDiffSummary now carries comparable and incomparable_reason, separating no provider billing records imported from records exist but total $0.00. countBillingRecords shares one period predicate with queryBillingSummary, deliberately: a verdict about a total is only safe while both reads select the same rows.

Live proof, both directions

empty billing table:
  ! billing drift month: UNKNOWN - no provider billing records imported for this period (estimated $0.00); run: economy billing sync

populated, agreeing ($10.00 vs $10.00):
  ✓ billing drift month: 0.0%

The second line is the control: a fix proven only on the broken case can silently break the healthy path. A populated table that disagrees beyond threshold still fails.

3. The same ternary, written three times

actual > 0 ? ... : 0 appeared independently in the diff, in doctor, and in billing show — and every copy rendered its fallback as a measured 0.0%. All three now call one billingDeltaPct helper that returns null rather than 0, so the next caller cannot reintroduce it by writing the obvious thing.

billing show on an empty table now prints Difference: $0.00 (n/a) with a note, instead of (+0.0%).

The dashboard's Reconciliation tab rendered the same delta_pct and would have shown the same false 0.0%; it now shows n/a with a banner. SDK and dashboard type contracts carry the new fields.

The 503 report does NOT reproduce, and I did not "fix" it

The row's third item was a 503 presenting as an empty result. Measured against an ephemeral always-503 server with the real CLI:

rc=1
stdout bytes: 0
stderr bytes: 70
economy: cloud API request failed (GET /billing?period=month -> 503).

Positive control, identical probe against a 200: rc=0, 488 bytes stdout, 0 bytes stderr. So the CLI already distinguishes transport failure from empty data, via exit code and a named stderr line, and the probe can produce both outcomes.

The observed "0 bytes on stdout" is the correct contract rather than a defect: errors belong on stderr so that a --json consumer never parses an error object as data. Emitting one on stdout would be the anti-pattern this fleet has already been bitten by — an error object that a consumer's d if isinstance(d, list) else ... turns into an empty list. A caller that reads stdout while ignoring $? is a capture-path problem on the caller's side. No change made; recorded so nobody re-files it.

Known remaining instance, named rather than silently widened

BillingDiffRow.delta_pct (the per-agent rows) still uses the same zero fallback. Making it nullable changes a type the dashboard renders with .toFixed(1), which is beyond this row's scope. Named here so it is tracked rather than assumed handled.

Validation

• bun test — 384 pass, 0 fail, 1663 expect() calls, 44 files
• bun run typecheck — rc=0
• cd dashboard && bun run build — rc=0
• staged secrets scan — 0 findings

Both regression tests were written first and are pasted failing in the task record: the --since test failed with "future": 3 where 0 was required, and the doctor test failed to import a billingDriftCheck that did not yet exist.

All probing ran against an ephemeral db under /tmp; the live ~/.hasna/economy/economy.db mtime was unchanged before and after. No economy sync verb was run.

Agent: agent-chief-strategy

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

[REVIEW] GO — #32 @ 0d51eeb — lens: period-filter-and-health-check, reviewer Aurelius (1 of 1)

Independent review, fresh context. Every claim below came from the worktree at /home/hasna/.hasna/repos/worktrees/open-economy/68346b5f — the open-economy sibling, not economy/. The shared checkout at /home/hasna/workspace/hasna/opensource/open-economy was flagged stale and was never read.

Base has not moved: origin/main resolves to 5bcdb6a081097fea728b8cda2e9599647358c96d, equal to the PR's own baseRefOid, so the merge-result staleness check is not triggered.

Probes ran against ephemeral /tmp databases. The live database is byte-identical before and after:

before  2026-08-06 18:28:48.184978798 +0300 522883072 /home/hasna/.hasna/economy/economy.db
after   2026-08-06 18:28:48.184978798 +0300 522883072 /home/hasna/.hasna/economy/economy.db

The branch working tree was never mutated — both trees were materialised into /tmp with git archive, and git status --short is empty at head.

---

1. Does the filter fix cover every grouping?

Yes, all nine values the route accepts. Independent harness, not the PR's test: a real Bun.serve over a fresh database seeded with one 2020 row and one 2026 row per dimension.

HEAD:

dimension        | no filter | mid 2021 | future 2099 | VERDICT
(default)        |         6 |        3 |           0 | FILTERS
by=model         |         6 |        3 |           0 | FILTERS
by=agent         |         6 |        3 |           0 | FILTERS
by=account       |         6 |        3 |           0 | FILTERS
by=project       |         6 |        3 |           0 | FILTERS
by=cost-center   |         6 |        3 |           0 | FILTERS
by=loop          |         2 |        1 |           0 | FILTERS
by=app           |         2 |        1 |           0 | FILTERS
by=repo          |         2 |        1 |           0 | FILTERS

BASE reproduces the defect, one dimension wider than the PR description:

dimension        | no filter | mid 2021 | future 2099 | VERDICT
(default)        |         6 |        6 |           6 | INERT/BROKEN
by=model         |         6 |        6 |           6 | INERT/BROKEN
by=agent         |         6 |        6 |           6 | INERT/BROKEN
by=account       |         6 |        6 |           6 | INERT/BROKEN
by=project       |         6 |        3 |           0 | FILTERS
by=cost-center   |         6 |        6 |           6 | INERT/BROKEN
by=loop          |         2 |        2 |           2 | INERT/BROKEN
by=app           |         2 |        2 |           2 | INERT/BROKEN
by=repo          |         2 |        2 |           2 | INERT/BROKEN

Eight of nine inert on base, nine of nine filtering on head. The cost-center aliases loop / app / repo were affected too and are fixed by the same wiring; the PR summary names cost-center without them.

Wiring rather than new SQL is confirmed: queryModelBreakdownSince, queryAgentBreakdownSince and queryAccountBreakdownSince pre-date this PR at src/db/database.ts:1251/1268/1281, and queryCostCenterBreakdown already accepted a since field in its filter bag at line 1147.

2. Is the negative control load-bearing?

Both legs hold on every dimension. The future cursor returns exactly 0, so the filter is not disabled. The mid-range cursor returns strictly fewer and non-zero — 6 to 3, and 2 to 1 — so a fix that returned zero for everything is excluded. The PR's own test asserts the same two properties.

3. Does the null return break a consumer?

No gap found. BillingDiffSummary.delta_pct stays number; only the new helper returns number | null, and both its call sites branch on it. The SDK change is purely additive.

Every consumer across src, sdk, dashboard/src, mcp: src/cli/index.ts:1517 and src/cli/commands/extras.ts:339 both branch and render n/a; ReconciliationTab.tsx:96 passes null and the renderer at line 99 handles item.value == null before calling toFixed. The unguarded row.delta_pct.toFixed(1) at line 125 cannot throw, because that field was never made nullable — see P3-C.

bun test (full suite)                          384 pass  0 fail  1663 expect()  rc=0
bun test serve.test.ts billing-diff.test.ts     72 pass  0 fail                 rc=0
npx tsc --noEmit -p tsconfig.json                                               rc=0
npx tsc --noEmit -p dashboard/tsconfig.app.json                                 rc=0

4. Does doctor still report healthy when it should?

Five states, one ephemeral database, default threshold 15%. HEAD:

STATE 1  telemetry $100, zero billing rows
   ! billing drift month: UNKNOWN — no provider billing records imported for this period (estimated $100.00); run: economy billing sync
   comparable=false reason=no_billing_records delta_pct=0 is_alert=false

STATE 2  telemetry $100, one billing row totalling $0.00
   ! billing drift month: UNKNOWN — provider billing records exist but total $0.00 (estimated $100.00)
   comparable=false reason=zero_actual_billing delta_pct=0 is_alert=false

STATE 3  telemetry $100, billing $100  (healthy)
   ✓ billing drift month: 0.0%
   comparable=true reason=null delta_pct=0 is_alert=false

STATE 4  telemetry $100, billing $10   (drift under threshold)
   ✓ billing drift month: 9.1%
   comparable=true reason=null delta_pct=-9.090909090909092 is_alert=false

STATE 5  telemetry $100, billing $510  (drift over threshold)
   ! billing drift month: 80.4%
   comparable=true reason=null delta_pct=-80.3921568627451 is_alert=true

BASE collapses three distinguishable states of the world into one output:

STATE 1  ✓ billing drift month: 0.0%
STATE 2  ✓ billing drift month: 0.0%
STATE 3  ✓ billing drift month: 0.0%
STATE 4  ✓ billing drift month: 9.1%
STATE 5  ! billing drift month: 80.4%

The mirror defect was the specific risk and it does not occur. States 3 and 4 stay green, and 4 is green for the right reason — a real 9.1% drift under threshold. State 5 proves the check can still fail on genuine divergence. ok:false renders as a yellow ! and sets no exit code, so an operator without a billing integration gets a persistent warning naming its own remedy rather than a hard failure — strictly better than a green tick certifying a check that never ran.

Item 3 — the refutation holds; my dispatcher's row was wrong

Three-leg discrimination matrix, real CLI, real HTTP, fake upstream:

503 upstream          rc=1   stdout    1 byte   stderr 85 bytes
                      economy: cloud API request failed (GET /breakdown?by=model&since=2021-01-01 -> 503).
200 with rows         rc=0   stdout  255 bytes  stderr  0 bytes    "total": 1
200 with zero rows    rc=0   stdout   74 bytes  stderr  0 bytes    "total": 0

Transport failure and empty data are distinguishable on both exit status and stderr. HasnaHttpError is thrown from src/lib/contracts-client/transport.ts:473 once 503 exhausts its retry budget; nothing degrades to an empty list. The second half of the author's argument is right on the merits too — emitting an error object on stdout would reproduce the parses-as-empty-list anti-pattern. The filed row was wrong.

Item 5 — the deployed-version inference

Out of scope; answering only the reviewable half. Nothing in the repo pins a deployed version: .github/workflows holds only ci.yml and release-menubar.yml, compose builds from source, and Dockerfile.runtime assembles host-built artifacts. But the version is discoverable at runtime rather than by inference — src/server/serve.ts:231 serves GET /version returning packageMetadata.version, and /health returns status, version and backend. So the conditional is checkable with one read against the running service after release. Package version at head is 0.3.9; this fix is unreleased. I did not query production.

---

Findings

Blocking: none.

P2-A — the machine filter is silently dropped alongside a since cursor, on by=agent and by=account

Introduced here. Probed with a machine id matching no seeded row, so 0 means honoured and non-zero means dropped.

                    HEAD                          BASE
by=model         since+machine=3  machine=6     since+machine=6  machine=6
by=agent         since+machine=3  machine=0     since+machine=0  machine=0
by=account       since+machine=3  machine=0     since+machine=0  machine=0
by=project       since+machine=0  machine=0     since+machine=0  machine=0
by=cost-center   since+machine=0  machine=0     since+machine=0  machine=0

by=agent and by=account moved from 0 to 3 — honoured before, dropped now. Structural: queryAgentBreakdownSince and queryAccountBreakdownSince take no machine parameter, unlike queryProjectBreakdownSince. The PR does thread it through for cost-center and project, so this is a gap in those two helpers rather than in the wiring. by=model drops it in both columns and never accepted one.

Non-blocking because no shipped consumer reaches it: the CLI breakdown command declares no machine flag, ApiStore.agentBreakdown and ApiStore.accountBreakdown send only by, period and since, and the dashboard sends only by and period. Reachable only by a direct HTTP caller — a latent trap rather than a live one.

P2-B — by=model ignores period entirely, and the dashboard does reach that one

Pre-existing and identical on base, so out of scope here. Recorded because it is the same defect class one parameter over, and unlike P2-A it is reachable from a shipped consumer.

HEAD  by=model   all=2  year=2  today=2  yesterday=2      inert
BASE  by=model   all=2  year=2  today=2  yesterday=2      identical
HEAD  by=agent   all=2  year=1  today=1  yesterday=0      period works

queryModelBreakdown at src/db/database.ts:622 takes no period, and the dashboard's getBreakdown accepts by: 'model' together with a period and sends both — so the dashboard's model breakdown silently ignores the period selector and returns the all-time table at HTTP 200.

P3-C — the fourth copy of the ternary, and why this PR makes it visible

src/lib/billing-diff.ts:96 still reads const rowPct = actualUsd > 0 ? (rowDelta / actualUsd) * 100 : 0 for the per-agent rows. Correctly out of scope — making that field nullable changes a rendered type. Worth recording that the consequence now lands on one screen. Measured with telemetry and no billing:

SUMMARY  billing drift month: UNKNOWN — no provider billing records imported for this period (estimated $100.00)
   agent=claude estimated=$100 actual=$0 delta_pct=0 -> rendered "0.0%"

The reconciliation tab shows the amber "Drift is UNKNOWN, not zero" banner above a table row reading 0.0%. The fix is right and it surfaces the remaining copy rather than hiding it — this is the strongest follow-up.

P3-D — two required fields added to a published SDK interface

BillingDiffSummary gains comparable and incomparable_reason as required. Additive for readers, breaking for external code constructing the object as a literal. A semver note for the release, not a defect.

---

What I did not check

Production at economy.hasna.xyz — not queried. Numeric correctness of by_agent rows beyond how the percentage renders. The dashboard in a browser; typecheck and code read only. The postgres branch of startServer — my harness used the SQLite path, so the cloud database open, the API-key authenticator and the readiness check are unexercised. by=service and by=team, which share the branch that loop / app / repo exercise. The 2026-07-07 ingest split-brain, out of scope.

Verdict

GO. Both fixes do what they claim, on evidence that can fail in both directions and that reproduces the defect on base. Full suite and both typechecks green. The findings above are non-blocking follow-ups.