fix(cli): keep identifier fields intact in list projection shortening - #173
Merged
Conversation
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.
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.
Defect
The compact list projections (
incident list,incident similar,alert-event listin--output-format json|toon) bound their output to the 16 KiB list budget by applying one "fair" per-field byte cap to every string value. When a page overflows badly enough that the fair cap lands below an identifier's own length, identifier fields —incident_id,alert_id,event_id,alert_key, … — are clipped mid-value with a trailing....User-visible symptom: a consumer that filters the JSON output by exact match (
jq 'select(.alert_key == "...")') or passes an id back into a follow-updetailcall gets nothing — a clipped identifier matches no real value, and the empty result is indistinguishable from "nothing matched". Whether an id survives depends on what else happened to overflow on that page, so exact-match filtering over list output is unreliable by default.Root cause
boundProjectedList(internal/cli/fieldproject.go) computedmaxLen, ran itsfits()trials, and applied the final cap uniformly over all string values — no distinction between identifiers (values a consumer matches, filters, or passes back verbatim) and free text (values a human reads).Fix
Add
isIdentifierField(key)(keys ending in_idor_key) and exempt those fields at all three points ofboundProjectedList:maxLensizing scan skips identifier keys;fits()copies identifier values verbatim into the trial rows;total++, so the shortening note's denominator counts only shortenable strings.Identifiers now survive byte-intact — or, when a page's irreducible content (identifier strings + keys + envelope) alone exceeds the budget, the existing narrowing error fires (
request fewer rows (--limit) or fewer --fields) instead of shipping clipped ids. Note format and error text are unchanged;boundProjectedDetail, all projection call sites, flag help, and skill docs are untouched.Tests
TestBoundProjectedListNeverShortensIdentifierFields(json + toon subtests): 10 rows of{event_id, alert_key, title(500B)}at a 1400-byte budget — both id fields byte-identical in every row, every title shortened with the...marker, the note namestitlebut not the identifier fields, and the re-marshaled output fits the budget. Against the old implementation the json subtest fails: the fair cap lands at 27 bytes and clips the 32-bytealert_key.TestBoundProjectedListIdentifierOnlyOverflowErrors: 12 rows of{incident_id}at a 512-byte budget — errors with "request fewer rows" and leaves every row unmutated. Against the old implementation it fails: the old code clipped every 24-byte id to 12 bytes and returned success.TestBoundProjectedListNeverEmitsUnmarkedTruncation(pre-existing): its 100-row fixture only ever fit because the old code clipped the ids to fit the budget — under the exemption that page errors by design, so the fixture is resized to 90 rows, keeping it in the "titles shorten, ids intact" regime the test is about. This is the intended contract: a page whose irreducible identifier content alone exceeds the budget now fails with the narrowing error instead of shipping mangled ids.Verification