Skip to content

Commit b8a1ed9

Browse files
committed
feat(client): show request log events, and keep a watch inside the window
## Summary ### Why? Two things were wrong with what a watch showed, and one of them got worse the more there was to show. **Events were dropped.** A request's history holds two kinds of entry: a **status** is a position it reached — `batched`, `speculating`, `landed` — and an **event** is something that happened while it sat at one, like a build starting or a passed path waiting on a dependency. `digest` skipped every entry whose status was empty, which is exactly what an event is. Two requests that both read `speculating → speculated → landed` could have done wildly different amounts of work, one build or eight, and the table said the same thing about both. **`list` showed less still.** It reads the queue's receipts rather than a history per request, so its trail was empty and its `STAGE` column rendered `…` for every row — including rows whose current status it already had in hand. **And a watch of more requests than the window holds repainted the whole screen on every redraw.** The renderer never knew how tall the terminal was: `terminalSize` asked for the size and threw the height away, so a frame could be any number of lines. Each redraw moves the cursor up by the number of lines it emitted, on the assumption they are still on screen — and once a frame is taller than the window, they are not. Drawing it scrolls the window, the cursor cannot move above its first row, and the terminal clamps the jump; every frame after that starts from the wrong origin, overwriting the top of the screen and stranding the rest of the previous frame below. The threshold is the window height, which is why it looked like a size problem rather than a bug. ### What? **Events are shown against the status they happened under**, rather than as steps of their own, because they are not positions and treating them as such would imply the request moved: ``` batched → speculating [building ×8, built ×8, waiting] → speculated → landing → landed ``` Repeats are counted rather than listed. A batch runs one build per speculation path, so a request that speculated widely records `building` many times over, and spelling each one out would say less than the count does while pushing the rest of the row off the line. **`list` shows the position each request holds** instead of `…`. It still fetches no histories — that is what keeps a listing one round trip — so it reports where a request is without claiming to know how it got there. **The renderer reads the height along with the width**, re-reading both before every draw so a window resized mid-watch is picked up, and trims a frame that will not fit. Settled requests are dropped first: they have stopped changing, so leaving them out costs a reader nothing `land-list` will not tell them, while dropping the ones still moving would hide the only part of the table that is doing anything. Order is kept within each group so rows do not jump between redraws, and a line reports how many are not shown. Two cases deliberately keep every row, because nothing is drawn over them and the terminal's own scrollback is the right answer: redirected output, which is a log rather than a window, and a one-shot `list`, which is drawn once and scrolled back through. ## Test Plan - ✅ `make demo-requests COUNT=4 FOLDERS=1` against a live stack, which forces every change to conflict and so produces real speculation. The deepest request in the chain rendered `speculating [building ×8, built ×8, waiting]` while the first rendered `speculating [building, built]` — the difference this exists to show - ✅ `make land-list` mid-flight reports `speculating` where it used to report `…`, and `landed` once the run settled - ✅ 30 requests in a simulated 20-row window: the frame is 19 lines and the cursor moves back 19. The same run emitted over 60 lines into that window before - ✅ `make land-list LIMIT=30` in that same 20-row window still prints all 30 rows - ✅ new `digest` cases: events attach to the status they occurred under, repeats are counted, a status repeated around its own events stays one step, each status collects only its own events, an event does not move the request off its status, an event before any status is dropped, and an error carried by an event is still surfaced - ✅ new frame cases: a frame never exceeds the window and `lastLines` matches what was emitted; an unsettled row survives the trim; piped output and one-shot listings keep every row - ✅ mutation-checked the height test by making the trim a no-op, confirming it fails without the fix rather than passing vacuously - ✅ `make test` (105 targets), `make lint`, `make gazelle`
1 parent 04445d2 commit b8a1ed9

3 files changed

Lines changed: 392 additions & 28 deletions

File tree

doc/howto/QUICKSTART.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,16 @@ make land-list SINCE=24h LIMIT=200 # a wider window
105105
make land-watch # follow them until they settle
106106
```
107107

108-
Both draw the same table `make demo-requests` does — the demo tool and the CLI share it — but against whatever the queue already holds, so watching a queue no longer means adding to it. They do not carry the same information, though: `list` is a one-shot read of the queue's receipts and does not fetch histories, so its `STAGE` column is always ``, while `watch` follows the history API and fills the trail in as each request moves.
108+
Both draw the same table `make demo-requests` does — the demo tool and the CLI share it — but against whatever the queue already holds, so watching a queue no longer means adding to it. They do not carry the same information, though: `list` is a one-shot read of the queue's receipts and does not fetch a history per request, so its `STAGE` column shows where each request is and not how it got there, while `watch` follows the history API and fills the whole trail in as each one moves.
109+
110+
That trail carries more than positions. A request records events while it sits at one — a build starting or finishing, a passed path waiting on a dependency — and those are shown against the status they happened under, with repeats counted:
111+
112+
```
113+
accepted → started → validating → validated → batching → batched →
114+
speculating [building ×8, built ×8, waiting] → speculated → landing → landed
115+
```
116+
117+
Eight builds means the batch was speculating down eight paths at once, and `waiting` means one of them passed and then sat on a dependency that had not resolved. A request that sailed through reads `speculating [building, built]` instead — the same position, a very different amount of work behind it.
109118

110119
`land-watch` fixes its set when it starts and exits non-zero if any request in that set finishes anywhere other than `landed`, which makes it usable from a script. A request accepted after the watch begins is not picked up: a watch that grew as the queue did would never finish.
111120

submitqueue/client/view.go

Lines changed: 194 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,19 @@ func (rw *Row) elapsed() string {
116116
return fmt.Sprintf("%ds", int(end.Sub(rw.Submitted).Seconds()))
117117
}
118118

119-
// stage is the path the request has taken, as the gateway recorded it. The
120-
// waiting marker covers the gap between acceptance and the first recorded
121-
// event, so an accepted request is never shown as though nothing happened.
119+
// stage is the path the request has taken, as the gateway recorded it.
120+
//
121+
// A one-shot listing reads the queue's receipts and does not fetch a history
122+
// per request, so it has the position each one holds but not how it got there.
123+
// That is worth showing on its own: a column of "…" says nothing about a queue
124+
// whose rows are mostly `speculating`.
122125
func (rw *Row) stage() string {
123126
if len(rw.Trail) > 0 {
124127
return strings.Join(rw.Trail, " → ")
125128
}
129+
if rw.Status != "" {
130+
return rw.Status
131+
}
126132
if rw.SQID != "" {
127133
return "…"
128134
}
@@ -133,34 +139,110 @@ func (rw *Row) stage() string {
133139
// a caller following requests as they move uses a Tracker instead, which owns
134140
// the rows and redraws them.
135141
func Draw(rows []*Row, status string) {
136-
newRenderer().draw(rows, status)
142+
r := newRenderer()
143+
// Nothing will be drawn over this, so a table taller than the window simply
144+
// scrolls — which is what a reader of a long listing wants, and why the
145+
// height limit a redrawing watch lives under does not apply here.
146+
r.oneShot = true
147+
r.draw(rows, status)
137148
}
138149

139150
// digest reduces a request's recorded history to the trail worth showing, the
140151
// status it currently holds, and the error the latest event carried. A status
141152
// recorded more than once in a row is one step in the trail, not several.
153+
//
154+
// The history holds two kinds of entry. A status is a position the request
155+
// reached, and those are the trail's spine. An event is something that happened
156+
// while it sat at one — a build starting, a passed path waiting on a dependency
157+
// — and never changes the position, so each is shown against the status it
158+
// occurred under rather than as a step of its own:
159+
//
160+
// batched → speculating [building ×2, built] → speculated
161+
//
162+
// Repeats are counted rather than listed. A batch runs one build per
163+
// speculation path, so a request that speculated widely records `building` many
164+
// times, and a trail that spelled each one out would say less than the count
165+
// does while pushing the rest of the row off the line.
142166
func digest(events []*pb.HistoryEvent) (trail []string, status, note string) {
143167
if len(events) == 0 {
144168
return nil, "", ""
145169
}
170+
171+
// Events seen since the last status, in first-seen order with their counts,
172+
// so they can be attached once the step they belong to is complete.
173+
var pending []string
174+
var last string
175+
counts := make(map[string]int)
176+
177+
flush := func() {
178+
if len(trail) == 0 || len(pending) == 0 {
179+
pending, counts = nil, make(map[string]int)
180+
return
181+
}
182+
trail[len(trail)-1] += " [" + strings.Join(annotate(pending, counts), ", ") + "]"
183+
pending, counts = nil, make(map[string]int)
184+
}
185+
146186
for _, e := range events {
147-
if e == nil || e.Status == "" {
187+
if e == nil {
148188
continue
149189
}
150-
if len(trail) > 0 && trail[len(trail)-1] == e.Status {
190+
if e.Status == "" {
191+
if e.Event == "" {
192+
continue
193+
}
194+
if counts[e.Event] == 0 {
195+
pending = append(pending, e.Event)
196+
}
197+
counts[e.Event]++
151198
continue
152199
}
200+
// A status repeated back-to-back is one step, but anything recorded
201+
// against it in between still belongs to that step.
202+
if e.Status == last {
203+
continue
204+
}
205+
flush()
153206
trail = append(trail, e.Status)
207+
last = e.Status
154208
}
209+
flush()
210+
155211
if last := events[len(events)-1]; last != nil {
156212
status, note = last.Status, last.LastError
157213
}
158-
if status == "" && len(trail) > 0 {
159-
status = trail[len(trail)-1]
214+
// The last entry may be an event, which leaves the request where it was.
215+
if status == "" {
216+
status = currentStatus(events)
160217
}
161218
return trail, status, note
162219
}
163220

221+
// annotate renders each event with its count, dropping the count when it
222+
// happened once.
223+
func annotate(order []string, counts map[string]int) []string {
224+
out := make([]string, 0, len(order))
225+
for _, event := range order {
226+
if counts[event] > 1 {
227+
out = append(out, fmt.Sprintf("%s ×%d", event, counts[event]))
228+
continue
229+
}
230+
out = append(out, event)
231+
}
232+
return out
233+
}
234+
235+
// currentStatus is the last position the request reached, ignoring anything
236+
// recorded while it sat there.
237+
func currentStatus(events []*pb.HistoryEvent) string {
238+
for i := len(events) - 1; i >= 0; i-- {
239+
if e := events[i]; e != nil && e.Status != "" {
240+
return e.Status
241+
}
242+
}
243+
return ""
244+
}
245+
164246
// outcome is the one-line verdict shown under the finished table.
165247
func outcome(rows []*Row) string {
166248
landed := 0
@@ -203,17 +285,29 @@ func summarize(rows []*Row) error {
203285
// the number of lines it *emitted* — so one wrapped line desyncs every redraw
204286
// after it. Everything wide is therefore wrapped deliberately, into lines the
205287
// renderer counts itself.
288+
//
289+
// A frame may not be taller than the window either, for the same reason in the
290+
// other axis. Drawing more lines than the window holds scrolls it, and the
291+
// cursor cannot then move back above the first row — every subsequent redraw
292+
// starts from the wrong place and repaints the whole screen instead of the
293+
// table. A watch of more requests than fit therefore shows as many as do and
294+
// says how many it is not showing.
206295
type renderer struct {
207296
inPlace bool
208297

209-
// width is the terminal's width, re-read before every draw. A watch runs for
210-
// minutes and a window can be resized inside them, so this is not a property
211-
// the process can sample once — see resize.
212-
width int
298+
// oneShot marks a renderer that draws once and returns, so its frame is
299+
// free to be taller than the window: no later frame has to line up with it.
300+
oneShot bool
301+
302+
// width and height are the terminal's, re-read before every draw. A watch
303+
// runs for minutes and a window can be resized inside them, so neither is a
304+
// property the process can sample once — see resize.
305+
width int
306+
height int
213307

214-
// size reports the terminal's width and whether it could be read. Held as a
215-
// field so a test can drive a resize without a terminal.
216-
size func() (int, bool)
308+
// size reports the terminal's width and height and whether they could be
309+
// read. Held as a field so a test can drive a resize without a terminal.
310+
size func() (int, int, bool)
217311

218312
wRequest int
219313
wChanges int
@@ -231,7 +325,7 @@ type renderer struct {
231325
}
232326

233327
func newRenderer() *renderer {
234-
width, sized := terminalSize()
328+
width, height, sized := terminalSize()
235329
return &renderer{
236330
// Redrawing in place requires knowing the width to wrap to. A terminal
237331
// that will not report its size is therefore treated as a log: emitting
@@ -240,6 +334,7 @@ func newRenderer() *renderer {
240334
// the lines that appeared, so one of those desyncs every frame after it.
241335
inPlace: sized,
242336
width: width,
337+
height: height,
243338
size: terminalSize,
244339
wRequest: len("REQUEST"),
245340
wChanges: len("CHANGES"),
@@ -256,12 +351,12 @@ func newRenderer() *renderer {
256351
// Anything that is not a sized terminal — a pipe, a file, a CI log — falls back
257352
// to a fixed width, since there is no width to discover and a log wants a
258353
// stable one anyway.
259-
func terminalSize() (int, bool) {
260-
w, _, err := term.GetSize(int(os.Stdout.Fd()))
354+
func terminalSize() (int, int, bool) {
355+
w, h, err := term.GetSize(int(os.Stdout.Fd()))
261356
if err != nil || w <= 0 {
262-
return defaultLineWidth, false
357+
return defaultLineWidth, 0, false
263358
}
264-
return w, true
359+
return w, h, true
265360
}
266361

267362
// lineWidth is the width to render to. It tolerates a renderer built without
@@ -289,14 +384,22 @@ func (r *renderer) resize() {
289384
if !r.inPlace || r.size == nil {
290385
return
291386
}
292-
if w, sized := r.size(); sized {
293-
r.width = w
387+
if w, h, sized := r.size(); sized {
388+
r.width, r.height = w, h
294389
}
295390
}
296391

297392
func (r *renderer) draw(rows []*Row, status string) {
298393
r.resize()
299-
body := r.body(rows)
394+
// Widths come from every row, not just the drawn ones, so a column does not
395+
// resize as rows come and go from view.
396+
r.fit(rows)
397+
398+
visible, hidden := r.visibleRows(rows)
399+
body := r.body(visible)
400+
if hidden > 0 {
401+
body = append(body, fmt.Sprintf(" … %d settled request(s) not shown; the window is too short", hidden))
402+
}
300403

301404
if !r.inPlace {
302405
sig := signature(rows)
@@ -325,6 +428,74 @@ func (r *renderer) draw(rows []*Row, status string) {
325428
r.drawn = true
326429
}
327430

431+
// frameOverhead is what a frame spends on lines other than the table: the blank
432+
// line and the status line below it, plus the row the cursor rests on, which
433+
// has to stay inside the window or the next redraw starts a line too low.
434+
const frameOverhead = 3
435+
436+
// visibleRows is the rows that fit in the window, and how many were left out.
437+
//
438+
// Settled requests are dropped first. They have stopped changing, so leaving
439+
// them out costs a reader nothing `land-list` will not tell them, whereas
440+
// dropping the ones still moving would hide the only part of the table that is
441+
// doing anything. Within each group the original order is kept, so rows do not
442+
// jump around between redraws.
443+
func (r *renderer) visibleRows(rows []*Row) ([]*Row, int) {
444+
// headerLines is the column header and its rule.
445+
const headerLines = 2
446+
447+
// Redirected output is a log, not a window: it scrolls, nothing is
448+
// overwritten, and a reader wants every row. So does a one-shot listing,
449+
// which is drawn once and scrolled back through rather than redrawn.
450+
if !r.inPlace || r.oneShot || r.height <= 0 {
451+
return rows, 0
452+
}
453+
budget := r.height - frameOverhead - headerLines
454+
if budget < 1 {
455+
return nil, len(rows)
456+
}
457+
458+
heights := make([]int, len(rows))
459+
total := 0
460+
for i, rw := range rows {
461+
heights[i] = len(r.rowLines(rw)) + len(r.noteLines(rw))
462+
total += heights[i]
463+
}
464+
if total <= budget {
465+
return rows, 0
466+
}
467+
// One line goes to the note saying what is not shown.
468+
budget--
469+
470+
// Keep the unsettled first, then settled, then restore the original order,
471+
// so what survives is the moving part of the table without being reordered.
472+
keep := make([]bool, len(rows))
473+
used := 0
474+
for _, settled := range []bool{false, true} {
475+
for i, rw := range rows {
476+
if rw.Done != settled || keep[i] {
477+
continue
478+
}
479+
if used+heights[i] > budget {
480+
continue
481+
}
482+
keep[i] = true
483+
used += heights[i]
484+
}
485+
}
486+
487+
visible := make([]*Row, 0, len(rows))
488+
hidden := 0
489+
for i, rw := range rows {
490+
if keep[i] {
491+
visible = append(visible, rw)
492+
continue
493+
}
494+
hidden++
495+
}
496+
return visible, hidden
497+
}
498+
328499
// body renders the header and one line per row.
329500
func (r *renderer) body(rows []*Row) []string {
330501
r.fit(rows)

0 commit comments

Comments
 (0)