Skip to content

Commit 612e7cb

Browse files
committed
feat(client): show request log events in list and watch
## Summary ### Why? A request's history holds two kinds of entry. A **status** is a position it reached — `batched`, `speculating`, `landed` — and a **event** is something that happened while it sat at one: a build starting, a build finishing, a passed path waiting on a dependency that has not resolved. The table only ever showed the first kind. `digest` skipped every entry whose status was empty, which is exactly what an event is, so the whole second half of the request log was dropped on the floor. 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. ### 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. Eight builds is the interesting fact; eight words are not. **`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 quickstart's description of the two commands is updated, including a claim about `list` that this makes false. ## 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 change exists to show - ✅ `make land-list` mid-flight reports `speculating` where it used to report `…`, and `landed` once the run settled - ✅ 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 `stage` cases for the bare position and for a fetched trail taking precedence - ✅ `make test` (105 targets), `make lint`, `make gazelle`
1 parent 04445d2 commit 612e7cb

3 files changed

Lines changed: 181 additions & 9 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: 84 additions & 7 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
}
@@ -139,28 +145,99 @@ func Draw(rows []*Row, status string) {
139145
// digest reduces a request's recorded history to the trail worth showing, the
140146
// status it currently holds, and the error the latest event carried. A status
141147
// recorded more than once in a row is one step in the trail, not several.
148+
//
149+
// The history holds two kinds of entry. A status is a position the request
150+
// reached, and those are the trail's spine. An event is something that happened
151+
// while it sat at one — a build starting, a passed path waiting on a dependency
152+
// — and never changes the position, so each is shown against the status it
153+
// occurred under rather than as a step of its own:
154+
//
155+
// batched → speculating [building ×2, built] → speculated
156+
//
157+
// Repeats are counted rather than listed. A batch runs one build per
158+
// speculation path, so a request that speculated widely records `building` many
159+
// times, and a trail that spelled each one out would say less than the count
160+
// does while pushing the rest of the row off the line.
142161
func digest(events []*pb.HistoryEvent) (trail []string, status, note string) {
143162
if len(events) == 0 {
144163
return nil, "", ""
145164
}
165+
166+
// Events seen since the last status, in first-seen order with their counts,
167+
// so they can be attached once the step they belong to is complete.
168+
var pending []string
169+
var last string
170+
counts := make(map[string]int)
171+
172+
flush := func() {
173+
if len(trail) == 0 || len(pending) == 0 {
174+
pending, counts = nil, make(map[string]int)
175+
return
176+
}
177+
trail[len(trail)-1] += " [" + strings.Join(annotate(pending, counts), ", ") + "]"
178+
pending, counts = nil, make(map[string]int)
179+
}
180+
146181
for _, e := range events {
147-
if e == nil || e.Status == "" {
182+
if e == nil {
148183
continue
149184
}
150-
if len(trail) > 0 && trail[len(trail)-1] == e.Status {
185+
if e.Status == "" {
186+
if e.Event == "" {
187+
continue
188+
}
189+
if counts[e.Event] == 0 {
190+
pending = append(pending, e.Event)
191+
}
192+
counts[e.Event]++
151193
continue
152194
}
195+
// A status repeated back-to-back is one step, but anything recorded
196+
// against it in between still belongs to that step.
197+
if e.Status == last {
198+
continue
199+
}
200+
flush()
153201
trail = append(trail, e.Status)
202+
last = e.Status
154203
}
204+
flush()
205+
155206
if last := events[len(events)-1]; last != nil {
156207
status, note = last.Status, last.LastError
157208
}
158-
if status == "" && len(trail) > 0 {
159-
status = trail[len(trail)-1]
209+
// The last entry may be an event, which leaves the request where it was.
210+
if status == "" {
211+
status = currentStatus(events)
160212
}
161213
return trail, status, note
162214
}
163215

216+
// annotate renders each event with its count, dropping the count when it
217+
// happened once.
218+
func annotate(order []string, counts map[string]int) []string {
219+
out := make([]string, 0, len(order))
220+
for _, event := range order {
221+
if counts[event] > 1 {
222+
out = append(out, fmt.Sprintf("%s ×%d", event, counts[event]))
223+
continue
224+
}
225+
out = append(out, event)
226+
}
227+
return out
228+
}
229+
230+
// currentStatus is the last position the request reached, ignoring anything
231+
// recorded while it sat there.
232+
func currentStatus(events []*pb.HistoryEvent) string {
233+
for i := len(events) - 1; i >= 0; i-- {
234+
if e := events[i]; e != nil && e.Status != "" {
235+
return e.Status
236+
}
237+
}
238+
return ""
239+
}
240+
164241
// outcome is the one-line verdict shown under the finished table.
165242
func outcome(rows []*Row) string {
166243
landed := 0

submitqueue/client/view_test.go

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,80 @@ func TestDigest(t *testing.T) {
8181
wantNote: "merge conflict",
8282
},
8383
{
84-
name: "events without a status do not become steps",
84+
name: "entries carrying neither a status nor an event are ignored",
8585
events: []*pb.HistoryEvent{{Status: ""}, {Status: "accepted"}, {Status: ""}},
8686
wantTrail: []string{"accepted"},
8787
wantStatus: "accepted",
8888
},
89+
{
90+
// An event is something that happened while the request sat at a
91+
// position, so it belongs to that step rather than being one.
92+
name: "an event is shown against the status it happened under",
93+
events: []*pb.HistoryEvent{
94+
{Status: "batched"},
95+
{Status: "speculating"},
96+
{Event: "building"},
97+
{Event: "built"},
98+
{Status: "speculated"},
99+
},
100+
wantTrail: []string{"batched", "speculating [building, built]", "speculated"},
101+
wantStatus: "speculated",
102+
},
103+
{
104+
// One build per speculation path, so a request that speculated
105+
// widely records this many times over.
106+
name: "repeats are counted rather than listed",
107+
events: []*pb.HistoryEvent{
108+
{Status: "speculating"},
109+
{Event: "building"}, {Event: "building"}, {Event: "building"},
110+
{Event: "built"},
111+
},
112+
wantTrail: []string{"speculating [building ×3, built]"},
113+
wantStatus: "speculating",
114+
},
115+
{
116+
name: "an event does not move the request off its status",
117+
events: []*pb.HistoryEvent{
118+
{Status: "speculating"}, {Event: "waiting"},
119+
},
120+
wantTrail: []string{"speculating [waiting]"},
121+
wantStatus: "speculating",
122+
},
123+
{
124+
name: "each status collects only the events recorded under it",
125+
events: []*pb.HistoryEvent{
126+
{Status: "speculating"}, {Event: "building"},
127+
{Status: "speculated"}, {Event: "invalidated"},
128+
{Status: "speculating"}, {Event: "building"}, {Event: "built"},
129+
},
130+
wantTrail: []string{
131+
"speculating [building]", "speculated [invalidated]", "speculating [building, built]",
132+
},
133+
wantStatus: "speculating",
134+
},
135+
{
136+
name: "a status repeated around its own events is still one step",
137+
events: []*pb.HistoryEvent{
138+
{Status: "speculating"}, {Event: "building"}, {Status: "speculating"}, {Event: "built"},
139+
},
140+
wantTrail: []string{"speculating [building, built]"},
141+
wantStatus: "speculating",
142+
},
143+
{
144+
name: "an event before any status has nothing to attach to",
145+
events: []*pb.HistoryEvent{{Event: "building"}, {Status: "accepted"}},
146+
wantTrail: []string{"accepted"},
147+
wantStatus: "accepted",
148+
},
149+
{
150+
name: "an error carried by an event is still reported",
151+
events: []*pb.HistoryEvent{
152+
{Status: "speculating"}, {Event: "building", LastError: "runner unreachable"},
153+
},
154+
wantTrail: []string{"speculating [building]"},
155+
wantStatus: "speculating",
156+
wantNote: "runner unreachable",
157+
},
89158
}
90159

91160
for _, tt := range tests {
@@ -165,6 +234,23 @@ func TestRowStage(t *testing.T) {
165234
row: Row{SQID: "demo-queue/17", Trail: []string{"accepted", "started", "landed"}},
166235
want: "accepted → started → landed",
167236
},
237+
{
238+
// A listing reads receipts rather than histories, so it knows where
239+
// a request is without knowing how it got there. That still beats
240+
// a column of nothing.
241+
name: "the position it holds, when the trail was never fetched",
242+
row: Row{SQID: "demo-queue/17", Status: "speculating"},
243+
want: "speculating",
244+
},
245+
{
246+
name: "a fetched trail is preferred over the bare position",
247+
row: Row{
248+
SQID: "demo-queue/17",
249+
Status: "landed",
250+
Trail: []string{"accepted", "landed"},
251+
},
252+
want: "accepted → landed",
253+
},
168254
}
169255

170256
for _, tt := range tests {

0 commit comments

Comments
 (0)