block / block/proto-fleet

Refactor: DRY miner state bucket derivation (3 copies → 1)

Open
#469 0 comments 0 reactions 0 assignees View on GitHub
enhancement server
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.