Skip to content

Commit 9fc2ac7

Browse files
Alexey Lesovskyclaude
andcommitted
feat(top): TUI papercuts — seven interactive-mode frictions removed
Sorting now scrolls the column window to the sort column; an active filter is announced permanently in the cmdline instead of only by a header marker that can itself scroll off-screen; `\` clears every filter of the current screen; the refresh interval is shown in the header; the verbose panels no longer waste a screen row and highlight their values like the base rows always did; and dialog prompts stay aligned with their input field in both display modes. Closes a live pre-existing crash on the way: the 93-character state-mask prompt put the dialog's input field past the right edge on an 80-column terminal, gocui rejected the coordinates and the whole UI was torn down and rebuilt. Also removes a pre-existing race where the cmdline clear timer touched a view buffer from a bare goroutine. Pause on Space was planned for this batch and split into its own feature: sorting runs in the collector goroutine, so a paused screen cannot re-sort, and returning from the pager rebuilds the UI with no frame left to repaint. Verified on a live terminal: all seven behaviours captured on a remote stand across three geometries, with a second binary built from master to separate regressions from long-standing behaviour. Acceptance: 27/27 user-spec and 11/11 tech-spec criteria. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0bfadfb commit 9fc2ac7

89 files changed

Lines changed: 7354 additions & 216 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/skills/project-knowledge/architecture.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ dedicated `view.View.Verbose bool` (rides `viewCh` to the collector) mirrored in
102102
into the four `SetView` calls. Compact (and the height-guard fallback) reproduces the historical
103103
literals (`4/4/3/5/4`) byte-identically; verbose grows the panels asymmetrically (`sysstat` +3,
104104
`pgstat` +5) and shifts `cmdline`/`dbstat` down. The height-guard refuses to expand when the band +
105-
cmdline + table header + ≥1 data row would not fit (threshold `maxY ≥ 13`), falling back to compact +
105+
cmdline + table header + ≥1 data row would not fit (threshold `maxY ≥ 12` since 015-feat-tui-papercuts,
106+
which tied the verbose table top to the cmdline the way the compact branch always did), falling back to compact +
106107
a one-shot cmdline hint. Pure-function, gocui-free, table-tested — the [009] `visibleColumns` precedent.
107108
- **All-three system collection branch (`internal/stat/stat.go:262`, `:401`).** When `view.Verbose` is
108109
set, `Collector.Update` runs a verbose-gated branch placed **after** the existing mutually-exclusive

.claude/skills/project-knowledge/patterns.md

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,21 @@ by driving pgcenter inside tmux on a dedicated stand over ssh. The regimen below
184184
from the sibling book project (`../postgresql-destruction-recovery-guide-book`, "Операционные
185185
правила стенда"), where it is already proven for capturing pgcenter screens.
186186

187-
**Stand.** A dedicated VM reachable over ssh, with PostgreSQL and tmux installed. The address and
188-
credentials are **not stored in this repository, and not worth recording anywhere else either**
189-
stands are ephemeral, with a TTL measured in hours, so a saved address is stale by the next
190-
session. Ask the author at the start of every run, and never assume the previous run's state
191-
survived.
187+
**Stand.** A dedicated VM reachable over ssh. The address and credentials are **not stored in this
188+
repository, and not worth recording anywhere else either** — stands are ephemeral, with a TTL
189+
measured in hours, so a saved address is stale by the next session. Ask the author at the start of
190+
every run, and never assume the previous run's state survived.
191+
192+
**Take an inventory before planning the run — the stand is not a fixed image.** Observed so far:
193+
Ubuntu 24.04 with tmux preinstalled, and Debian 12 with no tmux, no Go, and a Postgres Pro build
194+
whose `shared_preload_libraries` lacks `pg_stat_statements` (so the statements screens are simply
195+
unavailable). Check for `tmux`, the PostgreSQL flavour and the loaded libraries first; installing
196+
tmux is fine with the passwordless sudo these stands carry, but a missing extension may mean a
197+
screen cannot be exercised at all — decide that before writing the plan, not mid-run.
198+
199+
**Bring a second binary built from `master`.** Running the same scenario on both is what separates a
200+
regression introduced by the feature from behaviour that was always broken. In 015 this reclassified
201+
three of five findings as pre-existing; without it they would have been filed against the feature.
192202

193203
**The binary under test must be shipped explicitly.** The stand carries a pgcenter built from
194204
`master`; a feature branch's behaviour is not there until you copy the freshly built
@@ -217,18 +227,42 @@ Stripping ANSI from an `-e` capture for diffing: `sed 's/\x1b\[[0-9;]*m//g'`.
217227
**Leave the stand as you found it.** Server GUCs changed for an experiment are reset afterwards
218228
and the reset is verified with `SHOW`; the tmux session is killed at the end of the run.
219229

220-
## printCmdline() — Mutual Exclusion
221-
222-
`printCmdline(g, msg)` calls `g.Update` followed by `v.Clear`. If it is called twice in the
223-
same view-switch handler the second call immediately overwrites the first render. When a
224-
handler needs to show either a warning or a normal message, these two cases must be mutually
225-
exclusive — use an `if/else` branch, not two sequential calls. Calling `printCmdline(warning)`
226-
and then `printCmdline(v.Msg)` in the same code path will always discard the warning before
227-
the user can read it.
228-
229-
When multiple independent availability probes can fail (e.g., IO + delay accounting in
230-
`switchViewToProcPidStat`), use a 4-branch `switch` covering all combinations, with a combined
231-
message for the case where both are unavailable — still exactly one `printCmdline` call per path.
230+
## The cmdline: one composer, transient messages, persistent state (015-feat-tui-papercuts)
231+
232+
Everything written to the cmdline goes through **one composition point**. `printCmdline` keeps its
233+
historical signature and its 2-second clear timer; `printCmdlinePersist` is the same writer without
234+
the timer, for text that must survive until dismissed (the dialog prompt). Both delegate to a shared
235+
core that builds the line as *reserved state prefix* + *transient message* and clamps it to the
236+
terminal width in **runes**.
237+
238+
- **Persistent state is a token, not a message.** `cmdlineToken` carries renderings ordered
239+
longest-to-shortest; the composer degrades the rightmost token through its variants only when the
240+
line does not fit, then drops tokens from the right. A token with exactly one variant never
241+
shrinks — that is how a "must always be visible" indicator is expressed in data rather than in a
242+
special case. Adding a second token requires no change to the composer.
243+
- **Read state only on the gocui goroutine.** The composer reads its state from a package-level
244+
ambient `*config`, written once by a named setter from `RunMain` — deliberately not from `newApp`,
245+
which unit tests call repeatedly and would leave a stale pointer behind. Dereference only inside
246+
`g.Update` closures or key handlers. This is the package's only package-level mutable var; it
247+
exists so the 44 existing call sites keep their signature.
248+
- **The clear timer renders, it does not erase.** It re-renders the prefix-only line inside its own
249+
`g.Update` (removing an older race where a bare goroutine touched the view buffer) and is gated on
250+
an atomic UI-generation counter **captured into a local before the goroutine is spawned** — read
251+
inside, it would compare a value to itself and do nothing. `mainLoop` rebuilds the `Gui` without
252+
closing it on every pager/editor return, so an ungated timer pins the abandoned one.
253+
- **Sanitise anything that came from the server.** Column names reach the indicator, and
254+
`gocui.View.Clear()` resets the line buffer but *not* the escape-interpreter state — an
255+
unterminated sequence on this low-frequency surface survives the whole session, unlike the stats
256+
table, which repaints every tick amid correct sequences and self-heals.
257+
258+
**Still true, and now the sharper hazard:** two writes in one code path still leave only the last
259+
one visible, because `g.Update` enqueues each from its own goroutine and the order is not
260+
guaranteed. Keep exactly one call per path — an `if/else`, or the 4-branch `switch` that
261+
`switchViewToProcPidStat` uses for its two independent probes. Two known paths violate this and
262+
predate the composer: `dialogFinish` writes an empty line before its result, so **no message shown
263+
after a dialog closes is ever visible** (the action still happens), and the verbose height-guard
264+
hint loses to `collecting...` on the same keypress. Both are registered tech debt — do not treat a
265+
"missing" cmdline message as a new bug before checking whether it is one of these.
232266

233267
## Adding a Hybrid View (SQL + procfs enrichment)
234268

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
# Build artifacts
22
/bin/
3+
.test_coverage.txt

docs/decisions-log.md

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,3 +952,82 @@ without touching five files.
952952
**Alternatives considered:** branching at 14 (rejected — matches fixtures, not reality); adding a PG 13
953953
cluster (rejected — disproportionate, PG 13 is past EOL); renaming the constants (rejected — rested on
954954
a convention that does not exist).
955+
956+
---
957+
958+
## [015-feat-tui-papercuts] One cmdline composer fed by an ambient config
959+
960+
**Date:** 2026-08-02
961+
**Feature:** 015-feat-tui-papercuts
962+
**Status:** Accepted
963+
964+
**Context:** A persistent filter indicator has to appear on every cmdline write, but the writers are
965+
reached from 19 functions across 11 files, four of which (`showPgConfig`, `editPgConfig`,
966+
`showPgLog`, `resetStat`) have no `config` in scope at all. There are 44 existing call sites.
967+
968+
**Decision:** A package-level `cmdlineCfg *config` in `top/`, written exactly once by
969+
`setCmdlineConfig` called from `RunMain`, dereferenced only inside `g.Update` closures or key
970+
handlers. All existing call sites keep their signature.
971+
972+
**Rationale:** Threading the config explicitly costs 44 call-site edits plus four helper signatures
973+
and their keybinding registrations — a large mechanical diff across files that three waves were
974+
editing concurrently, for a dependency that is genuinely process-global in a single-instance TUI.
975+
Setting it from `RunMain` rather than `newApp` matters: `newApp` runs in unit tests, and an ambient
976+
set there would leave one test's config visible to every test that follows. That is safe today only
977+
because nothing in `top/` calls `t.Parallel()` — an undocumented invariant not worth depending on.
978+
979+
**Alternatives considered:** Explicit threading (rejected on diff size and cross-wave merge risk,
980+
not on principle). `sync.Once` (guards a second write that cannot happen). Note this is the first
981+
package-level mutable var in `top/`; `internal/version` and `internal/stat/procpidstat.go` have
982+
package vars, but they hold immutable data.
983+
984+
---
985+
986+
## [015-feat-tui-papercuts] Reserve the input field's room before composing the dialog prompt
987+
988+
**Date:** 2026-08-02
989+
**Feature:** 015-feat-tui-papercuts
990+
**Status:** Accepted
991+
992+
**Context:** The dialog input field was positioned from the raw prompt length, assuming an empty
993+
cmdline. With a persistent indicator that assumption breaks. It was already broken without one: the
994+
93-character state-mask prompt puts the field's left edge right of its right edge on an 80-column
995+
terminal, `gocui.SetView` rejects the coordinates, and the error propagates to `MainLoop`, which
996+
tears down and rebuilds the entire UI. Reproducible at startup on `master`.
997+
998+
**Decision:** Compose the prompt against a width budget that already excludes the field's reserved
999+
minimum, so an overlong prompt is **truncated** (with an ellipsis spent from the same budget) rather
1000+
than overlaid by the field. The x0 clamp remains as a backstop that provably never bites.
1001+
1002+
**Rationale:** Clamping alone keeps the dialog on screen but slides the field over the prompt's
1003+
tail, contradicting the alignment requirement. Truncating satisfies both at once. The prefix is
1004+
rendered at the same width the writer uses, so the measured line and the drawn line are the same
1005+
string by construction rather than by coincidence — the one place where two independently correct
1006+
pieces could silently disagree.
1007+
1008+
**Alternatives considered:** Clamp and accept the overlay (contradicts an approved criterion);
1009+
shorten the prompt strings themselves (changes user-visible text unrelated to the feature).
1010+
1011+
---
1012+
1013+
## [015-feat-tui-papercuts] Probe the existing window function instead of recomputing the offset
1014+
1015+
**Date:** 2026-08-02
1016+
**Feature:** 015-feat-tui-papercuts
1017+
**Status:** Accepted
1018+
1019+
**Context:** Auto-scrolling to the sort column needs the smallest offset at which that column is
1020+
visible. The admission rule lives in `visibleColumns`, whose marker-reservation arithmetic ADR
1021+
[009] records as error-prone — it shipped a bug invisible to unit tests.
1022+
1023+
**Decision:** Walk candidate offsets and ask `visibleColumns` whether the column is inside the
1024+
returned window; take the first that admits it. Where no offset admits it at all — a terminal
1025+
narrower than the frozen column — leave the offset untouched.
1026+
1027+
**Rationale:** Any independent computation would be a second implementation of the same subtle rule.
1028+
The function is pure and the column count is bounded by the widest screen. The research sketch
1029+
returned the last index in the no-fit case, which would jerk the window to an arbitrary edge; the
1030+
acceptance criterion overrode it.
1031+
1032+
**Alternatives considered:** Direct backward-walk arithmetic (a second source of truth for the
1033+
marker rule).

docs/features-catalog.md

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,10 @@ hidden to the left/right. Closes issue #14, the most-requested item, open since
185185
- Switch to another screen (including the per-process screen via `Shift+S`): the new screen
186186
opens unscrolled (offset reset to 0). The offset persists across auto-refresh ticks within one screen.
187187
- On a wide terminal where everything fits, `[`/`]` are no-ops and no markers are shown.
188-
- Sorting (``/``) and scrolling (`[`/`]`) are independent — the sort column may sit outside the
189-
visible window, which is expected (no auto-scroll to the sort column).
188+
- Sorting (``/``) and scrolling (`[`/`]`) are independent in the sense that a manual scroll is
189+
never undone by a refresh. **Superseded in part by [015-feat-tui-papercuts]:** changing the sort
190+
column now scrolls the window to bring that column into view, so "the sort column may sit outside
191+
the visible window" is no longer expected behaviour.
190192

191193
**Limitations:**
192194
- **Main stats table only.** The side extra-panels (iostat / netdev / fsstats / logtail) do not
@@ -336,3 +338,47 @@ is blank for some rows now places those rows last in both directions instead of
336338
zero. Most visible on [005-feat-replication-slots], whose default sort key is sparse.
337339
[009-feat-horizontal-scroll] is what makes the wider screen usable. The `report` replay path gained
338340
correct handling of a recorded version change, which matters for archives spanning a major upgrade.
341+
342+
---
343+
344+
### [015-feat-tui-papercuts] Interactive-mode Papercuts
345+
346+
**What it does:** Removes seven frictions that made `pgcenter top` slower to drive than it should
347+
be. Sorting now scrolls the column window to whatever column you sorted by; an active filter is
348+
announced permanently in the command line instead of only by a `*` on a header that may itself be
349+
off-screen; `\` clears every filter of the current screen at once; the refresh interval is shown in
350+
the header; the verbose panels stop wasting a screen row and highlight their values the way the
351+
normal rows always did; and dialog prompts line up with their input field in both display modes.
352+
353+
**Key scenarios:**
354+
- On a narrow terminal, press `` past the right edge: the window follows the sort column and brings
355+
it into view, highlight included. Scroll away with `[`/`]` afterwards and it stays where you put
356+
it — the window is only moved when the sort column changes.
357+
- Set a filter, then scroll its column off-screen or switch to another screen and back: the command
358+
line keeps showing `[F:datname]`, so a filtered view can no longer be mistaken for the whole
359+
picture. With several filters it lists them all: `[F:datname,usename]`.
360+
- Press `\` to drop every filter on the screen at once, instead of hunting down each column that set
361+
one. The message says how many were removed, or says plainly that there were none.
362+
- Press `v`: the verbose panels expand with no blank gap above the table, values are highlighted,
363+
and degraded `n/a` fields stay plain so a missing signal still looks different from a real number.
364+
- Open any dialog with a filter active: the prompt and the input field stay aligned, in both compact
365+
and verbose mode. An overlong prompt is cut with an ellipsis rather than covered by the field.
366+
367+
**Limitations:**
368+
- Pause on `Space` was planned for this batch and **split into its own feature** ([016] on the
369+
0.12.0 roadmap): sorting runs in the collector goroutine, so a paused screen cannot re-sort, and
370+
returning from the pager rebuilds the UI with no frame to repaint. Both need product decisions of
371+
their own.
372+
- The auto-scroll fires only when the sort column changes. Entering a screen whose default sort
373+
column sits off-screen leaves it there until the first arrow press — reachable only on terminals
374+
narrower than about 80 columns.
375+
- Filters still combine as OR, not AND. The indicator makes that visible for the first time but does
376+
not change it.
377+
- Messages printed after a dialog closes remain invisible (pre-existing, registered as tech debt),
378+
so one of the three filter messages can only be seen in tests, not on screen.
379+
- The verbose threshold moved from 13 terminal rows to 12 — verbose now engages one row earlier.
380+
381+
**Touches:** [009-feat-horizontal-scroll] — supersedes its "no auto-scroll to the sort column"
382+
limitation and reuses its column-window function. [010-feat-overview-dashboard] — fixes two defects
383+
in the verbose panels it introduced. The command-line composer added here is the foundation the
384+
deferred pause feature builds on.

docs/features/015-feat-tui-papercuts/015-feat-tui-papercuts-metrics.json

Lines changed: 0 additions & 63 deletions
This file was deleted.

docs/features/015-feat-tui-papercuts/015-feat-tui-papercuts-adequacy-review-round2.json renamed to docs/features/archive/015-feat-tui-papercuts/015-feat-tui-papercuts-adequacy-review-round2.json

File renamed without changes.

docs/features/015-feat-tui-papercuts/015-feat-tui-papercuts-adequacy-review.json renamed to docs/features/archive/015-feat-tui-papercuts/015-feat-tui-papercuts-adequacy-review.json

File renamed without changes.

docs/features/015-feat-tui-papercuts/015-feat-tui-papercuts-arch-review-round2.json renamed to docs/features/archive/015-feat-tui-papercuts/015-feat-tui-papercuts-arch-review-round2.json

File renamed without changes.

docs/features/015-feat-tui-papercuts/015-feat-tui-papercuts-arch-review.json renamed to docs/features/archive/015-feat-tui-papercuts/015-feat-tui-papercuts-arch-review.json

File renamed without changes.

0 commit comments

Comments
 (0)