Skip to content

fix(cli): say when dms, notes, feed, social and workflow reads come back at their limit - #5934

Open
Chessing234 wants to merge 12 commits into
block:mainfrom
Chessing234:feat/cli-limit-notes-everywhere
Open

fix(cli): say when dms, notes, feed, social and workflow reads come back at their limit#5934
Chessing234 wants to merge 12 commits into
block:mainfrom
Chessing234:feat/cli-limit-notes-everywhere

Conversation

@Chessing234

Copy link
Copy Markdown
Contributor

Refs #5595 — the rest of the read commands.

Stacked on #5721. It reuses effective_limit / truncation_notice, which that PR introduces, so its two commits are in this branch's history and the diff here is the eight commits above them. Please merge #5721 first, or say the word and I will fold this into it.

#5721 made messages get, search and thread say when a read came back at its bound. Every other list-shaped read has the same shape and the same failure: a full page is byte-for-byte a complete answer, so an agent enumerating conversations, notes, feed entries or workflow runs treats a prefix as the whole set.

  • refactor: the two helpers move out of commands/messages.rs into limits.rs — they were never message-specific, and a sibling command should not have to import from another command module to use them.
  • then one commit each for dms list (50/200), notes ls (50/200), feed get (20/50 — the tightest bounds here, and the read most likely to hit them), social notes (50/100) and workflows runs (20/100).
  • --limit help on those flags now names the default and the cap, which previously read "Maximum number of results to return" with no hint that either existed. notes ls already documented its bounds and is unchanged.

stdout is untouched everywhere: the note goes to stderr, so nothing parsing the JSON array changes. social notes prints the relay response verbatim, so it parses the response back to count what arrived rather than assuming the page was full.

Verified locally with CI's own gates: cargo clippy --workspace --all-targets -- -D warnings clean, cargo fmt --all --check clean, cargo test -p buzz-cli --lib (363, 6 new on the shared helper).

buzz messages get returns the newest 50 messages when --limit is
omitted, with nothing in the output to say the result is a prefix. A
truncated history is byte-for-byte indistinguishable from a complete
one, so an agent rebuilding context from a channel read reconstructs a
plausible-but-wrong past and acts on it.

Emit a note on stderr when the read comes back at its limit, naming the
bound that applied - the default, the requested --limit, or the cap it
was silently clamped to - and how to see more. stdout keeps the plain
JSON array, so the machine contract is unchanged.

The note says "may exist" rather than reporting a total: the relay
answers a filter, not a count, and a result set exactly the size of the
limit is possible. A short read is the only provably complete case and
stays silent.

Refs block#5595

Signed-off-by: Taksh <takshkothari09@gmail.com>
search defaults to 20 and hard-caps at 100, so `--limit 500` returns
exactly 100 with nothing indicating the flag was ignored. thread has
the same shape at 100/500. Both now emit the same stderr note.

thread counts only the replies: the root event rides along in the same
response but is not part of the reply page, so including it would
report truncation one reply early.

Also state the defaults and caps in --limit's help, which previously
read "Maximum number of results to return" with no hint that a default
existed, and mark --before / --since inclusive - `until` and `since`
are both inclusive comparisons in the filter matcher, so a naive pager
double-counts the boundary event without that note.

Closes block#5595

Signed-off-by: Taksh <takshkothari09@gmail.com>
`effective_limit` and `truncation_notice` are not specific to `messages`:
every list-shaped read has a default and a cap and can come back at either.
Give them a module so the other read commands can adopt them without
importing from a sibling command.

Signed-off-by: Taksh <takshkothari09@gmail.com>
`dms list` returns the newest 50 conversations by default and silently
clamps `--limit` at 200. A caller enumerating conversations to pick one has
no way to tell a full list from a page of it.

Signed-off-by: Taksh <takshkothari09@gmail.com>
Same shape as the other reads: 50 by default, clamped at 200, and a full
page is indistinguishable from the whole set.

Signed-off-by: Taksh <takshkothari09@gmail.com>
The feed defaults to 20 and caps at 50, the tightest bounds of any read
here, so a busy inbox hits the ceiling routinely and reads as "that is
everything addressed to me".

Signed-off-by: Taksh <takshkothari09@gmail.com>
Counts the events the relay actually returned rather than assuming the
page was full: this path prints the relay response verbatim, so the note
has to parse it back to know.

Signed-off-by: Taksh <takshkothari09@gmail.com>
Run history is the read most likely to be paged through, and the one where
a silent prefix reads as "the workflow stopped running".

Signed-off-by: Taksh <takshkothari09@gmail.com>
`notes ls` already documented its bounds; `dms list`, `feed get` and
`workflows runs` read "Maximum number of results to return" with no hint
that a default existed or that a larger value would be clamped.

Signed-off-by: Taksh <takshkothari09@gmail.com>
Moves the bound cases onto the module that now owns them: short reads stay
silent, a full default read names the default, a clamped --limit says so
and stops suggesting a larger one, and a relay that overshoots the bound
still warns.

Signed-off-by: Taksh <takshkothari09@gmail.com>
@Chessing234
Chessing234 requested a review from a team as a code owner August 15, 2026 07:31

@themiguelamador themiguelamador left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two correctness issues found across the full PR diff.

  • P1 — The hard-cap recovery instruction names flags most commands do not have. The shared notice universally tells users to page with --since / --before, but dms list, messages thread, notes ls, and workflows runs expose neither flag; feed get and messages search only expose --since, which cannot retrieve older results. Those commands therefore direct an agent into rejected or ineffective follow-up calls precisely when data may be missing. messages get now advertises only its real timestamp-window narrowing, social notes advertises its composite --before + --before-id cursor, and commands without a usable older-page mechanism say that they cannot request a larger page.
  • P2 — social notes silently suppresses the truncation warning on malformed relay JSON. It printed the raw body, parsed with unwrap_or(0), and treated any non-array response as zero results. That both emits invalid machine-readable output and makes a full/unknown response appear safely short. The command now validates the event array before writing stdout and returns a clear CLI error when it cannot determine the result count.

Fix: 589753e2b (review/pr-5934-fix).

Verification:

  • cargo test -p buzz-cli --lib — 364 passed
  • focused truncation helper/message tests — 15 passed
  • cargo clippy -p buzz-cli --all-targets -- -D warnings
  • cargo fmt --all
  • git diff --check

@Chessing234

Copy link
Copy Markdown
Contributor Author

Thanks — both fixed, in 0d1b882 (P1) and 1fe89fc (P2). Complear/buzz 404s for me, so 589753e2b was unreachable and these are written from your description.

P1 — advice that names real flags. You are right that the shared notice was worse than useless on most of these commands. Each read now declares its contract once, as a ReadLimits { default, max, paging }, and Paging is what the notice reads at the cap:

command flags it really has Paging
messages get --since, --before TimestampWindow
social notes --before, --before-id BeforeCursor
feed get, messages search --since only SinceOnly
dms list, notes ls, messages thread, workflows runs neither None

SinceOnly says "--since only narrows to newer results — this command cannot request an older page"; None says "this command has no windowing flags — it cannot request a larger page". Below the cap all four still say "pass a larger --limit", which is true regardless of paging, and there is a test asserting exactly that. The negative assertions matter as much as the positive ones, so the tests also check the None and SinceOnly notices contain no --before at all.

Replacing the loose (returned, requested, default, max) argument list with the struct is deliberate: the paging kind belongs with the numbers, and a fifth positional u32-adjacent argument is a transposition waiting to happen.

P2 — social notes on an uncountable body. Now parsed before anything reaches stdout, by count_events, which requires a JSON array whose every element is an object; anything else is a CliError saying the result count is unknown. One thing I tightened beyond the description: serde_json::from_str::<Vec<Value>> alone accepts ["a","b"], so the element check is what actually makes it a validation rather than a parse.

Verified on the pushed head: cargo test -p buzz-cli --lib 369 passed (was 364; the new cases cover each Paging variant at and below the cap, and the uncountable bodies), cargo clippy -p buzz-cli --all-targets -- -D warnings clean, cargo fmt --all -- --check, git diff --check.

The review points out the shared notice told every caller to page with
--since / --before, but dms list, messages thread, notes ls and workflows
runs expose neither, and feed get and messages search expose only --since,
which cannot reach older results. An agent following that advice burns a
call on a flag that does not exist, or believes data is reachable when it
is not.

Each command now declares its contract once as a ReadLimits — default, cap
and a Paging value naming what it can actually do:

- messages get   TimestampWindow (--since and --before)
- social notes   BeforeCursor    (--before + --before-id)
- feed get,      SinceOnly       (narrows to newer only; says it cannot
  messages search                 request an older page)
- dms list, notes ls,   None     (says it cannot request a larger page)
  messages thread, workflows runs

Below the cap every command still says 'pass a larger --limit', which is
true regardless of paging.

Signed-off-by: Taksh <takshkothari09@gmail.com>
The review points out the command printed the relay body verbatim and then
counted it with unwrap_or(0), so a non-array response both emitted invalid
machine-readable output and made an unknown result set look safely short —
suppressing the very notice this PR adds.

count_events now validates the body before anything is written to stdout:
it must parse as a JSON array whose every element is an object. Anything
else is a CLI error saying the result count is unknown.

Signed-off-by: Taksh <takshkothari09@gmail.com>
@Chessing234
Chessing234 force-pushed the feat/cli-limit-notes-everywhere branch from 1fe89fc to 45f5565 Compare August 16, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants