feat: add --page convenience option for trace list CLI#707
Open
VJ-yadav wants to merge 2 commits intoAltimateAI:mainfrom
Open
feat: add --page convenience option for trace list CLI#707VJ-yadav wants to merge 2 commits intoAltimateAI:mainfrom
VJ-yadav wants to merge 2 commits intoAltimateAI:mainfrom
Conversation
…teAI#669) The pagination UX previously required users to manually calculate offsets (--offset 20 --limit 20, then --offset 40 --limit 20). This adds --page N (-p) which computes the offset automatically: offset = (page - 1) * limit. - --page takes precedence over --offset when both are provided - Invalid values (0, negative, fractional) are clamped/truncated - Footer now shows "page X/Y" and "Next page: --page N" hints - 14 unit tests covering conversion logic and footer formatting
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a 1-based Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/test/cli/trace-page-option.test.ts (1)
12-17: Avoid duplicating the production pagination logic inside the test.
pageToOffset()andformatFooter()are local copies of the implementation, so this suite can stay green whileTraceCommand.handlerorlistTraces()drifts. Please extract a shared pure helper frompackages/opencode/src/cli/cmd/trace.ts, or drive the real command/output path here instead.Also applies to: 61-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/cli/trace-page-option.test.ts` around lines 12 - 17, Tests duplicate production pagination/formatting logic (functions pageToOffset and formatFooter) which can drift from the real implementation; refactor by extracting the shared pure helper from the production module (packages/opencode/src/cli/cmd/trace.ts) and import it into the test, or call the real output path via TraceCommand.handler / listTraces so tests exercise actual code paths; specifically remove the local pageToOffset/formatFooter in the test and reference the exported helper(s) from trace.ts (or invoke TraceCommand.handler/listTraces) to compute offsets/footers in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/cli/cmd/trace.ts`:
- Around line 64-66: The hint messages printed by UI.println use page-based
commands that assume page-aligned results and default limits; update the logic
that constructs the suggestion (the block using pagination.total,
pagination.limit, pagination.offset, totalPages and UI.println) to preserve the
active pagination shape: if pagination.offset % pagination.limit === 0, include
both --page <page> AND --limit ${pagination.limit} in the suggested command,
otherwise emit an offset-based suggestion using --offset ${pagination.offset}
--limit ${pagination.limit}; ensure both branches use pagination.limit
explicitly so the suggested commands never rely on implicit defaults.
- Around line 184-187: Compute and use a sanitized limit before calculating
offset so the page->offset math matches the downstream pagination clamp: replace
the current two-step use of const limit = args.limit ?? 20 and offset
calculation with logic that first derives a sanitizedLimit (apply the same
clamping/validation you pass to Trace.listTracesPaginated, e.g., ensure minimum
1 and default 20) and then compute offset = args.page != null ? (Math.max(1,
Math.trunc(args.page)) - 1) * sanitizedLimit : (args.offset ?? 0); use
sanitizedLimit wherever limit is passed to Trace.listTracesPaginated/listing so
offset and limit are consistent.
---
Nitpick comments:
In `@packages/opencode/test/cli/trace-page-option.test.ts`:
- Around line 12-17: Tests duplicate production pagination/formatting logic
(functions pageToOffset and formatFooter) which can drift from the real
implementation; refactor by extracting the shared pure helper from the
production module (packages/opencode/src/cli/cmd/trace.ts) and import it into
the test, or call the real output path via TraceCommand.handler / listTraces so
tests exercise actual code paths; specifically remove the local
pageToOffset/formatFooter in the test and reference the exported helper(s) from
trace.ts (or invoke TraceCommand.handler/listTraces) to compute offsets/footers
in the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f09b3b28-9735-4ee4-b36b-0a8cae98b334
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/trace.tspackages/opencode/test/cli/trace-page-option.test.ts
Comment on lines
+184
to
+187
| const limit = args.limit ?? 20 | ||
| const offset = args.page != null | ||
| ? (Math.max(1, Math.trunc(args.page)) - 1) * limit | ||
| : (args.offset ?? 0) |
There was a problem hiding this comment.
Normalize limit before converting page to offset.
Line 186 multiplies by the raw CLI value, but the comment above says Trace.listTracesPaginated() clamps invalid limits. That makes --page 2 --limit 0 compute offset = 0, then behave like page 1 after the downstream clamp to limit = 1. Compute the offset from the same sanitized limit you pass to pagination.
Suggested fix
- const limit = args.limit ?? 20
+ const limit = Math.max(1, Math.trunc(args.limit ?? 20))
const offset = args.page != null
? (Math.max(1, Math.trunc(args.page)) - 1) * limit
: (args.offset ?? 0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/trace.ts` around lines 184 - 187, Compute and
use a sanitized limit before calculating offset so the page->offset math matches
the downstream pagination clamp: replace the current two-step use of const limit
= args.limit ?? 20 and offset calculation with logic that first derives a
sanitizedLimit (apply the same clamping/validation you pass to
Trace.listTracesPaginated, e.g., ensure minimum 1 and default 20) and then
compute offset = args.page != null ? (Math.max(1, Math.trunc(args.page)) - 1) *
sanitizedLimit : (args.offset ?? 0); use sanitizedLimit wherever limit is passed
to Trace.listTracesPaginated/listing so offset and limit are consistent.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/src/cli/cmd/trace.ts">
<violation number="1" location="packages/opencode/src/cli/cmd/trace.ts:118">
P2: The generated `--page` hint drops the `--limit` value. If the user ran `trace list --page 2 --limit 10`, the suggested next-page command `--page 3` silently reverts to the default limit of 20, changing the result window. Include `--limit ${pagination.limit}` in the hint. Additionally, when the current offset isn't page-aligned (e.g. `--offset 5 --limit 10`), the computed `currentPage` is misleading and the `--page` hint will skip/overlap rows — fall back to an offset-based hint when `pagination.offset % pagination.limit !== 0`.</violation>
<violation number="2" location="packages/opencode/src/cli/cmd/trace.ts:185">
P2: `Math.max(1, NaN)` returns `NaN`, not `1`. If `args.page` is `NaN` (e.g., `--page foo`), the entire offset computation yields `NaN` and is passed to the API. Guard against `NaN` before the arithmetic.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Address PR review feedback: - Guard against NaN in --page arg (e.g. --page foo) using Number.isFinite() - Include --limit in next-page hint so custom limits are preserved - Fall back to offset-based hint when offset isn't page-aligned
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
1 participant
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.
What does this PR do?
Fixes #669 — adds
--page N(-p N) convenience option toaltimate-code trace listso users don't have to manually calculate--offsetvalues for pagination.Before:
altimate-code trace list --offset 20 --limit 20After:
altimate-code trace list --page 2Changes
packages/opencode/src/cli/cmd/trace.ts--page/-poption with page-to-offset conversion (offset = (page - 1) * limit). Updated pagination footer to showpage X/Yand--page Nhintspackages/opencode/test/cli/trace-page-option.test.tsBehavior
--page 1→ offset 0 (first page)--page 2 --limit 10→ offset 10--pagetakes precedence over--offsetwhen both providedShowing 1-20 of 50 trace(s) (page 1/3)andNext page: altimate-code trace list --page 2--offset/--limitusage unchangedScreenshots
Type of change
Issue for this PR
Closes #669
Related: #596 (original pagination infrastructure)
How did you verify your code works?
bun test test/cli/trace-page-option.test.ts)--page 1,--page 2,--page 6(last),--page 99(past end)--helpcorrectly shows the new-p, --pageoptionChecklist
Summary by cubic
Adds a
--page(-p) option toaltimate-code trace listso users can paginate by page number instead of calculating--offset. The footer now showspage X/Y, and hints preserve--limitand choose the right format.--pageconverts to offset ((page - 1) * limit) and overrides--offset.--pagevalues (0, negative, fractional) are clamped/truncated; non-numeric is ignored.page X/Y; next-page hint uses--pagewhen aligned, otherwise--offset, and keeps--limit.--page 1and shows total pages.--offset/--limit.Written for commit 779496a. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests