From 709921e69b28812e55c38c689608839f6381f7bb Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 15:53:04 +0200 Subject: [PATCH 01/10] docs: spec and plan for the tranche D layout follow-ups (#143, #144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured before writing, and the measurements reordered the work: the report header overflows at 60 columns too, not just at 40 (68 columns once the member note is there), and the 16 columns of background exposed beside the palette box at width 40 come from that header rather than from the table. So the header clamp goes first, and the compositor decision is made afterwards against the new numbers instead of against these. Also recorded: dimming the exposed background is byte-identical to today under the termenv.Ascii that TestMain pins, so no golden could verify it — the same trap as #141. --- .../plans/2026-07-30-layout-followups.md | 393 ++++++++++++++++++ .../2026-07-30-layout-followups-design.md | 154 +++++++ 2 files changed, 547 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-30-layout-followups.md create mode 100644 docs/superpowers/specs/2026-07-30-layout-followups-design.md diff --git a/docs/superpowers/plans/2026-07-30-layout-followups.md b/docs/superpowers/plans/2026-07-30-layout-followups.md new file mode 100644 index 0000000..f7758d9 --- /dev/null +++ b/docs/superpowers/plans/2026-07-30-layout-followups.md @@ -0,0 +1,393 @@ +# Follow-up di layout: piano di implementazione + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Chiudere #144 e #143, i due follow-up della tranche D. + +**Architecture:** Tre interventi indipendenti, in un ordine che non è +arbitrario: il Task 2 cambia i numeri che il Task 3 deve misurare. + +**Spec:** `docs/superpowers/specs/2026-07-30-layout-followups-design.md` + +## Global Constraints + +- `internal/report` e `internal/duration` restano **puri**: nessun I/O, nessun + import di `config`/`clickup`/`tui`. +- Nessuna funzione di stile in produzione chiama `lipgloss.NewStyle()`. +- Golden rigenerati con `go test ./internal/tui -update`, **mai** a mano, e + guardati dopo. +- Gate prima di ogni commit, tutti e cinque puliti: `gofmt -l .`, + `go vet ./...`, `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`, + `go build ./...`, `go test ./... -race`. +- Tutto in **inglese**, ortografia americana: codice, identificatori, commenti, + nomi e messaggi dei test, messaggi di commit. +- Conventional Commits. **MAI** `Co-Authored-By`. +- **Le larghezze si misurano rendendo, non calcolando.** Usa + `lipgloss.Width(strings.Split(, "\n")[0])`, o il ciclo su tutte le + righe quando quella più larga non è la prima. La tranche D ha prodotto due + numeri sbagliati perché sommati a mano: `reportAmountWidth` riprende spazio ad + Amount come ultima risorsa, e l'aritmetica a mano non lo vede. +- **Un test scritto contro un bug non è attendibile finché non lo si è visto + fallire contro quel bug**, e non deve asserire sotto il minimo che il codice + corretto può raggiungere: nella tranche D tre test asserivano l'impossibile. +- **Ogni numero scritto in un commento va misurato.** La tranche D ha corretto + quattordici commenti falsi. + +## Helper esistenti (verificati) + +- `truncateWidth(s string, cols int) string` e `cell(s string, cols int) string` + — `internal/tui/width.go`. `cols <= 0` ritorna `""` in entrambi. +- `testTheme(dark bool) theme` — `theme_test.go:15`. +- `goldenReport()`, `goldenPaletteModel()`, `golden(t, name, out)` — `golden_test.go`. +- `newReport(r report.Report, note string, series []float64) reportModel`. +- `reportModel.view(th theme, width int) string` — **già riceve la larghezza** + (`report.go:198`), chiamata da `app.go:980`. +- `renderBudgetBar(th theme, percentUsed float64) string` — `budget.go:60`. + Chiamanti: produzione `budget.go:149`, test `budget_test.go:43` e + `budget_bar_test.go:57`. +- `budgetLayout(lines []report.BudgetLine, width int) (nameW int, showRemaining bool)` + — `budget.go:116`. Misura `pctW` internamente e oggi non lo espone. + +## Ordine dei task + +`1` e `2` sono indipendenti. **`3` dipende da `2`**: il clamp +dell'intestazione cambia la riga più larga del body, e quindi l'esposizione che +il Task 3 deve misurare. + +--- + +### Task 1: allineare la percentuale del budget (#144) + +**Files:** +- Modify: `internal/tui/budget.go` (`renderBudgetBar`, `budgetLayout`, `view`) +- Modify: `internal/tui/budget_test.go:43`, `internal/tui/budget_bar_test.go:57` + (il terzo argomento) +- Test: `internal/tui/budget_test.go` +- Test: `internal/tui/testdata/budget.golden` + +**Interfaces:** +- Produces: `renderBudgetBar(th theme, percentUsed float64, pctW int) string`; + `budgetLayout` espone anche `pctW`. + +- [ ] **Step 1: Write the failing test** + +```go +// budgetLayout measures the widest percentage label across the real rows and +// uses it to size the name column, but the rendering never padded to it: a row +// at 62% started its figures one column left of a row at 104% (#144). The +// fixture needs percentages of DIFFERENT label widths — with two rows both +// under 100% the columns line up by accident and the test passes against the bug. +func TestBudgetViewAlignsFiguresAcrossRows(t *testing.T) { + t.Parallel() + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + {ListName: "Mobile app", Billed: 1040, Budget: 1000, Currency: "EUR", Remaining: -40, PercentUsed: 104}, + }) + var cols []int + for _, line := range strings.Split(bm.view(th, 100), "\n") { + if i := strings.Index(line, "/"); i >= 0 { + cols = append(cols, lipgloss.Width(line[:i])) + } + } + if len(cols) != 2 { + t.Fatalf("expected two budget rows, found %d figure columns", len(cols)) + } + if cols[0] != cols[1] { + t.Errorf("figures start at column %d on one row and %d on the other", cols[0], cols[1]) + } +} + +// A percentage narrower than the measured maximum is right-aligned into it, so +// the columns after it hold. pctW <= 0 keeps the natural width, which is what +// the callers that are not testing alignment pass. +func TestRenderBudgetBarRightAlignsThePercentage(t *testing.T) { + t.Parallel() + th := testTheme(true) + wide := renderBudgetBar(th, 104, 4) + narrow := renderBudgetBar(th, 62.5, 4) + if lipgloss.Width(wide) != lipgloss.Width(narrow) { + t.Errorf("bars are %d and %d columns wide, want equal", lipgloss.Width(wide), lipgloss.Width(narrow)) + } + if !strings.HasSuffix(narrow, " 62%") { + t.Errorf("narrow = %q, want it to end in a right-aligned \" 62%%\"", narrow) + } + natural := renderBudgetBar(th, 62.5, 0) + if !strings.HasSuffix(natural, "62%") || strings.HasSuffix(natural, " 62%") { + t.Errorf("natural = %q, want the unpadded \"62%%\" when pctW <= 0", natural) + } +} +``` + +- [ ] **Step 2: See the RED** + +I due test nominano `renderBudgetBar` con tre argomenti, quindi il package non +compila: `FAIL [build failed]` non è un RED. Per **vedere** il difetto, aggiungi +solo `TestBudgetViewAlignsFiguresAcrossRows` (che usa la firma attuale di `view`) +ed eseguilo. + +Run: `go test ./internal/tui -run TestBudgetViewAlignsFiguresAcrossRows -v` +Expected: FAIL, con due colonne diverse. **Riporta i due numeri** nel report: +sono la misura del difetto. + +- [ ] **Step 3: Implement** + +`renderBudgetBar` prende `pctW` e allinea a destra: + +```go +// pctW is the width the caller measured for the widest percentage label across +// all rows; the label is right-aligned into it so every row's figures start at +// the same column (#144). pctW <= 0 means natural width — the callers that are +// not testing alignment pass 0. +func renderBudgetBar(th theme, percentUsed float64, pctW int) string { + ... + label := fmt.Sprintf("%.0f%%", math.Floor(percentUsed)) + if pctW > lipgloss.Width(label) { + label = strings.Repeat(" ", pctW-lipgloss.Width(label)) + label + } + return fmt.Sprintf("%s %s", bar, label) +} +``` + +Non toccare `math.Floor`: il riempimento tronca, quindi la label deve +arrotondare per difetto o rivendicherebbe una soglia che la barra non ha +raggiunto (#136). + +`budgetLayout` espone il `pctW` che già misura, invece di lasciarlo interno. La +firma diventa `(nameW int, pctW int, showRemaining bool)` — oppure estrai la +misura in un helper che sia `budgetLayout` sia `view` chiamano. Scegli tu, ma la +quantità che `view` passa a `renderBudgetBar` deve essere **la stessa** che +l'aritmetica di `budgetLayout` ha usato, non una ricalcolata: se divergono, la +riga sfora di quanto divergono. Se estrai un helper, verifica che entrambi lo +chiamino e non ne resti una copia. + +`view` passa `pctW` alla `renderBudgetBar`. I due call site di test passano `0`. + +- [ ] **Step 4: Run and regenerate the golden** + +```bash +go test ./internal/tui -run 'TestBudget|TestRenderBudgetBar' -v +go test ./internal/tui -race +go test ./internal/tui -update +git diff internal/tui/testdata/ +``` +Expected: `budget.golden` ha una riga sola con `62%`, quindi `pctW` è 3 e non +cambia niente — **verificalo**, non assumerlo. Se cambia, guarda cosa è +cambiato e riportalo. + +- [ ] **Step 5: Gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "fix(tui): right-align the budget percentage so the figures line up (#144)" +``` + +--- + +### Task 2: troncare l'intestazione del report (#143, seconda metà) + +**Files:** +- Modify: `internal/tui/report.go` (`reportModel.view`, il titolo a riga ~204) +- Test: `internal/tui/report_test.go` +- Test: i golden stretti che mostrano l'intestazione + +**Interfaces:** +- Consumes: `truncateWidth` (`width.go`). + +- [ ] **Step 1: Write the failing test** + +```go +// The report table became width-aware in #66 and the budget screen in #136, but +// the header line above them never did: measured, it renders 68 columns on both +// a 40- and a 60-column terminal (the member note is what pushes it past the 54 +// it measures without one). Line 1 is the title's own MarginBottom padding and +// overflows with it, which is why the text is truncated BEFORE th.Title.Render +// rather than after — truncating the rendered string would leave that line long. +func TestReportViewHeaderFitsTheTerminal(t *testing.T) { + t.Parallel() + th := testTheme(true) + rm := newReport(goldenReport(), " (2/5 members)", []float64{1, 0, 3, 2, 8}) + for _, width := range []int{40, 60, 80} { + for i, line := range strings.Split(rm.view(th, width), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line %d rendered %d columns: %q", width, i, w, line) + } + } + } +} + +// Before the first WindowSizeMsg the width is 0 and nothing is sized against +// it: the header keeps its full text, because truncateWidth(s, 0) returns "" +// and would blank it. +func TestReportViewHeaderSurvivesZeroWidth(t *testing.T) { + t.Parallel() + th := testTheme(true) + rm := newReport(goldenReport(), "", nil) + if !strings.Contains(rm.view(th, 0), "Report ") { + t.Error("the header vanished at width 0") + } +} +``` + +- [ ] **Step 2: See the RED** + +Run: `go test ./internal/tui -run TestReportViewHeader -v` +Expected: il primo FAIL su width 40 e 60, riportando **68** colonne per la riga +0 e per la riga 1 — quattro errori in tutto. Il secondo PASS (oggi il titolo non +è troncato, quindi non può sparire). **Allega il transcript**: se i numeri non +sono 68, la fixture non è quella misurata dalla spec. + +- [ ] **Step 3: Implement** + +In `reportModel.view`, prima del `Render`: + +```go + // Clamped before Render, not after: th.Title carries MarginBottom(1), which + // emits a second line as wide as the content, so truncating the rendered + // string would shorten the text and leave that padding line overlong. + // width <= 0 is the first render, before the terminal has sent its size — + // truncateWidth(s, 0) returns "" and would blank the header. + head := fmt.Sprintf("Report %s — scope %s%s — grouped by %s — tz %s", + report.PeriodLabel(r.Start, r.End), r.Scope, rm.note, r.GroupBy, r.Timezone) + if width > 0 { + head = truncateWidth(head, width) + } + title := th.Title.Render(head) +``` + +- [ ] **Step 4: Run and regenerate the goldens** + +```bash +go test ./internal/tui -run TestReportViewHeader -v +go test ./internal/tui -race +go test ./internal/tui -update +git diff internal/tui/testdata/ +``` +Expected: si muovono i golden resi a una larghezza minore del titolo — +`palette_narrow` fra questi. Quelli a 80 non si muovono (68 ≤ 80). **Guarda ogni +golden cambiato** e riporta quali e come. Se un golden a 80 si muove, fermati: +significa che il clamp scatta quando non dovrebbe. + +- [ ] **Step 5: Gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "fix(tui): clamp the report header to the terminal width (#143)" +``` + +--- + +### Task 3: rimisurare l'esposizione accanto al box, e decidere (#143, prima metà) + +> **Dipende dal Task 2.** Il clamp dell'intestazione cambia la riga più larga +> del body, che è la quantità da misurare qui. Non eseguirlo prima. + +**Files:** +- Modify: `internal/tui/overlay.go` (`composite`) — **solo se la misura lo dice** +- Test: `internal/tui/overlay_test.go` +- Test: `internal/tui/testdata/palette_narrow.golden` + +- [ ] **Step 1: Misurare, prima di scrivere codice** + +Con una sonda usa-e-getta che poi cancelli, misura per ogni larghezza in +`{40, 50, 60, 80, 120}`: la riga più larga del body, la larghezza del box, il suo +`x`, e quante colonne del body restano esposte a destra del box +(`widestBodyLine - (x + boxW)`, se positivo). Prendi body, box, `x` e `y` dal +percorso vero: + +```go +m := goldenPaletteModel() +m.width = w +body := m.screenBody() +box, x, y := m.palette.layout(m.theme, m.width, m.height, strings.Count(body, "\n")+1) +``` + +Prima del Task 2 la misura era: 16 colonne esposte a larghezza 40, zero a ogni +larghezza da 60 in su. + +**Riporta la tabella nel report, e poi segui la misura:** + +- **Se l'esposizione massima è ≤ 1 colonna** a ogni larghezza: implementa il + ritaglio (Step 2). È il caso atteso. +- **Se è > 1 colonna** a qualche larghezza: **fermati**, riporta la tabella e + quale contenuto è esposto, e non implementare niente. Vuol dire che il Task 2 + non ha coperto tutta la causa e la decisione torna all'umano. + +- [ ] **Step 2: Il ritaglio, solo se la misura lo autorizza** + +Il segmento destro di `composite` non viene disegnato quando ciò che esporrebbe +è contenuto che sfonda comunque il terminale. Attenzione: `composite` non conosce +la larghezza del terminale, conosce solo body e box. Il ritaglio quindi non può +essere «oltre il terminale», deve essere «oltre il bordo destro del box» — che è +la stessa cosa solo quando il body non è più largo del terminale, cioè dopo il +Task 2. + +Scrivi il perché nel commento, e scrivi **anche** il caso in cui questa scelta +sarebbe sbagliata: se un giorno il body tornasse a essere più largo del +terminale, il ritaglio nasconderebbe contenuto invece di detriti. + +Il test deve inchiodare che il fondo esposto **non** compare, con una fixture in +cui prima compariva. Provalo per mutazione: rimetti il segmento destro e verifica +che il test fallisca. + +- [ ] **Step 3: Golden e gate** + +```bash +go test ./internal/tui -race +go test ./internal/tui -update +git diff internal/tui/testdata/palette_narrow.golden +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +``` +**Guarda il golden.** Deve mostrare il box senza i caratteri di cornice spuri a +destra. + +- [ ] **Step 4: Commit** + +```bash +git add internal/tui +git commit -m "fix(tui): stop drawing the background past the overlay's right edge (#143)" +``` + +--- + +### Task 4: CHANGELOG + +**Files:** +- Modify: `CHANGELOG.md` + +- [ ] **Step 1** Sotto `## [Unreleased]`, sezione `### Fixed`, una voce per issue + chiusa, nello stile delle voci già presenti: comportamento osservato, non + implementazione, e **niente garanzie assolute** — la tranche D ne ha dovuta + correggere una che prometteva «the table now always fits the terminal width». + Se il Task 3 si è fermato senza implementare, la voce sulla #143 copre solo il + clamp dell'intestazione e la issue resta aperta. + +- [ ] **Step 2** Gate completo, poi commit `docs: changelog for the layout follow-ups`. + +--- + +## Self-review del piano + +**Copertura:** spec §3.1 → Task 1; §3.2 → Task 2; §3.3 → Task 3; §5 è vincolo +globale; §4 (fuori scope) non genera task. + +**Dipendenze:** `3` dipende da `2` ed è dichiarato in testa al task, non solo +qui. `1` è indipendente da entrambi. + +**Il RED di ogni task è osservabile:** il Task 1 mette in staging il test che +compila contro la firma attuale; il Task 2 non ha simboli nuovi, quindi il RED è +diretto; il Task 3 non ha un RED perché è una misura che decide se c'è un +difetto da correggere — e il suo test nasce dopo la misura, con la prova per +mutazione al posto del red-green. + +**Le larghezze asserite** vengono tutte da §2 della spec, misurata rendendo. Il +Task 2 asserisce a 40, 60 e 80: le prime due sono le larghezze in cui oggi sfora, +la terza è il controllo che il clamp non scatti quando non serve. + +**Placeholder:** nessuno. Il Task 1 lascia una scelta esplicita +all'implementatore (esporre `pctW` da `budgetLayout` o estrarre un helper) con il +vincolo che la decide: la quantità passata a `renderBudgetBar` deve essere la +stessa usata dall'aritmetica. Il Task 3 è deliberatamente condizionale, e la +condizione è un numero misurato, non un giudizio. diff --git a/docs/superpowers/specs/2026-07-30-layout-followups-design.md b/docs/superpowers/specs/2026-07-30-layout-followups-design.md new file mode 100644 index 0000000..d9ae141 --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-layout-followups-design.md @@ -0,0 +1,154 @@ +# Follow-up di layout: allineamento budget e clamp dell'intestazione (design) + +> v1.9. Chiude **#144** e **#143**, i due follow-up emersi dalla review finale +> della tranche D. Spec breve: sono tre interventi piccoli, tutti già misurati +> prima di scriverla. + +## 1. Obiettivo + +Tre residui di larghezza che la tranche D ha scoperto senza chiudere. Nessuna +feature nuova, nessuna chiamata API nuova, nessun tasto nuovo. + +## 2. Evidenza misurata + +Ogni numero qui sotto viene dall'aver **reso** la schermata, non dall'aver +sommato colonne a mano. È la regola che la tranche D ha dovuto imparare: due +numeri del suo piano erano sbagliati perché calcolati. + +### 2.1 L'intestazione del report (#143, seconda metà) + +Righe della `reportModel.view` che sforano, misurate con `goldenReport()` più la +nota membri `" (2/5 members)"`: + +| terminale | riga 0 (titolo) | riga 1 (margine del titolo) | altre | +|---|---|---|---| +| 40 | **68** ✗ | **68** ✗ | tutte ≤ 39 | +| 60 | **68** ✗ | **68** ✗ | tutte ≤ 39 | +| 80 | 68 ✓ | 68 ✓ | tutte ≤ 39 | + +Tre cose che la issue non diceva: + +- **Il titolo sfora anche a 60 colonne**, non solo a 40. La issue era stata + scritta guardando `palette_narrow.golden`, dove il titolo è 54 colonne perché + quel modello non ha la nota membri; con la nota diventa 68. +- **La riga 1 è la riga di margine del titolo** (`th.Title` ha `MarginBottom(1)`, + che emette una seconda riga di spazi larga quanto il contenuto). Sfora insieme + al titolo, e si sistema insieme a lui: troncando il testo **prima** di + `th.Title.Render` la riga di margine si accorcia da sola. Troncare dopo il + `Render` la lascerebbe larga. +- **Nessun'altra riga sfora** in questa fixture: `summary` è 37 colonne e la nota + billable 38, entrambe sotto 40. Con contenuti più lunghi — molte valute nel + `SummaryLine` — potrebbero sforare anche loro, ma è fuori dallo scope di + questa issue, che nomina l'intestazione. Registrato qui perché la prossima + persona non lo riscopra da zero. + +### 2.2 L'esposizione del fondo accanto al box (#143, prima metà) + +Misurato rendendo dal percorso vero di `View()` (verificato: la variante «oggi» +riproduce `View()` byte per byte), per ogni larghezza: + +| terminale | box | esposto a destra | +|---|---|---| +| 40 | 36 @ x=2 | **16 colonne** | +| 60 | 52 @ x=4 | 0 | +| 80 | 52 @ x=14 | 0 | +| 120 | 52 @ x=34 | 0 | +| 160 | 52 @ x=54 | 0 | + +Il box è **centrato**, quindi da 60 colonne in su non resta esposto niente. E le +16 colonne a larghezza 40 sono dell'**intestazione**, non della tabella: la riga +esposta più larga è il titolo. Quindi §2.1 è la causa della maggior parte del +sintomo di §2.2, ed è per questo che va fatto prima. + +**L'attenuazione del fondo è invisibile a questa suite.** Misurato: ristilare il +segmento esposto con `th.Help` produce output **byte-identico** a oggi sotto +`termenv.Ascii`, che `TestMain` fissa per tutto il package — sia avvolgendo il +segmento sia strippandolo prima. Nessun golden potrebbe distinguerla. È la +trappola della #141, dove la prova ha dovuto essere un grep. + +### 2.3 L'allineamento del budget (#144) + +`budgetLayout` misura `pctW` come massimo delle larghezze delle etichette +percentuale sulle righe vere, lo usa nell'aritmetica che dimensiona la colonna +nome, e poi il rendering non ci allinea niente. Con righe a `62%` e `104%` le +cifre della prima partono una colonna più a sinistra. Mai un overflow — +l'aritmetica riserva per la più larga, quindi sbagliare per difetto è sicuro. + +`renderBudgetBar` produce barra e percentuale come una stringa sola e **non +conosce `pctW`**: è il motivo per cui l'allineamento non è mai avvenuto. + +## 3. Gli interventi + +### 3.1 `renderBudgetBar` riceve la larghezza della percentuale (#144) + +```go +func renderBudgetBar(th theme, percentUsed float64, pctW int) string +``` + +La percentuale viene allineata a destra dentro `pctW` colonne. `pctW <= 0` +significa larghezza naturale, che è il comportamento di oggi: i due call site di +test che non stanno misurando l'allineamento passano `0` e restano validi. + +Il chiamante di produzione (`budget.go:149`) passa il `pctW` che `budgetLayout` +ha già misurato — la stessa quantità, non una ricalcolata, così le due non +possono divergere. + +**L'etichetta continua ad arrotondare per difetto** (#136): il riempimento +tronca, quindi un'etichetta che arrotondasse per eccesso rivendicherebbe una +soglia che la barra non ha raggiunto. Non toccare quella parte. + +Le cifre restano non paddate: una volta fissata la percentuale sono sgranate a +destra e non a sinistra, e il box riempie comunque il bordo destro. Misurare +prima di cambiare idea. + +### 3.2 L'intestazione del report viene troncata (#143, seconda metà) + +In `reportModel.view`, il testo del titolo viene troncato con `truncateWidth` +**prima** di `th.Title.Render`, così anche la riga di margine si accorcia. + +`width <= 0` (primo render, prima del `WindowSizeMsg`) lascia il titolo intatto: +`truncateWidth(s, 0)` ritorna `""` e cancellerebbe l'intestazione. È lo stesso +fallback che `reportItemWidth` e `budgetLayout` hanno già. + +### 3.3 La colonna residua accanto al box (#143, prima metà) + +**Da decidere dopo §3.2, con i numeri nuovi.** Fatto il clamp +dell'intestazione, la riga più larga del body diventa la tabella e l'esposizione +attesa scende da 16 colonne a una. Il task rimisura e riporta; se resta una +colonna sola, il ritaglio è una riga di codice con un golden che lo prova, e va +fatto. Se resta di più, il task si ferma e riporta invece di decidere. + +Il ritaglio è sicuro in principio: l'unica cosa mai esposta è contenuto che già +sfonda il terminale. Ma va deciso contro una misura, non contro un'aspettativa. + +## 4. Fuori scope (dichiarato) + +- Il padding delle **cifre** del budget (§3.1): sgranate a destra, dove il box + riempie comunque. +- L'overflow di `summary` e della nota billable con contenuti molto lunghi + (§2.1): reale ma non in queste issue. +- L'**attenuazione** del fondo (§2.2): non implementabile in modo verificabile + da questa suite. + +## 5. Test + +- Ogni difetto ha un test **visto fallire** contro il difetto, col transcript. +- I test di larghezza usano le larghezze **misurate** in §2, non larghezze + scelte per prudenza: la tranche D ha prodotto tre test che asserivano sotto il + minimo raggiungibile dal codice corretto e quindi non potevano passare. +- Golden rigenerati con `go test ./internal/tui -update`, mai a mano, e + **guardati** dopo. +- Gate: `gofmt -l .`, `go vet ./...`, + `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`, `go build ./...`, + `go test ./... -race`. + +## 6. Vincoli globali + +- `internal/report` e `internal/duration` restano **puri**. +- Nessuna funzione di stile in produzione chiama `lipgloss.NewStyle()`. +- Tutto ciò che vive nel repo è in **inglese** — codice, commenti, stringhe UI, + nomi e messaggi dei test, messaggi di commit. Eccezione: i doc di design qui + sotto `docs/superpowers/`. +- **Ogni numero scritto in un commento va misurato rendendo.** La tranche D ha + corretto quattordici commenti che affermavano cose false. +- Conventional Commits. **MAI** `Co-Authored-By`. From c2b3c783c444c6e53bd9d1b96c0051f75547c464 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 16:11:05 +0200 Subject: [PATCH 02/10] docs: fold the adversarial review of the follow-up spec and plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review found three critical defects, all mine: - The exposure formula measured the wrong rows. The widest body line is the title at row 0, which sits ABOVE the box (paletteTopY = 2, covering rows 2-13), so "16 columns exposed beside the box" was the title's own overflow attributed to the compositor. Measured properly, iterating only over covered rows: the exposure is 1 column at width 40 and 0 everywhere else, it is the report table's right border, and clamping the header does not change it. The Task 3 dependency on Task 2 was an artifact of the same formula. - Clipping that column would break composite's documented layering contract (overlay.go:11-13) and three tests that pin it, and the safety argument for it was false: after the header clamp nothing overflows the terminal, and on a wide terminal the body legitimately extends between the box and the screen edge. So #143's compositor half closes without code, with a test that pins the column as intended. - Both prescribed budget tests could never pass. One anchored the figure column on "/" with fixture rows whose Billed values differ in width, asserting an alignment the spec deliberately does not provide; the other asserted the absence of a separator space that the prescribed implementation always emits. Also corrected: "dimming is unverifiable by this suite" is false — TestCompositeDoesNotLeakStyleIntoTheBox builds its own colored renderer for exactly the failure mode goldens cannot see. It stays cut, for the right reason. --- .../plans/2026-07-30-layout-followups.md | 321 ++++++++++-------- .../2026-07-30-layout-followups-design.md | 119 +++++-- 2 files changed, 261 insertions(+), 179 deletions(-) diff --git a/docs/superpowers/plans/2026-07-30-layout-followups.md b/docs/superpowers/plans/2026-07-30-layout-followups.md index f7758d9..68aeab3 100644 --- a/docs/superpowers/plans/2026-07-30-layout-followups.md +++ b/docs/superpowers/plans/2026-07-30-layout-followups.md @@ -4,11 +4,14 @@ **Goal:** Chiudere #144 e #143, i due follow-up della tranche D. -**Architecture:** Tre interventi indipendenti, in un ordine che non è -arbitrario: il Task 2 cambia i numeri che il Task 3 deve misurare. - **Spec:** `docs/superpowers/specs/2026-07-30-layout-followups-design.md` +> **Questa è la seconda versione.** La prima è stata bocciata da una review +> adversariale che ha trovato tre difetti critici: due test che non potevano +> passare contro il codice corretto, e una formula di misura che guardava le +> righe sbagliate e da cui dipendeva l'intero Task 3. Le correzioni sono +> incorporate qui e nella spec §2.2/§3.3. + ## Global Constraints - `internal/report` e `internal/duration` restano **puri**: nessun I/O, nessun @@ -22,67 +25,92 @@ arbitrario: il Task 2 cambia i numeri che il Task 3 deve misurare. - Tutto in **inglese**, ortografia americana: codice, identificatori, commenti, nomi e messaggi dei test, messaggi di commit. - Conventional Commits. **MAI** `Co-Authored-By`. -- **Le larghezze si misurano rendendo, non calcolando.** Usa - `lipgloss.Width(strings.Split(, "\n")[0])`, o il ciclo su tutte le - righe quando quella più larga non è la prima. La tranche D ha prodotto due - numeri sbagliati perché sommati a mano: `reportAmountWidth` riprende spazio ad - Amount come ultima risorsa, e l'aritmetica a mano non lo vede. +- **Le larghezze si misurano rendendo, non calcolando**, e si misurano sulle + righe **giuste**: la prima versione di questo piano ha misurato lo sforamento + del titolo credendo di misurare l'esposizione accanto al box, perché il titolo + è la riga più larga del body ma sta sopra il box, non accanto. - **Un test scritto contro un bug non è attendibile finché non lo si è visto - fallire contro quel bug**, e non deve asserire sotto il minimo che il codice - corretto può raggiungere: nella tranche D tre test asserivano l'impossibile. -- **Ogni numero scritto in un commento va misurato.** La tranche D ha corretto - quattordici commenti falsi. + fallire contro quel bug**, e non deve asserire qualcosa che il codice corretto + non può dare: due dei test della prima versione asserivano un allineamento che + la spec dichiara fuori scope, e uno asseriva l'assenza di uno spazio che + l'implementazione prescritta emette sempre. +- **Ogni numero scritto in un commento va misurato.** -## Helper esistenti (verificati) +## Helper esistenti (verificati contro il repo) -- `truncateWidth(s string, cols int) string` e `cell(s string, cols int) string` +- `truncateWidth(s string, cols int) string`, `cell(s string, cols int) string` — `internal/tui/width.go`. `cols <= 0` ritorna `""` in entrambi. - `testTheme(dark bool) theme` — `theme_test.go:15`. -- `goldenReport()`, `goldenPaletteModel()`, `golden(t, name, out)` — `golden_test.go`. -- `newReport(r report.Report, note string, series []float64) reportModel`. -- `reportModel.view(th theme, width int) string` — **già riceve la larghezza** - (`report.go:198`), chiamata da `app.go:980`. +- `golden(t, name, out)` — `golden_test.go:35`; `goldenReport()` — `:70`; + `goldenPaletteModel()` — `:397`. +- `newReport(r report.Report, note string, daily []float64) reportModel` + — `report.go:23`. Il terzo parametro si chiama **`daily`**. +- `reportModel.view(th theme, width int) string` — `report.go:198`, **già riceve + la larghezza**; chiamata da `app.go:980`. - `renderBudgetBar(th theme, percentUsed float64) string` — `budget.go:60`. Chiamanti: produzione `budget.go:149`, test `budget_test.go:43` e `budget_bar_test.go:57`. - `budgetLayout(lines []report.BudgetLine, width int) (nameW int, showRemaining bool)` - — `budget.go:116`. Misura `pctW` internamente e oggi non lo espone. + — `budget.go:116`. **Chiamanti di test: `budget_test.go:163`, `:171`, `:180`.** + Cambiarne la firma non compila finché non si toccano tutti e tre. +- `composite(body, box string, x, y int) string` — `internal/tui/overlay.go:30`. + Il suo contratto è documentato alle righe 11-13 e inchiodato da + `TestCompositeSplicesTheBoxIntoTheBody`, + `TestCompositeKeepsEveryLineWidth`, + `TestCompositeHandlesWideGlyphsOnBothEdges` e + `TestCompositeDoesNotLeakStyleIntoTheBox` (`overlay_test.go`). +- `paletteTopY = 2` — `palette.go:19`: il box comincia alla riga 2 del body. ## Ordine dei task -`1` e `2` sono indipendenti. **`3` dipende da `2`**: il clamp -dell'intestazione cambia la riga più larga del body, e quindi l'esposizione che -il Task 3 deve misurare. +`1`, `2` e `3` sono **indipendenti**. La prima versione dichiarava `3` dipendente +da `2`; era un artefatto della formula sbagliata — la riga coperta più larga è la +tabella, non il titolo, quindi il clamp del titolo non cambia niente per il +Task 3 (spec §2.2). --- ### Task 1: allineare la percentuale del budget (#144) **Files:** -- Modify: `internal/tui/budget.go` (`renderBudgetBar`, `budgetLayout`, `view`) -- Modify: `internal/tui/budget_test.go:43`, `internal/tui/budget_bar_test.go:57` - (il terzo argomento) +- Modify: `internal/tui/budget.go` (`renderBudgetBar`, la misura di `pctW`, `view`) +- Modify: `internal/tui/budget_test.go` — il terzo argomento a riga 43, e i tre + chiamanti di `budgetLayout` a righe **163, 171, 180** se ne cambi la firma +- Modify: `internal/tui/budget_bar_test.go:57` — il terzo argomento - Test: `internal/tui/budget_test.go` - Test: `internal/tui/testdata/budget.golden` **Interfaces:** -- Produces: `renderBudgetBar(th theme, percentUsed float64, pctW int) string`; - `budgetLayout` espone anche `pctW`. +- Produces: `renderBudgetBar(th theme, percentUsed float64, pctW int) string`. -- [ ] **Step 1: Write the failing test** +- [ ] **Step 1: Il test che si vede fallire, con la fixture giusta** + +La fixture è scelta, non casuale, e il perché va nel commento: i due `Billed` +hanno la **stessa** larghezza (`625.00` e `940.00`, sei caratteri) mentre le due +percentuali no (`62%` e `104%`). Così la posizione dello `/` isola esattamente il +padding della percentuale. Con `Billed` di larghezze diverse lo `/` divergerebbe +comunque, e il test asserirebbe l'allineamento delle **cifre**, che la spec §4 +dichiara fuori scope — cioè non potrebbe passare mai. ```go // budgetLayout measures the widest percentage label across the real rows and // uses it to size the name column, but the rendering never padded to it: a row -// at 62% started its figures one column left of a row at 104% (#144). The -// fixture needs percentages of DIFFERENT label widths — with two rows both -// under 100% the columns line up by accident and the test passes against the bug. +// at 62% started its figures one column left of a row at 104% (#144). +// +// The fixture is picked, not arbitrary. Both Billed values are six characters +// wide while the two percentage labels are three and four, so the column of the +// "/" isolates the percentage padding and nothing else. With Billed values of +// different widths the "/" would diverge anyway, and this test would be +// asserting that the FIGURES line up — which the spec deliberately does not do. +// +// Measured before the fix: the "/" sits at column 65 on the 62% row and 66 on +// the 104% row. func TestBudgetViewAlignsFiguresAcrossRows(t *testing.T) { t.Parallel() th := testTheme(true) bm := newBudget([]report.BudgetLine{ {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, - {ListName: "Mobile app", Billed: 1040, Budget: 1000, Currency: "EUR", Remaining: -40, PercentUsed: 104}, + {ListName: "Mobile app", Billed: 940, Budget: 900, Currency: "EUR", Remaining: -40, PercentUsed: 940.0 / 900 * 100}, }) var cols []int for _, line := range strings.Split(bm.view(th, 100), "\n") { @@ -99,8 +127,12 @@ func TestBudgetViewAlignsFiguresAcrossRows(t *testing.T) { } // A percentage narrower than the measured maximum is right-aligned into it, so -// the columns after it hold. pctW <= 0 keeps the natural width, which is what +// every column after it holds. pctW <= 0 keeps the natural width, which is what // the callers that are not testing alignment pass. +// +// The natural case is checked by WIDTH, not by suffix: renderBudgetBar always +// emits one space between the bar and the label, so "ends in \" 62%\"" is true +// of every conforming implementation and would assert nothing. func TestRenderBudgetBarRightAlignsThePercentage(t *testing.T) { t.Parallel() th := testTheme(true) @@ -109,41 +141,39 @@ func TestRenderBudgetBarRightAlignsThePercentage(t *testing.T) { if lipgloss.Width(wide) != lipgloss.Width(narrow) { t.Errorf("bars are %d and %d columns wide, want equal", lipgloss.Width(wide), lipgloss.Width(narrow)) } - if !strings.HasSuffix(narrow, " 62%") { - t.Errorf("narrow = %q, want it to end in a right-aligned \" 62%%\"", narrow) + if !strings.HasSuffix(narrow, " 62%") { + t.Errorf("narrow = %q, want the label right-aligned into 4 columns", narrow) } natural := renderBudgetBar(th, 62.5, 0) - if !strings.HasSuffix(natural, "62%") || strings.HasSuffix(natural, " 62%") { - t.Errorf("natural = %q, want the unpadded \"62%%\" when pctW <= 0", natural) + if got, want := lipgloss.Width(natural), budgetBarWidth+1+3; got != want { + t.Errorf("natural width = %d, want %d (bar + separator + \"62%%\", no padding)", got, want) } } ``` -- [ ] **Step 2: See the RED** +- [ ] **Step 2: Vedere il RED** -I due test nominano `renderBudgetBar` con tre argomenti, quindi il package non -compila: `FAIL [build failed]` non è un RED. Per **vedere** il difetto, aggiungi -solo `TestBudgetViewAlignsFiguresAcrossRows` (che usa la firma attuale di `view`) -ed eseguilo. +`TestRenderBudgetBarRightAlignsThePercentage` nomina `renderBudgetBar` con tre +argomenti, quindi il package non compila: `FAIL [build failed]` non è un RED. +Aggiungi quindi **solo** `TestBudgetViewAlignsFiguresAcrossRows`, che usa la +firma attuale di `view`, ed eseguilo. Run: `go test ./internal/tui -run TestBudgetViewAlignsFiguresAcrossRows -v` -Expected: FAIL, con due colonne diverse. **Riporta i due numeri** nel report: -sono la misura del difetto. - -- [ ] **Step 3: Implement** +Expected: FAIL con `figures start at column 65 on one row and 66 on the other`. +**Se i numeri non sono 65 e 66, fermati**: la fixture non è quella misurata. -`renderBudgetBar` prende `pctW` e allinea a destra: +- [ ] **Step 3: Implementare** ```go // pctW is the width the caller measured for the widest percentage label across // all rows; the label is right-aligned into it so every row's figures start at // the same column (#144). pctW <= 0 means natural width — the callers that are -// not testing alignment pass 0. +// not testing alignment pass 0, and so does the pre-WindowSizeMsg path below. func renderBudgetBar(th theme, percentUsed float64, pctW int) string { ... label := fmt.Sprintf("%.0f%%", math.Floor(percentUsed)) - if pctW > lipgloss.Width(label) { - label = strings.Repeat(" ", pctW-lipgloss.Width(label)) + label + if pad := pctW - lipgloss.Width(label); pad > 0 { + label = strings.Repeat(" ", pad) + label } return fmt.Sprintf("%s %s", bar, label) } @@ -153,17 +183,22 @@ Non toccare `math.Floor`: il riempimento tronca, quindi la label deve arrotondare per difetto o rivendicherebbe una soglia che la barra non ha raggiunto (#136). -`budgetLayout` espone il `pctW` che già misura, invece di lasciarlo interno. La -firma diventa `(nameW int, pctW int, showRemaining bool)` — oppure estrai la -misura in un helper che sia `budgetLayout` sia `view` chiamano. Scegli tu, ma la -quantità che `view` passa a `renderBudgetBar` deve essere **la stessa** che -l'aritmetica di `budgetLayout` ha usato, non una ricalcolata: se divergono, la -riga sfora di quanto divergono. Se estrai un helper, verifica che entrambi lo -chiamino e non ne resti una copia. +`view` deve passare **la stessa** `pctW` che l'aritmetica di `budgetLayout` ha +usato, non una ricalcolata: se divergono, la riga sfora di quanto divergono. Due +strade, scegli tu: -`view` passa `pctW` alla `renderBudgetBar`. I due call site di test passano `0`. +- allargare la firma di `budgetLayout` a `(nameW, pctW int, showRemaining bool)` + — ricorda i tre chiamanti di test a `budget_test.go:163, 171, 180`, o non + compila; +- estrarre la misura in un helper che **entrambe** chiamano — verifica che non + resti una copia della formula. -- [ ] **Step 4: Run and regenerate the golden** +**Vincolo che vale per entrambe:** a `width <= 0` (primo render, prima del +`WindowSizeMsg`) `budgetLayout` ritorna il fallback senza misurare niente, quindi +la label resta **naturale**, `pctW = 0`. Non misurare `pctW` in quel ramo: le due +strade divergerebbero là, e nessun test lo coprirebbe. + +- [ ] **Step 4: Eseguire e rigenerare il golden** ```bash go test ./internal/tui -run 'TestBudget|TestRenderBudgetBar' -v @@ -171,8 +206,8 @@ go test ./internal/tui -race go test ./internal/tui -update git diff internal/tui/testdata/ ``` -Expected: `budget.golden` ha una riga sola con `62%`, quindi `pctW` è 3 e non -cambia niente — **verificalo**, non assumerlo. Se cambia, guarda cosa è +Expected: `budget.golden` **non si muove** — ha una riga sola, al 62%, quindi +`pctW` è 3 e il padding è un no-op. Verificalo; se si muove, guarda cosa è cambiato e riportalo. - [ ] **Step 5: Gate + commit** @@ -190,12 +225,12 @@ git commit -m "fix(tui): right-align the budget percentage so the figures line u **Files:** - Modify: `internal/tui/report.go` (`reportModel.view`, il titolo a riga ~204) - Test: `internal/tui/report_test.go` -- Test: i golden stretti che mostrano l'intestazione +- Test: `internal/tui/testdata/palette_narrow.golden` **Interfaces:** -- Consumes: `truncateWidth` (`width.go`). +- Consumes: `truncateWidth`. -- [ ] **Step 1: Write the failing test** +- [ ] **Step 1: Il test che si vede fallire** ```go // The report table became width-aware in #66 and the budget screen in #136, but @@ -204,6 +239,10 @@ git commit -m "fix(tui): right-align the budget percentage so the figures line u // it measures without one). Line 1 is the title's own MarginBottom padding and // overflows with it, which is why the text is truncated BEFORE th.Title.Render // rather than after — truncating the rendered string would leave that line long. +// +// Measured: every other line of this view already fits at width 40 (summary 37, +// billable note 38, table 39, sparkline 15), so this test can assert on the +// whole view without depending on anything the spec left out of scope. func TestReportViewHeaderFitsTheTerminal(t *testing.T) { t.Parallel() th := testTheme(true) @@ -230,17 +269,14 @@ func TestReportViewHeaderSurvivesZeroWidth(t *testing.T) { } ``` -- [ ] **Step 2: See the RED** +- [ ] **Step 2: Vedere il RED** Run: `go test ./internal/tui -run TestReportViewHeader -v` -Expected: il primo FAIL su width 40 e 60, riportando **68** colonne per la riga -0 e per la riga 1 — quattro errori in tutto. Il secondo PASS (oggi il titolo non -è troncato, quindi non può sparire). **Allega il transcript**: se i numeri non -sono 68, la fixture non è quella misurata dalla spec. +Expected: il primo FAIL **quattro volte** — righe 0 e 1 a width 40, righe 0 e 1 a +width 60, ciascuna con **68** colonne. Il secondo PASS. Se i numeri non sono +questi, fermati e riporta. -- [ ] **Step 3: Implement** - -In `reportModel.view`, prima del `Render`: +- [ ] **Step 3: Implementare** ```go // Clamped before Render, not after: th.Title carries MarginBottom(1), which @@ -256,7 +292,7 @@ In `reportModel.view`, prima del `Render`: title := th.Title.Render(head) ``` -- [ ] **Step 4: Run and regenerate the goldens** +- [ ] **Step 4: Eseguire e rigenerare i golden** ```bash go test ./internal/tui -run TestReportViewHeader -v @@ -264,10 +300,9 @@ go test ./internal/tui -race go test ./internal/tui -update git diff internal/tui/testdata/ ``` -Expected: si muovono i golden resi a una larghezza minore del titolo — -`palette_narrow` fra questi. Quelli a 80 non si muovono (68 ≤ 80). **Guarda ogni -golden cambiato** e riporta quali e come. Se un golden a 80 si muove, fermati: -significa che il clamp scatta quando non dovrebbe. +Expected: si muove **solo `palette_narrow.golden`** — è l'unico golden reso a una +larghezza minore del titolo. Nessun golden a 80 si muove. **Guardalo.** Se si +muove un golden a 80, il clamp scatta quando non dovrebbe. - [ ] **Step 5: Gate + commit** @@ -279,23 +314,24 @@ git commit -m "fix(tui): clamp the report header to the terminal width (#143)" --- -### Task 3: rimisurare l'esposizione accanto al box, e decidere (#143, prima metà) +### Task 3: inchiodare la colonna esposta come comportamento voluto (#143, prima metà) + +**Nessuna modifica al codice di produzione.** La spec §3.3 chiude questa metà +della issue contro una misura: l'esposizione accanto al box è **una colonna** a +larghezza 40 e **zero** altrove, è il bordo destro della tabella — contenuto +legittimo dentro il terminale — e ritagliarla violerebbe il contratto di layering +della #59, rompendo tre test di `overlay_test.go`. -> **Dipende dal Task 2.** Il clamp dell'intestazione cambia la riga più larga -> del body, che è la quantità da misurare qui. Non eseguirlo prima. +Quello che manca è un test che lo **dica**, così che chi vedrà quella colonna nel +golden trovi scritto perché c'è invece di ri-aprire la issue. **Files:** -- Modify: `internal/tui/overlay.go` (`composite`) — **solo se la misura lo dice** - Test: `internal/tui/overlay_test.go` -- Test: `internal/tui/testdata/palette_narrow.golden` -- [ ] **Step 1: Misurare, prima di scrivere codice** +- [ ] **Step 1: Misurare, e confermare i numeri della spec** -Con una sonda usa-e-getta che poi cancelli, misura per ogni larghezza in -`{40, 50, 60, 80, 120}`: la riga più larga del body, la larghezza del box, il suo -`x`, e quante colonne del body restano esposte a destra del box -(`widestBodyLine - (x + boxW)`, se positivo). Prendi body, box, `x` e `y` dal -percorso vero: +Con una sonda usa-e-getta che poi cancelli, per ogni larghezza in +`{40, 50, 60, 80, 120}`, prendendo body, box, `x` e `y` dal percorso vero: ```go m := goldenPaletteModel() @@ -304,50 +340,43 @@ body := m.screenBody() box, x, y := m.palette.layout(m.theme, m.width, m.height, strings.Count(body, "\n")+1) ``` -Prima del Task 2 la misura era: 16 colonne esposte a larghezza 40, zero a ogni -larghezza da 60 in su. +misura **due** quantità distinte e riportale entrambe: -**Riporta la tabella nel report, e poi segui la misura:** +- la riga più larga di **tutto** il body, e quale indice ha; +- la riga più larga fra quelle che il box **copre davvero**, cioè con indice in + `[y, y+len(boxLines))`, e quanto ne resta esposto a destra + (`larghezza − (x + boxW)`, se positivo). -- **Se l'esposizione massima è ≤ 1 colonna** a ogni larghezza: implementa il - ritaglio (Step 2). È il caso atteso. -- **Se è > 1 colonna** a qualche larghezza: **fermati**, riporta la tabella e - quale contenuto è esposto, e non implementare niente. Vuol dire che il Task 2 - non ha coperto tutta la causa e la decisione torna all'umano. +Expected, dalla spec §2.2: la riga più larga del body è sempre la **0** (il +titolo, 54 colonne) e sta **sopra** il box, che comincia a `y=2`; la riga coperta +più larga è sempre la **7** (la tabella, 39 colonne); l'esposizione accanto al box +è **1** a larghezza 40 e **0** a 50, 60, 80 e 120. -- [ ] **Step 2: Il ritaglio, solo se la misura lo autorizza** +**Se i numeri non sono questi, fermati e riporta** invece di scrivere il test: la +spec ha già sbagliato una volta questa misura e la seconda versione va confermata, +non creduta. -Il segmento destro di `composite` non viene disegnato quando ciò che esporrebbe -è contenuto che sfonda comunque il terminale. Attenzione: `composite` non conosce -la larghezza del terminale, conosce solo body e box. Il ritaglio quindi non può -essere «oltre il terminale», deve essere «oltre il bordo destro del box» — che è -la stessa cosa solo quando il body non è più largo del terminale, cioè dopo il -Task 2. +- [ ] **Step 2: Il test che inchioda il comportamento** -Scrivi il perché nel commento, e scrivi **anche** il caso in cui questa scelta -sarebbe sbagliata: se un giorno il body tornasse a essere più largo del -terminale, il ritaglio nasconderebbe contenuto invece di detriti. +Un test in `overlay_test.go` che asserisce che, a larghezza 40, la colonna del +body oltre il bordo destro del box **c'è** e vale esattamente una colonna, con un +commento che spiega: è il bordo destro della tabella, sta dentro il terminale, e +il contratto della #59 dice che il body fuori dal rettangolo del box sopravvive +verbatim. Nomina i tre test che cadrebbero se qualcuno decidesse di ritagliare. -Il test deve inchiodare che il fondo esposto **non** compare, con una fixture in -cui prima compariva. Provalo per mutazione: rimetti il segmento destro e verifica -che il test fallisca. +Il test non ha un red-green — non corregge un difetto, fissa una scelta. La sua +prova è che fallisca se il comportamento cambia: **provalo** rendendo `composite` +un ritaglio (elimina il segmento destro), verifica che il tuo test nuovo +fallisca **e** che falliscano anche i tre test esistenti, poi ripristina. Metti +entrambi gli output nel report: sono la dimostrazione che il contratto è +protetto da più di un punto. -- [ ] **Step 3: Golden e gate** +- [ ] **Step 3: Gate + commit** ```bash -go test ./internal/tui -race -go test ./internal/tui -update -git diff internal/tui/testdata/palette_narrow.golden gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race -``` -**Guarda il golden.** Deve mostrare il box senza i caratteri di cornice spuri a -destra. - -- [ ] **Step 4: Commit** - -```bash git add internal/tui -git commit -m "fix(tui): stop drawing the background past the overlay's right edge (#143)" +git commit -m "test(tui): pin the body column beside the overlay as intended (#143)" ``` --- @@ -357,12 +386,16 @@ git commit -m "fix(tui): stop drawing the background past the overlay's right ed **Files:** - Modify: `CHANGELOG.md` -- [ ] **Step 1** Sotto `## [Unreleased]`, sezione `### Fixed`, una voce per issue - chiusa, nello stile delle voci già presenti: comportamento osservato, non - implementazione, e **niente garanzie assolute** — la tranche D ne ha dovuta - correggere una che prometteva «the table now always fits the terminal width». - Se il Task 3 si è fermato senza implementare, la voce sulla #143 copre solo il - clamp dell'intestazione e la issue resta aperta. +- [ ] **Step 1** Sotto `## [Unreleased]` → `### Fixed`, una voce per issue, + nello stile delle voci già presenti: comportamento osservato, non + implementazione, e **niente garanzie assolute** — la tranche D ha dovuto + correggere una voce che prometteva «the table now always fits the terminal + width». Due voci: + - il report non manda più a capo la propria intestazione su un terminale + stretto (#143); + - le cifre del burn-down budget si allineano fra le righe (#144). + + Non scrivere una voce per il Task 3: non cambia niente di osservabile. - [ ] **Step 2** Gate completo, poi commit `docs: changelog for the layout follow-ups`. @@ -370,24 +403,24 @@ git commit -m "fix(tui): stop drawing the background past the overlay's right ed ## Self-review del piano -**Copertura:** spec §3.1 → Task 1; §3.2 → Task 2; §3.3 → Task 3; §5 è vincolo -globale; §4 (fuori scope) non genera task. +**Copertura:** spec §3.1 → Task 1; §3.2 → Task 2; §3.3 → Task 3; §4 (fuori +scope) non genera task. -**Dipendenze:** `3` dipende da `2` ed è dichiarato in testa al task, non solo -qui. `1` è indipendente da entrambi. +**Dipendenze:** nessuna. La prima versione dichiarava `3` dipendente da `2`; la +misura corretta mostra che la riga coperta più larga è la tabella, quindi il +clamp del titolo non tocca il Task 3. -**Il RED di ogni task è osservabile:** il Task 1 mette in staging il test che -compila contro la firma attuale; il Task 2 non ha simboli nuovi, quindi il RED è -diretto; il Task 3 non ha un RED perché è una misura che decide se c'è un -difetto da correggere — e il suo test nasce dopo la misura, con la prova per -mutazione al posto del red-green. +**Ogni test prescritto può passare contro il codice corretto**, che è la cosa che +la prima versione sbagliava due volte: la fixture del Task 1 ha `Billed` di pari +larghezza perché l'allineamento delle cifre è fuori scope, e l'asserzione sul caso +naturale guarda la **larghezza** e non un suffisso che l'implementazione emette +sempre. -**Le larghezze asserite** vengono tutte da §2 della spec, misurata rendendo. Il -Task 2 asserisce a 40, 60 e 80: le prime due sono le larghezze in cui oggi sfora, -la terza è il controllo che il clamp non scatti quando non serve. +**Ogni RED è osservabile:** Task 1 mette in staging il test che compila con la +firma attuale; Task 2 non introduce simboli nuovi; Task 3 non ha un RED per +costruzione, e lo dice, con la prova per mutazione al suo posto. -**Placeholder:** nessuno. Il Task 1 lascia una scelta esplicita -all'implementatore (esporre `pctW` da `budgetLayout` o estrarre un helper) con il -vincolo che la decide: la quantità passata a `renderBudgetBar` deve essere la -stessa usata dall'aritmetica. Il Task 3 è deliberatamente condizionale, e la -condizione è un numero misurato, non un giudizio. +**Placeholder:** nessuno. Il Task 1 lascia una scelta con due vincoli espliciti +(i tre chiamanti di test, e `pctW = 0` a `width <= 0`); il Task 3 chiede una +conferma di misura prima di scrivere il test, e dice di fermarsi se la misura +smentisce la spec. diff --git a/docs/superpowers/specs/2026-07-30-layout-followups-design.md b/docs/superpowers/specs/2026-07-30-layout-followups-design.md index d9ae141..3187092 100644 --- a/docs/superpowers/specs/2026-07-30-layout-followups-design.md +++ b/docs/superpowers/specs/2026-07-30-layout-followups-design.md @@ -44,27 +44,46 @@ Tre cose che la issue non diceva: ### 2.2 L'esposizione del fondo accanto al box (#143, prima metà) -Misurato rendendo dal percorso vero di `View()` (verificato: la variante «oggi» -riproduce `View()` byte per byte), per ogni larghezza: - -| terminale | box | esposto a destra | -|---|---|---| -| 40 | 36 @ x=2 | **16 colonne** | -| 60 | 52 @ x=4 | 0 | -| 80 | 52 @ x=14 | 0 | -| 120 | 52 @ x=34 | 0 | -| 160 | 52 @ x=54 | 0 | - -Il box è **centrato**, quindi da 60 colonne in su non resta esposto niente. E le -16 colonne a larghezza 40 sono dell'**intestazione**, non della tabella: la riga -esposta più larga è il titolo. Quindi §2.1 è la causa della maggior parte del -sintomo di §2.2, ed è per questo che va fatto prima. - -**L'attenuazione del fondo è invisibile a questa suite.** Misurato: ristilare il -segmento esposto con `th.Help` produce output **byte-identico** a oggi sotto -`termenv.Ascii`, che `TestMain` fissa per tutto il package — sia avvolgendo il -segmento sia strippandolo prima. Nessun golden potrebbe distinguerla. È la -trappola della #141, dove la prova ha dovuto essere un grep. +**Questa sezione è stata riscritta dopo una review: la prima versione misurava +la cosa sbagliata, e il numero che ne era uscito è finito anche in un commento +sulla issue.** Vale la pena raccontare l'errore, perché è istruttivo. + +La formula era `rigaPiùLargaDelBody − (x + boxW)`. Ma la riga più larga del body +è la **0**, il titolo, e il box comincia a `paletteTopY = 2`: il titolo sta +**sopra** il box, non accanto. La formula misurava quindi lo sforamento del +titolo — cioè il difetto di §2.1 — e lo attribuiva al compositore. + +Misurato di nuovo, distinguendo le due quantità, iterando solo sulle righe che +il box copre davvero (`y .. y+len(boxLines)-1`, cioè 2..13): + +| terminale | box | riga più larga del body | la vecchia formula diceva | esposto **accanto al box** | +|---|---|---|---|---| +| 40 | 36 @ x=2 | riga 0, 54 col | 16 | **1** | +| 50 | 46 @ x=2 | riga 0, 54 col | 6 | **0** | +| 60 | 52 @ x=4 | riga 0, 54 col | 0 | **0** | +| 80 | 52 @ x=14 | riga 0, 54 col | 0 | **0** | +| 120 | 52 @ x=34 | riga 0, 54 col | 0 | **0** | + +La riga coperta più larga è sempre la **7**, la tabella a 39 colonne, a ogni +larghezza. Da cui tre conseguenze: + +- **L'esposizione accanto al box è una colonna sola, e solo a larghezza 40.** È + il bordo destro della tabella, contenuto legittimo dentro il terminale. +- **Il clamp dell'intestazione non la cambia**, perché la riga coperta più larga + è la tabella e non il titolo. La dipendenza §3.3 → §3.2 che la prima versione + dichiarava era un artefatto della formula sbagliata. +- **La vecchia formula sbagliava anche a 50 colonne**, dove diceva 6: quella + larghezza non era nello sweep originale, che saltava da 40 a 60. + +**Sull'attenuazione, una seconda correzione.** La prima versione diceva che +attenuare il fondo è «invisibile a questa suite» perché sotto `termenv.Ascii` +l'output è byte-identico. La premessa è vera per i **golden**, ma la conclusione +no: `TestCompositeDoesNotLeakStyleIntoTheBox` +(`internal/tui/overlay_test.go:95`) costruisce un renderer proprio con un +profilo colore reale e asserisce sui byte — il suo stesso commento dice che è +«the one failure mode a golden can never see». Con quella tecnica +l'attenuazione **sarebbe** verificabile. Resta tagliata, ma per la ragione +giusta: vedi §3.3. ### 2.3 L'allineamento del budget (#144) @@ -110,25 +129,55 @@ In `reportModel.view`, il testo del titolo viene troncato con `truncateWidth` `truncateWidth(s, 0)` ritorna `""` e cancellerebbe l'intestazione. È lo stesso fallback che `reportItemWidth` e `budgetLayout` hanno già. -### 3.3 La colonna residua accanto al box (#143, prima metà) - -**Da decidere dopo §3.2, con i numeri nuovi.** Fatto il clamp -dell'intestazione, la riga più larga del body diventa la tabella e l'esposizione -attesa scende da 16 colonne a una. Il task rimisura e riporta; se resta una -colonna sola, il ritaglio è una riga di codice con un golden che lo prova, e va -fatto. Se resta di più, il task si ferma e riporta invece di decidere. - -Il ritaglio è sicuro in principio: l'unica cosa mai esposta è contenuto che già -sfonda il terminale. Ma va deciso contro una misura, non contro un'aspettativa. +### 3.3 Il compositore non si tocca (#143, prima metà) + +**Chiuso senza modifiche al codice, e questa volta contro una misura.** Tre +ragioni, in ordine di peso. + +**Non c'è un difetto da correggere.** L'esposizione accanto al box è **una +colonna** a larghezza 40 e **zero** a ogni altra larghezza (§2.2). Quella colonna +è il bordo destro della tabella del report, contenuto legittimo che sta dentro il +terminale. Le sedici colonne che sembravano un problema del compositore erano lo +sforamento del titolo, cioè §3.2. + +**Il ritaglio violerebbe un contratto documentato.** `overlay.go:11-13` dice che +«cells of body outside the box's rectangle survive verbatim — that layering is +the whole point of #59», e tre test lo inchiodano: +`TestCompositeSplicesTheBoxIntoTheBody` pretende il segmento destro, +`TestCompositeKeepsEveryLineWidth` e +`TestCompositeHandlesWideGlyphsOnBothEdges` pretendono righe piene col box in +mezzo. Un ritaglio incondizionato al bordo destro del box li fa fallire tutti e +tre: non è una riga di codice, è un cambio di contratto. + +**E la giustificazione di sicurezza che avevo scritto era falsa.** Dicevo che +«l'unica cosa mai esposta è contenuto che già sfonda il terminale». Dopo §3.2 +niente sfonda più il terminale, e su un terminale largo con un report largo il +body sta legittimamente fra il bordo del box e il bordo dello schermo: il +ritaglio cancellerebbe proprio l'effetto di profondità che il contratto difende. + +**L'attenuazione** resta tagliata perché non risolve niente — non c'è un difetto +da attenuare — non perché non sia verificabile: come dice §2.2, lo sarebbe con +la tecnica di `TestCompositeDoesNotLeakStyleIntoTheBox`. + +Resta un lavoro, e non è codice: aggiungere un test che **inchioda** l'unica +colonna esposta a larghezza 40 come comportamento voluto, così che chi la vedrà +nel golden trovi scritto perché c'è. E chiudere la prima metà della #143 con la +misura. ## 4. Fuori scope (dichiarato) - Il padding delle **cifre** del budget (§3.1): sgranate a destra, dove il box - riempie comunque. + riempie comunque. **Conseguenza per i test:** un test che cerchi la colonna + delle cifre dovrà ancorarsi a qualcosa che non dipende dalla larghezza del + campo Billed, o usare una fixture in cui i Billed hanno la stessa larghezza — + altrimenti asserisce un allineamento che questa spec ha deciso di non dare. - L'overflow di `summary` e della nota billable con contenuti molto lunghi - (§2.1): reale ma non in queste issue. -- L'**attenuazione** del fondo (§2.2): non implementabile in modo verificabile - da questa suite. + (§2.1): reale ma non in queste issue. Misurato che nelle fixture usate dai + test non sforano (37 e 38 colonne a larghezza 40), quindi il fuori-scope non + rompe un test dentro lo scope. +- Il **ritaglio** e l'**attenuazione** del fondo esposto (§3.3): non c'è un + difetto da correggere, e il ritaglio violerebbe il contratto di layering + della #59. ## 5. Test From 355be7f996d8a93e06b962c5224bd5c4704d782c Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 16:27:33 +0200 Subject: [PATCH 03/10] fix(tui): right-align the budget percentage so the figures line up (#144) --- internal/tui/budget.go | 40 ++++++++++++++++++---- internal/tui/budget_bar_test.go | 2 +- internal/tui/budget_test.go | 59 ++++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 9 deletions(-) diff --git a/internal/tui/budget.go b/internal/tui/budget.go index 3a36250..8175594 100644 --- a/internal/tui/budget.go +++ b/internal/tui/budget.go @@ -57,7 +57,12 @@ func budgetFillStyle(th theme, percentUsed float64) lipgloss.Style { // the label is shown UNCLAMPED, so an over-100% burn stays visible in the // number. That asymmetry is the whole point: bubbles/progress clamps the // number too, which is why this screen does not use it. -func renderBudgetBar(th theme, percentUsed float64) string { +// +// pctW is the width the caller measured for the widest percentage label across +// all rows; the label is right-aligned into it so every row's figures start at +// the same column (#144). pctW <= 0 means natural width — the callers that are +// not testing alignment pass 0, and so does the pre-WindowSizeMsg path below. +func renderBudgetBar(th theme, percentUsed float64, pctW int) string { fillPct := min(max(percentUsed, 0), 100) filled := int(fillPct / 100 * budgetBarWidth) bar := budgetFillStyle(th, percentUsed).Render(strings.Repeat(string(gaugeFull), filled)) + @@ -68,7 +73,11 @@ func renderBudgetBar(th theme, percentUsed float64) string { // a full bar from 97.5%, and a full bar means the budget is spent. Above 100 // the label stays unclamped: that asymmetry is the whole point of this // function (see the doc comment). - return fmt.Sprintf("%s %.0f%%", bar, math.Floor(percentUsed)) + label := fmt.Sprintf("%.0f%%", math.Floor(percentUsed)) + if pad := pctW - lipgloss.Width(label); pad > 0 { + label = strings.Repeat(" ", pad) + label + } + return fmt.Sprintf("%s %s", bar, label) } const ( @@ -94,6 +103,19 @@ func budgetFigures(l report.BudgetLine, withRemaining bool) string { return fmt.Sprintf("%.2f / %.2f %s", l.Billed, l.Budget, l.Currency) } +// budgetPercentWidth measures the widest floored-percentage label across the +// rows, e.g. 3 for "62%" and 4 for "104%". budgetLayout uses it to size the +// row and view uses it to right-align renderBudgetBar's label into that same +// size (#144); it lives here, and not inline in either caller, so there is one +// formula for both to agree on rather than two that could drift apart. +func budgetPercentWidth(lines []report.BudgetLine) int { + pctW := 0 + for _, l := range lines { + pctW = max(pctW, lipgloss.Width(fmt.Sprintf("%.0f%%", math.Floor(l.PercentUsed)))) + } + return pctW +} + // budgetLayout resolves the list-name column and whether "(remaining …)" fits. // // Everything except the name is MEASURED from the real rows, because none of it @@ -117,10 +139,7 @@ func budgetLayout(lines []report.BudgetLine, width int) (nameW int, showRemainin if width <= 0 { // before the first WindowSizeMsg nothing is sized against it return budgetNameWidth, true } - pctW := 0 - for _, l := range lines { - pctW = max(pctW, lipgloss.Width(fmt.Sprintf("%.0f%%", math.Floor(l.PercentUsed)))) - } + pctW := budgetPercentWidth(lines) widestFigures := func(withRemaining bool) int { w := 0 for _, l := range lines { @@ -143,10 +162,17 @@ func (bm budgetModel) view(th theme, width int) string { return title + "\n\n" + th.Box.Render("No budgets configured.") } nameW, showRemaining := budgetLayout(bm.lines, width) + // Same pctW budgetLayout's own arithmetic used, not a recomputed one that + // could diverge from it — width <= 0 is budgetLayout's fallback branch, + // which never measures pctW, so the label there stays natural (#144). + pctW := 0 + if width > 0 { + pctW = budgetPercentWidth(bm.lines) + } var rows strings.Builder for _, l := range bm.lines { rows.WriteString(fmt.Sprintf("%s %s %s\n", - cell(l.ListName, nameW), renderBudgetBar(th, l.PercentUsed), budgetFigures(l, showRemaining))) + cell(l.ListName, nameW), renderBudgetBar(th, l.PercentUsed, pctW), budgetFigures(l, showRemaining))) } body := th.Box.Render(strings.TrimRight(rows.String(), "\n")) return title + "\n\n" + body diff --git a/internal/tui/budget_bar_test.go b/internal/tui/budget_bar_test.go index 53962c2..844212b 100644 --- a/internal/tui/budget_bar_test.go +++ b/internal/tui/budget_bar_test.go @@ -54,7 +54,7 @@ var budgetBarFixtures = []struct { func TestBudgetBarLabelAgreesWithTheFill(t *testing.T) { th := testTheme(true) for _, tc := range budgetBarFixtures { - bar := renderBudgetBar(th, tc.pct) + bar := renderBudgetBar(th, tc.pct, 0) full := strings.Count(bar, string(gaugeFull)) if full != tc.wantFull { t.Errorf("pct=%.1f: %d full blocks, want %d (%q)", tc.pct, full, tc.wantFull, bar) diff --git a/internal/tui/budget_test.go b/internal/tui/budget_test.go index c3c5fcb..c29811f 100644 --- a/internal/tui/budget_test.go +++ b/internal/tui/budget_test.go @@ -40,7 +40,7 @@ func TestBudgetViewEmptyShowsMessage(t *testing.T) { // why this screen keeps its own bar. func TestRenderBudgetBarClampsFillNotPercent(t *testing.T) { t.Parallel() - out := renderBudgetBar(testTheme(true), 150) + out := renderBudgetBar(testTheme(true), 150, 0) if !strings.Contains(out, "150%") { t.Errorf("renderBudgetBar(150) = %q, want the unclamped 150%% in the label", out) } @@ -150,6 +150,63 @@ func TestBudgetViewHoldsItsWidthWithWideRunes(t *testing.T) { } } +// budgetLayout measures the widest percentage label across the real rows and +// uses it to size the name column, but the rendering never padded to it: a row +// at 62% started its figures one column left of a row at 104% (#144). +// +// The fixture is picked, not arbitrary. Both Billed values are six characters +// wide while the two percentage labels are three and four, so the column of the +// "/" isolates the percentage padding and nothing else. With Billed values of +// different widths the "/" would diverge anyway, and this test would be +// asserting that the FIGURES line up — which the spec deliberately does not do. +// +// Measured before the fix: the "/" sits at column 65 on the 62% row and 66 on +// the 104% row. +func TestBudgetViewAlignsFiguresAcrossRows(t *testing.T) { + t.Parallel() + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + {ListName: "Mobile app", Billed: 940, Budget: 900, Currency: "EUR", Remaining: -40, PercentUsed: 940.0 / 900 * 100}, + }) + var cols []int + for _, line := range strings.Split(bm.view(th, 100), "\n") { + if i := strings.Index(line, "/"); i >= 0 { + cols = append(cols, lipgloss.Width(line[:i])) + } + } + if len(cols) != 2 { + t.Fatalf("expected two budget rows, found %d figure columns", len(cols)) + } + if cols[0] != cols[1] { + t.Errorf("figures start at column %d on one row and %d on the other", cols[0], cols[1]) + } +} + +// A percentage narrower than the measured maximum is right-aligned into it, so +// every column after it holds. pctW <= 0 keeps the natural width, which is what +// the callers that are not testing alignment pass. +// +// The natural case is checked by WIDTH, not by suffix: renderBudgetBar always +// emits one space between the bar and the label, so "ends in \" 62%\"" is true +// of every conforming implementation and would assert nothing. +func TestRenderBudgetBarRightAlignsThePercentage(t *testing.T) { + t.Parallel() + th := testTheme(true) + wide := renderBudgetBar(th, 104, 4) + narrow := renderBudgetBar(th, 62.5, 4) + if lipgloss.Width(wide) != lipgloss.Width(narrow) { + t.Errorf("bars are %d and %d columns wide, want equal", lipgloss.Width(wide), lipgloss.Width(narrow)) + } + if !strings.HasSuffix(narrow, " 62%") { + t.Errorf("narrow = %q, want the label right-aligned into 4 columns", narrow) + } + natural := renderBudgetBar(th, 62.5, 0) + if got, want := lipgloss.Width(natural), budgetBarWidth+1+3; got != want { + t.Errorf("natural width = %d, want %d (bar + separator + \"62%%\", no padding)", got, want) + } +} + // "(remaining X)" is the most redundant field on the row (it is budget minus // billed), so it is what gives way when the name column would fall below the // floor. One pass: compute with it, and if the name is too narrow, recompute From 563b2798b9f561ae1fea67a3ce86d3b0ff05f7fb Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 16:37:54 +0200 Subject: [PATCH 04/10] fix(tui): clamp the report header to the terminal width (#143) --- internal/tui/report.go | 13 ++++++-- internal/tui/report_test.go | 36 +++++++++++++++++++++ internal/tui/testdata/palette_narrow.golden | 4 +-- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/internal/tui/report.go b/internal/tui/report.go index 2a60d8e..40a8426 100644 --- a/internal/tui/report.go +++ b/internal/tui/report.go @@ -201,8 +201,17 @@ func (rm reportModel) view(th theme, width int) string { // "Local" (time.Local.String()), not a portable IANA name — accepted, see // the task's binding amendments. Users wanting a stable zone name across // machines should set `timezone` in the config. - title := th.Title.Render(fmt.Sprintf("Report %s — scope %s%s — grouped by %s — tz %s", - report.PeriodLabel(r.Start, r.End), r.Scope, rm.note, r.GroupBy, r.Timezone)) + // Clamped before Render, not after: th.Title carries MarginBottom(1), which + // emits a second line as wide as the content, so truncating the rendered + // string would shorten the text and leave that padding line overlong. + // width <= 0 is the first render, before the terminal has sent its size — + // truncateWidth(s, 0) returns "" and would blank the header. + head := fmt.Sprintf("Report %s — scope %s%s — grouped by %s — tz %s", + report.PeriodLabel(r.Start, r.End), r.Scope, rm.note, r.GroupBy, r.Timezone) + if width > 0 { + head = truncateWidth(head, width) + } + title := th.Title.Render(head) summary := th.Accent.Render(export.SummaryLine(r)) var body string diff --git a/internal/tui/report_test.go b/internal/tui/report_test.go index e2ecd89..67dad16 100644 --- a/internal/tui/report_test.go +++ b/internal/tui/report_test.go @@ -7,6 +7,7 @@ import ( "time" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/marcoarnulfo/clickup-cli/internal/clickup" "github.com/marcoarnulfo/clickup-cli/internal/config" "github.com/marcoarnulfo/clickup-cli/internal/report" @@ -351,3 +352,38 @@ func TestTruncateEmptyStringAlwaysEmpty(t *testing.T) { } } } + +// The report table became width-aware in #66 and the budget screen in #136, but +// the header line above them never did: measured, it renders 68 columns on both +// a 40- and a 60-column terminal (the member note is what pushes it past the 54 +// it measures without one). Line 1 is the title's own MarginBottom padding and +// overflows with it, which is why the text is truncated BEFORE th.Title.Render +// rather than after — truncating the rendered string would leave that line long. +// +// Measured: every other line of this view already fits at width 40 (summary 37, +// billable note 38, table 39, sparkline 15), so this test can assert on the +// whole view without depending on anything the spec left out of scope. +func TestReportViewHeaderFitsTheTerminal(t *testing.T) { + t.Parallel() + th := testTheme(true) + rm := newReport(goldenReport(), " (2/5 members)", []float64{1, 0, 3, 2, 8}) + for _, width := range []int{40, 60, 80} { + for i, line := range strings.Split(rm.view(th, width), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line %d rendered %d columns: %q", width, i, w, line) + } + } + } +} + +// Before the first WindowSizeMsg the width is 0 and nothing is sized against +// it: the header keeps its full text, because truncateWidth(s, 0) returns "" +// and would blank it. +func TestReportViewHeaderSurvivesZeroWidth(t *testing.T) { + t.Parallel() + th := testTheme(true) + rm := newReport(goldenReport(), "", nil) + if !strings.Contains(rm.view(th, 0), "Report ") { + t.Error("the header vanished at width 0") + } +} diff --git a/internal/tui/testdata/palette_narrow.golden b/internal/tui/testdata/palette_narrow.golden index c5cdb74..084d88b 100644 --- a/internal/tui/testdata/palette_narrow.golden +++ b/internal/tui/testdata/palette_narrow.golden @@ -1,5 +1,5 @@ -Report July 2026 — scope me — grouped by list — tz UTC - +Report July 2026 — scope me — grouped b… + ╭─ Command palette ────────────────╮ 0 │ > │ ├──────────────────────────────────┤ From ca43ae90656723b7a2320b74bfad9e287337b01f Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 16:47:24 +0200 Subject: [PATCH 05/10] test(tui): add margin integrity test to protect truncation order (#143) --- internal/tui/report_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/internal/tui/report_test.go b/internal/tui/report_test.go index 67dad16..d51ee85 100644 --- a/internal/tui/report_test.go +++ b/internal/tui/report_test.go @@ -387,3 +387,34 @@ func TestReportViewHeaderSurvivesZeroWidth(t *testing.T) { t.Error("the header vanished at width 0") } } + +// TestReportViewHeaderMarginProtectsTruncationOrder ensures truncation happens +// before th.Title.Render, not after. When truncation occurs after rendering, +// ansi.Truncate cuts across the multi-line rendered string (text + MarginBottom), +// corrupting the margin line with ellipsis or wrong spacing. This test verifies +// that line 1 (the margin) is all spaces and matches line 0's width — the only +// outcome when text is truncated before rendering. If truncation is inverted, +// the margin line will be shorter than line 0, and the test fails. +func TestReportViewHeaderMarginProtectsTruncationOrder(t *testing.T) { + t.Parallel() + th := testTheme(true) + rm := newReport(goldenReport(), " (2/5 members)", []float64{1, 0, 3, 2, 8}) + for _, width := range []int{40, 60, 80} { + lines := strings.Split(rm.view(th, width), "\n") + if len(lines) < 2 { + t.Fatalf("width %d: view has fewer than 2 lines", width) + } + line0, line1 := lines[0], lines[1] + w0, w1 := lipgloss.Width(line0), lipgloss.Width(line1) + + // Line 1 must be all spaces (margin produced by th.Title.MarginBottom). + if line1 != strings.Repeat(" ", w1) { + t.Errorf("width %d: line 1 is not all spaces: %q", width, line1) + } + + // Line 1 must match line 0's width (how MarginBottom works). + if w1 != w0 { + t.Errorf("width %d: margin line width %d != text line width %d", width, w1, w0) + } + } +} From 9372f4243add0332d4b53a9df2df3c0501526afd Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 16:57:43 +0200 Subject: [PATCH 06/10] test(tui): pin the body column beside the overlay as intended (#143) --- internal/tui/overlay_test.go | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/internal/tui/overlay_test.go b/internal/tui/overlay_test.go index c6f420c..d10199d 100644 --- a/internal/tui/overlay_test.go +++ b/internal/tui/overlay_test.go @@ -112,6 +112,55 @@ func TestCompositeDoesNotLeakStyleIntoTheBox(t *testing.T) { } } +// TestCompositeDoesNotClipTheReportTableBesideANarrowPalette pins, rather than +// fixes, a choice: at a 40-column terminal the palette box does not span the +// full width, and the one column of body it leaves exposed on its right is +// legitimate content — the report table's own right border — not an +// artifact to crop. +// +// Measured from goldenPaletteModel at width 40 (screenBody + palette.layout, +// the real render path, not a hand-built fixture): the box sits at x=2 with +// boxW=36, so its footprint ends at column 38. Row 7, the table's top border, +// is the widest row the box actually covers (39 cells, against 38 for every +// other covered row) and is one cell wider than that footprint, so column 38 +// survives — the same "╮" that closes the table's own border. At 50, 60, 80, +// and 120 columns the box's footprint reaches at least that 39th column, so +// nothing is left exposed there; only 40 shows this column at all. +// +// Why it is there rather than cropped: overlay.go:11-13 documents that cells +// of body outside the box's rectangle survive verbatim, "that layering is the +// whole point of #59" — the same contract that already leaves the body +// visible to the LEFT of the box on this very screen. An overlay that instead +// cropped the body to the box's rectangle would fail this test, and would +// also fail three tests that already pin the splice this relies on: +// TestCompositeSplicesTheBoxIntoTheBody (the box lands inside the body rather +// than replacing it), TestCompositeKeepsEveryLineWidth (a crop changes line +// widths, which that test forbids), and +// TestCompositeHandlesWideGlyphsOnBothEdges (same property against wide +// glyphs). +func TestCompositeDoesNotClipTheReportTableBesideANarrowPalette(t *testing.T) { + t.Parallel() + m := goldenPaletteModel() + m.width = 40 + body := m.screenBody() + box, x, y := m.palette.layout(m.theme, m.width, m.height, strings.Count(body, "\n")+1) + got := composite(body, box, x, y) + + boxW := 0 + for _, l := range strings.Split(box, "\n") { + boxW = max(boxW, lipgloss.Width(l)) + } + + row := strings.Split(got, "\n")[7] // the table's top border, ╭───...───╮ + exposed := ansi.Cut(row, x+boxW, lipgloss.Width(row)) + if w := lipgloss.Width(exposed); w != 1 { + t.Fatalf("column exposed beside the box is %d cells wide, want 1: %q", w, exposed) + } + if exposed != "╮" { + t.Errorf("exposed cell = %q, want the table's own top-right corner %q", exposed, "╮") + } +} + // Past the end of a line the right-hand segment is skipped by construction // (composite's x+boxW < rowW guard), not cut and discovered empty: without // that guard the straddle correction would still run against Cut's own empty From f9e8052b2110e0626698efe3611f2ee4c6ce9a19 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 17:07:11 +0200 Subject: [PATCH 07/10] test(tui): name the row-index assumption in the new overlay test (#143) --- internal/tui/overlay_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/tui/overlay_test.go b/internal/tui/overlay_test.go index d10199d..fac060f 100644 --- a/internal/tui/overlay_test.go +++ b/internal/tui/overlay_test.go @@ -151,7 +151,19 @@ func TestCompositeDoesNotClipTheReportTableBesideANarrowPalette(t *testing.T) { boxW = max(boxW, lipgloss.Width(l)) } - row := strings.Split(got, "\n")[7] // the table's top border, ╭───...───╮ + // Guards the "row 7" index above, not just the measurement: if the report + // screen ever gains or loses a line above the table, row 7 stops being the + // table's top border and this fails with a numeric delta that hides the + // real cause. Fail here instead, naming it, so a future reader lands on + // "the index moved" rather than "the count is off by one." + const tableTopBorderRow = 7 + if r := strings.Split(body, "\n")[tableTopBorderRow]; !strings.HasPrefix(r, "╭") || !strings.HasSuffix(r, "╮") { + t.Fatalf("row %d of the body is no longer the report table's top border (got %q); "+ + "the report screen's layout above the table changed, so this test's row index needs "+ + "updating, not its numbers", tableTopBorderRow, r) + } + + row := strings.Split(got, "\n")[tableTopBorderRow] // the table's top border, ╭───...───╮ exposed := ansi.Cut(row, x+boxW, lipgloss.Width(row)) if w := lipgloss.Width(exposed); w != 1 { t.Fatalf("column exposed beside the box is %d cells wide, want 1: %q", w, exposed) From 6683ea75ced9875a942aefb2c83fd93820bd5388 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 17:10:39 +0200 Subject: [PATCH 08/10] docs: changelog for the layout follow-ups --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dca861..181835b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The report header ("Report — scope …") is now truncated on narrow + terminals instead of wrapping to a second line; with an ellipsis marking the + truncation (#143). +- Budget burn-down view now aligns the percentage labels right-justified, so the + figures they precede start at the same column across rows instead of ragged; + this complements the visual alignment the gauge itself already provided (#144). - Removed the duplicate "Total" data row that appeared directly above the TOTAL row when the report is grouped as a single total (#137). - Budget burn-down view now sizes its list-name column to the available width From 7b98a4690393594d5d527a2c80eb30d10aa1e28e Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 17:19:44 +0200 Subject: [PATCH 09/10] docs: changelog precision fixes for layout follow-ups --- CHANGELOG.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 181835b..9870e83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,12 +27,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- The report header ("Report — scope …") is now truncated on narrow - terminals instead of wrapping to a second line; with an ellipsis marking the - truncation (#143). -- Budget burn-down view now aligns the percentage labels right-justified, so the - figures they precede start at the same column across rows instead of ragged; - this complements the visual alignment the gauge itself already provided (#144). +- The report header ("Report — scope …") is now truncated when it no + longer fits the terminal width instead of wrapping to a second line, with an + ellipsis marking where it was cut (#143). +- Budget burn-down view now right-justifies the percentage labels, so the + figures they precede start at the same column across rows instead of ragged (#144). - Removed the duplicate "Total" data row that appeared directly above the TOTAL row when the report is grouped as a single total (#137). - Budget burn-down view now sizes its list-name column to the available width From 95043febcb9e6069dd9e92d896a5e082d73d23b9 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Thu, 30 Jul 2026 17:39:33 +0200 Subject: [PATCH 10/10] test(tui): fix stale comment, add zero-width budget test, tighten changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - overlay_test.go: correct the comment on the narrow-palette clipping test — row 7 is not "the widest row, against 38 for every other covered row"; the six table rows below it are 39 cells too and each exposes a cell, so what survives beside the box is the table's whole right-border strip, not one stray cell. Remeasured from goldenPaletteModel at width 40 to confirm. - budget_test.go: add TestBudgetViewLeavesPercentLabelsNaturalAtZeroWidth, the untested half of the #144 invariant documented on budget.go's view — that width <= 0 never pads the percentage label. Verified it fails without the guard in view (pctW measured unconditionally) and passes with it. - CHANGELOG.md: finish the dangling "instead of ragged" into "instead of a ragged column". --- CHANGELOG.md | 3 ++- internal/tui/budget_test.go | 32 ++++++++++++++++++++++++++++++++ internal/tui/overlay_test.go | 18 ++++++++++++------ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9870e83..fc5bfad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 longer fits the terminal width instead of wrapping to a second line, with an ellipsis marking where it was cut (#143). - Budget burn-down view now right-justifies the percentage labels, so the - figures they precede start at the same column across rows instead of ragged (#144). + figures they precede start at the same column across rows instead of a + ragged column (#144). - Removed the duplicate "Total" data row that appeared directly above the TOTAL row when the report is grouped as a single total (#137). - Budget burn-down view now sizes its list-name column to the available width diff --git a/internal/tui/budget_test.go b/internal/tui/budget_test.go index c29811f..b23c153 100644 --- a/internal/tui/budget_test.go +++ b/internal/tui/budget_test.go @@ -183,6 +183,38 @@ func TestBudgetViewAlignsFiguresAcrossRows(t *testing.T) { } } +// TestBudgetViewLeavesPercentLabelsNaturalAtZeroWidth protects the other half +// of the #144 fallback: view's doc comment claims width <= 0 is budgetLayout's +// own fallback branch, which never measures pctW, so the percentage label +// should stay at its natural width there instead of quietly starting to align +// once nothing else in the layout does either. TestBudgetViewAlignsFiguresAcrossRows +// covers the sized side (equal columns); this is the zero-width side, where the +// columns must differ instead. +// +// Same fixture as that test, same reasoning for why: the two rows' percentages +// are three and four digits long ("62%" vs "104%"), so a natural label shifts +// the "/" one column between rows while a padded one would not. +func TestBudgetViewLeavesPercentLabelsNaturalAtZeroWidth(t *testing.T) { + t.Parallel() + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + {ListName: "Mobile app", Billed: 940, Budget: 900, Currency: "EUR", Remaining: -40, PercentUsed: 940.0 / 900 * 100}, + }) + var cols []int + for _, line := range strings.Split(bm.view(th, 0), "\n") { + if i := strings.Index(line, "/"); i >= 0 { + cols = append(cols, lipgloss.Width(line[:i])) + } + } + if len(cols) != 2 { + t.Fatalf("expected two budget rows, found %d figure columns", len(cols)) + } + if cols[0] == cols[1] { + t.Errorf("figures start at the same column (%d) at width 0; want the percentage label unpadded, one column apart between the 62%% and 104%% rows", cols[0]) + } +} + // A percentage narrower than the measured maximum is right-aligned into it, so // every column after it holds. pctW <= 0 keeps the natural width, which is what // the callers that are not testing alignment pass. diff --git a/internal/tui/overlay_test.go b/internal/tui/overlay_test.go index fac060f..83aee65 100644 --- a/internal/tui/overlay_test.go +++ b/internal/tui/overlay_test.go @@ -120,12 +120,18 @@ func TestCompositeDoesNotLeakStyleIntoTheBox(t *testing.T) { // // Measured from goldenPaletteModel at width 40 (screenBody + palette.layout, // the real render path, not a hand-built fixture): the box sits at x=2 with -// boxW=36, so its footprint ends at column 38. Row 7, the table's top border, -// is the widest row the box actually covers (39 cells, against 38 for every -// other covered row) and is one cell wider than that footprint, so column 38 -// survives — the same "╮" that closes the table's own border. At 50, 60, 80, -// and 120 columns the box's footprint reaches at least that 39th column, so -// nothing is left exposed there; only 40 shows this column at all. +// boxW=36, so its footprint ends at column 38. Row 7 is the table's top +// border, 39 cells wide — one cell wider than that footprint, so column 38 +// survives as the table's own "╮". It is not alone: the six rows below it +// (8-13 — the header, the separator, the two data rows, TOTAL, and the +// bottom border) are the rest of the table and are 39 cells wide too, each +// exposing that same column with its own border glyph. No row this box +// covers is 38 cells. So what stays visible beside the box is not a single +// stray cell but the whole strip of the table's right border; row 7's "╮" is +// just that strip's top corner, and this test pins it as a stand-in for the +// rest. At 50, 60, 80, and 120 columns the box's footprint reaches at least +// that 39th column, so nothing is left exposed there; only 40 shows this +// column at all. // // Why it is there rather than cropped: overlay.go:11-13 documents that cells // of body outside the box's rectangle survive verbatim, "that layering is the