Skip to content

Commit f4df3c3

Browse files
committed
fix(cli): keep identifier fields intact in list projection shortening
boundProjectedList applied one fair byte cap to every string value, so an overflowing page silently clipped identifier fields (keys ending in _id/_key, e.g. alert_key, incident_id) mid-value. A clipped identifier silently defeats the consumer's exact-match filter or follow-up detail call: the query matches nothing and looks like an empty result. Add isIdentifierField and exempt _id/_key fields at all three points of boundProjectedList: the maxLen sizing scan, the fits() trials, and the final apply loop (skipped before the note's denominator, so it counts only shortenable strings). Identifiers now survive byte-intact; when a page's irreducible content alone overflows the budget, the existing narrowing error fires instead of shipping clipped ids.
1 parent 2e19d39 commit f4df3c3

2 files changed

Lines changed: 126 additions & 8 deletions

File tree

internal/cli/fieldproject.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,25 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error {
180180
len(encoded), maxBytes, largest)
181181
}
182182

183+
// isIdentifierField reports whether a projected field is an identifier:
184+
// keys ending in _id or _key (incident_id, alert_key, …). Identifier values
185+
// are matched, filtered, and passed back verbatim by the consumer — a jq
186+
// exact-match over --json output, or a follow-up detail call — so clipping
187+
// one silently defeats that consumer: an identifier either survives a
188+
// projection intact or the projection errors out.
189+
func isIdentifierField(key string) bool {
190+
return strings.HasSuffix(key, "_id") || strings.HasSuffix(key, "_key")
191+
}
192+
183193
// boundProjectedList shortens a list projection's string values fairly when
184194
// the compact rows themselves overflow the budget: it finds the largest
185195
// per-field byte cap that still makes everything fit, then applies that one
186-
// cap to every string value across every row. A field already shorter than
187-
// the cap is left completely untouched — only the field(s) actually
188-
// responsible for the overflow (typically a long title) get shortened, each
189-
// marked with "...". The cap never drops low enough to make the "..."
196+
// cap to every shortenable string value across every row. A field already
197+
// shorter than the cap is left completely untouched — only the field(s)
198+
// actually responsible for the overflow (typically a long title) get
199+
// shortened, each marked with "...". Identifier fields (keys ending in _id
200+
// or _key) are exempt at every step — sizing, fitting, and applying — so
201+
// they survive byte-intact. The cap never drops low enough to make the "..."
190202
// marker itself disappear, so a shortened value is always distinguishable
191203
// from a genuinely short one; if no cap at or above that floor fits, the
192204
// command fails with a small error instead of emitting values that look
@@ -213,7 +225,10 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
213225

214226
maxLen := 0
215227
for _, row := range rows {
216-
for _, value := range row {
228+
for key, value := range row {
229+
if isIdentifierField(key) {
230+
continue
231+
}
217232
if text, ok := value.(string); ok && len(text) > maxLen {
218233
maxLen = len(text)
219234
}
@@ -228,7 +243,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
228243
for i, row := range rows {
229244
trialRow := make(map[string]any, len(row))
230245
for key, value := range row {
231-
if text, ok := value.(string); ok {
246+
if text, ok := value.(string); ok && !isIdentifierField(key) {
232247
trialRow[key] = truncateUTF8Bytes(text, limit)
233248
} else {
234249
trialRow[key] = value
@@ -280,6 +295,9 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) {
280295
fields := map[string]bool{}
281296
for _, row := range rows {
282297
for key, value := range row {
298+
if isIdentifierField(key) {
299+
continue
300+
}
283301
text, ok := value.(string)
284302
if !ok {
285303
continue

internal/cli/fieldproject_test.go

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -781,12 +781,16 @@ func TestAlertEventListFieldsProjectionUnchanged(t *testing.T) {
781781
// dropped to 3 bytes or below, truncateUTF8Bytes's no-room-for-a-marker
782782
// fallback returned raw, unmarked bytes indistinguishable from a genuinely
783783
// short value. Even under extreme row/field pressure that forces every
784-
// string field to shrink, every shortened value must carry the "..." marker.
784+
// shortenable string field to shrink, every shortened value must carry the
785+
// "..." marker. (Row count is sized so the exempt _id fields and the JSON
786+
// envelope still fit at the truncation floor — more rows would tip the page
787+
// into the identifier-overflow error pinned by
788+
// TestBoundProjectedListIdentifierOnlyOverflowErrors.)
785789
func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) {
786790
saveAndResetGlobals(t)
787791
flagOutputFormat = "json"
788792

789-
rows := make([]map[string]any, 100)
793+
rows := make([]map[string]any, 90)
790794
for i := range rows {
791795
rows[i] = map[string]any{
792796
"event_id": fmt.Sprintf("%024x", i),
@@ -919,3 +923,99 @@ func TestBoundProjectedListErrorNamesLargestFields(t *testing.T) {
919923
t.Fatalf("list overflow error = %q, want it to name the largest fields", err)
920924
}
921925
}
926+
927+
// TestBoundProjectedListNeverShortensIdentifierFields pins the identifier
928+
// exemption: keys ending in _id/_key carry values a consumer matches,
929+
// filters, or passes back verbatim (a jq exact-match over --json output, a
930+
// follow-up detail call), so shortening one silently defeats that consumer.
931+
// Even when a page overflows badly enough that the fair cap lands below an
932+
// identifier's own length, identifiers must come back byte-identical and
933+
// only free-text fields shorten; the note must name only the clipped fields.
934+
func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) {
935+
for _, format := range []string{"json", "toon"} {
936+
t.Run(format, func(t *testing.T) {
937+
saveAndResetGlobals(t)
938+
flagOutputFormat = format
939+
940+
rows := make([]map[string]any, 10)
941+
wantIDs := make([]map[string]string, len(rows))
942+
for i := range rows {
943+
eventID := fmt.Sprintf("%024x", i)
944+
alertKey := fmt.Sprintf("%032x", i)
945+
rows[i] = map[string]any{
946+
"event_id": eventID,
947+
"alert_key": alertKey,
948+
"title": strings.Repeat("a", 500),
949+
}
950+
wantIDs[i] = map[string]string{"event_id": eventID, "alert_key": alertKey}
951+
}
952+
953+
const budget = 1400
954+
note, err := boundProjectedOutput(rows, budget)
955+
if err != nil {
956+
t.Fatalf("bound projected output: %v", err)
957+
}
958+
959+
for i, row := range rows {
960+
for _, field := range []string{"event_id", "alert_key"} {
961+
if got := row[field].(string); got != wantIDs[i][field] {
962+
t.Errorf("row %d %s was shortened: got %q, want byte-identical %q", i, field, got, wantIDs[i][field])
963+
}
964+
}
965+
if title := row["title"].(string); !strings.HasSuffix(title, "...") {
966+
t.Errorf("row %d title should be shortened with the \"...\" marker, got %q", i, title)
967+
}
968+
}
969+
970+
if note == "" {
971+
t.Fatal("shortened projection returned no note; caller cannot tell values were clipped")
972+
}
973+
if !strings.Contains(note, "title") {
974+
t.Errorf("note = %q, want it to name the shortened field (title)", note)
975+
}
976+
if strings.Contains(note, "event_id") || strings.Contains(note, "alert_key") {
977+
t.Errorf("note = %q, want it to name only shortened fields, never exempt identifiers", note)
978+
}
979+
980+
encoded, err := marshalStructured(rows)
981+
if err != nil {
982+
t.Fatalf("marshal bounded output: %v", err)
983+
}
984+
if len(encoded)+1 >= budget {
985+
t.Errorf("bounded %s output is %d bytes, want <%d", format, len(encoded)+1, budget)
986+
}
987+
})
988+
}
989+
}
990+
991+
// TestBoundProjectedListIdentifierOnlyOverflowErrors pins the other half of
992+
// the identifier exemption: when a page carries nothing shortenable and its
993+
// identifier content alone overflows the budget, the command must fail with
994+
// the narrowing error instead of clipping identifiers to fit — and the rows
995+
// must come back untouched.
996+
func TestBoundProjectedListIdentifierOnlyOverflowErrors(t *testing.T) {
997+
saveAndResetGlobals(t)
998+
flagOutputFormat = "json"
999+
1000+
// Sized so the full rows overflow 512 bytes while rows with ids clipped
1001+
// to the truncation floor would still fit: the old fair cap "succeeded"
1002+
// by shipping mangled ids, the exemption must instead refuse.
1003+
rows := make([]map[string]any, 12)
1004+
for i := range rows {
1005+
rows[i] = map[string]any{"incident_id": fmt.Sprintf("%024x", i)}
1006+
}
1007+
originals := make([]string, len(rows))
1008+
for i, row := range rows {
1009+
originals[i] = row["incident_id"].(string)
1010+
}
1011+
1012+
_, err := boundProjectedOutput(rows, 512)
1013+
if err == nil || !strings.Contains(err.Error(), "request fewer rows") {
1014+
t.Fatalf("identifier-only overflow error = %v, want bounded guidance", err)
1015+
}
1016+
for i, row := range rows {
1017+
if got := row["incident_id"].(string); got != originals[i] {
1018+
t.Errorf("row %d incident_id was mutated despite the error: got %q, want %q", i, got, originals[i])
1019+
}
1020+
}
1021+
}

0 commit comments

Comments
 (0)