fix(comments): address code review feedback on pagination
1. Update CLAUDE.md template to document --limit, --offset, --since params and guide agents to use pagination when comments are large 2. Add GetJSONWithHeaders to API client; CLI now prints "Showing X of Y comments" to stderr when paginating 3. Cap --since without --limit at 500 server-side to prevent unbounded result sets
This commit is contained in:
parent
0bbc6bc1c5
commit
c39470a53f
4 changed files with 49 additions and 4 deletions
|
|
@ -556,8 +556,19 @@ func runIssueCommentList(cmd *cobra.Command, args []string) error {
|
|||
}
|
||||
|
||||
var comments []map[string]any
|
||||
if err := client.GetJSON(ctx, path, &comments); err != nil {
|
||||
return fmt.Errorf("list comments: %w", err)
|
||||
isPaginated := len(params) > 0
|
||||
if isPaginated {
|
||||
headers, getErr := client.GetJSONWithHeaders(ctx, path, &comments)
|
||||
if getErr != nil {
|
||||
return fmt.Errorf("list comments: %w", getErr)
|
||||
}
|
||||
if total := headers.Get("X-Total-Count"); total != "" {
|
||||
fmt.Fprintf(os.Stderr, "Showing %d of %s comments.\n", len(comments), total)
|
||||
}
|
||||
} else {
|
||||
if err := client.GetJSON(ctx, path, &comments); err != nil {
|
||||
return fmt.Errorf("list comments: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
output, _ := cmd.Flags().GetString("output")
|
||||
|
|
|
|||
|
|
@ -77,6 +77,34 @@ func (c *APIClient) GetJSON(ctx context.Context, path string, out any) error {
|
|||
return json.NewDecoder(resp.Body).Decode(out)
|
||||
}
|
||||
|
||||
// GetJSONWithHeaders performs a GET request, decodes the JSON response, and
|
||||
// returns the response headers. Useful when callers need header values like
|
||||
// X-Total-Count for pagination.
|
||||
func (c *APIClient) GetJSONWithHeaders(ctx context.Context, path string, out any) (http.Header, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.BaseURL+path, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
c.setHeaders(req)
|
||||
|
||||
resp, err := c.HTTPClient.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode >= 400 {
|
||||
data, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
|
||||
return nil, fmt.Errorf("GET %s returned %d: %s", path, resp.StatusCode, strings.TrimSpace(string(data)))
|
||||
}
|
||||
if out != nil {
|
||||
if err := json.NewDecoder(resp.Body).Decode(out); err != nil {
|
||||
return resp.Header, err
|
||||
}
|
||||
}
|
||||
return resp.Header, nil
|
||||
}
|
||||
|
||||
// DeleteJSON performs a DELETE request.
|
||||
func (c *APIClient) DeleteJSON(ctx context.Context, path string) error {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodDelete, c.BaseURL+path, nil)
|
||||
|
|
|
|||
|
|
@ -47,7 +47,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
|
|||
b.WriteString("### Read\n")
|
||||
b.WriteString("- `multica issue get <id> --output json` — Get full issue details (title, description, status, priority, assignee)\n")
|
||||
b.WriteString("- `multica issue list [--status X] [--priority X] [--assignee X] --output json` — List issues in workspace\n")
|
||||
b.WriteString("- `multica issue comment list <issue-id> --output json` — List all comments on an issue (includes id, parent_id for threading)\n")
|
||||
b.WriteString("- `multica issue comment list <issue-id> [--limit N] [--offset N] [--since <RFC3339>] --output json` — List comments on an issue (supports pagination; includes id, parent_id for threading)\n")
|
||||
b.WriteString("- `multica workspace get --output json` — Get workspace details and context\n")
|
||||
b.WriteString("- `multica agent list --output json` — List agents in workspace\n")
|
||||
b.WriteString("- `multica issue runs <issue-id> --output json` — List all execution runs for an issue (status, timestamps, errors)\n")
|
||||
|
|
@ -83,6 +83,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
|
|||
b.WriteString("**This task was triggered by a comment.** Your primary job is to respond.\n\n")
|
||||
fmt.Fprintf(&b, "1. Run `multica issue get %s --output json` to understand the issue context\n", ctx.IssueID)
|
||||
fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the conversation\n", ctx.IssueID)
|
||||
b.WriteString(" - If the output is very large or truncated, use pagination: `--limit 30` to get the latest 30 comments, or `--since <timestamp>` to fetch only recent ones\n")
|
||||
fmt.Fprintf(&b, "3. Find the triggering comment (ID: `%s`) and understand what is being asked\n", ctx.TriggerCommentID)
|
||||
fmt.Fprintf(&b, "4. Reply: `multica issue comment add %s --parent %s --content \"...\"`\n", ctx.IssueID, ctx.TriggerCommentID)
|
||||
b.WriteString("5. If the comment requests code changes or further work, do the work first, then reply with your results\n")
|
||||
|
|
|
|||
|
|
@ -109,11 +109,16 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
|
|||
Offset: offset,
|
||||
})
|
||||
case sinceTime.Valid:
|
||||
comments, err = h.Queries.ListCommentsSince(r.Context(), db.ListCommentsSinceParams{
|
||||
// Apply a server-side cap to prevent unbounded result sets when
|
||||
// --since is used without --limit.
|
||||
comments, err = h.Queries.ListCommentsSincePaginated(r.Context(), db.ListCommentsSincePaginatedParams{
|
||||
IssueID: issue.ID,
|
||||
WorkspaceID: issue.WorkspaceID,
|
||||
CreatedAt: sinceTime,
|
||||
Limit: 500,
|
||||
Offset: 0,
|
||||
})
|
||||
hasPagination = true
|
||||
case hasPagination:
|
||||
if limit == 0 {
|
||||
limit = 50
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue