Skip to content

Fix billable messages warning #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
umair-ably wants to merge 1 commit into
base: main
Choose a base branch
from
+2 −2
Open

Fix billable messages warning #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ All output helpers use the `format` prefix and are exported from `src/utils/outp
- **Limit warnings**: `formatLimitWarning(count, limit, "items")` — yellow warning if results truncated. Only show when `hasMore === true`.
- **Pagination collection**: `collectPaginatedResults(firstPage, limit)` — walks cursor-based pages until `limit` items are collected. Returns `{ items, hasMore, pagesConsumed }`. Use for both SDK and HTTP paginated commands.
- **Filtered pagination**: `collectFilteredPaginatedResults(firstPage, limit, filter, maxPages?)` — same as above but applies a client-side filter. Use for rooms/spaces list where channels need prefix filtering. `maxPages` (default: 20) prevents runaway requests.
- **Pagination warning**: `formatPaginationLog(pagesConsumed, itemCount, isBillable?)` — shows "Fetched N pages" when `pagesConsumed > 1`. Pass `isBillable: true` for history commands (billable API calls). Guard with `!this.shouldOutputJson(flags)`.
- **Pagination warning**: `formatPaginationLog(pagesConsumed, itemCount, isBillable?)` — shows "Fetched N pages" when `pagesConsumed > 1`. Pass `isBillable: true` for history commands (each message retrieved counts as a billable message). Guard with `!this.shouldOutputJson(flags)`.
- **Pagination next hint**: `buildPaginationNext(hasMore, lastTimestamp?)` — returns `{ hint, start? }` for JSON output when `hasMore` is true. Pass `lastTimestamp` only for history commands (which have `--start`).
- **JSON guard**: All human-readable output (progress, success, listening messages) must be wrapped in `if (!this.shouldOutputJson(flags))` so it doesn't pollute `--json` output. Only JSON payloads should be emitted when `--json` is active.
- **JSON envelope**: Use `this.logJsonResult(data, flags)` for one-shot results, `this.logJsonEvent(data, flags)` for streaming events, and `this.logJsonStatus(status, message, flags)` for hold/status signals in long-running commands. The envelope adds top-level fields (`type`, `command`, `success?`). Nest domain data under a **domain key** (see "JSON data nesting convention" below). Do NOT add ad-hoc `success: true/false` — the envelope handles it. `--json` produces compact single-line output (NDJSON for streaming). `--pretty-json` is unchanged.
Expand Down
2 changes: 1 addition & 1 deletion src/utils/pagination.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export function formatPaginationLog(
const message = `Fetched ${pagesConsumed} pages to retrieve ${itemCount} results.`;
if (isBillable) {
return formatWarning(
`${message} Each page request counts as a billable message.`,
`${message} Each message retrieved counts as a billable message.`,
Copy link
Contributor

@sacOO7 sacOO7 Mar 25, 2026

Choose a reason for hiding this comment

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

@coderabbitai can you check if warning is also shown in json mode?

Copy link
Contributor

@sacOO7 sacOO7 Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Seems, it's not printed in JSON mode

      const paginationWarning = formatPaginationLog(
        pagesConsumed,
        subscriptions.length,
      );
      if (paginationWarning && !this.shouldOutputJson(flags)) {
        this.log(paginationWarning);
      }

Copy link
Contributor

@sacOO7 sacOO7 Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Would be useful for someone viewing nested messages in pretty-json mode or
even for LLMs to understand message retrieval pricing.

Copy link
Collaborator Author

@umair-ably umair-ably Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

there's a bunch of inconsistencies with JSON mode on the same lines that Andy has highlighted in the dog fooding thread here...

I've picked this up as a separate task. It doesn't fit with this PR here.

);
}

Expand Down
Loading