From a4ffb6426896b2d40cc4e9317f49945eb60c0339 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Tue, 11 Aug 2026 06:23:21 -0700 Subject: [PATCH] fix(cli): stop compact list projection from mangling short fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/cli/fieldproject.go | 83 +++++++++++---- internal/cli/fieldproject_test.go | 169 +++++++++++++++++++++++++++++- 2 files changed, 231 insertions(+), 21 deletions(-) diff --git a/internal/cli/fieldproject.go b/internal/cli/fieldproject.go index b754875..c8bd01e 100644 --- a/internal/cli/fieldproject.go +++ b/internal/cli/fieldproject.go @@ -143,11 +143,17 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error { len(encoded), maxBytes, strings.Join(largest, ", ")) } -// boundProjectedList shortens a list projection's string values fairly -// (across all rows) when the compact rows themselves overflow the budget, -// marking shortened values with "...". If keys and non-string values alone -// exceed the budget, the command fails with a small error instead of -// emitting an oversized payload. +// boundProjectedList shortens a list projection's string values fairly when +// the compact rows themselves overflow the budget: it finds the largest +// per-field byte cap that still makes everything fit, then applies that one +// cap to every string value across every row. A field already shorter than +// the cap is left completely untouched — only the field(s) actually +// responsible for the overflow (typically a long title) get shortened, each +// marked with "...". The cap never drops low enough to make the "..." +// marker itself disappear, so a shortened value is always distinguishable +// from a genuinely short one; if no cap at or above that floor fits, the +// command fails with a small error instead of emitting values that look +// real but aren't. func boundProjectedList(rows []map[string]any, maxBytes int) error { encoded, err := marshalStructured(rows) if err != nil { @@ -157,40 +163,79 @@ func boundProjectedList(rows []map[string]any, maxBytes int) error { return nil } - stringCount := 0 + maxLen := 0 for _, row := range rows { for _, value := range row { - if _, ok := value.(string); ok { - stringCount++ + if text, ok := value.(string); ok && len(text) > maxLen { + maxLen = len(text) } } } - if stringCount == 0 { + if maxLen == 0 { return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes) } - fieldLimit := maxBytes / stringCount - for { - for _, row := range rows { + fits := func(limit int) (bool, error) { + trial := make([]map[string]any, len(rows)) + for i, row := range rows { + trialRow := make(map[string]any, len(row)) for key, value := range row { if text, ok := value.(string); ok { - row[key] = truncateUTF8Bytes(text, fieldLimit) + trialRow[key] = truncateUTF8Bytes(text, limit) + } else { + trialRow[key] = value } } + trial[i] = trialRow } + trialEncoded, err := marshalStructured(trial) + if err != nil { + return false, err + } + return len(trialEncoded)+1 < maxBytes, nil + } - encoded, err = marshalStructured(rows) + // minMarkedTruncationCap is the smallest cap for which truncateUTF8Bytes + // still appends "..." (it needs 3 bytes of headroom beyond the marker + // itself); below it a truncated value would be indistinguishable from a + // genuinely short one, which is the defect this function must not + // reintroduce. + const minMarkedTruncationCap = 4 + if maxLen <= minMarkedTruncationCap { + return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes) + } + if ok, err := fits(minMarkedTruncationCap); err != nil { + return err + } else if !ok { + return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes) + } + + // Binary search for the largest cap that still fits: fits(limit) is true + // for small limits (more shortened) and false for large ones (less + // shortened, up to and including maxLen, which is the untouched size we + // already know overflows), so the boundary is unique. + lo, hi := minMarkedTruncationCap, maxLen-1 + for lo < hi { + mid := lo + (hi-lo+1)/2 + ok, err := fits(mid) if err != nil { return err } - if len(encoded)+1 < maxBytes { - return nil + if ok { + lo = mid + } else { + hi = mid - 1 } - if fieldLimit == 0 { - return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes) + } + + for _, row := range rows { + for key, value := range row { + if text, ok := value.(string); ok { + row[key] = truncateUTF8Bytes(text, lo) + } } - fieldLimit /= 2 } + return nil } func truncateUTF8Bytes(value string, maxBytes int) string { diff --git a/internal/cli/fieldproject_test.go b/internal/cli/fieldproject_test.go index 3437219..c82c048 100644 --- a/internal/cli/fieldproject_test.go +++ b/internal/cli/fieldproject_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "encoding/json" + "fmt" "reflect" "strings" "testing" @@ -195,9 +196,9 @@ func TestIncidentListStructuredDefaultUsesCompactProjection(t *testing.T) { row["title"] = strings.Repeat("数据库故障", 5000) stub.data = map[string]any{"items": []any{row}, "total": 1} - out, err := execCommand("incident", "list", "--output-format", format) + out, _, err := execCommandSplit("incident", "list", "--output-format", format) if err != nil { - t.Fatalf("execCommand: %v", err) + t.Fatalf("execCommandSplit: %v", err) } if len([]byte(out)) >= compactListOutputLimit { 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) { } +// alertEventOutlierFixture builds n alert-event rows with long (Mongo +// ObjectID-shaped) ids: the first `outliers` rows carry a pathologically long +// multi-word/CJK title (the field actually responsible for a page overflowing +// the compact-list budget), the rest carry short, realistic titles. It +// returns the row list alongside the exact ids and titles a correct +// projection must preserve or shorten. +func alertEventOutlierFixture(n, outliers int) (items []any, ids, shortTitles []string) { + longTitle := strings.Repeat("K8S pod tcp 接收队列大于2000 / cluster-prod-a / node-17 / namespace kube-system / pod coredns-7db6d8ff4d-abcde ", 40) + normalTitles := []string{ + "ERROR Detected / VMLogs-Prod", + "CPU利用率较高 / fc-n9e-plus-18001", + "服务器 dev-flasheye-01 连续飘红", + "Disk usage high / db-02", + } + + items = make([]any, n) + ids = make([]string, 0, n*2) + for i := range items { + eventID := fmt.Sprintf("%024x", i) + alertID := fmt.Sprintf("%024x", i+1_000_000) + ids = append(ids, eventID, alertID) + + title := normalTitles[i%len(normalTitles)] + if i >= outliers { + shortTitles = append(shortTitles, title) + } else { + title = fmt.Sprintf("%s (row %d)", longTitle, i) + } + + items[i] = map[string]any{ + "event_id": eventID, + "alert_id": alertID, + "event_severity": "Warning", + "event_status": "Triggered", + "event_time": 1712000000 + i, + "title": title, + } + } + return items, ids, shortTitles +} + +// TestAlertEventListDefaultProjectionPreservesShortFields is the regression +// guard for the original defect: a minority of pathologically long titles +// pushing a page over the 16 KiB compact-list budget must never mangle the +// other rows' long (Mongo ObjectID-shaped) ids, or the short title rows on +// the same page — the shortening must land entirely on the field(s) actually +// responsible for the overflow. +func TestAlertEventListDefaultProjectionPreservesShortFields(t *testing.T) { + for _, format := range []string{"json", "toon"} { + t.Run(format, func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + items, ids, shortTitles := alertEventOutlierFixture(30, 3) + stub.data = map[string]any{"items": items, "total": len(items)} + + out, stderrText, err := execCommandSplit("alert-event", "list", "--output-format", format) + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if len(out) >= compactListOutputLimit { + t.Fatalf("compact alert-event output is %d bytes, want <%d", len(out), compactListOutputLimit) + } + if !strings.Contains(stderrText, "note: rows projected to default compact fields") { + t.Errorf("default projection should announce itself on stderr, got:\n%s", stderrText) + } + + for _, id := range ids { + if !strings.Contains(out, id) { + t.Errorf("id %q was shortened; only the oversized outlier titles should shrink, got:\n%s", id, out) + } + } + for _, title := range shortTitles { + if !strings.Contains(out, title) { + t.Errorf("short title %q was shortened even though it never exceeded the budget on its own, got:\n%s", title, out) + } + } + if !strings.Contains(out, "...") { + t.Errorf("expected the outlier titles to be visibly marked with \"...\", got:\n%s", out) + } + }) + } +} + +// TestAlertEventListFieldsProjectionUnchanged is the conductor constraint for +// alert-event list's --fields path: it must keep selecting exactly the named +// fields, unaffected by the default-projection truncation logic. +func TestAlertEventListFieldsProjectionUnchanged(t *testing.T) { + for _, format := range []string{"json", "toon"} { + t.Run(format, func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + items, ids, _ := alertEventOutlierFixture(30, 3) + stub.data = map[string]any{"items": items, "total": len(items)} + + out, stderrText, err := execCommandSplit("alert-event", "list", "--fields", "event_id,alert_id", "--output-format", format) + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if strings.Contains(stderrText, "note: rows projected to default compact fields") { + t.Errorf("explicit --fields must not print the default-projection note, got:\n%s", stderrText) + } + for _, id := range ids { + if !strings.Contains(out, id) { + t.Errorf("id %q missing from --fields output, got:\n%s", id, out) + } + } + if strings.Contains(out, "event_severity") || strings.Contains(out, "title") { + t.Errorf("--fields output should contain only the requested fields, got:\n%s", out) + } + }) + } +} + +// TestBoundProjectedListNeverEmitsUnmarkedTruncation is the regression guard +// for the original defect's silent-corruption half: the old algorithm +// repeatedly halved a single shared per-field byte cap, and once that cap +// dropped to 3 bytes or below, truncateUTF8Bytes's no-room-for-a-marker +// fallback returned raw, unmarked bytes indistinguishable from a genuinely +// short value. Even under extreme row/field pressure that forces every +// string field to shrink, every shortened value must carry the "..." marker. +func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + + rows := make([]map[string]any, 100) + for i := range rows { + rows[i] = map[string]any{ + "event_id": fmt.Sprintf("%024x", i), + "alert_id": fmt.Sprintf("%024x", i+1_000_000), + "event_severity": "Info", + "event_status": "Ok", + "title": strings.Repeat(fmt.Sprintf("row %d compound alert title with extra detail 详情 ", i), 3), + } + } + originals := make([]map[string]any, len(rows)) + for i, row := range rows { + clone := make(map[string]any, len(row)) + for k, v := range row { + clone[k] = v + } + originals[i] = clone + } + + if err := boundProjectedOutput(rows, compactListOutputLimit); err != nil { + t.Fatalf("bound: %v", err) + } + + for i, row := range rows { + for key, value := range row { + text, ok := value.(string) + if !ok { + continue + } + original := originals[i][key].(string) + if text == original { + continue + } + if !strings.HasSuffix(text, "...") { + t.Fatalf("row %d field %q was shortened to %q without the \"...\" marker (original was %d bytes)", i, key, text, len(original)) + } + } + } +} + func TestStructuredFieldsEmptyErrors(t *testing.T) { cases := []struct { name string