Skip to content

Commit a4ffb64

Browse files
committed
fix(cli): stop compact list projection from mangling short fields
The default compact projection used by alert-event list, incident list, and incident similar (in json/toon mode, when --fields is omitted) computed a single per-field byte cap by dividing the total budget by the total count of string values across every row, then geometrically halved that cap whenever the aggregate output still overflowed 16 KiB. Each halving re-applied the shrinking cap to every string field on every row, including fields that were never responsible for the overflow (e.g. Mongo ObjectID-shaped ids, or short enum-like severity/ status strings), clipping them down toward a 1-byte cap even though a single long field (typically the title) was the actual cause. Once the cap dropped to 3 bytes or below, the truncation helper had no room left for its "..." marker and fell back to returning raw, unmarked bytes — making a shortened value indistinguishable from a genuinely short one. Piping such output to jq/grep for an exact id or status match then silently returns no hits, with no indication that the field was ever truncated. Replace the per-field cap computation with a search for the largest single cap that lets the whole page fit, then apply it once. A field already shorter than that cap is left completely untouched, so only the field(s) actually responsible for the overflow get shortened, and the cap is never allowed to drop low enough to lose the "..." marker. When no such cap exists, the command now fails with an actionable error instead of emitting values that look real but aren't. Add regression coverage: a fixture with long ids and a minority of oversized multi-word/CJK titles confirms the ids and short titles stay intact in both json and toon output while only the oversized titles are marked-truncated; a --fields path test confirms explicit field selection is unaffected; and a low-level test forces every string field to shrink and asserts the "..." marker is never dropped.
1 parent 6344780 commit a4ffb64

2 files changed

Lines changed: 231 additions & 21 deletions

File tree

internal/cli/fieldproject.go

Lines changed: 64 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,17 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error {
143143
len(encoded), maxBytes, strings.Join(largest, ", "))
144144
}
145145

146-
// boundProjectedList shortens a list projection's string values fairly
147-
// (across all rows) when the compact rows themselves overflow the budget,
148-
// marking shortened values with "...". If keys and non-string values alone
149-
// exceed the budget, the command fails with a small error instead of
150-
// emitting an oversized payload.
146+
// boundProjectedList shortens a list projection's string values fairly when
147+
// the compact rows themselves overflow the budget: it finds the largest
148+
// per-field byte cap that still makes everything fit, then applies that one
149+
// cap to every string value across every row. A field already shorter than
150+
// the cap is left completely untouched — only the field(s) actually
151+
// responsible for the overflow (typically a long title) get shortened, each
152+
// marked with "...". The cap never drops low enough to make the "..."
153+
// marker itself disappear, so a shortened value is always distinguishable
154+
// from a genuinely short one; if no cap at or above that floor fits, the
155+
// command fails with a small error instead of emitting values that look
156+
// real but aren't.
151157
func boundProjectedList(rows []map[string]any, maxBytes int) error {
152158
encoded, err := marshalStructured(rows)
153159
if err != nil {
@@ -157,40 +163,79 @@ func boundProjectedList(rows []map[string]any, maxBytes int) error {
157163
return nil
158164
}
159165

160-
stringCount := 0
166+
maxLen := 0
161167
for _, row := range rows {
162168
for _, value := range row {
163-
if _, ok := value.(string); ok {
164-
stringCount++
169+
if text, ok := value.(string); ok && len(text) > maxLen {
170+
maxLen = len(text)
165171
}
166172
}
167173
}
168-
if stringCount == 0 {
174+
if maxLen == 0 {
169175
return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
170176
}
171177

172-
fieldLimit := maxBytes / stringCount
173-
for {
174-
for _, row := range rows {
178+
fits := func(limit int) (bool, error) {
179+
trial := make([]map[string]any, len(rows))
180+
for i, row := range rows {
181+
trialRow := make(map[string]any, len(row))
175182
for key, value := range row {
176183
if text, ok := value.(string); ok {
177-
row[key] = truncateUTF8Bytes(text, fieldLimit)
184+
trialRow[key] = truncateUTF8Bytes(text, limit)
185+
} else {
186+
trialRow[key] = value
178187
}
179188
}
189+
trial[i] = trialRow
180190
}
191+
trialEncoded, err := marshalStructured(trial)
192+
if err != nil {
193+
return false, err
194+
}
195+
return len(trialEncoded)+1 < maxBytes, nil
196+
}
181197

182-
encoded, err = marshalStructured(rows)
198+
// minMarkedTruncationCap is the smallest cap for which truncateUTF8Bytes
199+
// still appends "..." (it needs 3 bytes of headroom beyond the marker
200+
// itself); below it a truncated value would be indistinguishable from a
201+
// genuinely short one, which is the defect this function must not
202+
// reintroduce.
203+
const minMarkedTruncationCap = 4
204+
if maxLen <= minMarkedTruncationCap {
205+
return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
206+
}
207+
if ok, err := fits(minMarkedTruncationCap); err != nil {
208+
return err
209+
} else if !ok {
210+
return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
211+
}
212+
213+
// Binary search for the largest cap that still fits: fits(limit) is true
214+
// for small limits (more shortened) and false for large ones (less
215+
// shortened, up to and including maxLen, which is the untouched size we
216+
// already know overflows), so the boundary is unique.
217+
lo, hi := minMarkedTruncationCap, maxLen-1
218+
for lo < hi {
219+
mid := lo + (hi-lo+1)/2
220+
ok, err := fits(mid)
183221
if err != nil {
184222
return err
185223
}
186-
if len(encoded)+1 < maxBytes {
187-
return nil
224+
if ok {
225+
lo = mid
226+
} else {
227+
hi = mid - 1
188228
}
189-
if fieldLimit == 0 {
190-
return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
229+
}
230+
231+
for _, row := range rows {
232+
for key, value := range row {
233+
if text, ok := value.(string); ok {
234+
row[key] = truncateUTF8Bytes(text, lo)
235+
}
191236
}
192-
fieldLimit /= 2
193237
}
238+
return nil
194239
}
195240

196241
func truncateUTF8Bytes(value string, maxBytes int) string {

internal/cli/fieldproject_test.go

Lines changed: 167 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli
33
import (
44
"bytes"
55
"encoding/json"
6+
"fmt"
67
"reflect"
78
"strings"
89
"testing"
@@ -195,9 +196,9 @@ func TestIncidentListStructuredDefaultUsesCompactProjection(t *testing.T) {
195196
row["title"] = strings.Repeat("数据库故障", 5000)
196197
stub.data = map[string]any{"items": []any{row}, "total": 1}
197198

198-
out, err := execCommand("incident", "list", "--output-format", format)
199+
out, _, err := execCommandSplit("incident", "list", "--output-format", format)
199200
if err != nil {
200-
t.Fatalf("execCommand: %v", err)
201+
t.Fatalf("execCommandSplit: %v", err)
201202
}
202203
if len([]byte(out)) >= compactListOutputLimit {
203204
t.Fatalf("bounded %s incident list is %d bytes, want <%d", format, len([]byte(out)), compactListOutputLimit)
@@ -656,6 +657,170 @@ func TestAlertEventListStructuredProjection(t *testing.T) {
656657

657658
}
658659

660+
// alertEventOutlierFixture builds n alert-event rows with long (Mongo
661+
// ObjectID-shaped) ids: the first `outliers` rows carry a pathologically long
662+
// multi-word/CJK title (the field actually responsible for a page overflowing
663+
// the compact-list budget), the rest carry short, realistic titles. It
664+
// returns the row list alongside the exact ids and titles a correct
665+
// projection must preserve or shorten.
666+
func alertEventOutlierFixture(n, outliers int) (items []any, ids, shortTitles []string) {
667+
longTitle := strings.Repeat("K8S pod tcp 接收队列大于2000 / cluster-prod-a / node-17 / namespace kube-system / pod coredns-7db6d8ff4d-abcde ", 40)
668+
normalTitles := []string{
669+
"ERROR Detected / VMLogs-Prod",
670+
"CPU利用率较高 / fc-n9e-plus-18001",
671+
"服务器 dev-flasheye-01 连续飘红",
672+
"Disk usage high / db-02",
673+
}
674+
675+
items = make([]any, n)
676+
ids = make([]string, 0, n*2)
677+
for i := range items {
678+
eventID := fmt.Sprintf("%024x", i)
679+
alertID := fmt.Sprintf("%024x", i+1_000_000)
680+
ids = append(ids, eventID, alertID)
681+
682+
title := normalTitles[i%len(normalTitles)]
683+
if i >= outliers {
684+
shortTitles = append(shortTitles, title)
685+
} else {
686+
title = fmt.Sprintf("%s (row %d)", longTitle, i)
687+
}
688+
689+
items[i] = map[string]any{
690+
"event_id": eventID,
691+
"alert_id": alertID,
692+
"event_severity": "Warning",
693+
"event_status": "Triggered",
694+
"event_time": 1712000000 + i,
695+
"title": title,
696+
}
697+
}
698+
return items, ids, shortTitles
699+
}
700+
701+
// TestAlertEventListDefaultProjectionPreservesShortFields is the regression
702+
// guard for the original defect: a minority of pathologically long titles
703+
// pushing a page over the 16 KiB compact-list budget must never mangle the
704+
// other rows' long (Mongo ObjectID-shaped) ids, or the short title rows on
705+
// the same page — the shortening must land entirely on the field(s) actually
706+
// responsible for the overflow.
707+
func TestAlertEventListDefaultProjectionPreservesShortFields(t *testing.T) {
708+
for _, format := range []string{"json", "toon"} {
709+
t.Run(format, func(t *testing.T) {
710+
saveAndResetGlobals(t)
711+
stub := newGFStub(t)
712+
items, ids, shortTitles := alertEventOutlierFixture(30, 3)
713+
stub.data = map[string]any{"items": items, "total": len(items)}
714+
715+
out, stderrText, err := execCommandSplit("alert-event", "list", "--output-format", format)
716+
if err != nil {
717+
t.Fatalf("execCommandSplit: %v", err)
718+
}
719+
if len(out) >= compactListOutputLimit {
720+
t.Fatalf("compact alert-event output is %d bytes, want <%d", len(out), compactListOutputLimit)
721+
}
722+
if !strings.Contains(stderrText, "note: rows projected to default compact fields") {
723+
t.Errorf("default projection should announce itself on stderr, got:\n%s", stderrText)
724+
}
725+
726+
for _, id := range ids {
727+
if !strings.Contains(out, id) {
728+
t.Errorf("id %q was shortened; only the oversized outlier titles should shrink, got:\n%s", id, out)
729+
}
730+
}
731+
for _, title := range shortTitles {
732+
if !strings.Contains(out, title) {
733+
t.Errorf("short title %q was shortened even though it never exceeded the budget on its own, got:\n%s", title, out)
734+
}
735+
}
736+
if !strings.Contains(out, "...") {
737+
t.Errorf("expected the outlier titles to be visibly marked with \"...\", got:\n%s", out)
738+
}
739+
})
740+
}
741+
}
742+
743+
// TestAlertEventListFieldsProjectionUnchanged is the conductor constraint for
744+
// alert-event list's --fields path: it must keep selecting exactly the named
745+
// fields, unaffected by the default-projection truncation logic.
746+
func TestAlertEventListFieldsProjectionUnchanged(t *testing.T) {
747+
for _, format := range []string{"json", "toon"} {
748+
t.Run(format, func(t *testing.T) {
749+
saveAndResetGlobals(t)
750+
stub := newGFStub(t)
751+
items, ids, _ := alertEventOutlierFixture(30, 3)
752+
stub.data = map[string]any{"items": items, "total": len(items)}
753+
754+
out, stderrText, err := execCommandSplit("alert-event", "list", "--fields", "event_id,alert_id", "--output-format", format)
755+
if err != nil {
756+
t.Fatalf("execCommandSplit: %v", err)
757+
}
758+
if strings.Contains(stderrText, "note: rows projected to default compact fields") {
759+
t.Errorf("explicit --fields must not print the default-projection note, got:\n%s", stderrText)
760+
}
761+
for _, id := range ids {
762+
if !strings.Contains(out, id) {
763+
t.Errorf("id %q missing from --fields output, got:\n%s", id, out)
764+
}
765+
}
766+
if strings.Contains(out, "event_severity") || strings.Contains(out, "title") {
767+
t.Errorf("--fields output should contain only the requested fields, got:\n%s", out)
768+
}
769+
})
770+
}
771+
}
772+
773+
// TestBoundProjectedListNeverEmitsUnmarkedTruncation is the regression guard
774+
// for the original defect's silent-corruption half: the old algorithm
775+
// repeatedly halved a single shared per-field byte cap, and once that cap
776+
// dropped to 3 bytes or below, truncateUTF8Bytes's no-room-for-a-marker
777+
// fallback returned raw, unmarked bytes indistinguishable from a genuinely
778+
// short value. Even under extreme row/field pressure that forces every
779+
// string field to shrink, every shortened value must carry the "..." marker.
780+
func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) {
781+
saveAndResetGlobals(t)
782+
flagOutputFormat = "json"
783+
784+
rows := make([]map[string]any, 100)
785+
for i := range rows {
786+
rows[i] = map[string]any{
787+
"event_id": fmt.Sprintf("%024x", i),
788+
"alert_id": fmt.Sprintf("%024x", i+1_000_000),
789+
"event_severity": "Info",
790+
"event_status": "Ok",
791+
"title": strings.Repeat(fmt.Sprintf("row %d compound alert title with extra detail 详情 ", i), 3),
792+
}
793+
}
794+
originals := make([]map[string]any, len(rows))
795+
for i, row := range rows {
796+
clone := make(map[string]any, len(row))
797+
for k, v := range row {
798+
clone[k] = v
799+
}
800+
originals[i] = clone
801+
}
802+
803+
if err := boundProjectedOutput(rows, compactListOutputLimit); err != nil {
804+
t.Fatalf("bound: %v", err)
805+
}
806+
807+
for i, row := range rows {
808+
for key, value := range row {
809+
text, ok := value.(string)
810+
if !ok {
811+
continue
812+
}
813+
original := originals[i][key].(string)
814+
if text == original {
815+
continue
816+
}
817+
if !strings.HasSuffix(text, "...") {
818+
t.Fatalf("row %d field %q was shortened to %q without the \"...\" marker (original was %d bytes)", i, key, text, len(original))
819+
}
820+
}
821+
}
822+
}
823+
659824
func TestStructuredFieldsEmptyErrors(t *testing.T) {
660825
cases := []struct {
661826
name string

0 commit comments

Comments
 (0)