diff --git a/cmd/flashduty/main_test.go b/cmd/flashduty/main_test.go index 4f52b74..013d8ea 100644 --- a/cmd/flashduty/main_test.go +++ b/cmd/flashduty/main_test.go @@ -152,7 +152,7 @@ func TestProjectionOverflowFailsHard(t *testing.T) { if stdout.Len() != 0 { t.Errorf("[#79] a failed projection must write nothing to stdout, got %d bytes:\n%s", stdout.Len(), stdout.String()) } - if !strings.Contains(stderr.String(), "Error: projected list is") || !strings.Contains(stderr.String(), "exceeds the 16384-byte limit") { + if !strings.Contains(stderr.String(), "Error: projected list is") || !strings.Contains(stderr.String(), "16384-byte structured-output limit") { t.Errorf("[#79] stderr should report the byte-limit refusal, got:\n%s", stderr.String()) } } diff --git a/internal/cli/command_test.go b/internal/cli/command_test.go index e65232e..9187b25 100644 --- a/internal/cli/command_test.go +++ b/internal/cli/command_test.go @@ -1721,13 +1721,13 @@ func TestCommandListProjectionOverflowFails(t *testing.T) { stub.data = map[string]any{"items": []any{row}, "total": 1} out, stderrText, err := execCommandSplit("incident", "list", "--fields", "incident_id,labels", "--output-format", "json") - if err == nil || !strings.Contains(err.Error(), "exceeds the 16384-byte limit") { + if err == nil || !strings.Contains(err.Error(), "16384-byte structured-output limit") { t.Fatalf("irreducible projection error = %v, want the byte-limit refusal", err) } if out != "" { t.Errorf("a failed projection must write nothing to stdout, got %d bytes", len(out)) } - if strings.Contains(stderrText, "exceeds the") { + if strings.Contains(stderrText, "structured-output limit") { t.Errorf("the error is returned for the entrypoint to report, not printed mid-run, got:\n%s", stderrText) } }) @@ -1747,7 +1747,7 @@ func TestCommandListProjectionOverflowFails(t *testing.T) { stub.data = map[string]any{"items": []any{row}, "total": 1} out, _, err := execCommandSplit("alert-event", "list", "--fields", "event_id,labels", "--output-format", "json") - if err == nil || !strings.Contains(err.Error(), "exceeds the 16384-byte limit") { + if err == nil || !strings.Contains(err.Error(), "16384-byte structured-output limit") { t.Fatalf("irreducible projection error = %v, want the byte-limit refusal", err) } if out != "" { diff --git a/internal/cli/fieldproject.go b/internal/cli/fieldproject.go index 1f9ae82..47245d0 100644 --- a/internal/cli/fieldproject.go +++ b/internal/cli/fieldproject.go @@ -98,7 +98,8 @@ type projectionBound struct { shortened int valuesTotal int fields []string - // maxBytes is the budget the reduction was sized against. + // maxBytes is the published limit the payload had to fit under — not the + // rows' share of it, which is smaller when they ride inside an envelope. maxBytes int } @@ -203,20 +204,29 @@ func noteProjectionBound(cmd *cobra.Command, bound projectionBound) { // callers turn it into the command-appropriate failure with // explainProjectionOverflow. type projectionOverflow struct { - detail bool - bytes int - rows int + detail bool + bytes int + rows int + // maxBytes is the published limit, so the error names that cap rather than + // the rows' internal share of it. maxBytes int largest string } func (o *projectionOverflow) Error() string { if o.detail { - return fmt.Sprintf("projected detail is %d bytes, exceeds the %d-byte limit; largest fields: %s", + return fmt.Sprintf("projected detail is %d bytes and does not fit under the %d-byte structured-output limit; largest fields: %s", o.bytes, o.maxBytes, o.largest) } - return fmt.Sprintf("projected list is %d bytes across %d rows, exceeds the %d-byte limit; largest fields: %s", - o.bytes, o.rows, o.maxBytes, o.largest) + // The count is the rows' own encoding, the limit the payload's: inside an + // envelope the two live in different spaces, so the sentence states the + // size and the refusal separately rather than comparing them. + rowNoun := "rows" + if o.rows == 1 { + rowNoun = "row" + } + return fmt.Sprintf("projected list is %d bytes across %d %s and cannot be reduced to fit under the %d-byte structured-output limit; largest fields: %s", + o.bytes, o.rows, rowNoun, o.maxBytes, o.largest) } // explainProjectionOverflow returns err with the remedy for cmd appended when @@ -259,7 +269,7 @@ func boundProjectedOutput(data any, maxBytes int) (any, projectionBound, error) case map[string]any: return value, projectionBound{}, boundProjectedDetail(value, maxBytes) case []map[string]any: - return boundProjectedList(value, maxBytes) + return boundProjectedList(value, maxBytes, 0) default: return nil, projectionBound{}, fmt.Errorf("internal error: unsupported projected output %T", data) } @@ -325,7 +335,9 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error { if err != nil { return err } - return &projectionOverflow{detail: true, bytes: len(encoded), maxBytes: maxBytes, largest: largest} + // +1 for the trailing newline the printer appends, so the reported size is + // the one the fit test just measured. + return &projectionOverflow{detail: true, bytes: len(encoded) + 1, maxBytes: maxBytes, largest: largest} } // isIdentifierField reports whether a projected field is an identifier: @@ -356,12 +368,21 @@ func isIdentifierField(key string) bool { // command fails with a small error instead of emitting values that look // real but aren't. Whatever it reduces or clips, it reports back in the // returned projectionBound. -func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, projectionBound, error) { +// +// The rows are sized against limit minus framing, where framing is the bytes +// the payload costs around them besides the row array (0 when the rows are the +// payload), while the returned facts name limit: the caller's note quotes the +// published cap, not an internal remainder. The input rows are never modified, +// so a caller that re-bounds them against a smaller budget (the envelope +// re-fit) measures every pass from the original values and reports facts that +// match the payload it prints. +func boundProjectedList(rows []map[string]any, limit, framing int) ([]map[string]any, projectionBound, error) { + budget := limit - framing encoded, err := marshalStructured(rows) if err != nil { return nil, projectionBound{}, err } - if len(encoded)+1 < maxBytes { + if len(encoded)+1 < budget { return rows, projectionBound{}, nil } @@ -372,15 +393,17 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, if err != nil { return nil, projectionBound{}, err } - return nil, projectionBound{}, &projectionOverflow{bytes: len(encoded), rows: len(rows), maxBytes: maxBytes, largest: largest} + // +1 for the trailing newline the printer appends, so the reported size + // is the one the fit test just measured. + return nil, projectionBound{}, &projectionOverflow{bytes: len(encoded) + 1, rows: len(rows), maxBytes: limit, largest: largest} } - kept, err := largestFittingPrefix(rows, maxBytes) + kept, err := largestFittingPrefix(rows, budget) if err != nil { return nil, projectionBound{}, err } if kept > 0 { - return rows[:kept], projectionBound{rowsEmitted: kept, rowsTotal: len(rows), maxBytes: maxBytes}, nil + return rows[:kept], projectionBound{rowsEmitted: kept, rowsTotal: len(rows), maxBytes: limit}, nil } maxLen := 0 @@ -415,7 +438,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, if err != nil { return false, err } - return len(trialEncoded)+1 < maxBytes, nil + return len(trialEncoded)+1 < budget, nil } // minMarkedTruncationCap is the smallest cap for which truncateUTF8Bytes @@ -453,13 +476,13 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, shortened, total := 0, 0 fields := map[string]bool{} - for _, row := range rows { + bounded := make([]map[string]any, len(rows)) + for i, row := range rows { + boundedRow := make(map[string]any, len(row)) for key, value := range row { - if isIdentifierField(key) { - continue - } text, ok := value.(string) - if !ok { + if !ok || isIdentifierField(key) { + boundedRow[key] = value continue } total++ @@ -468,8 +491,9 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, shortened++ fields[key] = true } - row[key] = clipped + boundedRow[key] = clipped } + bounded[i] = boundedRow } if shortened == 0 { return rows, projectionBound{}, nil @@ -479,7 +503,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, names = append(names, name) } sort.Strings(names) - return rows, projectionBound{shortened: shortened, valuesTotal: total, fields: names, maxBytes: maxBytes}, nil + return bounded, projectionBound{shortened: shortened, valuesTotal: total, fields: names, maxBytes: limit}, nil } // largestFittingPrefix returns the largest n < len(rows) whose encoded prefix diff --git a/internal/cli/fieldproject_test.go b/internal/cli/fieldproject_test.go index 907a0ad..ee4f5a8 100644 --- a/internal/cli/fieldproject_test.go +++ b/internal/cli/fieldproject_test.go @@ -38,22 +38,34 @@ func TestBoundProjectedOutputCapsStructuredFormats(t *testing.T) { t.Run(format, func(t *testing.T) { saveAndResetGlobals(t) flagOutputFormat = format + longTitle := strings.Repeat("数据库故障", 2000) rows := []map[string]any{{ "incident_id": "inc-1", - "title": strings.Repeat("数据库故障", 2000), + "title": longTitle, }} - if _, _, err := boundProjectedOutput(rows, 512); err != nil { + bounded, _, err := boundProjectedOutput(rows, 512) + if err != nil { t.Fatalf("bound projected output: %v", err) } - encoded, err := marshalStructured(rows) + boundedRows, ok := bounded.([]map[string]any) + if !ok { + t.Fatalf("bounded output = %T, want []map[string]any", bounded) + } + encoded, err := marshalStructured(boundedRows) if err != nil { t.Fatalf("marshal bounded output: %v", err) } if len(encoded)+1 >= 512 { t.Fatalf("bounded %s output is %d bytes, want <512", format, len(encoded)+1) } - title := rows[0]["title"].(string) + // The input stays untouched: the caller prints the returned value, + // and the envelope re-fit re-bounds the same rows against a smaller + // budget, so every pass must see the original data. + if original, _ := rows[0]["title"].(string); original != longTitle { + t.Fatalf("bounding modified the input row: title is %d bytes, want it untouched", len(original)) + } + title := boundedRows[0]["title"].(string) if !utf8.ValidString(title) || !strings.HasSuffix(title, "...") { t.Fatalf("truncated title = %q, want valid UTF-8 with marker", title) } @@ -61,6 +73,36 @@ func TestBoundProjectedOutputCapsStructuredFormats(t *testing.T) { } } +// TestBoundProjectedOutputRefusesOversizeDetail pins the detail half of the +// refusal: a single-object projection is never modified (a clipped id or status +// would pass for a real value), so one that overflows the detail limit fails +// instead, and its message reports the rendered size the fit test measured +// against the detail cap — the two numbers live in the same space here, so the +// sentence can state them together. +func TestBoundProjectedOutputRefusesOversizeDetail(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + row := map[string]any{ + "incident_id": "inc-1", + "counts": make([]int, 5000), + } + + _, _, err := boundProjectedOutput(row, compactDetailOutputLimit) + if err == nil { + t.Fatal("oversize detail = nil error, want the overflow refusal") + } + if !strings.Contains(err.Error(), "projected detail is ") { + t.Fatalf("detail refusal should name the detail payload, got: %v", err) + } + want := fmt.Sprintf("does not fit under the %d-byte structured-output limit", compactDetailOutputLimit) + if !strings.Contains(err.Error(), want) || !strings.Contains(err.Error(), "counts") { + t.Fatalf("detail refusal = %v, want %q and the largest field", err, want) + } + if strings.Contains(err.Error(), "exceeds") { + t.Fatalf("the refusal must not claim a size exceeds a limit it was measured against in another space, got: %v", err) + } +} + func TestBoundProjectedOutputRejectsIrreducibleMetadata(t *testing.T) { saveAndResetGlobals(t) flagOutputFormat = "json" @@ -73,7 +115,7 @@ func TestBoundProjectedOutputRejectsIrreducibleMetadata(t *testing.T) { if err == nil { t.Fatal("irreducible output = nil error, want the overflow refusal") } - if !strings.Contains(err.Error(), "exceeds the 512-byte limit") || !strings.Contains(err.Error(), "counts") { + if !strings.Contains(err.Error(), "cannot be reduced to fit under the 512-byte structured-output limit") || !strings.Contains(err.Error(), "counts") { t.Fatalf("irreducible output error = %v, want the byte budget and the largest field", err) } // The byte-bounder's own message is deliberately flag-neutral: it knows diff --git a/internal/cli/gen_support.go b/internal/cli/gen_support.go index 7ca3dab..91e1e59 100644 --- a/internal/cli/gen_support.go +++ b/internal/cli/gen_support.go @@ -380,7 +380,7 @@ func printBoundedGenericResult(ctx *RunContext, data any) error { if !ok { return ctx.Printer.Print(data, nil) } - bounded, bound, err := boundProjectedList(rows, compactListOutputLimit) + bounded, bound, err := boundProjectedList(rows, compactListOutputLimit, 0) if err != nil { return explainProjectionOverflow(ctx.Cmd, err) } @@ -399,30 +399,36 @@ func printBoundedGenericResult(ctx *RunContext, data any) error { if !ok { return ctx.Printer.Print(data, nil) } - // boundProjectedList sizes the rows standalone, but printed inside the - // envelope they share the budget with the pagination siblings (and, in - // indented JSON, sit one indent level deeper). Fit against the full - // limit, then re-fit with the observed envelope overhead subtracted - // until the whole payload is under it. - budget := compactListOutputLimit + // The rows are first sized against the whole limit, but printed inside + // the envelope they share the budget with the pagination siblings (and, + // in indented JSON, sit one indent level deeper). Each re-fit sizes + // them against the limit minus the framing seen so far, growing that + // framing by each observed overflow until the whole payload fits. + framing := 0 for { - bounded, bound, err := boundProjectedList(rows, budget) + bounded, bound, err := boundProjectedList(rows, compactListOutputLimit, framing) if err != nil { return explainProjectionOverflow(ctx.Cmd, err) } value[key] = bounded + // In-payload, not just on stderr: scripts discard stderr, and + // the pagination siblings keep describing the server page, so a + // reduced page would otherwise read as complete. emitted_rows + // is set only when rows were withheld (a prefix was dropped), + // which is the case paging can repair; when instead long values + // were clipped, truncated rides alone — re-requesting cannot + // restore them, narrowing --fields can. Stamp both keys from this + // pass alone, so a key left over from an earlier re-fit cannot + // describe a reduction this payload does not carry. if bound.reduced() { - // In-payload, not just on stderr: scripts discard stderr, and - // the pagination siblings keep describing the server page, so a - // reduced page would otherwise read as complete. emitted_rows - // is set only when rows were WITHHELD (a prefix was dropped), - // which is the case paging can repair; when instead long values - // were clipped, truncated rides alone — re-requesting cannot - // restore them, narrowing --fields can. value["truncated"] = true - if len(bounded) < len(rows) { - value["emitted_rows"] = len(bounded) - } + } else { + delete(value, "truncated") + } + if len(bounded) < len(rows) { + value["emitted_rows"] = len(bounded) + } else { + delete(value, "emitted_rows") } out, err := marshalStructured(value) if err != nil { @@ -432,7 +438,7 @@ func printBoundedGenericResult(ctx *RunContext, data any) error { noteProjectionBound(ctx.Cmd, bound) return ctx.Printer.Print(value, nil) } - budget -= len(out) + 2 - compactListOutputLimit + framing += len(out) + 2 - compactListOutputLimit } default: return ctx.Printer.Print(data, nil) diff --git a/internal/cli/gen_support_test.go b/internal/cli/gen_support_test.go index 96d0b20..a1a26d4 100644 --- a/internal/cli/gen_support_test.go +++ b/internal/cli/gen_support_test.go @@ -314,3 +314,88 @@ func TestPrintGenericResultCompleteEnvelopeUnmarked(t *testing.T) { t.Errorf("within-budget envelope must not announce a reduction, got:\n%s", stderrText) } } + +// TestBoundedEnvelopeReductionFactsMatchPayload pins the reduction contract a +// --json consumer walks: emitted_rows appears only when rows were WITHHELD +// with every emitted value intact (a prefix reduction paging can repair), and a +// clipped value (it ends in "...") never rides under that marker, because +// paging cannot restore a clipped value — only a narrower --fields can. The +// fixture sweeps the first row across the band where the envelope re-fit +// switches between the two reductions, since that is where the reported facts +// and the printed payload can drift apart. +func TestBoundedEnvelopeReductionFactsMatchPayload(t *testing.T) { + for _, titleBytes := range []int{15000, 15200, 15400, 15600, 15800, 16000, 16200, 16400, 16600, 16800} { + t.Run(fmt.Sprintf("title=%d", titleBytes), func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = map[string]any{ + "items": []any{ + map[string]any{"incident_id": "inc-1", "title": strings.Repeat("x", titleBytes), "severity": "Critical"}, + map[string]any{"incident_id": "inc-2", "title": "small follower", "severity": "Info"}, + }, + "total": 2, + "has_next_page": true, + } + + out, stderrText, err := execCommandSplit("insight", "incident-list", + "--start-time", "7d", "--end-time", "now", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if len([]byte(out)) >= compactListOutputLimit { + t.Errorf("bounded envelope is %d bytes, want <%d", len([]byte(out)), compactListOutputLimit) + } + var envelope map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &envelope); err != nil { + t.Fatalf("bounded output is not a JSON object: %v\n%s", err, out) + } + items, ok := envelope["items"].([]any) + if !ok { + t.Fatalf("bounded output lost the items array: %v", envelope) + } + _, rowsWithheld := envelope["emitted_rows"] + clippedAt := "" + for _, item := range items { + row, ok := item.(map[string]any) + if !ok { + t.Fatalf("emitted row is not an object: %v", item) + } + for key, value := range row { + if text, ok := value.(string); ok && strings.HasSuffix(text, "...") { + clippedAt = fmt.Sprintf("%s (%d bytes emitted)", key, len(text)) + } + } + } + mode := fmt.Sprintf("items=%d emitted_rows=%v clipped=%q", len(items), envelope["emitted_rows"], clippedAt) + clipped := clippedAt != "" + t.Log(mode) + + if envelope["truncated"] != true { + t.Errorf("reduced envelope must carry truncated: %s", mode) + } + if !rowsWithheld && !clipped { + t.Errorf("a marked payload was reduced one way or the other: %s", mode) + } + if rowsWithheld && clipped { + t.Errorf("emitted_rows tells a paging walk the withheld rows are recoverable, but the payload also carries a clipped value that paging cannot restore: %s", mode) + } + if clipped { + if strings.Contains(stderrText, "every value intact") { + t.Errorf("a clipped value cannot be announced as \"every value intact\": %s\ngot: %s", mode, stderrText) + } + if !strings.Contains(stderrText, "were shortened to fit") { + t.Errorf("a clipped value should be announced as shortened on stderr: %s\ngot: %s", mode, stderrText) + } + // The re-fit sizes the rows against the limit minus the + // envelope's framing, but the note names the published cap: + // an internal remainder tells the reader nothing they can act + // on, and it contradicts the documented limit. + if published := fmt.Sprintf("the %d-byte limit", compactListOutputLimit); !strings.Contains(stderrText, published) { + t.Errorf("the shortened note should name the published limit (%s): %s\ngot: %s", published, mode, stderrText) + } + } else if !strings.Contains(stderrText, "every value intact") { + t.Errorf("a rows-withheld reduction should announce intact values on stderr: %s\ngot: %s", mode, stderrText) + } + }) + } +}