Refactor: DRY miner state bucket derivation (3 copies → 1)
- Dominant language
- Go
- Stars
- 55
- Forks
- 16
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 87
Description
## Context
The miner state bucket priority ladder (`broken` / `hashing` / `sleeping` / `offline`) is duplicated in three places today:
1. `server/sqlc/queries/device.sql:840-899` — `CountMinersByState` sqlc query (read path, flat by device ID).
2. `server/internal/domain/stores/sqlstores/device.go:1353-1434` — `GetMinerStateCountsByCollections` inline SQL (read path, collection-scoped).
3. `server/sqlc/queries/miner_state_snapshots.sql:15-31` — `CASE` expressions written into the `state` column at snapshot insert time (write path).
The rules:
| Bucket | Rule (top wins) |
|---|---|
| broken | `pairing_status = AUTH_NEEDED` OR `status IN (ERROR, NEEDS_MINING_POOL, UPDATING, REBOOT_REQUIRED)` OR has open actionable errors |
| hashing | `status = ACTIVE` AND no AUTH_NEEDED AND no open errors |
| sleeping | `status IN (MAINTENANCE, INACTIVE)` AND not AUTH_NEEDED |
| offline | `status = OFFLINE` OR `status IS NULL` (fall-through) |
Drift between the three copies would manifest as the health column on one tab disagreeing with another for the same fleet.
## Why not bundled with #372
#372 already lands two helper migrations (temperature rollup into `devicerollup.AggregateLatestMetrics`, component-error counts generalized to a scoped helper). Bundling a third cross-cutting refactor compounds the rack-tab regression risk and pushes the PR past a reasonable review size. The bucket dedup is purely existing-code cleanup with no user-visible effect, so it's the cleanest follow-up.
## Proposed approach (Option A — snapshot-backed)
Read-side queries stop recomputing buckets from `device_status` + `device_pairing` + `errors` joins, and instead `GROUP BY snapshot.state` against the `miner_state_snapshots` hypertable. The snapshot writer becomes the single source of truth for the CASE rules. Telemetry rollups (`GetLatestDeviceMetrics`) are already snapshot-based, so the staleness model is consistent.
- Rewrite `GetMinerStateCountsByCollections` (`sqlstores/device.go:1353-1434`) and `CountMinersByState` (`device.sql:840-899`) to group by `snapshot.state`.
- Leave the `CASE` in `miner_state_snapshots.sql` as the canonical owner; add a comment noting it's the single source of truth.
- Generalize to `GetMinerStateCountsByScope(collection | site | building)` so the site + building handlers stop using the flat-by-ID path.
- Golden-output parity tests on `GetCollectionStats` against the pre-refactor implementation before the refactor lands — the racks tab health column is live, so any drift is a regression.
Alternative considered (Option B — Postgres function `derive_miner_state(...)`): keeps the live `device_status`-join read path but introduces a SQL function as a new dependency. Rejected for the staleness-model-mismatch with the rest of stats.
## Estimated size
~8–9 files, ~125 net LOC added, ~80 LOC duplication removed.
## Out of scope
- Changes to bucket priority rules themselves. This is a pure DRY refactor — observable values must match the pre-refactor implementation across all three handlers.
- The snapshot `state` enum already exists; no schema migration needed.
## Depends on
- #372 — lands first so the helper-factoring work it does (temperature + component errors) doesn't conflict with this refactor.
Contributor guide
Research direction
Read the CASE rules in server/sqlc/queries/miner_state_snapshots.sql:15-31, then compare the read paths in server/sqlc/queries/device.sql:840-899 and server/internal/domain/stores/sqlstores/device.go:1353-1434. Review the existing site and building handlers and the GetCollectionStats path before changing scope. Done means all three handlers group by snapshot.state, share the canonical rules, and golden-output parity tests show no observable drift.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, postgresql, sql
- Domain
- backend, database
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100