Skip to content

perf(api): cache and opt in to comment reaction reads #366

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

Merged
stefan-ernst merged 1 commit into from
Oct 21, 2026
+220 −24
Merged

perf(api): cache and opt in to comment reaction reads #366

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
36 changes: 35 additions & 1 deletion api/openapi-v2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65625,6 +65625,10 @@
"PostCommentsBatchPayload": {
"additionalProperties": false,
"properties": {
"include_reactions": {
"description": "The include reactions value.",
"type": "boolean"
},
"item_ids": {
"description": "The item ids value.",
"items": {
Expand Down Expand Up @@ -103802,6 +103806,16 @@
"minimum": 1,
"type": "integer"
}
},
{
"description": "Attach grouped emoji reactions to the returned comment. Values true and 1 enable; omitted or other values leave reactions out.",
"in": "query",
"name": "include_reactions",
"required": false,
"schema": {
"default": false,
"type": "boolean"
}
}
],
"responses": {
Expand Down Expand Up @@ -103857,6 +103871,16 @@
"minimum": 1,
"type": "integer"
}
},
{
"description": "Attach grouped emoji reactions to the returned comment. Values true and 1 enable; omitted or other values leave reactions out.",
"in": "query",
"name": "include_reactions",
"required": false,
"schema": {
"default": false,
"type": "boolean"
}
}
],
"requestBody": {
Expand Down Expand Up @@ -108121,7 +108145,7 @@
},
"/items/{item_id}/comments": {
"get": {
"description": "Returns an authorization-filtered comment feed with at most 200 entries. Opaque next_cursor and refresh_cursor values preserve the exclusive timestamp-and-ID boundary without exposing its representation; partial comment pages are never returned on failure.",
"description": "Returns an authorization-filtered comment feed with at most 200 entries. Opaque next_cursor and refresh_cursor values preserve the exclusive timestamp-and-ID boundary without exposing its representation; partial comment pages are never returned on failure. Grouped reactions are attached only when include_reactions is true.",
"operationId": "getItemsItemIdComments",
"parameters": [
{
Expand Down Expand Up @@ -108155,6 +108179,16 @@
"minLength": 1,
"type": "string"
}
},
{
"description": "Attach grouped emoji reactions to each comment. Values true and 1 enable; omitted or other values leave reactions out.",
"in": "query",
"name": "include_reactions",
"required": false,
"schema": {
"default": false,
"type": "boolean"
}
}
],
"responses": {
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/lib/api/misc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ export const getComments = (itemId, params = {}) => {
const searchParams = new URLSearchParams();
if (params.limit) searchParams.set('page_size', params.limit);
if (params.cursor) searchParams.set('cursor', params.cursor);
// Reactions are opt-in so feeds that do not render chips skip the extra read.
if (params.include_reactions) searchParams.set('include_reactions', 'true');
const query = searchParams.toString();
return fetchV2Data(`/items/${itemId}/comments${query ? `?${query}` : ''}`);
};
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/lib/features/items/Comments.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
const version = ++reconciliationVersion;
let response;
try {
response = await loadAttributedComments(itemId, { limit: COMMENT_PAGE_SIZE });
response = await loadAttributedComments(itemId, { limit: COMMENT_PAGE_SIZE, include_reactions: true });
} catch (err) {
if (version !== reconciliationVersion) return;
// Navigation can unload the document while this request is in flight.
Expand Down Expand Up @@ -203,6 +203,7 @@
try {
response = await loadAttributedComments(itemId, {
limit: COMMENT_PAGE_SIZE,
include_reactions: true,
...(refreshCursor ? { cursor: refreshCursor } : {})
});
} catch (err) {
Expand Down Expand Up @@ -256,6 +257,7 @@
const isFirstPage = firstResponse === null;
const response = await loadAttributedComments(itemId, {
limit: Math.min(100, targetCount - fresh.length),
include_reactions: true,
...(cursor ? { cursor } : {})
});
if (version !== reconciliationVersion) return;
Expand Down Expand Up @@ -313,7 +315,8 @@
try {
const response = await loadAttributedComments(itemId, {
limit: COMMENT_PAGE_SIZE,
cursor: olderCursor
cursor: olderCursor,
include_reactions: true
});
comments = mergeCommentRows(comments, response?.comments || []);
hasMore = Boolean(response?.has_more);
Expand Down
35 changes: 25 additions & 10 deletions internal/restapi/v2/comments.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ type commentCreateRequest struct {
const maxCommentBatchItems = 500

type commentBatchRequest struct {
ItemIDs []int `json:"item_ids"`
PageSize int `json:"page_size"`
ItemIDs []int `json:"item_ids"`
PageSize int `json:"page_size"`
IncludeReactions bool `json:"include_reactions,omitempty"`
}

type commentBatchEntry struct {
Expand Down Expand Up @@ -115,8 +116,10 @@ func commentsBatch(deps Deps) jsonOperation[commentBatchRequest, []commentBatchE
if err != nil {
return nil, internalError(err)
}
if err := attachBatchReactions(deps, pages); err != nil {
return nil, internalError(err)
if input.IncludeReactions {
if err := attachBatchReactions(deps, pages); err != nil {
return nil, internalError(err)
}
}
entries := make([]commentBatchEntry, 0, len(visible))
for _, itemID := range visible {
Expand Down Expand Up @@ -199,8 +202,12 @@ func listComments(deps Deps) readOperation[commentFeedDTO] {
if err := renderCommentHTML(page.Comments); err != nil {
return commentFeedDTO{}, internalError(err)
}
if err := deps.Comments.AttachReactions(page.Comments); err != nil {
return commentFeedDTO{}, internalError(err)
if includeReactions, err := optionalBoolQuery(r, "include_reactions"); err != nil {
return commentFeedDTO{}, err
} else if includeReactions {
if err := deps.Comments.AttachReactions(page.Comments); err != nil {
return commentFeedDTO{}, internalError(err)
}
}
var total *int
if options.Before == nil && options.Since == nil {
Expand Down Expand Up @@ -257,8 +264,12 @@ func getComment(deps Deps) readOperation[models.Comment] {
return models.Comment{}, err
}
body := []models.Comment{comment.Comment}
if err := deps.Comments.AttachReactions(body); err != nil {
return models.Comment{}, internalError(err)
if includeReactions, err := optionalBoolQuery(r, "include_reactions"); err != nil {
return models.Comment{}, err
} else if includeReactions {
if err := deps.Comments.AttachReactions(body); err != nil {
return models.Comment{}, internalError(err)
}
}
body[0].ContentHTML, err = markdown.Render(body[0].Content)
return body[0], commentError(err)
Expand Down Expand Up @@ -286,8 +297,12 @@ func updateComment(deps Deps) jsonOperation[commentPatchRequest, models.Comment]
return models.Comment{}, commentError(err)
}
body := []models.Comment{*updated}
if err := deps.Comments.AttachReactions(body); err != nil {
return models.Comment{}, internalError(err)
if includeReactions, err := optionalBoolQuery(r, "include_reactions"); err != nil {
return models.Comment{}, err
} else if includeReactions {
if err := deps.Comments.AttachReactions(body); err != nil {
return models.Comment{}, internalError(err)
}
}
body[0].ContentHTML, err = markdown.Render(body[0].Content)
return body[0], commentError(err)
Expand Down
32 changes: 31 additions & 1 deletion internal/restapi/v2/contract-metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3788,6 +3788,16 @@
"minimum": 1,
"type": "integer"
}
},
{
"Name": "include_reactions",
"In": "query",
"Description": "Attach grouped emoji reactions to the returned comment. Values true and 1 enable; omitted or other values leave reactions out.",
"Required": false,
"Schema": {
"default": false,
"type": "boolean"
}
}
],
"responses": {
Expand All @@ -3814,6 +3824,16 @@
"minimum": 1,
"type": "integer"
}
},
{
"Name": "include_reactions",
"In": "query",
"Description": "Attach grouped emoji reactions to the returned comment. Values true and 1 enable; omitted or other values leave reactions out.",
"Required": false,
"Schema": {
"default": false,
"type": "boolean"
}
}
],
"responses": {
Expand Down Expand Up @@ -5967,7 +5987,7 @@
},
"/items/{item_id}/comments": {
"get": {
"description": "Returns an authorization-filtered comment feed with at most 200 entries. Opaque next_cursor and refresh_cursor values preserve the exclusive timestamp-and-ID boundary without exposing its representation; partial comment pages are never returned on failure.",
"description": "Returns an authorization-filtered comment feed with at most 200 entries. Opaque next_cursor and refresh_cursor values preserve the exclusive timestamp-and-ID boundary without exposing its representation; partial comment pages are never returned on failure. Grouped reactions are attached only when include_reactions is true.",
"parameters": [
{
"Name": "item_id",
Expand Down Expand Up @@ -6000,6 +6020,16 @@
"minLength": 1,
"type": "string"
}
},
{
"Name": "include_reactions",
"In": "query",
"Description": "Attach grouped emoji reactions to each comment. Values true and 1 enable; omitted or other values leave reactions out.",
"Required": false,
"Schema": {
"default": false,
"type": "boolean"
}
}
],
"responses": {
Expand Down
7 changes: 6 additions & 1 deletion internal/restapi/v2/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,11 @@ func applyParameterCorrections(route *Route) {
defaultSize = 200
}
upsertParameter(route, integerQuery("page_size", "Maximum number of resources to return.", 200, defaultSize))
if route.Path == "/items/{item_id}/comments" {
upsertParameter(route, booleanQuery("include_reactions", "Attach grouped emoji reactions to each comment. Values true and 1 enable; omitted or other values leave reactions out.", false))
}
case "GET /comments/{comment_id}", "PATCH /comments/{comment_id}":
upsertParameter(route, booleanQuery("include_reactions", "Attach grouped emoji reactions to the returned comment. Values true and 1 enable; omitted or other values leave reactions out.", false))
case "POST /items/batch", "POST /assets/summaries", "POST /milestones/test-statistics", "POST /iterations/progress":
removeQueryParameter(route, "ids")
}
Expand Down Expand Up @@ -1203,7 +1208,7 @@ func applyParameterCorrections(route *Route) {
case "GET /links/batch":
route.Description = "Returns one-hop links for at most 100 explicit item IDs, or for one paged query-language selection. Explicit IDs retain first-occurrence order. Each item is capped independently and exposes an operation-bound next_cursor when more links exist; inaccessible links are omitted."
case "GET /items/{item_id}/comments":
route.Description = "Returns an authorization-filtered comment feed with at most 200 entries. Opaque next_cursor and refresh_cursor values preserve the exclusive timestamp-and-ID boundary without exposing its representation; partial comment pages are never returned on failure."
route.Description = "Returns an authorization-filtered comment feed with at most 200 entries. Opaque next_cursor and refresh_cursor values preserve the exclusive timestamp-and-ID boundary without exposing its representation; partial comment pages are never returned on failure. Grouped reactions are attached only when include_reactions is true."
case "GET /agent-runs/{run_id}/events":
route.Description = "Returns up to 200 visible run events after an operation-bound opaque cursor. Events use stable ascending ID order, next_cursor resumes exclusively after the final returned event, and partial pages are never returned on failure."
case "GET /workspaces/{workspace_id}/agent-runs", "GET /items/{item_id}/agent-runs":
Expand Down
Loading
Loading