diff --git a/internal/cli/alert_event.go b/internal/cli/alert_event.go index a707ba5..7abb232 100644 --- a/internal/cli/alert_event.go +++ b/internal/cli/alert_event.go @@ -97,12 +97,12 @@ func newAlertEventListCmd() *cobra.Command { if err != nil { return err } - bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit) + bounded, bound, err := boundProjectedOutput(proj, compactListOutputLimit) if err != nil { - return err + return explainProjectionOverflow(cmd, err) } proj = bounded.([]map[string]any) - noteProjectionBound(cmd.ErrOrStderr(), note) + noteProjectionBound(cmd, bound) effectiveLimit := limit if len(proj) < len(result.Items) { effectiveLimit = len(proj) @@ -125,6 +125,7 @@ func newAlertEventListCmd() *cobra.Command { cmd.Flags().IntVar(&limit, "limit", 20, "Max results (max 100)") cmd.Flags().IntVar(&page, "page", 1, "Page number") cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. event_id,alert_id,event_severity,event_status,event_time,title); ignored in table mode. Defaults to these compact event fields. If the page would exceed 16 KiB, only the leading rows that fit are emitted, with every value intact (announced on stderr).") + declareOutputNarrowing(cmd, "fields", "limit") return cmd } diff --git a/internal/cli/channel.go b/internal/cli/channel.go index 6d56e08..0355d15 100644 --- a/internal/cli/channel.go +++ b/internal/cli/channel.go @@ -195,12 +195,12 @@ func newChannelEscalateRuleListCmd() *cobra.Command { if err != nil { return err } - bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit) + bounded, bound, err := boundProjectedOutput(proj, compactListOutputLimit) if err != nil { - return err + return explainProjectionOverflow(cmd, err) } proj = bounded.([]map[string]any) - noteProjectionBound(cmd.ErrOrStderr(), note) + noteProjectionBound(cmd, bound) return ctx.PrintTotal(proj, nil, len(proj)) } @@ -218,5 +218,6 @@ func newChannelEscalateRuleListCmd() *cobra.Command { cmd.Flags().Int64Var(&fChannelID, "channel-id", 0, "Channel to list rules for. (required)") cmd.Flags().StringVar(&dataJSON, "data", "", "Full request body as JSON; positional arguments and typed flags override its fields. Accepts inline JSON, or - to read stdin.") cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. rule_id,rule_name,status,priority); ignored in table mode. Use to avoid dumping the full nested record.") + declareOutputNarrowing(cmd, "fields", "") return cmd } diff --git a/internal/cli/fieldproject.go b/internal/cli/fieldproject.go index 44a86e7..1f9ae82 100644 --- a/internal/cli/fieldproject.go +++ b/internal/cli/fieldproject.go @@ -1,12 +1,15 @@ package cli import ( + "errors" "fmt" "io" "reflect" "sort" "strings" "unicode/utf8" + + "github.com/spf13/cobra" ) const ( @@ -80,16 +83,159 @@ func noteDefaultProjection(w io.Writer, fields []string) { strings.Join(fields, ",")) } -// noteProjectionBound relays a boundProjectedOutput note to the caller on -// stderr. Without it a reduced page or 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 bounded one. -func noteProjectionBound(w io.Writer, note string) { - if note == "" { - return +// projectionBound reports how boundProjectedList reduced an over-budget +// projection — the facts, not the prose. The byte-bounding helper knows bytes +// and nothing about the command it runs under, so it cannot name the flags a +// caller should narrow; the caller that owns the command turns this into the +// stderr note via noteProjectionBound. A prefix reduction and a value +// shortening are mutually exclusive, and the zero value means nothing was +// reduced. +type projectionBound struct { + // A prefix reduction kept rowsEmitted of rowsTotal rows, every value intact. + rowsEmitted int + rowsTotal int + // A value shortening clipped shortened of valuesTotal string values, in fields. + shortened int + valuesTotal int + fields []string + // maxBytes is the budget the reduction was sized against. + maxBytes int +} + +// reduced reports whether the payload was actually reduced — rows withheld or +// values clipped. The zero value means it was emitted intact, which is what the +// in-payload truncation marker keys off. +func (b projectionBound) reduced() bool { + return b.rowsTotal > 0 || b.shortened > 0 +} + +// Commands declare here which of their flags narrow structured output, so the +// projection note and overflow error can name them. Nothing infers this from a +// flag's name: a verb may spell a request-body write selector --fields, or +// accept a --limit the server ignores. Only a command whose own definition +// states what its flags do sets these, via declareOutputNarrowing. +const ( + narrowsByProjection = "narrows-output-projection" + narrowsByRows = "narrows-output-rows" +) + +// declareOutputNarrowing records, on cmd, the flags its own definition declares +// as narrowing its structured output. projectionFlag reduces how much each row +// carries (e.g. --fields); rowsFlag reduces how many rows are requested (e.g. +// --limit). Pass "" for a control the command does not expose. +func declareOutputNarrowing(cmd *cobra.Command, projectionFlag, rowsFlag string) { + if cmd.Annotations == nil { + cmd.Annotations = map[string]string{} + } + if projectionFlag != "" { + cmd.Annotations[narrowsByProjection] = projectionFlag + } + if rowsFlag != "" { + cmd.Annotations[narrowsByRows] = rowsFlag + } +} + +// declaredNarrowing returns the flag cmd declared under annotation, or "" when +// it declared none. A declaration whose flag the command no longer carries (a +// renamed flag) is dropped, so the note can never name a flag that is not there. +func declaredNarrowing(cmd *cobra.Command, annotation string) string { + name := cmd.Annotations[annotation] + if name == "" || cmd.Flags().Lookup(name) == nil { + return "" + } + return name +} + +// projectionRemedy names the flags cmd declared as able to shrink an over-budget +// projection, suffixed with what they buy (purpose). rowsHelp is false for a +// single-row shortening, where a smaller page cannot shrink the one row that +// overflows, so a rows flag is not offered there. A command that declared no +// usable flag is told so rather than sent to one it may reject or misuse. +func projectionRemedy(cmd *cobra.Command, rowsHelp bool, purpose string) string { + projection := declaredNarrowing(cmd, narrowsByProjection) + rows := declaredNarrowing(cmd, narrowsByRows) + if !rowsHelp { + rows = "" + } + var offered []string + if projection != "" { + offered = append(offered, "narrow --"+projection) + } + if rows != "" { + offered = append(offered, "lower --"+rows) + } + if len(offered) == 0 { + return "this command declares no flag that narrows the output" + } + remedy := strings.Join(offered, " or ") + if purpose != "" { + remedy += " " + purpose + } + return remedy +} + +// noteProjectionBound announces on stderr how an over-budget projection was +// reduced. Without it a reduced page or 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 bounded one. +// +// The remedy comes from cmd's own narrowing declaration, not from the +// byte-bounding helper: the helper knows bytes and nothing about the command, +// so a sentence composed there could name a flag the command rejects +// (unknown flag), ignores, or means for something else entirely. +func noteProjectionBound(cmd *cobra.Command, bound projectionBound) { + w := cmd.ErrOrStderr() + switch { + case bound.rowsTotal > 0: + _, _ = fmt.Fprintf(w, "note: emitted %d of %d projected rows (every value intact) to stay below the %d-byte structured-output limit; %s — the rows past the first %d were not emitted\n", + bound.rowsEmitted, bound.rowsTotal, bound.maxBytes, projectionRemedy(cmd, true, "to fit more rows per page"), bound.rowsEmitted) + case bound.shortened > 0: + _, _ = fmt.Fprintf(w, "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 — %s\n", + bound.shortened, bound.valuesTotal, bound.maxBytes, strings.Join(bound.fields, ", "), projectionRemedy(cmd, false, "for untruncated values")) + } +} + +// projectionOverflow is the byte-bounding failure for a projection that no +// reduction can bring under the limit: no leading prefix fits and no row can be +// shortened (detail marks the single-object projection, which is refused rather +// than truncated). It carries the facts; its own message is flag-neutral, and +// callers turn it into the command-appropriate failure with +// explainProjectionOverflow. +type projectionOverflow struct { + detail bool + bytes int + rows int + 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", + 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) +} + +// explainProjectionOverflow returns err with the remedy for cmd appended when +// err is a projectionOverflow, so the failure names only flags cmd declared as +// output-narrowing; any other error passes through unchanged. Callers wrap each +// error returned from the bounding path with it. +func explainProjectionOverflow(cmd *cobra.Command, err error) error { + var overflow *projectionOverflow + if !errors.As(err, &overflow) { + return err + } + if overflow.detail { + projection := declaredNarrowing(cmd, narrowsByProjection) + if projection == "" { + return fmt.Errorf("%w; this command declares no flag that narrows the output", overflow) + } + return fmt.Errorf("%w; request fewer --%s, or omit --%s for the full, unbounded detail", overflow, projection, projection) } - _, _ = fmt.Fprintln(w, note) + return fmt.Errorf("%w; %s", overflow, projectionRemedy(cmd, true, "")) } // boundProjectedOutput keeps the new agent-oriented projections below their @@ -104,18 +250,18 @@ func noteProjectionBound(w io.Writer, note string) { // error instead. // // It returns the bounded data with the same type it was given, plus a -// caller-printable note (empty when nothing was reduced or shortened), 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) (any, string, error) { +// projectionBound describing what was reduced (its zero value when nothing +// was), so the caller can announce the loss on stderr via noteProjectionBound +// — 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) (any, projectionBound, error) { switch value := data.(type) { case map[string]any: - return value, "", boundProjectedDetail(value, maxBytes) + return value, projectionBound{}, boundProjectedDetail(value, maxBytes) case []map[string]any: return boundProjectedList(value, maxBytes) default: - return nil, "", fmt.Errorf("internal error: unsupported projected output %T", data) + return nil, projectionBound{}, fmt.Errorf("internal error: unsupported projected output %T", data) } } @@ -162,10 +308,10 @@ func largestProjectedFields(rows []map[string]any) (string, error) { 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. +// boundProjectedDetail rejects an oversized single-object projection instead of +// truncating it, naming the largest fields so the request can be narrowed in one +// pass. Its message is flag-neutral; the caller appends the command-appropriate +// remedy via explainProjectionOverflow. func boundProjectedDetail(row map[string]any, maxBytes int) error { encoded, err := marshalStructured(row) if err != nil { @@ -179,8 +325,7 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error { 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, largest) + return &projectionOverflow{detail: true, bytes: len(encoded), maxBytes: maxBytes, largest: largest} } // isIdentifierField reports whether a projected field is an identifier: @@ -210,34 +355,32 @@ func isIdentifierField(key string) bool { // 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. Whatever it reduces or clips, it reports back in the -// returned note. -func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, string, error) { +// returned projectionBound. +func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, projectionBound, error) { encoded, err := marshalStructured(rows) if err != nil { - return nil, "", err + return nil, projectionBound{}, err } if len(encoded)+1 < maxBytes { - return rows, "", nil + return rows, projectionBound{}, 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() ([]map[string]any, string, error) { + tooBig := func() ([]map[string]any, projectionBound, error) { largest, err := largestProjectedFields(rows) if err != nil { - return nil, "", err + return nil, projectionBound{}, err } - return nil, "", 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) + return nil, projectionBound{}, &projectionOverflow{bytes: len(encoded), rows: len(rows), maxBytes: maxBytes, largest: largest} } kept, err := largestFittingPrefix(rows, maxBytes) if err != nil { - return nil, "", err + return nil, projectionBound{}, err } if kept > 0 { - return rows[:kept], fmt.Sprintf("note: emitted %d of %d projected rows (every value intact) to stay below the %d-byte structured-output limit; narrow --fields or lower --limit to fit more rows per page — the rows past the first %d were not emitted", - kept, len(rows), maxBytes, kept), nil + return rows[:kept], projectionBound{rowsEmitted: kept, rowsTotal: len(rows), maxBytes: maxBytes}, nil } maxLen := 0 @@ -285,7 +428,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, return tooBig() } if ok, err := fits(minMarkedTruncationCap); err != nil { - return nil, "", err + return nil, projectionBound{}, err } else if !ok { return tooBig() } @@ -299,7 +442,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, mid := lo + (hi-lo+1)/2 ok, err := fits(mid) if err != nil { - return nil, "", err + return nil, projectionBound{}, err } if ok { lo = mid @@ -329,15 +472,14 @@ func boundProjectedList(rows []map[string]any, maxBytes int) ([]map[string]any, } } if shortened == 0 { - return rows, "", nil + return rows, projectionBound{}, nil } names := make([]string, 0, len(fields)) for name := range fields { names = append(names, name) } sort.Strings(names) - return rows, 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 + return rows, projectionBound{shortened: shortened, valuesTotal: total, fields: names, maxBytes: maxBytes}, 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 610ca70..907a0ad 100644 --- a/internal/cli/fieldproject_test.go +++ b/internal/cli/fieldproject_test.go @@ -70,8 +70,19 @@ func TestBoundProjectedOutputRejectsIrreducibleMetadata(t *testing.T) { rows := []map[string]any{{"counts": make([]int, 500)}} _, _, err := boundProjectedOutput(rows, 512) - if err == nil || !strings.Contains(err.Error(), "request fewer rows") { - t.Fatalf("irreducible output error = %v, want bounded guidance", err) + 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") { + 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 + // bytes, not the command, so a remedy can only be appended by a caller that + // owns the command (explainProjectionOverflow). + for _, flag := range []string{"--limit", "--fields"} { + if strings.Contains(err.Error(), flag) { + t.Fatalf("bounding error must not name a flag itself, got: %v", err) + } } } @@ -916,7 +927,7 @@ func TestAlertEventListFieldsProjectionUnchanged(t *testing.T) { // for the original defect's silent-corruption half: a page that overflows the // budget is reduced to the leading rows that fit, so no value is ever // shortened — every emitted row is byte-identical to the fixture, no "..." -// marker appears anywhere, the note names the emitted count, and the encoded +// marker appears anywhere, the report records the emitted count, and the encoded // output stays under the budget. func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) { saveAndResetGlobals(t) @@ -941,7 +952,7 @@ func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) { originals[i] = clone } - bounded, note, err := boundProjectedOutput(rows, compactListOutputLimit) + bounded, bound, err := boundProjectedOutput(rows, compactListOutputLimit) if err != nil { t.Fatalf("bound: %v", err) } @@ -979,10 +990,11 @@ func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) { if len(encoded)+1 >= compactListOutputLimit { t.Fatalf("bounded output is %d bytes, want <%d", len(encoded)+1, compactListOutputLimit) } - wantNote := fmt.Sprintf("note: emitted %d of %d projected rows (every value intact) to stay below the %d-byte structured-output limit; narrow --fields or lower --limit to fit more rows per page — the rows past the first %d were not emitted", - len(kept), len(rows), compactListOutputLimit, len(kept)) - if note != wantNote { - t.Fatalf("note = %q, want %q", note, wantNote) + if bound.rowsEmitted != len(kept) || bound.rowsTotal != len(rows) || bound.maxBytes != compactListOutputLimit { + t.Fatalf("bound = %+v, want a %d-of-%d prefix reduction against the %d-byte limit", bound, len(kept), len(rows), compactListOutputLimit) + } + if bound.shortened != 0 { + t.Fatalf("bound reported %d shortened values; a page reduction must shorten none", bound.shortened) } } @@ -1013,15 +1025,15 @@ 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. The fixture is a single oversized row -// so the run lands on the shortening fallback (a multi-row page would be -// reduced, not shortened). -func TestBoundProjectedListAnnouncesShortening(t *testing.T) { +// TestBoundProjectedListReportsShortening pins that a list projection which had +// to clip values reports the loss as data. 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 +// indistinguishable from "nothing matched" — the expensive failure the caller's +// stderr note exists to prevent, keyed off these facts. The fixture is a single +// oversized row so the run lands on the shortening fallback (a multi-row page +// would be reduced, not shortened). +func TestBoundProjectedListReportsShortening(t *testing.T) { for _, format := range []string{"json", "toon"} { t.Run(format, func(t *testing.T) { saveAndResetGlobals(t) @@ -1031,33 +1043,33 @@ func TestBoundProjectedListAnnouncesShortening(t *testing.T) { "title": strings.Repeat("payment-gateway timeout ", 200), }} - _, note, err := boundProjectedOutput(rows, 512) + _, bound, 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 bound.shortened == 0 || bound.valuesTotal == 0 { + t.Fatalf("shortened projection reported no shortening: %+v", bound) } - if !strings.Contains(note, "title") { - t.Fatalf("note = %q, want it to name the shortened field (title)", note) + if len(bound.fields) != 1 || bound.fields[0] != "title" { + t.Fatalf("bound.fields = %v, want exactly the shortened field (title)", bound.fields) } }) } } -// TestBoundProjectedListNoNoteWhenNothingShortened keeps the note honest: a +// TestBoundProjectedListReportsNothingWhenFits keeps the report honest: a // projection that fits must not claim anything was clipped. -func TestBoundProjectedListNoNoteWhenNothingShortened(t *testing.T) { +func TestBoundProjectedListReportsNothingWhenFits(t *testing.T) { saveAndResetGlobals(t) flagOutputFormat = "json" rows := []map[string]any{{"incident_id": "inc-1", "title": "disk full"}} - _, note, err := boundProjectedOutput(rows, 512) + _, bound, 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) + if bound.rowsTotal != 0 || bound.shortened != 0 || len(bound.fields) != 0 { + t.Fatalf("fitting projection reported a reduction: %+v", bound) } } @@ -1259,13 +1271,238 @@ func TestChannelEscalateRuleListStructuredProjection(t *testing.T) { }) } +// TestReductionAdviceNamesOnlyDeclaredFlags pins that the projection note names +// a flag only when the command's own definition declares that flag as +// narrowing its structured output. --fields and --limit are registered per verb +// and their names do not imply their effect: a request-body write selector +// spelled --fields, or a --limit the server ignores, must never be offered as a +// way to shrink the output. +func TestReductionAdviceNamesOnlyDeclaredFlags(t *testing.T) { + // alert-event list declares both a projection --fields and a paging --limit, + // so the note may offer both. + t.Run("declared projection and rows", func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + items := make([]any, 40) + for i := range items { + items[i] = map[string]any{ + "event_id": fmt.Sprintf("%024x", i), + "title": strings.Repeat(fmt.Sprintf("row %d fat event title 详情 ", i), 20), + } + } + stub.data = map[string]any{"items": items, "total": len(items)} + + _, stderrText, err := execCommandSplit("alert-event", "list", "--limit", "40", + "--fields", "event_id,title", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(stderrText, "narrow --fields or lower --limit to fit more rows per page") { + t.Fatalf("both declared flags should be offered, got:\n%s", stderrText) + } + }) + + // channel escalate-rule-list declares a projection --fields but no paging + // flag, so the note may offer --fields alone. + t.Run("declared projection only", func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + items := make([]any, 60) + for i := range items { + items[i] = map[string]any{ + "rule_id": fmt.Sprintf("%024x", i), + "rule_name": strings.Repeat(fmt.Sprintf("rule %d ", i), 40), + } + } + stub.data = map[string]any{"items": items} + + _, stderrText, err := execCommandSplit("channel", "escalate-rule-list", "4201", + "--fields", "rule_id,rule_name", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(stderrText, "narrow --fields to fit more rows per page") { + t.Fatalf("advice should name the declared --fields, got:\n%s", stderrText) + } + if strings.Contains(stderrText, "--limit") { + t.Fatalf("advice must not name --limit; this verb declares no paging flag, got:\n%s", stderrText) + } + }) + + // monit rule-update-fields spells a request-body write selector --fields and + // declares no narrowing: the note must not offer --fields, which would + // silently change a write instead of the output. + t.Run("write-selector fields is not a projection control", func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + rows := make([]any, 60) + for i := range rows { + rows[i] = map[string]any{ + "message": "", + "name": strings.Repeat(fmt.Sprintf("rule %d ", i), 40), + } + } + stub.data = rows + + _, stderrText, err := execCommandSplit("monit", "rule-update-fields", + "--data", `{"ids":[50001],"fields":["enabled"]}`, "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(stderrText, "note: emitted") { + t.Fatalf("over-budget response should announce the reduction, got:\n%s", stderrText) + } + if strings.Contains(stderrText, "--fields") { + t.Fatalf("advice must not name --fields; here it selects rule fields to WRITE, got:\n%s", stderrText) + } + }) + + // channel silence-rule-list is a generated verb that declares no narrowing + // flag: the note must not invent one. + t.Run("no narrowing flag", func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + items := make([]any, 60) + for i := range items { + items[i] = map[string]any{ + "rule_id": fmt.Sprintf("%024x", i), + "rule_name": fmt.Sprintf("silence rule %d", i), + "description": strings.Repeat(fmt.Sprintf("row %d noise-suppression detail ", i), 40), + } + } + stub.data = map[string]any{"items": items} + + _, stderrText, err := execCommandSplit("channel", "silence-rule-list", "4201", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(stderrText, "note: emitted") { + t.Fatalf("over-budget page should announce the reduction, got:\n%s", stderrText) + } + if !strings.Contains(stderrText, "declares no flag that narrows the output") { + t.Fatalf("advice should say no flag narrows this command, got:\n%s", stderrText) + } + for _, flag := range []string{"--fields", "--limit"} { + if strings.Contains(stderrText, flag) { + t.Fatalf("advice must not name %s on a verb that does not narrow with it, got:\n%s", flag, stderrText) + } + } + }) +} + +// hugeTimeFilters returns a recurring-window list large enough that a single row +// carrying it cannot fit the compact budget even after its string values are +// shortened — the shape that lands the run on the irreducible-overflow path. +func hugeTimeFilters() []any { + out := make([]any, 400) + for i := range out { + out[i] = map[string]any{"start": "00:00", "end": "23:59", "repeat": []any{1, 2, 3, 4, 5, 6, 7}} + } + return out +} + +// TestShorteningAdviceOmitsRowsFlag pins that the single-row shortening branch +// offers only the projection control: requesting fewer rows cannot shrink the +// one row that already overflows, so a paging flag declared by the same command +// must not appear in that remedy. +func TestShorteningAdviceOmitsRowsFlag(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + stub.data = map[string]any{ + "items": []any{map[string]any{ + "event_id": fmt.Sprintf("%024x", 1), + "title": strings.Repeat("payment-gateway timeout 详情 ", 800), + }}, + "total": 1, + } + + _, stderrText, err := execCommandSplit("alert-event", "list", "--fields", "event_id,title", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(stderrText, "were shortened to fit") { + t.Fatalf("a clipped value should be announced, got:\n%s", stderrText) + } + if !strings.Contains(stderrText, "narrow --fields for untruncated values") { + t.Fatalf("the shortening remedy should name the declared projection, got:\n%s", stderrText) + } + if strings.Contains(stderrText, "--limit") { + t.Fatalf("a smaller page cannot shrink the overflowing row, so --limit must not be offered, got:\n%s", stderrText) + } +} + +// TestOverflowErrorNamesOnlyDeclaredFlags pins that the irreducible-overflow +// failure carries the same command-scoped remedy as the note: it is reachable by +// the same generated verbs, so it must not name a flag the command does not +// declare as narrowing. +func TestOverflowErrorNamesOnlyDeclaredFlags(t *testing.T) { + // A row whose bulk is a non-string field cannot be shortened, so the command + // fails. channel silence-rule-list declares no narrowing flag. + t.Run("no narrowing flag", func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + stub.data = map[string]any{"items": []any{map[string]any{ + "rule_id": fmt.Sprintf("%024x", 1), + "rule_name": "silence everything", + "time_filters": hugeTimeFilters(), + }}} + + _, _, err := execCommandSplit("channel", "silence-rule-list", "4201", "--output-format", "json") + if err == nil { + t.Fatal("irreducible single row should fail, got nil") + } + if !strings.Contains(err.Error(), "largest fields:") { + t.Fatalf("error should name the largest fields, got: %v", err) + } + if !strings.Contains(err.Error(), "declares no flag that narrows the output") { + t.Fatalf("error should state no flag narrows this command, got: %v", err) + } + for _, flag := range []string{"--fields", "--limit"} { + if strings.Contains(err.Error(), flag) { + t.Fatalf("error must not name %s on a verb that does not narrow with it, got: %v", flag, err) + } + } + }) + + // channel escalate-rule-list declares a projection --fields, so the failure + // may name it and must not name the paging flag the verb does not have. + t.Run("declared projection", func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + stub := newGFStub(t) + stub.data = map[string]any{"items": []any{map[string]any{ + "rule_id": "6621b23f4a2c5e0012ab34d0", + "rule_name": "P1 on-call", + "time_filters": hugeTimeFilters(), + }}} + + _, _, err := execCommandSplit("channel", "escalate-rule-list", "4201", + "--fields", "rule_id,rule_name,time_filters", "--output-format", "json") + if err == nil { + t.Fatal("irreducible single row should fail, got nil") + } + if !strings.Contains(err.Error(), "narrow --fields") { + t.Fatalf("error should name the declared --fields, got: %v", err) + } + if strings.Contains(err.Error(), "--limit") { + t.Fatalf("error must not name --limit; this verb declares no paging flag, got: %v", err) + } + }) +} + // TestBoundProjectedListNeverShortensIdentifierFields pins the identifier // exemption: keys ending in _id/_key carry values a consumer matches, // filters, or passes back verbatim (a jq exact-match over --json output, a // follow-up detail call), so shortening one silently defeats that consumer. // The fixture is a single row whose oversized title overflows the budget on // its own, landing the run on the shortening fallback: the identifiers must -// come back byte-identical, only the free-text title shortens, and the note +// come back byte-identical, only the free-text title shortens, and the report // must name only the clipped field. func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) { for _, format := range []string{"json", "toon"} { @@ -1282,7 +1519,7 @@ func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) { }} const budget = 1400 - bounded, note, err := boundProjectedOutput(rows, budget) + bounded, bound, err := boundProjectedOutput(rows, budget) if err != nil { t.Fatalf("bound projected output: %v", err) } @@ -1301,14 +1538,11 @@ func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) { t.Errorf("title should be shortened with the \"...\" marker, got %q", title) } - if note == "" { - t.Fatal("shortened projection returned no note; caller cannot tell values were clipped") - } - if !strings.Contains(note, "title") { - t.Errorf("note = %q, want it to name the shortened field (title)", note) + if len(bound.fields) != 1 || bound.fields[0] != "title" { + t.Fatalf("bound.fields = %v, want only the shortened non-identifier field (title)", bound.fields) } - if strings.Contains(note, "event_id") || strings.Contains(note, "alert_key") { - t.Errorf("note = %q, want it to name only shortened fields, never exempt identifiers", note) + if bound.shortened != 1 { + t.Fatalf("bound.shortened = %d, want exactly the one clipped value", bound.shortened) } encoded, err := marshalStructured(kept) @@ -1325,7 +1559,7 @@ func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) { // TestBoundProjectedListIdentifierOnlyOverflowReducesPage pins the other half // of the identifier exemption: a page carrying nothing shortenable (only // identifier content) that overflows the budget is reduced to the leading -// rows that fit — identifiers stay byte-identical and the note names the +// rows that fit — identifiers stay byte-identical and the report records the // emitted count — instead of clipping identifiers or erroring out. func TestBoundProjectedListIdentifierOnlyOverflowReducesPage(t *testing.T) { saveAndResetGlobals(t) @@ -1340,7 +1574,7 @@ func TestBoundProjectedListIdentifierOnlyOverflowReducesPage(t *testing.T) { originals[i] = row["incident_id"].(string) } - bounded, note, err := boundProjectedOutput(rows, 512) + bounded, bound, err := boundProjectedOutput(rows, 512) if err != nil { t.Fatalf("identifier-only overflow should reduce the page, not error: %v", err) } @@ -1356,9 +1590,8 @@ func TestBoundProjectedListIdentifierOnlyOverflowReducesPage(t *testing.T) { t.Errorf("row %d incident_id was mutated: got %q, want byte-identical %q", i, got, originals[i]) } } - wantNote := fmt.Sprintf("emitted %d of %d", len(kept), len(rows)) - if !strings.Contains(note, wantNote) { - t.Fatalf("note = %q, want it to name the emitted count (%q)", note, wantNote) + if bound.rowsEmitted != len(kept) || bound.rowsTotal != len(rows) { + t.Fatalf("bound = %+v, want it to report emitting %d of %d rows", bound, len(kept), len(rows)) } encoded, err := marshalStructured(kept) if err != nil { diff --git a/internal/cli/gen_support.go b/internal/cli/gen_support.go index db1439f..7ca3dab 100644 --- a/internal/cli/gen_support.go +++ b/internal/cli/gen_support.go @@ -380,13 +380,13 @@ func printBoundedGenericResult(ctx *RunContext, data any) error { if !ok { return ctx.Printer.Print(data, nil) } - bounded, note, err := boundProjectedList(rows, compactListOutputLimit) + bounded, bound, err := boundProjectedList(rows, compactListOutputLimit) if err != nil { - return err + return explainProjectionOverflow(ctx.Cmd, err) } // A bare array has nowhere to carry the marker the envelope branch // adds; the stderr note is its only signal. - noteProjectionBound(ctx.Cmd.ErrOrStderr(), note) + noteProjectionBound(ctx.Cmd, bound) return ctx.Printer.Print(bounded, nil) case map[string]any: key, ok := listEnvelopeKey(value) @@ -406,12 +406,12 @@ func printBoundedGenericResult(ctx *RunContext, data any) error { // until the whole payload is under it. budget := compactListOutputLimit for { - bounded, note, err := boundProjectedList(rows, budget) + bounded, bound, err := boundProjectedList(rows, budget) if err != nil { - return err + return explainProjectionOverflow(ctx.Cmd, err) } value[key] = bounded - if note != "" { + 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 @@ -429,7 +429,7 @@ func printBoundedGenericResult(ctx *RunContext, data any) error { return err } if len(out)+1 < compactListOutputLimit { - noteProjectionBound(ctx.Cmd.ErrOrStderr(), note) + noteProjectionBound(ctx.Cmd, bound) return ctx.Printer.Print(value, nil) } budget -= len(out) + 2 - compactListOutputLimit diff --git a/internal/cli/gen_support_test.go b/internal/cli/gen_support_test.go index b31b1d1..96d0b20 100644 --- a/internal/cli/gen_support_test.go +++ b/internal/cli/gen_support_test.go @@ -76,6 +76,16 @@ func TestPrintGenericResultBoundsListEnvelope(t *testing.T) { if !strings.Contains(stderrText, "note: emitted") { t.Errorf("reduced %s page should announce itself on stderr, got:\n%s", format, stderrText) } + // insight incident-list is a generated verb: it declares no narrowing + // flag, so even though it carries a --limit, the note names none. + if !strings.Contains(stderrText, "declares no flag that narrows the output") { + t.Errorf("a generated verb should get the flagless note, got:\n%s", stderrText) + } + for _, flag := range []string{"--limit", "--fields"} { + if strings.Contains(stderrText, flag) { + t.Errorf("advice must not name %s on a verb that declares no narrowing flag, got:\n%s", flag, stderrText) + } + } // The first row survives intact; the last was dropped by the // prefix reduction. if !strings.Contains(out, ids[0]) { @@ -154,6 +164,15 @@ func TestPrintGenericResultBoundsTopLevelArray(t *testing.T) { if !strings.Contains(stderrText, "note: emitted") { t.Errorf("reduced page should announce itself on stderr, got:\n%s", stderrText) } + // monit rule-list-basic is a generated verb whose --limit sizes the response + // only alongside --include-descendants: it declares no narrowing flag, so the + // note must not offer --limit. + if !strings.Contains(stderrText, "declares no flag that narrows the output") { + t.Errorf("a generated verb should get the flagless note, got:\n%s", stderrText) + } + if strings.Contains(stderrText, "--limit") { + t.Errorf("advice must not name --limit; this verb declares no narrowing flag, got:\n%s", stderrText) + } var decoded []map[string]any if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &decoded); err != nil { t.Fatalf("bounded top-level array is not a JSON array: %v\n%s", err, out) diff --git a/internal/cli/incident.go b/internal/cli/incident.go index a4dceb3..f4833d5 100644 --- a/internal/cli/incident.go +++ b/internal/cli/incident.go @@ -137,12 +137,12 @@ func newIncidentListCmd() *cobra.Command { if err != nil { return err } - bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit) + bounded, bound, err := boundProjectedOutput(proj, compactListOutputLimit) if err != nil { - return err + return explainProjectionOverflow(cmd, err) } proj = bounded.([]map[string]any) - noteProjectionBound(cmd.ErrOrStderr(), note) + noteProjectionBound(cmd, bound) effectiveLimit := limit if len(proj) < len(result.Items) { effectiveLimit = len(proj) @@ -176,6 +176,7 @@ func newIncidentListCmd() *cobra.Command { cmd.Flags().IntVar(&limit, "limit", 20, "Max results (max 100)") cmd.Flags().IntVar(&page, "page", 1, "Page number") cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. incident_id,title,incident_severity,progress,start_time); ignored in table mode. Use to avoid dumping the full nested record.") + declareOutputNarrowing(cmd, "fields", "limit") return cmd } @@ -638,12 +639,12 @@ func newIncidentSimilarCmd() *cobra.Command { if err != nil { return err } - bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit) + bounded, bound, err := boundProjectedOutput(proj, compactListOutputLimit) if err != nil { - return err + return explainProjectionOverflow(cmd, err) } proj = bounded.([]map[string]any) - noteProjectionBound(cmd.ErrOrStderr(), note) + noteProjectionBound(cmd, bound) return ctx.Printer.Print(proj, nil) } @@ -654,6 +655,7 @@ func newIncidentSimilarCmd() *cobra.Command { cmd.Flags().IntVar(&limit, "limit", 5, "Max results") cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. incident_id,title,incident_severity,progress,start_time); ignored in table mode. Defaults to a compact incident summary. If the page would exceed 16 KiB, only the leading rows that fit are emitted, with every value intact (announced on stderr).") + declareOutputNarrowing(cmd, "fields", "limit") return cmd } @@ -1613,11 +1615,11 @@ func newIncidentDetailCmd() *cobra.Command { if err != nil { return err } - _, note, err := boundProjectedOutput(proj[0], compactDetailOutputLimit) + _, bound, err := boundProjectedOutput(proj[0], compactDetailOutputLimit) if err != nil { - return err + return explainProjectionOverflow(cmd, err) } - noteProjectionBound(cmd.ErrOrStderr(), note) + noteProjectionBound(cmd, bound) return ctx.Printer.Print(proj[0], nil) } return ctx.Printer.Print(result, nil) @@ -1629,6 +1631,7 @@ func newIncidentDetailCmd() *cobra.Command { }, } cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. incident_id,title,incident_severity,progress,root_cause); ignored in table mode. The projection must fit within 8 KiB or the command fails and names the largest fields; omit --fields for the full, unbounded detail.") + declareOutputNarrowing(cmd, "fields", "") return cmd } diff --git a/internal/cli/insight.go b/internal/cli/insight.go index d404df8..97a3c99 100644 --- a/internal/cli/insight.go +++ b/internal/cli/insight.go @@ -129,12 +129,12 @@ func newInsightIncidentsCmd() *cobra.Command { if err != nil { return err } - bounded, note, err := boundProjectedOutput(proj, compactListOutputLimit) + bounded, bound, err := boundProjectedOutput(proj, compactListOutputLimit) if err != nil { - return err + return explainProjectionOverflow(cmd, err) } proj = bounded.([]map[string]any) - noteProjectionBound(cmd.ErrOrStderr(), note) + noteProjectionBound(cmd, bound) effectiveLimit := limit if len(proj) < len(result.Items) { effectiveLimit = len(proj) @@ -176,6 +176,7 @@ func newInsightIncidentsCmd() *cobra.Command { cmd.Flags().IntVar(&limit, "limit", 20, "Max results (max 100)") cmd.Flags().IntVar(&page, "page", 1, "Page number") cmd.Flags().StringVar(&fields, "fields", "", "Comma-separated fields to project in json/toon output (e.g. incident_id,title,severity); ignored in table mode. Use to avoid dumping the full nested record.") + declareOutputNarrowing(cmd, "fields", "limit") return cmd } diff --git a/internal/cmd/cligen/main.go b/internal/cmd/cligen/main.go index a3e0b48..a219808 100644 --- a/internal/cmd/cligen/main.go +++ b/internal/cmd/cligen/main.go @@ -1317,6 +1317,13 @@ func emitCmd(fn string, s service, o specOp, mi methodInfo) string { call += ")" } + // Generated commands emit no declareOutputNarrowing call. The spec gives no + // structured contract for whether a --limit bounds the emitted rows (the + // knowledge file-list documents its --limit as ignored; rule-list-basic + // honors its own only alongside --include-descendants), and a flag's name is + // not evidence of its effect. With no declaration the projection note names + // no flag, which is the honest default; a hand-written command that knows + // what its flags do declares them itself. switch { case mi.HasData: fmt.Fprintf(&b, "\t\t\t\tout, _, err := %s\n", call)