fix(cli): announce shortened projection values instead of clipping silently - #156
Merged
Conversation
…lently An over-budget list projection shortens string values and marks them with "...", on the reasoning that a marked value is distinguishable from a genuinely short one. That holds for a reader. It does not hold for the consumer `--json` exists to serve: a jq filter or exact match runs over the value itself and never sees the marker, so a clipped title turns a matching query into an empty result that looks exactly like "nothing matched". The cost lands on whoever wrote the filter, who re-runs variations of a query that was correct all along. boundProjectedOutput now returns a note naming the clipped fields, and the three list call sites print it on stderr next to the existing default- projection note. Behaviour is otherwise unchanged: the same values are shortened, with the same fair per-field cap and the same marker. The list-overflow error also gains what the detail path already had — the largest fields by aggregate byte size. Finding the oversized field previously meant re-running the query once per field until one of them was dropped; the error now names it outright. Both paths share one helper.
The stderr note added in this branch tells a caller when list values were clipped, but the cheaper answer is not to match on them at all: incident list already takes --query, a server-side full-text search over title/labels/content that is unaffected by whatever the projection did to the rendered value. Filtering a page of list output locally is the pattern that runs into clipped titles in the first place, and it also costs a page walk the server could have skipped. Say so on the card, next to the existing --fields advice that encourages projecting list scans down.
Two cards quoted behaviour this branch changes. incident.md reproduced the
old flat error string verbatim ("request fewer rows or fields"), which no
longer appears in the output, and described the list path as failing without
naming anything — it now names the largest fields, like detail always has.
Both incident.md and alert.md also documented the "..." marker as the only
sign that a value was shortened. That is what made the marker a trap: it is
visible when you read a row, not when you filter one. Say that the stderr
note exists and name what it is for.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
boundProjectedListshortens string values when a projection overflows its byte budget and marks each clipped value with"...". The reasoning in the code is sound as far as it goes: a marked value is distinguishable from a genuinely short one.It is distinguishable to a reader. It is not distinguishable to the consumer
--jsonexists for. A caller pipesincident list --fields title,... --jsonintojq 'select(.title | test("..."))'— the filter runs over the value, never sees the marker, and returns nothing. An empty result from a clipped value is byte-identical to an empty result from a genuine non-match, so the caller concludes the data isn't there and starts re-running variations of a query that was correct all along.Two related papercuts on the same path:
structured projection exceeds 16384-byte limit; request fewer rows or fields. It doesn't say which field is oversized, so isolating it means re-running the query once per field, dropping one each time. The detail path already solves this — it names the top-3 largest fields — the list path just never got it.The change
Code
boundProjectedOutputreturns a caller-printable note (empty when nothing was clipped) naming the shortened fields and how many values were affected. The three list call sites print it on stderr, right where the existing default-projection note from feat(cli): announce the default compact projection on stderr #139 goes.boundProjectedDetailandboundProjectedListnow share onelargestProjectedFieldshelper.Cards (same defect, other side)
incident.md: search with--queryrather than substring-matchingtitlefrom list output.--queryis a server-side full-text search over title/labels/content, so it is unaffected by whatever the projection did to the rendered value — and it saves the page walk that local filtering requires. This is the fix that stops the problem arising at all; the stderr note is the safety net for when it does.incident.md+alert.md: both quoted behaviour this branch changes — the old flat error string verbatim, and the"..."marker as the only sign a value was shortened. Synced to the new error text and to the existence of the note.Behaviour is otherwise unchanged: the same values get shortened, with the same fair per-field cap, the same
"..."marker, and the sameminMarkedTruncationCapfloor. Nothing that used to succeed now fails.Why announce rather than refuse in
--jsonRefusing to shorten in JSON was the first thing I tried, and it turns silent corruption into a hard wall: four existing end-to-end cases (
incident list,incident similar,alert-event list) go from returning rows to returning an error. Whether a display byte budget should apply to a machine format at all is a real question —alert list --fieldsis already uncapped, and the budget is a second guard behind the--fieldsprojection itself — but that is a policy call, not this fix. Announcing the loss removes the failure that was actually observed without betting on the answer.Verification
go build ./...,go vet ./...cleango test ./...— all 9 packages passincident listlong-title end-to-end case now also asserts the note reaches stderr through the real command path, in both formats — so this is verified end-to-end, not just at the helpergrep -rn "request fewer rows or fields" skills/returns nothing: no card still quotes the removed string