From 40e4292d65c75615cc6eabfa10dfef0509710699 Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Mon, 7 Sep 2026 00:14:54 +0100 Subject: [PATCH] fix: cap the bar sparkline one grade short of full height --- cmd/spinloop/metrics_render.go | 12 ++-- cmd/spinloop/metrics_render_test.go | 50 ++++++++++++-- .../2026-09-06-bar-glyph-gap/.openspec.yaml | 2 + .../2026-09-06-bar-glyph-gap/design.md | 65 +++++++++++++++++++ .../2026-09-06-bar-glyph-gap/proposal.md | 48 ++++++++++++++ .../specs/remote-metrics-bar-format/spec.md | 28 ++++++++ .../archive/2026-09-06-bar-glyph-gap/tasks.md | 14 ++++ .../specs/remote-metrics-bar-format/spec.md | 27 ++++++++ 8 files changed, 234 insertions(+), 12 deletions(-) create mode 100644 openspec/changes/archive/2026-09-06-bar-glyph-gap/.openspec.yaml create mode 100644 openspec/changes/archive/2026-09-06-bar-glyph-gap/design.md create mode 100644 openspec/changes/archive/2026-09-06-bar-glyph-gap/proposal.md create mode 100644 openspec/changes/archive/2026-09-06-bar-glyph-gap/specs/remote-metrics-bar-format/spec.md create mode 100644 openspec/changes/archive/2026-09-06-bar-glyph-gap/tasks.md diff --git a/cmd/spinloop/metrics_render.go b/cmd/spinloop/metrics_render.go index b373561..79877c6 100644 --- a/cmd/spinloop/metrics_render.go +++ b/cmd/spinloop/metrics_render.go @@ -139,11 +139,13 @@ const barLineW = 40 // full row fits the tile exactly and the clip never takes the percentage. const dashBarLineW = 25 -// barGlyphs is the eight block elements the sparkline draws with, lightest to -// heaviest: a series' value maps to the one whose fill height is nearest. A -// rune slice, not a string — the elements are multibyte, so byte indexing -// would not land on glyph boundaries. -var barGlyphs = []rune("▁▂▃▄▅▆▇█") +// barGlyphs is the seven sub-full block elements the sparkline draws with, +// lightest to heaviest: a series' value maps to the one whose fill height is +// nearest. The set stops one grade short of the full block, so the tallest row +// leaves a sliver of space above it and adjacent maxed rows read as separate +// bars instead of one solid block. A rune slice, not a string — the elements +// are multibyte, so byte indexing would not land on glyph boundaries. +var barGlyphs = []rune("▁▂▃▄▅▆▇") // barGlyph picks the block element for a 0-100% value. func barGlyph(pct float64) rune { diff --git a/cmd/spinloop/metrics_render_test.go b/cmd/spinloop/metrics_render_test.go index 5d569aa..00ee879 100644 --- a/cmd/spinloop/metrics_render_test.go +++ b/cmd/spinloop/metrics_render_test.go @@ -16,8 +16,8 @@ func TestBarGlyph(t *testing.T) { pct float64 want rune }{ - {0, '▁'}, {12.4, '▁'}, {12.5, '▂'}, {30, '▃'}, {50, '▅'}, - {75, '▇'}, {87.4, '▇'}, {87.5, '█'}, {100, '█'}, + {0, '▁'}, {12.4, '▁'}, {12.5, '▁'}, {30, '▃'}, {50, '▄'}, + {75, '▆'}, {87.4, '▇'}, {87.5, '▇'}, {100, '▇'}, } for _, c := range cases { if got := barGlyph(c.pct); got != c.want { @@ -26,6 +26,20 @@ func TestBarGlyph(t *testing.T) { } } +// The sparkline never draws a full block: the highest value caps at the +// seven-eighths glyph, so a maxed row leaves a sliver of space above it and +// adjacent rows read as separate bars. +func TestBarGlyphNeverFullBlock(t *testing.T) { + for pct := 0.0; pct <= 100.0; pct += 0.5 { + if g := barGlyph(pct); g == '█' { + t.Fatalf("barGlyph(%v) = full block, want seven-eighths or less", pct) + } + } + if g := barGlyph(100); g != '▇' { + t.Errorf("barGlyph(100) = %c, want ▇", g) + } +} + func TestPoolMaxKeepsPeaks(t *testing.T) { // A pool keeps its highest reading, so the spike a threshold is about // survives the downsampling instead of being averaged away. @@ -76,9 +90,9 @@ func TestRenderSparklineColoursOnlyTheLastPoint(t *testing.T) { last float64 want string }{ - {79.9, ansiGreen + "▇" + ansiReset + " 80%\n"}, - {85, ansiYellow + "▇" + ansiReset + " 85%\n"}, - {95, ansiRed + "█" + ansiReset + " 95%\n"}, + {79.9, ansiGreen + "▆" + ansiReset + " 80%\n"}, + {85, ansiYellow + "▆" + ansiReset + " 85%\n"}, + {95, ansiRed + "▇" + ansiReset + " 95%\n"}, } for _, c := range cases { var b bytes.Buffer @@ -152,14 +166,36 @@ func TestRenderStatBarsDrawsHistory(t *testing.T) { if len(lines) != 2 { t.Fatalf("drew %d lines, want CPU and RAM: %q", len(lines), b.String()) } - if !strings.Contains(lines[0], ansiRed+"█"+ansiReset+" 95%") { + if !strings.Contains(lines[0], ansiRed+"▇"+ansiReset+" 95%") { t.Errorf("CPU line lost its spike or its red last point: %q", lines[0]) } - if !strings.Contains(lines[1], ansiGreen+"▄"+ansiReset+" 40%") { + if !strings.Contains(lines[1], ansiGreen+"▃"+ansiReset+" 40%") { t.Errorf("RAM line: %q", lines[1]) } } +// Two adjacent series both pinned at 100% draw at seven-eighths, so neither +// row reaches the top of its cell: the rows read as separate bars rather than +// one solid block. The full block must not appear anywhere in the render. +func TestRenderStatBarsMaxedRowsStopShortOfFull(t *testing.T) { + history := []metrics.HistorySample{ + {Time: 1, CPU: ptrPct(100), Mem: ptrPct(100)}, + {Time: 2, CPU: ptrPct(100), Mem: ptrPct(100)}, + } + var b bytes.Buffer + renderStatBars(&b, nil, nil, nil, history, barLineW) + out := b.String() + if strings.Contains(out, "█") { + t.Errorf("a maxed series drew a full block:\n%s", out) + } + if !strings.Contains(out, "▇") { + t.Errorf("a maxed series did not draw the seven-eighths block:\n%s", out) + } + if !strings.Contains(out, " 100%") { + t.Errorf("a maxed series lost its trailing figure:\n%s", out) + } +} + // The fallback is per series: a series with retained readings draws them, a // series without falls back to its current reading on the same screen. func TestRenderStatBarsFallsBackPerSeries(t *testing.T) { diff --git a/openspec/changes/archive/2026-09-06-bar-glyph-gap/.openspec.yaml b/openspec/changes/archive/2026-09-06-bar-glyph-gap/.openspec.yaml new file mode 100644 index 0000000..1a62d62 --- /dev/null +++ b/openspec/changes/archive/2026-09-06-bar-glyph-gap/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-06 diff --git a/openspec/changes/archive/2026-09-06-bar-glyph-gap/design.md b/openspec/changes/archive/2026-09-06-bar-glyph-gap/design.md new file mode 100644 index 0000000..6b1e9e3 --- /dev/null +++ b/openspec/changes/archive/2026-09-06-bar-glyph-gap/design.md @@ -0,0 +1,65 @@ +## Context + +The bar format draws each resource series as a sparkline via `renderSparkline`, +which maps each sample to a block glyph with `barGlyph` over the set `barGlyphs` +(`cmd/spinloop/metrics_render.go`). That one function is the shared drawing path +for `remote metrics --format=bar`, `fleet metrics --format=bar`, and the +dashboard tiles (all route through `renderStatBars`). The `gauge` format is a +separate drawing (`renderGauge`) — a horizontal filled/unfilled bar. Today +`barGlyphs` is the eight block elements `▁▂▃▄▅▆▇█`, so a value at 100% fills the +whole cell with `█` and adjacent maxed rows merge into one solid block (see +proposal.md — Why). + +## Goals / Non-Goals + +**Goals:** +- Cap the sparkline's tallest glyph at the seven-eighths block (`▇`) so a maxed + row always leaves a visible gap above it. +- Make "never draws a full block" a property of the glyph set, not a runtime + clamp. + +**Non-Goals:** +- Change the `gauge` format or the bar format's no-history gauge-style fallback. +- Change the colour thresholds, the downsampled-extremes pooling, or the + trailing percentage. + +## Decisions + +1. Reduce `barGlyphs` to the seven sub-full block elements `▁▂▃▄▅▆▇` and keep the + existing `int(pct/100 * len(barGlyphs))` mapping. + - `len(barGlyphs)` is now 7, so 100% maps to index 7, clamped to 6, which is + `▇`. The set simply contains no `█`, so a full block is unrepresentable + rather than clamped away. + - The seven grades split 0–100% into even bands of roughly 14.3%. + - Alternative considered: keep the eight elements and clamp the index to 6. + That changes only the top band (87.5–100% becomes `▇`) and leaves every + other value's glyph identical — a smaller visible diff — but it expresses + the cap as a clamp and leaves the top band twice as wide as the rest. + Rejected: the structural invariant and even scale are cleaner, and the + mid-range shift is invisible because the trailing percentage still carries + the exact value. + +2. Scope the change to the sparkline only. + - `renderGauge` keeps `█` as its filled character. The gauge is a horizontal + progress bar where a full fill at 100% is the natural, expected reading; + capping it would make a 100% gauge look under-filled. The request and its + example are about the bar-format sparkline, so the gauge is left as-is. + +## Risks / Trade-offs + +- [Mid-range glyphs shift one grade] (e.g. 50% `▅` → `▄`) under the seven-grade + split → the trailing percentage carries the exact value and the sparkline is a + shape/pressure read, not a per-sample meter; the shift is not something a + reader relies on. Accepted. +- [The no-history fallback in bar format still draws full `█`] at 100%, since it + reuses the gauge style → that is a transient pre-history state (a fresh daemon + with no retained samples); if the same gap is wanted there, it is a small + follow-up. Out of scope for this change. +- [Existing tests pin specific glyphs] to the old eight-grade boundaries → + updated in the apply phase; they are the regression net, not a constraint on + the mapping. + +## Open Questions + +None. The gauge scope is decided (out of scope); capping it would be a separate +change. diff --git a/openspec/changes/archive/2026-09-06-bar-glyph-gap/proposal.md b/openspec/changes/archive/2026-09-06-bar-glyph-gap/proposal.md new file mode 100644 index 0000000..7a687b9 --- /dev/null +++ b/openspec/changes/archive/2026-09-06-bar-glyph-gap/proposal.md @@ -0,0 +1,48 @@ +## Why + +The bar format draws each resource series as a sparkline of block glyphs, and a +series at 100% fills the whole cell with the full block (`█`). When several rows +are all high — GPU util and GPU mem pinned, say — their full-height glyphs touch +edge to edge and the rows read as one solid block with no separation between +them, so the eye can't tell where one series ends and the next begins. + +## What Changes + +- Cap the sparkline's tallest glyph at the seven-eighths block (`▇`) instead of + the full block (`█`), so the highest value in a series always leaves one-eighth + of the cell height unfilled above it. Adjacent rows at their maximum now read + as separate bars with a visible gap between the bottom of one and the top of + the next. +- The trailing percentage is unchanged and still carries the exact value, so the + glyph is a visual approximation that may sit one grade below full height. +- The change applies to the sparkline drawing shared by `remote metrics + --format=bar`, `fleet metrics --format=bar`, and the dashboard tiles. The + `gauge` format (a horizontal filled/unfilled progress bar) is a different + drawing and is left as-is. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `remote-metrics-bar-format`: the sparkline's glyph-for-value mapping gains a + ceiling — the highest value renders as the seven-eighths block (`▇`), never the + full block (`█`) — so a maxed row leaves a visible gap above it. + +## Impact + +- Code: `cmd/spinloop/metrics_render.go` — the sparkline's glyph set + (`barGlyphs`) and the value-to-glyph mapping (`barGlyph`) used by + `renderSparkline`. No change to `renderGauge`. +- Surfaces: `spinloop remote metrics --format=bar`, `spinloop fleet metrics + --format=bar`, and the dashboard tiles (all draw the sparkline through + `renderStatBars`). +- Tests: expectations in `cmd/spinloop/metrics_render_test.go` (and any + dashboard/fleet assertions on specific sparkline glyphs) that pin a value to a + full-block glyph or to the old eight-grade bucket boundaries. +- Not breaking in any data or API sense: the change is to a visual approximation; + the exact value is still reported by the trailing percentage and by the other + formats. diff --git a/openspec/changes/archive/2026-09-06-bar-glyph-gap/specs/remote-metrics-bar-format/spec.md b/openspec/changes/archive/2026-09-06-bar-glyph-gap/specs/remote-metrics-bar-format/spec.md new file mode 100644 index 0000000..d192dfb --- /dev/null +++ b/openspec/changes/archive/2026-09-06-bar-glyph-gap/specs/remote-metrics-bar-format/spec.md @@ -0,0 +1,28 @@ +## ADDED Requirements + +### Requirement: Sparkline never draws a full block + +The sparkline SHALL map a series' 0–100% values to Unicode block elements of one +grade per utilisation level, capped at the seven-eighths block (`▇`): the highest +value in a series renders as `▇`, and no value renders as the full block (`█`). +Capping the tallest glyph one grade short of full height leaves the top eighth of +each cell unfilled, so two adjacent series at their maximum read as separate bars +with a visible gap between the bottom of one row and the top of the next, instead +of merging into a single solid block. The exact value is carried by the trailing +percentage, so a glyph may sit one grade below full height without losing +information. + +#### Scenario: A value at the top of the range caps at seven-eighths + +- **WHEN** a sample in a series reaches 100% +- **THEN** its glyph is the seven-eighths block (`▇`), not the full block (`█`) + +#### Scenario: No sparkline glyph is a full block + +- **WHEN** the sparkline draws any series from its retained samples +- **THEN** none of the glyphs it draws is the full block (`█`) + +#### Scenario: Adjacent maxed rows keep a visible gap + +- **WHEN** two adjacent series are both at their maximum across the window +- **THEN** each row's tallest glyph leaves the top eighth of its cell unfilled, so a blank strip separates the rows rather than the rows reading as one solid block diff --git a/openspec/changes/archive/2026-09-06-bar-glyph-gap/tasks.md b/openspec/changes/archive/2026-09-06-bar-glyph-gap/tasks.md new file mode 100644 index 0000000..e348410 --- /dev/null +++ b/openspec/changes/archive/2026-09-06-bar-glyph-gap/tasks.md @@ -0,0 +1,14 @@ +## 1. Core change + +- [x] 1.1 In `cmd/spinloop/metrics_render.go`, reduce `barGlyphs` from the eight block elements `▁▂▃▄▅▆▇█` to the seven sub-full `▁▂▃▄▅▆▇`, and update its doc comment to say the set stops one grade short of the full block so the tallest row leaves a sliver of space above it. `barGlyph` already keys off `len(barGlyphs)`, so its body is unchanged. Verify: `go build ./cmd/spinloop` succeeds. + +## 2. Tests + +- [x] 2.1 Update `TestBarGlyph` in `cmd/spinloop/metrics_render_test.go` to the seven-grade mapping — 0→`▁`, 12.5→`▁`, 30→`▃`, 50→`▄`, 75→`▆`, 87.4→`▇`, 87.5→`▇`, 100→`▇` — and verify `go test -run TestBarGlyph ./cmd/spinloop` passes. +- [x] 2.2 Add a test asserting `barGlyph` returns `▇` at 100 and never returns the full block `█` across the 0–100 range, covering the spec's "no sparkline glyph is a full block" scenario; verify it passes under `go test ./cmd/spinloop`. +- [x] 2.3 Update the glyph expectations in `TestRenderSparklineColoursOnlyTheLastPoint` (79.9→`▆`, 85→`▆`, 95→`▇`) and `TestRenderStatBarsDrawsHistory` (95→`▇`, 40→`▃`) in `cmd/spinloop/metrics_render_test.go`, and verify `go test ./cmd/spinloop` passes. +- [x] 2.4 Run `go test ./...` and fix any remaining assertions that pinned the old eight-grade sparkline mapping (e.g. the sparkline `Contains` checks in `cmd/spinloop/fleet_test.go`), leaving the gauge-based expectations (`dashBar` in `cmd/spinloop/fleet_dashboard_test.go`) untouched; verify the full suite is green. + +## 3. Final checks + +- [x] 3.1 Run `gofmt -l ./cmd/spinloop` (expect no files listed) and `go test ./... -cover` (expect total coverage >= 80%) and verify both hold. diff --git a/openspec/specs/remote-metrics-bar-format/spec.md b/openspec/specs/remote-metrics-bar-format/spec.md index 01f3a2b..efe2b63 100644 --- a/openspec/specs/remote-metrics-bar-format/spec.md +++ b/openspec/specs/remote-metrics-bar-format/spec.md @@ -140,3 +140,30 @@ Where the daemon reports no history — a daemon that predates the feature, or a - **WHEN** the user runs `spinloop remote metrics --format=bar` against a daemon that reports no history - **THEN** each series is drawn from the current reading in the gauge's filled style +### Requirement: Sparkline never draws a full block + +The sparkline SHALL map a series' 0–100% values to Unicode block elements of one +grade per utilisation level, capped at the seven-eighths block (`▇`): the highest +value in a series renders as `▇`, and no value renders as the full block (`█`). +Capping the tallest glyph one grade short of full height leaves the top eighth of +each cell unfilled, so two adjacent series at their maximum read as separate bars +with a visible gap between the bottom of one row and the top of the next, instead +of merging into a single solid block. The exact value is carried by the trailing +percentage, so a glyph may sit one grade below full height without losing +information. + +#### Scenario: A value at the top of the range caps at seven-eighths + +- **WHEN** a sample in a series reaches 100% +- **THEN** its glyph is the seven-eighths block (`▇`), not the full block (`█`) + +#### Scenario: No sparkline glyph is a full block + +- **WHEN** the sparkline draws any series from its retained samples +- **THEN** none of the glyphs it draws is the full block (`█`) + +#### Scenario: Adjacent maxed rows keep a visible gap + +- **WHEN** two adjacent series are both at their maximum across the window +- **THEN** each row's tallest glyph leaves the top eighth of its cell unfilled, so a blank strip separates the rows rather than the rows reading as one solid block +