diff --git a/README.md b/README.md index 0ae066e..d38ff69 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,8 @@ and enabled groups union. - `get_change` — one change in review-relevant detail: status, owner, labels with votes, current revision, messages. - `list_change_files` — files touched by a revision, with per-file change stats. - `get_file_diff` — the diff of one file in a revision. -- `get_change_comments` — comment threads on a change, with resolution state and comment ids. +- `get_change_comments` — comment threads on a change, with resolution state and comment ids. Returns unresolved + threads only by default; `status=all` fetches the full history, `status=resolved` the settled threads. - `post_comments` — publish a review in one call: optional top-level message plus inline, range, file-level, and reply comments; replies anchor to comment ids from `get_change_comments`; `resolved` toggles the thread state. - `set_vote` — set a label vote (e.g. `Code-Review`) with an optional message; value `0` clears an own vote. diff --git a/internal/tools/get-change-comments.go b/internal/tools/get-change-comments.go index 539d975..1088110 100644 --- a/internal/tools/get-change-comments.go +++ b/internal/tools/get-change-comments.go @@ -26,7 +26,7 @@ var errInvalidStatus = e.New("invalid status filter, expected all, resolved, or type getChangeCommentsInput struct { Change string `json:"change" jsonschema:"Change identifier: change number (123), project~number (myproject~123), or Change-Id (I8473b95...)"` - Status string `json:"status,omitempty" jsonschema:"Thread filter: all, resolved, or unresolved; default all"` + Status string `json:"status,omitempty" jsonschema:"Thread filter: all, resolved, or unresolved; default unresolved"` } func getChangeComments(c *gerritclient.Client) Tool { @@ -38,16 +38,22 @@ func getChangeComments(c *gerritclient.Client) Tool { Description: "List a Gerrit change's inline review comments — the code discussion " + "anchored to files and lines, distinct from the change messages get_change " + "shows — grouped by file and reconstructed into threads with their resolved " + - "state. The calling account's unpublished draft comments are included, marked " + - "draft=\"true\" and invisible to anyone else; thread state accounts for them, " + - "matching what this account's Gerrit UI shows. Unresolved threads render " + - "before resolved ones; threads follow their latest activity and comments " + - "their history. Comment ids are the reply anchors for post_comments; filter " + - "status=unresolved to see what still needs action.", + "state. By default only unresolved threads return: the discussion still " + + "needing action. status=all adds the settled history — on long reviews that " + + "can be very large, so reach for it only when the resolved context matters; " + + "status=resolved lists the settled threads alone. The calling account's " + + "unpublished draft comments are included, marked draft=\"true\" and invisible " + + "to anyone else; thread state accounts for them, matching what this account's " + + "Gerrit UI shows. Unresolved threads render before resolved ones; threads " + + "follow their latest activity and comments their history. Comment ids are the " + + "reply anchors for post_comments.", }, func(ctx context.Context, _ *mcp.CallToolRequest, in getChangeCommentsInput, ) (*mcp.CallToolResult, any, error) { + // Unresolved is the default: it is the actionable subset, and + // the full history of a long review can dwarf the context it + // lands in. if in.Status == "" { - in.Status = statusAll + in.Status = statusUnresolved } if in.Status != statusAll && in.Status != statusResolved && in.Status != statusUnresolved { diff --git a/internal/tools/read-tools_test.go b/internal/tools/read-tools_test.go index 8547d58..6c78fbb 100644 --- a/internal/tools/read-tools_test.go +++ b/internal/tools/read-tools_test.go @@ -160,11 +160,22 @@ func Test_GetChangeComments(t *testing.T) { cs := commentsSession(t, fixture, emptyDraftsJSON) - out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"}) + out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"}) golden(t, "change-comments-all", out) }) + t.Run("default returns only unresolved threads", func(t *testing.T) { + t.Parallel() + + cs := commentsSession(t, fixture, emptyDraftsJSON) + + out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"}) + + // Same golden as the explicit filter: the default IS unresolved. + golden(t, "change-comments-unresolved", out) + }) + t.Run("unresolved filter drops resolved threads", func(t *testing.T) { t.Parallel() @@ -241,7 +252,7 @@ func Test_GetChangeComments(t *testing.T) { cs := commentsSession(t, published, emptyDraftsJSON) - out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"}) + out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"}) golden(t, "change-comments-renamed", out) }) @@ -273,7 +284,7 @@ func Test_GetChangeComments(t *testing.T) { cs := commentsSession(t, published, emptyDraftsJSON) - out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"}) + out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"}) golden(t, "change-comments-ui-fork", out) }) @@ -301,7 +312,7 @@ func Test_GetChangeComments(t *testing.T) { cs := commentsSession(t, published, emptyDraftsJSON) - out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"}) + out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"}) golden(t, "change-comments-latest-order", out) }) @@ -329,7 +340,7 @@ func Test_GetChangeComments(t *testing.T) { cs := commentsSession(t, published, emptyDraftsJSON) - out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123"}) + out := callTool(t, cs, "get_change_comments", map[string]any{"change": "123", "status": "all"}) golden(t, "change-comments-tiebreak", out) })