Merge pull request #156 from flashcatcloud/fix/json-projection-never-shortened

fix(cli): announce shortened projection values instead of clipping silently

Author: ysyneu

internal/cli/alert_event.go

@@ -97,9 +97,11 @@ func newAlertEventListCmd() *cobra.Command {
 					if err != nil {
 						return err
 					}
-					if err := boundProjectedOutput(proj, compactListOutputLimit); err != nil {
+					note, err := boundProjectedOutput(proj, compactListOutputLimit)
+					if err != nil {
 						return err
 					}
+					noteProjectionShortening(cmd.ErrOrStderr(), note)
 					return ctx.PrintList(proj, nil, len(result.Items), page, int(result.Total))
 				}
 

internal/cli/fieldproject.go

@@ -80,6 +80,18 @@ func noteDefaultProjection(w io.Writer, fields []string) {
 		strings.Join(fields, ","))
 }
 
+// noteProjectionShortening tells the caller, on stderr, that some values came
+// back clipped. Without it a shortened value is only visible to a reader, not
+// to the jq filter or exact match a --json consumer runs over it, so a query
+// that silently matches nothing looks like an empty result rather than a
+// truncated one.
+func noteProjectionShortening(w io.Writer, note string) {
+	if note == "" {
+		return
+	}
+	_, _ = fmt.Fprintln(w, note)
+}
+
 // boundProjectedOutput keeps the new agent-oriented projections below their
 // command budget without changing the selected keys. List rows (many small
 // records) are shortened fairly when they overflow the budget, with
@@ -88,44 +100,49 @@ func noteDefaultProjection(w io.Writer, fields []string) {
 // a genuinely short value, so silently shortening it would hand the caller
 // wrong data instead of a compact one. If a detail projection doesn't fit,
 // the command fails with an error instead.
-func boundProjectedOutput(data any, maxBytes int) error {
+//
+// It returns a caller-printable note (empty when nothing was shortened) that
+// names the clipped fields, so the caller can announce the loss on stderr —
+// the "..." marker is only visible to something that reads the value, never
+// to the filter a --json consumer runs over it.
+func boundProjectedOutput(data any, maxBytes int) (string, error) {
 	switch value := data.(type) {
 	case map[string]any:
-		return boundProjectedDetail(value, maxBytes)
+		return "", boundProjectedDetail(value, maxBytes)
 	case []map[string]any:
 		return boundProjectedList(value, maxBytes)
 	default:
-		return fmt.Errorf("internal error: unsupported projected output %T", data)
+		return "", fmt.Errorf("internal error: unsupported projected output %T", data)
 	}
 }
 
-// boundProjectedDetail rejects an oversized single-object projection instead
-// of truncating it, naming the largest fields so the caller can fix the
-// request in one pass: drop some of them from --fields, or drop --fields
-// entirely for the full, unbounded detail.
-func boundProjectedDetail(row map[string]any, maxBytes int) error {
-	encoded, err := marshalStructured(row)
-	if err != nil {
-		return err
-	}
-	if len(encoded)+1 < maxBytes {
-		return nil
+// largestProjectedFields names the up to three fields carrying the most bytes
+// in a projection, so an over-budget request can be narrowed in one pass
+// instead of one re-run per field. Sizes are summed per field across every
+// row, which is what makes it meaningful for a list: the field responsible
+// for the overflow is the one that is big in aggregate, not in any one row.
+// Ties break on name so the same oversized request always names the same
+// fields, despite Go's randomized map iteration order.
+func largestProjectedFields(rows []map[string]any) (string, error) {
+	totals := map[string]int{}
+	for _, row := range rows {
+		for key, value := range row {
+			encoded, err := marshalStructured(map[string]any{key: value})
+			if err != nil {
+				return "", err
+			}
+			totals[key] += len(encoded)
+		}
 	}
 
 	type fieldSize struct {
 		name string
 		size int
 	}
-	sizes := make([]fieldSize, 0, len(row))
-	for key, value := range row {
-		fieldEncoded, err := marshalStructured(map[string]any{key: value})
-		if err != nil {
-			return err
-		}
-		sizes = append(sizes, fieldSize{key, len(fieldEncoded)})
+	sizes := make([]fieldSize, 0, len(totals))
+	for name, size := range totals {
+		sizes = append(sizes, fieldSize{name, size})
 	}
-	// Ties break on name so the same oversized request always names the same
-	// fields, despite Go's randomized map iteration order.
 	sort.Slice(sizes, func(i, j int) bool {
 		if sizes[i].size != sizes[j].size {
 			return sizes[i].size > sizes[j].size
@@ -139,8 +156,28 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error {
 	for i, f := range sizes {
 		largest[i] = fmt.Sprintf("%s (%d bytes)", f.name, f.size)
 	}
+	return strings.Join(largest, ", "), nil
+}
+
+// boundProjectedDetail rejects an oversized single-object projection instead
+// of truncating it, naming the largest fields so the caller can fix the
+// request in one pass: drop some of them from --fields, or drop --fields
+// entirely for the full, unbounded detail.
+func boundProjectedDetail(row map[string]any, maxBytes int) error {
+	encoded, err := marshalStructured(row)
+	if err != nil {
+		return err
+	}
+	if len(encoded)+1 < maxBytes {
+		return nil
+	}
+
+	largest, err := largestProjectedFields([]map[string]any{row})
+	if err != nil {
+		return err
+	}
 	return fmt.Errorf("projected detail is %d bytes, exceeds the %d-byte limit; largest fields: %s; request fewer --fields, or omit --fields for the full, unbounded detail",
-		len(encoded), maxBytes, strings.Join(largest, ", "))
+		len(encoded), maxBytes, largest)
 }
 
 // boundProjectedList shortens a list projection's string values fairly when
@@ -153,14 +190,25 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error {
 // 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 {
+// real but aren't. Whatever it clips, it reports back in the returned note.
+func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
 	encoded, err := marshalStructured(rows)
 	if err != nil {
-		return err
+		return "", err
 	}
 	if len(encoded)+1 < maxBytes {
-		return nil
+		return "", nil
+	}
+
+	// The overflow error names the fields responsible, exactly as the detail
+	// path does, so the request can be narrowed in one pass.
+	tooBig := func() (string, error) {
+		largest, err := largestProjectedFields(rows)
+		if err != nil {
+			return "", err
+		}
+		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",
+			len(encoded), len(rows), maxBytes, largest)
 	}
 
 	maxLen := 0
@@ -172,7 +220,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) error {
 		}
 	}
 	if maxLen == 0 {
-		return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
+		return tooBig()
 	}
 
 	fits := func(limit int) (bool, error) {
@@ -202,12 +250,12 @@ func boundProjectedList(rows []map[string]any, maxBytes int) error {
 	// reintroduce.
 	const minMarkedTruncationCap = 4
 	if maxLen <= minMarkedTruncationCap {
-		return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
+		return tooBig()
 	}
 	if ok, err := fits(minMarkedTruncationCap); err != nil {
-		return err
+		return "", err
 	} else if !ok {
-		return fmt.Errorf("structured projection exceeds %d-byte limit; request fewer rows or fields", maxBytes)
+		return tooBig()
 	}
 
 	// Binary search for the largest cap that still fits: fits(limit) is true
@@ -219,7 +267,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) error {
 		mid := lo + (hi-lo+1)/2
 		ok, err := fits(mid)
 		if err != nil {
-			return err
+			return "", err
 		}
 		if ok {
 			lo = mid
@@ -228,14 +276,33 @@ func boundProjectedList(rows []map[string]any, maxBytes int) error {
 		}
 	}
 
+	shortened, total := 0, 0
+	fields := map[string]bool{}
 	for _, row := range rows {
 		for key, value := range row {
-			if text, ok := value.(string); ok {
-				row[key] = truncateUTF8Bytes(text, lo)
+			text, ok := value.(string)
+			if !ok {
+				continue
+			}
+			total++
+			clipped := truncateUTF8Bytes(text, lo)
+			if clipped != text {
+				shortened++
+				fields[key] = true
 			}
+			row[key] = clipped
 		}
 	}
-	return nil
+	if shortened == 0 {
+		return "", nil
+	}
+	names := make([]string, 0, len(fields))
+	for name := range fields {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	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",
+		shortened, total, maxBytes, strings.Join(names, ", ")), nil
 }
 
 func truncateUTF8Bytes(value string, maxBytes int) string {

internal/cli/fieldproject_test.go

@@ -42,7 +42,7 @@ func TestBoundProjectedOutputCapsStructuredFormats(t *testing.T) {
 				"title":       strings.Repeat("数据库故障", 2000),
 			}}
 
-			if err := boundProjectedOutput(rows, 512); err != nil {
+			if _, err := boundProjectedOutput(rows, 512); err != nil {
 				t.Fatalf("bound projected output: %v", err)
 			}
 			encoded, err := marshalStructured(rows)
@@ -68,8 +68,8 @@ func TestBoundProjectedOutputRejectsIrreducibleMetadata(t *testing.T) {
 		rows[i] = map[string]any{"count": i}
 	}
 
-	err := boundProjectedOutput(rows, 512)
-	if err == nil || !strings.Contains(err.Error(), "request fewer rows or fields") {
+	_, err := boundProjectedOutput(rows, 512)
+	if err == nil || !strings.Contains(err.Error(), "request fewer rows") {
 		t.Fatalf("irreducible output error = %v, want bounded guidance", err)
 	}
 }
@@ -92,7 +92,7 @@ func TestBoundProjectedOutputDetailWithinBudgetLeavesValuesUnchanged(t *testing.
 		"progress":    "Triggered",
 	}
 
-	if err := boundProjectedOutput(row, compactDetailOutputLimit); err != nil {
+	if _, err := boundProjectedOutput(row, compactDetailOutputLimit); err != nil {
 		t.Fatalf("bound projected output: %v", err)
 	}
 	if !reflect.DeepEqual(row, want) {
@@ -119,7 +119,7 @@ func TestBoundProjectedOutputDetailOversizedErrorsWithoutMutating(t *testing.T)
 		"root_cause":  strings.Repeat("disk exhaustion details ", 3000),
 	}
 
-	err := boundProjectedOutput(row, 512)
+	_, err := boundProjectedOutput(row, 512)
 	if err == nil {
 		t.Fatal("expected an error for an oversized detail projection, got nil")
 	}
@@ -151,7 +151,7 @@ func TestBoundProjectedOutputDetailErrorIsDeterministic(t *testing.T) {
 			"delta":   strings.Repeat("d", 400),
 			"echo":    strings.Repeat("e", 400),
 		}
-		err := boundProjectedOutput(row, 512)
+		_, err := boundProjectedOutput(row, 512)
 		if err == nil {
 			t.Fatal("expected an error for an oversized detail projection, got nil")
 		}
@@ -196,7 +196,7 @@ func TestIncidentListStructuredDefaultUsesCompactProjection(t *testing.T) {
 			row["title"] = strings.Repeat("数据库故障", 5000)
 			stub.data = map[string]any{"items": []any{row}, "total": 1}
 
-			out, _, err := execCommandSplit("incident", "list", "--output-format", format)
+			out, stderrText, err := execCommandSplit("incident", "list", "--output-format", format)
 			if err != nil {
 				t.Fatalf("execCommandSplit: %v", err)
 			}
@@ -206,6 +206,11 @@ func TestIncidentListStructuredDefaultUsesCompactProjection(t *testing.T) {
 			if !utf8.ValidString(out) || !strings.Contains(out, "...") {
 				t.Fatalf("bounded %s incident list must retain valid UTF-8 and show truncation", format)
 			}
+			// The clipped value must be announced, not just marked: a --json
+			// consumer filters on the value and never sees the "..." itself.
+			if !strings.Contains(stderrText, "were shortened to fit") || !strings.Contains(stderrText, "title") {
+				t.Errorf("shortened %s incident list should announce the clipped field on stderr, got:\n%s", format, stderrText)
+			}
 		})
 	}
 
@@ -800,7 +805,7 @@ func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) {
 		originals[i] = clone
 	}
 
-	if err := boundProjectedOutput(rows, compactListOutputLimit); err != nil {
+	if _, err := boundProjectedOutput(rows, compactListOutputLimit); err != nil {
 		t.Fatalf("bound: %v", err)
 	}
 
@@ -847,3 +852,70 @@ func TestStructuredFieldsEmptyErrors(t *testing.T) {
 		})
 	}
 }
+
+// TestBoundProjectedListAnnouncesShortening pins that a list projection which
+// had to clip values says so on the caller's side. The "..." marker alone is
+// only visible to something that READS the value; a --json consumer runs a jq
+// filter or an exact match over it, where a clipped string produces an empty
+// result that is indistinguishable from "nothing matched" — the expensive
+// failure this note exists to prevent.
+func TestBoundProjectedListAnnouncesShortening(t *testing.T) {
+	for _, format := range []string{"json", "toon"} {
+		t.Run(format, func(t *testing.T) {
+			saveAndResetGlobals(t)
+			flagOutputFormat = format
+			rows := []map[string]any{{
+				"incident_id": "inc-1",
+				"title":       strings.Repeat("payment-gateway timeout ", 200),
+			}}
+
+			note, err := boundProjectedOutput(rows, 512)
+			if err != nil {
+				t.Fatalf("bound projected output: %v", err)
+			}
+			if note == "" {
+				t.Fatalf("shortened projection returned no note; caller cannot tell values were clipped")
+			}
+			if !strings.Contains(note, "title") {
+				t.Fatalf("note = %q, want it to name the shortened field (title)", note)
+			}
+		})
+	}
+}
+
+// TestBoundProjectedListNoNoteWhenNothingShortened keeps the note honest: a
+// projection that fits must not claim anything was clipped.
+func TestBoundProjectedListNoNoteWhenNothingShortened(t *testing.T) {
+	saveAndResetGlobals(t)
+	flagOutputFormat = "json"
+	rows := []map[string]any{{"incident_id": "inc-1", "title": "disk full"}}
+
+	note, err := boundProjectedOutput(rows, 512)
+	if err != nil {
+		t.Fatalf("bound projected output: %v", err)
+	}
+	if note != "" {
+		t.Fatalf("fitting projection returned note %q, want none", note)
+	}
+}
+
+// TestBoundProjectedListErrorNamesLargestFields pins that a list projection
+// which cannot fit at all says WHICH fields are responsible, exactly as the
+// detail path already does. Without it the only way to find the oversized
+// field is to re-run the query once per field.
+func TestBoundProjectedListErrorNamesLargestFields(t *testing.T) {
+	saveAndResetGlobals(t)
+	flagOutputFormat = "json"
+	rows := make([]map[string]any, 200)
+	for i := range rows {
+		rows[i] = map[string]any{"count": i, "score": i * 2}
+	}
+
+	_, err := boundProjectedOutput(rows, 512)
+	if err == nil {
+		t.Fatalf("irreducible projection = nil error, want refusal")
+	}
+	if !strings.Contains(err.Error(), "largest fields:") {
+		t.Fatalf("list overflow error = %q, want it to name the largest fields", err)
+	}
+}