Skip to content

Commit abd120d

Browse files
authored
Merge pull request #175 from flashcatcloud/fix/list-auto-reduce-page-fit
fix(cli): reduce over-budget projected list pages instead of shortening values
2 parents a42780c + 525f253 commit abd120d

9 files changed

Lines changed: 410 additions & 183 deletions

File tree

cmd/flashduty/main_test.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -112,24 +112,23 @@ func TestSetVersionInfoBeforeExecute(t *testing.T) {
112112
}
113113
}
114114

115-
// Test 79: When a compact list projection overflows its byte budget, the
116-
// binary exits non-zero, writes nothing to stdout, and reports the error on
117-
// stderr — a pipeline reading stdout must see a failed call, never an empty
118-
// page masquerading as "no data".
115+
// Test 79: When a compact list projection cannot fit its byte budget at
116+
// all — a single row that overflows on its own and carries nothing
117+
// shortenable — the binary exits non-zero, writes nothing to stdout, and
118+
// reports the error on stderr: a pipeline reading stdout must see a failed
119+
// call, never an empty page masquerading as "no data". (A multi-row page
120+
// that overflows is instead reduced to the leading rows that fit, announced
121+
// on stderr.)
119122
func TestProjectionOverflowFailsHard(t *testing.T) {
120123
binPath := buildTestBinary(t, "")
121124

122-
// Stub the alert-event list endpoint with a page whose projection stays
123-
// over the 16 KiB budget even after value shortening.
125+
// Stub the alert-event list endpoint with one row whose projected labels
126+
// blob alone exceeds the 16 KiB budget; a labels map is not a string, so
127+
// no value shortening can rescue it.
124128
var body strings.Builder
125-
body.WriteString(`{"request_id":"r","error":{"code":"OK","message":""},"data":{"total":100,"items":[`)
126-
for i := 0; i < 100; i++ {
127-
if i > 0 {
128-
body.WriteByte(',')
129-
}
130-
fmt.Fprintf(&body, `{"event_id":"%024x","alert_id":"%024x","event_severity":"Warning","event_status":"Triggered","event_time":1712000000,"title":%q}`,
131-
i, i+1_000_000, strings.Repeat("x", 200))
132-
}
129+
body.WriteString(`{"request_id":"r","error":{"code":"OK","message":""},"data":{"total":1,"items":[`)
130+
fmt.Fprintf(&body, `{"event_id":"%024x","alert_id":"%024x","event_severity":"Warning","event_status":"Triggered","event_time":1712000000,"title":"disk full","labels":{"payload":%q}}`,
131+
1, 1_000_001, strings.Repeat("x", 20000))
133132
body.WriteString(`]}}`)
134133

135134
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -138,7 +137,7 @@ func TestProjectionOverflowFailsHard(t *testing.T) {
138137
}))
139138
defer srv.Close()
140139

141-
run := exec.Command(binPath, "alert-event", "list", "--limit", "100",
140+
run := exec.Command(binPath, "alert-event", "list", "--fields", "event_id,labels",
142141
"--output-format", "json", "--app-key", "test-key", "--base-url", srv.URL)
143142
// Isolate HOME so the test never reads the developer's real CLI config.
144143
run.Env = append(os.Environ(), "HOME="+t.TempDir())

internal/cli/alert_event.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,17 @@ func newAlertEventListCmd() *cobra.Command {
9797
if err != nil {
9898
return err
9999
}
100-
note, err := boundProjectedOutput(proj, compactListOutputLimit)
100+
bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit)
101101
if err != nil {
102102
return err
103103
}
104-
noteProjectionShortening(cmd.ErrOrStderr(), note)
105-
return ctx.PrintList(proj, nil, len(result.Items), page, limit, int(result.Total))
104+
proj = bounded.([]map[string]any)
105+
noteProjectionBound(cmd.ErrOrStderr(), note)
106+
effectiveLimit := limit
107+
if len(proj) < len(result.Items) {
108+
effectiveLimit = len(proj)
109+
}
110+
return ctx.PrintList(proj, nil, len(proj), page, effectiveLimit, int(result.Total))
106111
}
107112

108113
return ctx.PrintList(result.Items, cols, len(result.Items), page, limit, int(result.Total))
@@ -119,7 +124,7 @@ func newAlertEventListCmd() *cobra.Command {
119124
cmd.Flags().StringVar(&until, "until", "now", "End time")
120125
cmd.Flags().IntVar(&limit, "limit", 20, "Max results (max 100)")
121126
cmd.Flags().IntVar(&page, "page", 1, "Page number")
122-
cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. event_id,alert_id,event_severity,event_status,event_time,title); ignored in table mode. Defaults to these compact event fields. Long strings are truncated as needed to keep structured output below 16 KiB.")
127+
cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. event_id,alert_id,event_severity,event_status,event_time,title); ignored in table mode. Defaults to these compact event fields. If the page would exceed 16 KiB, only the leading rows that fit are emitted, with every value intact (announced on stderr).")
123128

124129
return cmd
125130
}

internal/cli/channel.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,12 @@ func newChannelEscalateRuleListCmd() *cobra.Command {
195195
if err != nil {
196196
return err
197197
}
198-
note, err := boundProjectedOutput(proj, compactListOutputLimit)
198+
bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit)
199199
if err != nil {
200200
return err
201201
}
202-
noteProjectionShortening(cmd.ErrOrStderr(), note)
202+
proj = bounded.([]map[string]any)
203+
noteProjectionBound(cmd.ErrOrStderr(), note)
203204
return ctx.PrintTotal(proj, nil, len(proj))
204205
}
205206

internal/cli/command_test.go

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1705,24 +1705,22 @@ func TestCommandAlertListStructuredAnnouncesTruncation(t *testing.T) {
17051705
// ---------------------------------------------------------------------------
17061706

17071707
// TestCommandListProjectionOverflowFails pins that a compact list projection
1708-
// which cannot fit the byte budget fails the command instead of emitting
1709-
// anything: Execute returns the error and stdout stays empty, so a pipeline
1710-
// reading stdout sees a failed call, never an empty page masquerading as
1711-
// "no data".
1708+
// which cannot fit the byte budget at all — a single row that overflows on
1709+
// its own and carries nothing shortenable — fails the command instead of
1710+
// emitting anything: Execute returns the error and stdout stays empty, so a
1711+
// pipeline reading stdout sees a failed call, never an empty page
1712+
// masquerading as "no data". (A multi-row page that overflows is instead
1713+
// reduced to the leading rows that fit — see
1714+
// TestAlertEventListAutoReducesPageAtLargeLimit.)
17121715
func TestCommandListProjectionOverflowFails(t *testing.T) {
17131716
t.Run("incident list", func(t *testing.T) {
17141717
saveAndResetGlobals(t)
17151718
stub := newGFStub(t)
1716-
items := make([]any, 100)
1717-
for i := range items {
1718-
row := incidentRow()
1719-
row["incident_id"] = fmt.Sprintf("inc-%024d", i)
1720-
row["title"] = strings.Repeat("x", 200)
1721-
items[i] = row
1722-
}
1723-
stub.data = map[string]any{"items": items, "total": len(items)}
1719+
row := incidentRow()
1720+
row["labels"] = map[string]any{"payload": strings.Repeat("x", 20000)}
1721+
stub.data = map[string]any{"items": []any{row}, "total": 1}
17241722

1725-
out, stderrText, err := execCommandSplit("incident", "list", "--limit", "100", "--output-format", "json")
1723+
out, stderrText, err := execCommandSplit("incident", "list", "--fields", "incident_id,labels", "--output-format", "json")
17261724
if err == nil || !strings.Contains(err.Error(), "exceeds the 16384-byte limit") {
17271725
t.Fatalf("irreducible projection error = %v, want the byte-limit refusal", err)
17281726
}
@@ -1737,20 +1735,18 @@ func TestCommandListProjectionOverflowFails(t *testing.T) {
17371735
t.Run("alert-event list", func(t *testing.T) {
17381736
saveAndResetGlobals(t)
17391737
stub := newGFStub(t)
1740-
items := make([]any, 100)
1741-
for i := range items {
1742-
items[i] = map[string]any{
1743-
"event_id": fmt.Sprintf("%024x", i),
1744-
"alert_id": fmt.Sprintf("%024x", i+1_000_000),
1745-
"event_severity": "Warning",
1746-
"event_status": "Triggered",
1747-
"event_time": 1712000000 + i,
1748-
"title": strings.Repeat("x", 200),
1749-
}
1738+
row := map[string]any{
1739+
"event_id": fmt.Sprintf("%024x", 1),
1740+
"alert_id": fmt.Sprintf("%024x", 1_000_001),
1741+
"event_severity": "Warning",
1742+
"event_status": "Triggered",
1743+
"event_time": 1712000000,
1744+
"title": "disk full",
1745+
"labels": map[string]any{"payload": strings.Repeat("x", 20000)},
17501746
}
1751-
stub.data = map[string]any{"items": items, "total": len(items)}
1747+
stub.data = map[string]any{"items": []any{row}, "total": 1}
17521748

1753-
out, _, err := execCommandSplit("alert-event", "list", "--limit", "100", "--output-format", "json")
1749+
out, _, err := execCommandSplit("alert-event", "list", "--fields", "event_id,labels", "--output-format", "json")
17541750
if err == nil || !strings.Contains(err.Error(), "exceeds the 16384-byte limit") {
17551751
t.Fatalf("irreducible projection error = %v, want the byte-limit refusal", err)
17561752
}

internal/cli/fieldproject.go

Lines changed: 85 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -80,39 +80,42 @@ func noteDefaultProjection(w io.Writer, fields []string) {
8080
strings.Join(fields, ","))
8181
}
8282

83-
// noteProjectionShortening tells the caller, on stderr, that some values came
84-
// back clipped. Without it a shortened value is only visible to a reader, not
85-
// to the jq filter or exact match a --json consumer runs over it, so a query
86-
// that silently matches nothing looks like an empty result rather than a
87-
// truncated one.
88-
func noteProjectionShortening(w io.Writer, note string) {
83+
// noteProjectionBound relays a boundProjectedOutput note to the caller on
84+
// stderr. Without it a reduced page or a shortened value is only visible to
85+
// a reader, not to the jq filter or exact match a --json consumer runs over
86+
// it, so a query that silently matches nothing looks like an empty result
87+
// rather than a bounded one.
88+
func noteProjectionBound(w io.Writer, note string) {
8989
if note == "" {
9090
return
9191
}
9292
_, _ = fmt.Fprintln(w, note)
9393
}
9494

9595
// boundProjectedOutput keeps the new agent-oriented projections below their
96-
// command budget without changing the selected keys. List rows (many small
97-
// records) are shortened fairly when they overflow the budget, with
98-
// shortened values marked with "...". A single-object detail projection is
99-
// never modified: a truncated id or status string is indistinguishable from
100-
// a genuinely short value, so silently shortening it would hand the caller
101-
// wrong data instead of a compact one. If a detail projection doesn't fit,
102-
// the command fails with an error instead.
96+
// command budget without changing the selected keys. A list projection that
97+
// overflows the budget is first reduced to the leading rows that fit with
98+
// every value intact; only a single row that overflows the budget on its own
99+
// is shortened fairly, with shortened values marked with "...". A
100+
// single-object detail projection is never modified: a truncated id or
101+
// status string is indistinguishable from a genuinely short value, so
102+
// silently shortening it would hand the caller wrong data instead of a
103+
// compact one. If a detail projection doesn't fit, the command fails with an
104+
// error instead.
103105
//
104-
// It returns a caller-printable note (empty when nothing was shortened) that
105-
// names the clipped fields, so the caller can announce the loss on stderr —
106-
// the "..." marker is only visible to something that reads the value, never
107-
// to the filter a --json consumer runs over it.
108-
func boundProjectedOutput(data any, maxBytes int) (string, error) {
106+
// It returns the bounded data with the same type it was given, plus a
107+
// caller-printable note (empty when nothing was reduced or shortened), so
108+
// the caller can announce the loss on stderr — the "..." marker is only
109+
// visible to something that reads the value, never to the filter a --json
110+
// consumer runs over it.
111+
func boundProjectedOutput(data any, maxBytes int) (any, string, error) {
109112
switch value := data.(type) {
110113
case map[string]any:
111-
return "", boundProjectedDetail(value, maxBytes)
114+
return value, "", boundProjectedDetail(value, maxBytes)
112115
case []map[string]any:
113116
return boundProjectedList(value, maxBytes)
114117
default:
115-
return "", fmt.Errorf("internal error: unsupported projected output %T", data)
118+
return nil, "", fmt.Errorf("internal error: unsupported projected output %T", data)
116119
}
117120
}
118121

@@ -190,10 +193,14 @@ func isIdentifierField(key string) bool {
190193
return strings.HasSuffix(key, "_id") || strings.HasSuffix(key, "_key")
191194
}
192195

193-
// boundProjectedList shortens a list projection's string values fairly when
194-
// the compact rows themselves overflow the budget: it finds the largest
195-
// per-field byte cap that still makes everything fit, then applies that one
196-
// cap to every shortenable string value across every row. A field already
196+
// boundProjectedList keeps a list projection below the budget. A page that
197+
// overflows is first reduced to the largest leading prefix of rows that
198+
// fits, with every value intact: a --json consumer filters and matches on
199+
// the values, so a partial page of intact rows serves it, while a full page
200+
// of "..."-clipped rows silently defeats the filter. Only when one row alone
201+
// overflows the budget does it shorten that row's string values fairly: it
202+
// finds the largest per-field byte cap that still makes the row fit, then
203+
// applies that one cap to every shortenable string value. A field already
197204
// shorter than the cap is left completely untouched — only the field(s)
198205
// actually responsible for the overflow (typically a long title) get
199206
// shortened, each marked with "...". Identifier fields (keys ending in _id
@@ -202,27 +209,37 @@ func isIdentifierField(key string) bool {
202209
// marker itself disappear, so a shortened value is always distinguishable
203210
// from a genuinely short one; if no cap at or above that floor fits, the
204211
// command fails with a small error instead of emitting values that look
205-
// real but aren't. Whatever it clips, it reports back in the returned note.
206-
func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
212+
// real but aren't. Whatever it reduces or clips, it reports back in the
213+
// returned note.
214+
func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, string, error) {
207215
encoded, err := marshalStructured(rows)
208216
if err != nil {
209-
return "", err
217+
return nil, "", err
210218
}
211219
if len(encoded)+1 < maxBytes {
212-
return "", nil
220+
return rows, "", nil
213221
}
214222

215223
// The overflow error names the fields responsible, exactly as the detail
216224
// path does, so the request can be narrowed in one pass.
217-
tooBig := func() (string, error) {
225+
tooBig := func() ([]map[string]any, string, error) {
218226
largest, err := largestProjectedFields(rows)
219227
if err != nil {
220-
return "", err
228+
return nil, "", err
221229
}
222-
return "", fmt.Errorf("projected list is %d bytes across %d rows, exceeds the %d-byte limit; largest fields: %s; request fewer rows (--limit) or fewer --fields",
230+
return nil, "", fmt.Errorf("projected list is %d bytes across %d rows, exceeds the %d-byte limit; largest fields: %s; request fewer rows (--limit) or fewer --fields",
223231
len(encoded), len(rows), maxBytes, largest)
224232
}
225233

234+
kept, err := largestFittingPrefix(rows, maxBytes)
235+
if err != nil {
236+
return nil, "", err
237+
}
238+
if kept > 0 {
239+
return rows[:kept], fmt.Sprintf("note: emitted %d of %d projected rows (every value intact) to stay below the %d-byte structured-output limit; narrow --fields or lower --limit to fit more rows per page — the rows past the first %d were not emitted",
240+
kept, len(rows), maxBytes, kept), nil
241+
}
242+
226243
maxLen := 0
227244
for _, row := range rows {
228245
for key, value := range row {
@@ -268,7 +285,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
268285
return tooBig()
269286
}
270287
if ok, err := fits(minMarkedTruncationCap); err != nil {
271-
return "", err
288+
return nil, "", err
272289
} else if !ok {
273290
return tooBig()
274291
}
@@ -282,7 +299,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
282299
mid := lo + (hi-lo+1)/2
283300
ok, err := fits(mid)
284301
if err != nil {
285-
return "", err
302+
return nil, "", err
286303
}
287304
if ok {
288305
lo = mid
@@ -312,17 +329,50 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
312329
}
313330
}
314331
if shortened == 0 {
315-
return "", nil
332+
return rows, "", nil
316333
}
317334
names := make([]string, 0, len(fields))
318335
for name := range fields {
319336
names = append(names, name)
320337
}
321338
sort.Strings(names)
322-
return fmt.Sprintf("note: %d of %d string values were shortened to fit the %d-byte limit and now end with \"...\" (fields: %s); matching or filtering on those fields will miss — narrow --fields or --limit for untruncated values",
339+
return rows, fmt.Sprintf("note: %d of %d string values were shortened to fit the %d-byte limit and now end with \"...\" (fields: %s); matching or filtering on those fields will miss — narrow --fields or --limit for untruncated values",
323340
shortened, total, maxBytes, strings.Join(names, ", ")), nil
324341
}
325342

343+
// largestFittingPrefix returns the largest n < len(rows) whose encoded prefix
344+
// rows[:n] fits the budget, or 0 when even one row overflows it. Prefix size
345+
// is monotone — appending a row never shrinks the encoding — so the boundary
346+
// is found by binary search between lo=1 (known fitting, checked first) and
347+
// hi=len(rows) (known not to fit: the caller only reaches here on overflow).
348+
func largestFittingPrefix(rows []map[string]any, maxBytes int) (int, error) {
349+
fits := func(n int) (bool, error) {
350+
encoded, err := marshalStructured(rows[:n])
351+
if err != nil {
352+
return false, err
353+
}
354+
return len(encoded)+1 < maxBytes, nil
355+
}
356+
ok, err := fits(1)
357+
if err != nil || !ok {
358+
return 0, err
359+
}
360+
lo, hi := 1, len(rows) // fits(lo) holds, fits(hi) does not
361+
for hi-lo > 1 {
362+
mid := lo + (hi-lo)/2
363+
ok, err := fits(mid)
364+
if err != nil {
365+
return 0, err
366+
}
367+
if ok {
368+
lo = mid
369+
} else {
370+
hi = mid
371+
}
372+
}
373+
return lo, nil
374+
}
375+
326376
func truncateUTF8Bytes(value string, maxBytes int) string {
327377
if len(value) <= maxBytes {
328378
return value

0 commit comments

Comments
 (0)