Skip to content

Commit c069ea1

Browse files
DeRaowlclaude
andauthored
fix(percy): full build-items pagination and diff precision (#351)
- percy_get_build detail=snapshots/changes fetched only the first page (page[limit]=30, cursor never followed) and reported partial counts as complete. Now fetches the full set and follows meta.pagination.next_cursor with stall/page-cap guards, surfacing a warning if truncated. - detail=changes returned 0 items: the API resolves filter[category]=changed through filter[subcategories][]; now sent (unreviewed, changes_requested, approved). Same fix in percy_search_builds. - Diff ratios were rounded with toFixed(1), hiding small diffs (0.02% -> 0.0%). New formatDiffPercent shows two decimals with a <0.01% floor, applied across v2 tools and v1 workflows. - percyGet now supports repeated array params (filter[...][]), also fixing percy_search_builds dropping all but the last browser_ids/widths value. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 6fd2c2e commit c069ea1

9 files changed

Lines changed: 149 additions & 52 deletions

File tree

src/lib/percy-api/build-items.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* Shared helpers for the /build-items endpoint.
3+
*
4+
* Pagination: the API returns everything when page[limit] is omitted, but may
5+
* still paginate (meta.pagination.has_more + next_cursor) on very large
6+
* builds or if server-side defaults change. fetchAllBuildItems handles both:
7+
* it starts with an unbounded request and follows next_cursor if the response
8+
* is paginated anyway.
9+
*/
10+
11+
import { percyGet } from "./percy-auth.js";
12+
import { BrowserStackConfig } from "../types.js";
13+
14+
const MAX_PAGES = 50;
15+
16+
export interface BuildItemsResult {
17+
items: any[];
18+
/** True if pagination could not be exhausted (page cap or cursor stall). */
19+
truncated: boolean;
20+
}
21+
22+
/**
23+
* Fetch ALL build items for the given filters, following
24+
* meta.pagination.next_cursor until has_more is false.
25+
*/
26+
export async function fetchAllBuildItems(
27+
config: BrowserStackConfig,
28+
baseParams: Record<string, string | string[]>,
29+
): Promise<BuildItemsResult> {
30+
const items: any[] = [];
31+
let cursor: string | undefined;
32+
let pages = 0;
33+
34+
for (;;) {
35+
const params: Record<string, string | string[]> = { ...baseParams };
36+
if (cursor) params["page[cursor]"] = cursor;
37+
38+
const response = await percyGet("/build-items", config, params);
39+
items.push(...(response?.data || []));
40+
pages += 1;
41+
42+
const pagination = response?.meta?.pagination;
43+
const nextCursor = pagination?.next_cursor;
44+
if (!pagination?.has_more || !nextCursor) {
45+
return { items, truncated: false };
46+
}
47+
// Guard against a stalled cursor or runaway loop.
48+
if (nextCursor === cursor || pages >= MAX_PAGES) {
49+
return { items, truncated: true };
50+
}
51+
cursor = String(nextCursor);
52+
}
53+
}
54+
55+
/**
56+
* Filter params that make filter[category]=changed actually return results.
57+
* The API only maps the changed category to review states via
58+
* filter[subcategories][]; without it the scope resolves to zero rows.
59+
*/
60+
export const CHANGED_CATEGORY_PARAMS: Record<string, string | string[]> = {
61+
"filter[category]": "changed",
62+
"filter[subcategories][]": ["unreviewed", "changes_requested", "approved"],
63+
};
64+
65+
/**
66+
* Format a 0..1 diff ratio as a percentage without hiding small diffs:
67+
* 0 → "0%", tiny → "<0.01%", otherwise two decimals (e.g. "0.02%").
68+
*/
69+
export function formatDiffPercent(ratio: number | null | undefined): string {
70+
if (ratio == null) return "—";
71+
const pct = ratio * 100;
72+
if (pct === 0) return "0%";
73+
if (pct < 0.01) return "<0.01%";
74+
return pct.toFixed(2) + "%";
75+
}

src/lib/percy-api/percy-auth.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,18 @@ const PERCY_API_BASE = "https://percy.io/api/v1";
4949
export async function percyGet(
5050
path: string,
5151
config: BrowserStackConfig,
52-
params?: Record<string, string>,
52+
params?: Record<string, string | string[]>,
5353
): Promise<any> {
5454
const headers = getPercyAuthHeaders(config);
5555
const url = new URL(`${PERCY_API_BASE}${path}`);
5656

5757
if (params) {
5858
for (const [key, value] of Object.entries(params)) {
59-
url.searchParams.set(key, value);
59+
if (Array.isArray(value)) {
60+
value.forEach((v) => url.searchParams.append(key, v));
61+
} else {
62+
url.searchParams.set(key, value);
63+
}
6064
}
6165
}
6266

src/tools/percy-mcp/v2/get-build-detail.ts

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
*/
1313

1414
import { percyGet, percyPost } from "../../../lib/percy-api/percy-auth.js";
15+
import {
16+
fetchAllBuildItems,
17+
formatDiffPercent,
18+
CHANGED_CATEGORY_PARAMS,
19+
} from "../../../lib/percy-api/build-items.js";
1520
import { BrowserStackConfig } from "../../../lib/types.js";
1621
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
1722
import { setActiveBuild } from "../../../lib/percy-api/percy-session.js";
@@ -320,14 +325,11 @@ async function getChanges(
320325
buildId: string,
321326
config: BrowserStackConfig,
322327
): Promise<CallToolResult> {
323-
const response = await percyGet("/build-items", config, {
328+
const { items, truncated } = await fetchAllBuildItems(config, {
324329
"filter[build-id]": buildId,
325-
"filter[category]": "changed",
326-
"page[limit]": "30",
330+
...CHANGED_CATEGORY_PARAMS,
327331
});
328332

329-
const items = response?.data || [];
330-
331333
if (!items.length) {
332334
return {
333335
content: [
@@ -347,17 +349,17 @@ async function getChanges(
347349
const name = a["cover-snapshot-name"] || a.coverSnapshotName || "?";
348350
const displayName =
349351
a["cover-snapshot-display-name"] || a.coverSnapshotDisplayName || "";
350-
const diff =
351-
(a["max-diff-ratio"] ?? a.maxDiffRatio) != null
352-
? ((a["max-diff-ratio"] ?? a.maxDiffRatio) * 100).toFixed(1) + "%"
353-
: "—";
352+
const diff = formatDiffPercent(a["max-diff-ratio"] ?? a.maxDiffRatio);
354353
const bugs =
355354
a["max-bug-total-potential-bugs"] ?? a.maxBugTotalPotentialBugs ?? 0;
356355
const review = a["review-state"] || a.reviewState || "?";
357356
const count = a["item-count"] || a.itemCount || 1;
358357
output += `| ${i + 1} | ${name} | ${displayName || "—"} | ${diff} | ${bugs} | ${review} | ${count} |\n`;
359358
});
360359

360+
if (truncated) {
361+
output += `\n⚠️ Result may be incomplete — pagination could not be fully exhausted.\n`;
362+
}
361363
output += `\nUse \`percy_get_snapshot\` with a snapshot ID from above for full details.\n`;
362364

363365
return { content: [{ type: "text", text: output }] };
@@ -637,13 +639,10 @@ async function getSnapshots(
637639
buildId: string,
638640
config: BrowserStackConfig,
639641
): Promise<CallToolResult> {
640-
const response = await percyGet("/build-items", config, {
642+
const { items, truncated } = await fetchAllBuildItems(config, {
641643
"filter[build-id]": buildId,
642-
"page[limit]": "30",
643644
});
644645

645-
const items = response?.data || [];
646-
647646
if (!items.length) {
648647
return {
649648
content: [
@@ -670,10 +669,7 @@ async function getSnapshots(
670669
const name = a["cover-snapshot-name"] || a.coverSnapshotName || "?";
671670
const display =
672671
a["cover-snapshot-display-name"] || a.coverSnapshotDisplayName || "—";
673-
const diff =
674-
(a["max-diff-ratio"] ?? a.maxDiffRatio) != null
675-
? ((a["max-diff-ratio"] ?? a.maxDiffRatio) * 100).toFixed(1) + "%"
676-
: "—";
672+
const diff = formatDiffPercent(a["max-diff-ratio"] ?? a.maxDiffRatio);
677673
const bugs =
678674
a["max-bug-total-potential-bugs"] ?? a.maxBugTotalPotentialBugs ?? "—";
679675
const review = a["review-state"] || a.reviewState || "?";
@@ -686,6 +682,9 @@ async function getSnapshots(
686682
output += `| ${i + 1} | ${name} | ${display} | ${diff} | ${bugs} | ${review} | ${count} | ${snapIds}${more} |\n`;
687683
});
688684

685+
if (truncated) {
686+
output += `\n⚠️ Result may be incomplete — pagination could not be fully exhausted.\n`;
687+
}
689688
output += `\nUse \`percy_get_snapshot\` with a snapshot ID for full comparison details.\n`;
690689

691690
return { content: [{ type: "text", text: output }] };

src/tools/percy-mcp/v2/get-comparison.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { percyGet } from "../../../lib/percy-api/percy-auth.js";
2+
import { formatDiffPercent } from "../../../lib/percy-api/build-items.js";
23
import { BrowserStackConfig } from "../../../lib/types.js";
34
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
45

@@ -43,8 +44,8 @@ export async function percyGetComparison(
4344
output += `| **Browser** | ${browserName} |\n`;
4445
output += `| **Width** | ${attrs.width || "?"}px |\n`;
4546
output += `| **State** | ${attrs.state || "?"} |\n`;
46-
output += `| **Diff ratio** | ${attrs["diff-ratio"] != null ? (attrs["diff-ratio"] * 100).toFixed(2) + "%" : "—"} |\n`;
47-
output += `| **AI diff ratio** | ${attrs["ai-diff-ratio"] != null ? (attrs["ai-diff-ratio"] * 100).toFixed(2) + "%" : "—"} |\n`;
47+
output += `| **Diff ratio** | ${formatDiffPercent(attrs["diff-ratio"])} |\n`;
48+
output += `| **AI diff ratio** | ${formatDiffPercent(attrs["ai-diff-ratio"])} |\n`;
4849
output += `| **AI state** | ${attrs["ai-processing-state"] || "—"} |\n`;
4950
output += `| **Potential bugs** | ${ai["total-potential-bugs"] ?? "—"} |\n`;
5051
output += `| **AI visual diffs** | ${ai["total-ai-visual-diffs"] ?? "—"} |\n`;

src/tools/percy-mcp/v2/get-snapshot.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { percyGet } from "../../../lib/percy-api/percy-auth.js";
2+
import { formatDiffPercent } from "../../../lib/percy-api/build-items.js";
23
import { BrowserStackConfig } from "../../../lib/types.js";
34
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
45

@@ -29,7 +30,7 @@ export async function percyGetSnapshot(
2930

3031
output += `| Field | Value |\n|---|---|\n`;
3132
output += `| **Review** | ${attrs["review-state"] || "—"} (${attrs["review-state-reason"] || "—"}) |\n`;
32-
output += `| **Diff ratio** | ${attrs["diff-ratio"] != null ? (attrs["diff-ratio"] * 100).toFixed(2) + "%" : "—"} |\n`;
33+
output += `| **Diff ratio** | ${formatDiffPercent(attrs["diff-ratio"])} |\n`;
3334
output += `| **Test case** | ${attrs["test-case-name"] || "none"} |\n`;
3435
output += `| **Comments** | ${attrs["total-open-comments"] ?? 0} |\n`;
3536
output += `| **Layout** | ${attrs["enable-layout"] ? "enabled" : "disabled"} |\n`;
@@ -65,14 +66,8 @@ export async function percyGetSnapshot(
6566
const ca = c.attributes || {};
6667
const browserId = c.relationships?.browser?.data?.id;
6768
const browserName = browsers.get(browserId) || "?";
68-
const diff =
69-
ca["diff-ratio"] != null
70-
? (ca["diff-ratio"] * 100).toFixed(1) + "%"
71-
: "—";
72-
const aiDiff =
73-
ca["ai-diff-ratio"] != null
74-
? (ca["ai-diff-ratio"] * 100).toFixed(1) + "%"
75-
: "—";
69+
const diff = formatDiffPercent(ca["diff-ratio"]);
70+
const aiDiff = formatDiffPercent(ca["ai-diff-ratio"]);
7671
const aiState = ca["ai-processing-state"] || "—";
7772
const bugs = ca["ai-details"]?.["total-potential-bugs"] ?? "—";
7873
output += `| ${browserName} | ${ca.width || "?"}px | ${diff} | ${aiDiff} | ${aiState} | ${bugs} |\n`;

src/tools/percy-mcp/v2/search-build-items.ts

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
11
import { percyGet } from "../../../lib/percy-api/percy-auth.js";
2+
import {
3+
fetchAllBuildItems,
4+
formatDiffPercent,
5+
CHANGED_CATEGORY_PARAMS,
6+
} from "../../../lib/percy-api/build-items.js";
27
import { BrowserStackConfig } from "../../../lib/types.js";
38
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
49

@@ -15,25 +20,37 @@ export async function percySearchBuildItems(
1520
},
1621
config: BrowserStackConfig,
1722
): Promise<CallToolResult> {
18-
const params: Record<string, string> = { "filter[build-id]": args.build_id };
19-
if (args.category) params["filter[category]"] = args.category;
23+
const params: Record<string, string | string[]> = {
24+
"filter[build-id]": args.build_id,
25+
};
26+
if (args.category === "changed") {
27+
// The API resolves the changed category through subcategories; without
28+
// them filter[category]=changed matches nothing.
29+
Object.assign(params, CHANGED_CATEGORY_PARAMS);
30+
} else if (args.category) {
31+
params["filter[category]"] = args.category;
32+
}
2033
if (args.sort_by) params["filter[sort_by]"] = args.sort_by;
21-
if (args.limit) params["page[limit]"] = String(args.limit);
2234

2335
// Array filters
2436
if (args.browser_ids)
25-
args.browser_ids.split(",").forEach((id) => {
26-
params[`filter[browser_ids][]`] = id.trim();
27-
});
37+
params["filter[browser_ids][]"] = args.browser_ids
38+
.split(",")
39+
.map((id) => id.trim());
2840
if (args.widths)
29-
args.widths.split(",").forEach((w) => {
30-
params[`filter[widths][]`] = w.trim();
31-
});
41+
params["filter[widths][]"] = args.widths.split(",").map((w) => w.trim());
3242
if (args.os) params["filter[os]"] = args.os;
3343
if (args.device_name) params["filter[device_name]"] = args.device_name;
3444

35-
const response = await percyGet("/build-items", config, params);
36-
const items = response?.data || [];
45+
let items: any[];
46+
let truncated = false;
47+
if (args.limit) {
48+
params["page[limit]"] = String(args.limit);
49+
const response = await percyGet("/build-items", config, params);
50+
items = response?.data || [];
51+
} else {
52+
({ items, truncated } = await fetchAllBuildItems(config, params));
53+
}
3754

3855
if (!items.length) {
3956
return {
@@ -48,14 +65,17 @@ export async function percySearchBuildItems(
4865
items.forEach((item: any, i: number) => {
4966
const attrs = item.attributes || item;
5067
const name = attrs.coverSnapshotName || attrs["cover-snapshot-name"] || "?";
51-
const diff =
52-
attrs.maxDiffRatio != null
53-
? `${(attrs.maxDiffRatio * 100).toFixed(1)}%`
54-
: "—";
68+
const diff = formatDiffPercent(
69+
attrs.maxDiffRatio ?? attrs["max-diff-ratio"],
70+
);
5571
const review = attrs.reviewState || attrs["review-state"] || "?";
5672
const count = attrs.itemCount || attrs["item-count"] || 1;
5773
output += `| ${i + 1} | ${name} | ${diff} | ${review} | ${count} |\n`;
5874
});
5975

76+
if (truncated) {
77+
output += `\n⚠️ Result may be incomplete — pagination could not be fully exhausted.\n`;
78+
}
79+
6080
return { content: [{ type: "text", text: output }] };
6181
}

src/tools/percy-mcp/workflows/auto-triage.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { PercyClient } from "../../../lib/percy-api/client.js";
2+
import { formatDiffPercent } from "../../../lib/percy-api/build-items.js";
23
import { BrowserStackConfig } from "../../../lib/types.js";
34
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
45

@@ -56,14 +57,14 @@ export async function percyAutoTriage(
5657
if (critical.length > 0) {
5758
output += `### CRITICAL — Potential Bugs (${critical.length})\n`;
5859
critical.forEach((e, i) => {
59-
output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff, ${e.potentialBugs} bug(s)\n`;
60+
output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff, ${e.potentialBugs} bug(s)\n`;
6061
});
6162
output += "\n";
6263
}
6364
if (reviewRequired.length > 0) {
6465
output += `### REVIEW REQUIRED (${reviewRequired.length})\n`;
6566
reviewRequired.forEach((e, i) => {
66-
output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff\n`;
67+
output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff\n`;
6768
});
6869
output += "\n";
6970
}

src/tools/percy-mcp/workflows/diff-explain.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { PercyClient } from "../../../lib/percy-api/client.js";
2+
import { formatDiffPercent } from "../../../lib/percy-api/build-items.js";
23
import { pollUntil } from "../../../lib/percy-api/polling.js";
34
import { BrowserStackConfig } from "../../../lib/types.js";
45
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
@@ -36,9 +37,9 @@ export async function percyDiffExplain(
3637
// Basic diff info
3738
const diffRatio = comparison.diffRatio ?? 0;
3839
const aiDiffRatio = comparison.aiDiffRatio;
39-
output += `**Diff:** ${(diffRatio * 100).toFixed(1)}%`;
40+
output += `**Diff:** ${formatDiffPercent(diffRatio)}`;
4041
if (aiDiffRatio !== null && aiDiffRatio !== undefined) {
41-
output += ` | **AI Diff:** ${(aiDiffRatio * 100).toFixed(1)}%`;
42+
output += ` | **AI Diff:** ${formatDiffPercent(aiDiffRatio)}`;
4243
const reduction =
4344
diffRatio > 0 ? ((1 - aiDiffRatio / diffRatio) * 100).toFixed(0) : "0";
4445
output += ` (${reduction}% noise filtered)`;

src/tools/percy-mcp/workflows/pr-visual-report.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { PercyClient } from "../../../lib/percy-api/client.js";
2+
import { formatDiffPercent } from "../../../lib/percy-api/build-items.js";
23
import { percyCache } from "../../../lib/percy-api/cache.js";
34
import { formatBuild } from "../../../lib/percy-api/formatter.js";
45
import { BrowserStackConfig } from "../../../lib/types.js";
@@ -162,23 +163,23 @@ export async function percyPrVisualReport(
162163
if (critical.length > 0) {
163164
output += `**CRITICAL — Potential Bugs (${critical.length}):**\n`;
164165
critical.forEach((e, i) => {
165-
output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff, ${e.potentialBugs} bug(s) flagged\n`;
166+
output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff, ${e.potentialBugs} bug(s) flagged\n`;
166167
});
167168
output += "\n";
168169
}
169170

170171
if (review.length > 0) {
171172
output += `**REVIEW REQUIRED (${review.length}):**\n`;
172173
review.forEach((e, i) => {
173-
output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff\n`;
174+
output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff\n`;
174175
});
175176
output += "\n";
176177
}
177178

178179
if (expected.length > 0) {
179180
output += `**EXPECTED CHANGES (${expected.length}):**\n`;
180181
expected.forEach((e, i) => {
181-
output += `${i + 1}. ${e.name}${(e.diffRatio * 100).toFixed(1)}% diff\n`;
182+
output += `${i + 1}. ${e.name}${formatDiffPercent(e.diffRatio)} diff\n`;
182183
});
183184
output += "\n";
184185
}

0 commit comments

Comments
 (0)