Skip to content

Commit 2397b0a

Browse files
committed
default get_change_comments to unresolved threads
BREAKING: a bare get_change_comments call now returns unresolved threads only; pass status=all for the previous behavior. The full comment history of a long review can dwarf the context it lands in -- a change with a hundred-plus comments overflows the client's tool-result budget and forces a re-read. The actionable subset is the unresolved discussion, so that is what a bare call returns; status=all and status=resolved stay available unchanged, and the tool description steers the model to reach for the history only when the settled context matters. Notification payloads are unaffected: their thread selection is by new comment, not by resolution state.
1 parent 218b220 commit 2397b0a

3 files changed

Lines changed: 32 additions & 14 deletions

File tree

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,8 @@ and enabled groups union.
177177
- `get_change` — one change in review-relevant detail: status, owner, labels with votes, current revision, messages.
178178
- `list_change_files` — files touched by a revision, with per-file change stats.
179179
- `get_file_diff` — the diff of one file in a revision.
180-
- `get_change_comments` — comment threads on a change, with resolution state and comment ids.
180+
- `get_change_comments` — comment threads on a change, with resolution state and comment ids. Returns unresolved
181+
threads only by default; `status=all` fetches the full history, `status=resolved` the settled threads.
181182
- `post_comments` — publish a review in one call: optional top-level message plus inline, range, file-level, and
182183
reply comments; replies anchor to comment ids from `get_change_comments`; `resolved` toggles the thread state.
183184
- `set_vote` — set a label vote (e.g. `Code-Review`) with an optional message; value `0` clears an own vote.

internal/tools/get-change-comments.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ var errInvalidStatus = e.New("invalid status filter, expected all, resolved, or
2626

2727
type getChangeCommentsInput struct {
2828
Change string `json:"change" jsonschema:"Change identifier: change number (123), project~number (myproject~123), or Change-Id (I8473b95...)"`
29-
Status string `json:"status,omitempty" jsonschema:"Thread filter: all, resolved, or unresolved; default all"`
29+
Status string `json:"status,omitempty" jsonschema:"Thread filter: all, resolved, or unresolved; default unresolved"`
3030
}
3131

3232
func getChangeComments(c *gerritclient.Client) Tool {
@@ -38,16 +38,22 @@ func getChangeComments(c *gerritclient.Client) Tool {
3838
Description: "List a Gerrit change's inline review comments — the code discussion " +
3939
"anchored to files and lines, distinct from the change messages get_change " +
4040
"shows — grouped by file and reconstructed into threads with their resolved " +
41-
"state. The calling account's unpublished draft comments are included, marked " +
42-
"draft=\"true\" and invisible to anyone else; thread state accounts for them, " +
43-
"matching what this account's Gerrit UI shows. Unresolved threads render " +
44-
"before resolved ones; threads follow their latest activity and comments " +
45-
"their history. Comment ids are the reply anchors for post_comments; filter " +
46-
"status=unresolved to see what still needs action.",
41+
"state. By default only unresolved threads return: the discussion still " +
42+
"needing action. status=all adds the settled history — on long reviews that " +
43+
"can be very large, so reach for it only when the resolved context matters; " +
44+
"status=resolved lists the settled threads alone. The calling account's " +
45+
"unpublished draft comments are included, marked draft=\"true\" and invisible " +
46+
"to anyone else; thread state accounts for them, matching what this account's " +
47+
"Gerrit UI shows. Unresolved threads render before resolved ones; threads " +
48+
"follow their latest activity and comments their history. Comment ids are the " +
49+
"reply anchors for post_comments.",
4750
}, func(ctx context.Context, _ *mcp.CallToolRequest, in getChangeCommentsInput,
4851
) (*mcp.CallToolResult, any, error) {
52+
// Unresolved is the default: it is the actionable subset, and
53+
// the full history of a long review can dwarf the context it
54+
// lands in.
4955
if in.Status == "" {
50-
in.Status = statusAll
56+
in.Status = statusUnresolved
5157
}
5258

5359
if in.Status != statusAll && in.Status != statusResolved && in.Status != statusUnresolved {

internal/tools/read-tools_test.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,22 @@ func Test_GetChangeComments(t *testing.T) {
160160

161161
cs := commentsSession(t, fixture, emptyDraftsJSON)
162162

163-
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"})
163+
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"})
164164

165165
golden(t, "change-comments-all", out)
166166
})
167167

168+
t.Run("default returns only unresolved threads", func(t *testing.T) {
169+
t.Parallel()
170+
171+
cs := commentsSession(t, fixture, emptyDraftsJSON)
172+
173+
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"})
174+
175+
// Same golden as the explicit filter: the default IS unresolved.
176+
golden(t, "change-comments-unresolved", out)
177+
})
178+
168179
t.Run("unresolved filter drops resolved threads", func(t *testing.T) {
169180
t.Parallel()
170181

@@ -241,7 +252,7 @@ func Test_GetChangeComments(t *testing.T) {
241252

242253
cs := commentsSession(t, published, emptyDraftsJSON)
243254

244-
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"})
255+
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"})
245256

246257
golden(t, "change-comments-renamed", out)
247258
})
@@ -273,7 +284,7 @@ func Test_GetChangeComments(t *testing.T) {
273284

274285
cs := commentsSession(t, published, emptyDraftsJSON)
275286

276-
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"})
287+
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"})
277288

278289
golden(t, "change-comments-ui-fork", out)
279290
})
@@ -301,7 +312,7 @@ func Test_GetChangeComments(t *testing.T) {
301312

302313
cs := commentsSession(t, published, emptyDraftsJSON)
303314

304-
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"})
315+
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"})
305316

306317
golden(t, "change-comments-latest-order", out)
307318
})
@@ -329,7 +340,7 @@ func Test_GetChangeComments(t *testing.T) {
329340

330341
cs := commentsSession(t, published, emptyDraftsJSON)
331342

332-
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"})
343+
out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"})
333344

334345
golden(t, "change-comments-tiebreak", out)
335346
})

0 commit comments

Comments
 (0)