From 1482a60b2e477020522d97905106a562eea1c44c Mon Sep 17 00:00:00 2001 From: DeRaowl Date: Tue, 21 Jul 2026 18:24:14 +0530 Subject: [PATCH] fix(percy): full build-items pagination and diff precision - 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 --- src/lib/percy-api/build-items.ts | 75 +++++++++++++++++++ src/lib/percy-api/percy-auth.ts | 8 +- src/tools/percy-mcp/v2/get-build-detail.ts | 33 ++++---- src/tools/percy-mcp/v2/get-comparison.ts | 5 +- src/tools/percy-mcp/v2/get-snapshot.ts | 13 +--- src/tools/percy-mcp/v2/search-build-items.ts | 50 +++++++++---- src/tools/percy-mcp/workflows/auto-triage.ts | 5 +- src/tools/percy-mcp/workflows/diff-explain.ts | 5 +- .../percy-mcp/workflows/pr-visual-report.ts | 7 +- 9 files changed, 149 insertions(+), 52 deletions(-) create mode 100644 src/lib/percy-api/build-items.ts diff --git a/src/lib/percy-api/build-items.ts b/src/lib/percy-api/build-items.ts new file mode 100644 index 0000000..930d251 --- /dev/null +++ b/src/lib/percy-api/build-items.ts @@ -0,0 +1,75 @@ +/** + * Shared helpers for the /build-items endpoint. + * + * Pagination: the API returns everything when page[limit] is omitted, but may + * still paginate (meta.pagination.has_more + next_cursor) on very large + * builds or if server-side defaults change. fetchAllBuildItems handles both: + * it starts with an unbounded request and follows next_cursor if the response + * is paginated anyway. + */ + +import { percyGet } from "./percy-auth.js"; +import { BrowserStackConfig } from "../types.js"; + +const MAX_PAGES = 50; + +export interface BuildItemsResult { + items: any[]; + /** True if pagination could not be exhausted (page cap or cursor stall). */ + truncated: boolean; +} + +/** + * Fetch ALL build items for the given filters, following + * meta.pagination.next_cursor until has_more is false. + */ +export async function fetchAllBuildItems( + config: BrowserStackConfig, + baseParams: Record, +): Promise { + const items: any[] = []; + let cursor: string | undefined; + let pages = 0; + + for (;;) { + const params: Record = { ...baseParams }; + if (cursor) params["page[cursor]"] = cursor; + + const response = await percyGet("/build-items", config, params); + items.push(...(response?.data || [])); + pages += 1; + + const pagination = response?.meta?.pagination; + const nextCursor = pagination?.next_cursor; + if (!pagination?.has_more || !nextCursor) { + return { items, truncated: false }; + } + // Guard against a stalled cursor or runaway loop. + if (nextCursor === cursor || pages >= MAX_PAGES) { + return { items, truncated: true }; + } + cursor = String(nextCursor); + } +} + +/** + * Filter params that make filter[category]=changed actually return results. + * The API only maps the changed category to review states via + * filter[subcategories][]; without it the scope resolves to zero rows. + */ +export const CHANGED_CATEGORY_PARAMS: Record = { + "filter[category]": "changed", + "filter[subcategories][]": ["unreviewed", "changes_requested", "approved"], +}; + +/** + * Format a 0..1 diff ratio as a percentage without hiding small diffs: + * 0 → "0%", tiny → "<0.01%", otherwise two decimals (e.g. "0.02%"). + */ +export function formatDiffPercent(ratio: number | null | undefined): string { + if (ratio == null) return "—"; + const pct = ratio * 100; + if (pct === 0) return "0%"; + if (pct < 0.01) return "<0.01%"; + return pct.toFixed(2) + "%"; +} diff --git a/src/lib/percy-api/percy-auth.ts b/src/lib/percy-api/percy-auth.ts index c792b0b..e74b2e7 100644 --- a/src/lib/percy-api/percy-auth.ts +++ b/src/lib/percy-api/percy-auth.ts @@ -49,14 +49,18 @@ const PERCY_API_BASE = "https://percy.io/api/v1"; export async function percyGet( path: string, config: BrowserStackConfig, - params?: Record, + params?: Record, ): Promise { const headers = getPercyAuthHeaders(config); const url = new URL(`${PERCY_API_BASE}${path}`); if (params) { for (const [key, value] of Object.entries(params)) { - url.searchParams.set(key, value); + if (Array.isArray(value)) { + value.forEach((v) => url.searchParams.append(key, v)); + } else { + url.searchParams.set(key, value); + } } } diff --git a/src/tools/percy-mcp/v2/get-build-detail.ts b/src/tools/percy-mcp/v2/get-build-detail.ts index 2fc183d..f13a01b 100644 --- a/src/tools/percy-mcp/v2/get-build-detail.ts +++ b/src/tools/percy-mcp/v2/get-build-detail.ts @@ -12,6 +12,11 @@ */ import { percyGet, percyPost } from "../../../lib/percy-api/percy-auth.js"; +import { + fetchAllBuildItems, + formatDiffPercent, + CHANGED_CATEGORY_PARAMS, +} from "../../../lib/percy-api/build-items.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { setActiveBuild } from "../../../lib/percy-api/percy-session.js"; @@ -320,14 +325,11 @@ async function getChanges( buildId: string, config: BrowserStackConfig, ): Promise { - const response = await percyGet("/build-items", config, { + const { items, truncated } = await fetchAllBuildItems(config, { "filter[build-id]": buildId, - "filter[category]": "changed", - "page[limit]": "30", + ...CHANGED_CATEGORY_PARAMS, }); - const items = response?.data || []; - if (!items.length) { return { content: [ @@ -347,10 +349,7 @@ async function getChanges( const name = a["cover-snapshot-name"] || a.coverSnapshotName || "?"; const displayName = a["cover-snapshot-display-name"] || a.coverSnapshotDisplayName || ""; - const diff = - (a["max-diff-ratio"] ?? a.maxDiffRatio) != null - ? ((a["max-diff-ratio"] ?? a.maxDiffRatio) * 100).toFixed(1) + "%" - : "—"; + const diff = formatDiffPercent(a["max-diff-ratio"] ?? a.maxDiffRatio); const bugs = a["max-bug-total-potential-bugs"] ?? a.maxBugTotalPotentialBugs ?? 0; const review = a["review-state"] || a.reviewState || "?"; @@ -358,6 +357,9 @@ async function getChanges( output += `| ${i + 1} | ${name} | ${displayName || "—"} | ${diff} | ${bugs} | ${review} | ${count} |\n`; }); + if (truncated) { + output += `\n⚠️ Result may be incomplete — pagination could not be fully exhausted.\n`; + } output += `\nUse \`percy_get_snapshot\` with a snapshot ID from above for full details.\n`; return { content: [{ type: "text", text: output }] }; @@ -637,13 +639,10 @@ async function getSnapshots( buildId: string, config: BrowserStackConfig, ): Promise { - const response = await percyGet("/build-items", config, { + const { items, truncated } = await fetchAllBuildItems(config, { "filter[build-id]": buildId, - "page[limit]": "30", }); - const items = response?.data || []; - if (!items.length) { return { content: [ @@ -670,10 +669,7 @@ async function getSnapshots( const name = a["cover-snapshot-name"] || a.coverSnapshotName || "?"; const display = a["cover-snapshot-display-name"] || a.coverSnapshotDisplayName || "—"; - const diff = - (a["max-diff-ratio"] ?? a.maxDiffRatio) != null - ? ((a["max-diff-ratio"] ?? a.maxDiffRatio) * 100).toFixed(1) + "%" - : "—"; + const diff = formatDiffPercent(a["max-diff-ratio"] ?? a.maxDiffRatio); const bugs = a["max-bug-total-potential-bugs"] ?? a.maxBugTotalPotentialBugs ?? "—"; const review = a["review-state"] || a.reviewState || "?"; @@ -686,6 +682,9 @@ async function getSnapshots( output += `| ${i + 1} | ${name} | ${display} | ${diff} | ${bugs} | ${review} | ${count} | ${snapIds}${more} |\n`; }); + if (truncated) { + output += `\n⚠️ Result may be incomplete — pagination could not be fully exhausted.\n`; + } output += `\nUse \`percy_get_snapshot\` with a snapshot ID for full comparison details.\n`; return { content: [{ type: "text", text: output }] }; diff --git a/src/tools/percy-mcp/v2/get-comparison.ts b/src/tools/percy-mcp/v2/get-comparison.ts index 05c65ad..4e5fd19 100644 --- a/src/tools/percy-mcp/v2/get-comparison.ts +++ b/src/tools/percy-mcp/v2/get-comparison.ts @@ -1,4 +1,5 @@ import { percyGet } from "../../../lib/percy-api/percy-auth.js"; +import { formatDiffPercent } from "../../../lib/percy-api/build-items.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -43,8 +44,8 @@ export async function percyGetComparison( output += `| **Browser** | ${browserName} |\n`; output += `| **Width** | ${attrs.width || "?"}px |\n`; output += `| **State** | ${attrs.state || "?"} |\n`; - output += `| **Diff ratio** | ${attrs["diff-ratio"] != null ? (attrs["diff-ratio"] * 100).toFixed(2) + "%" : "—"} |\n`; - output += `| **AI diff ratio** | ${attrs["ai-diff-ratio"] != null ? (attrs["ai-diff-ratio"] * 100).toFixed(2) + "%" : "—"} |\n`; + output += `| **Diff ratio** | ${formatDiffPercent(attrs["diff-ratio"])} |\n`; + output += `| **AI diff ratio** | ${formatDiffPercent(attrs["ai-diff-ratio"])} |\n`; output += `| **AI state** | ${attrs["ai-processing-state"] || "—"} |\n`; output += `| **Potential bugs** | ${ai["total-potential-bugs"] ?? "—"} |\n`; output += `| **AI visual diffs** | ${ai["total-ai-visual-diffs"] ?? "—"} |\n`; diff --git a/src/tools/percy-mcp/v2/get-snapshot.ts b/src/tools/percy-mcp/v2/get-snapshot.ts index 9424fce..ee1d5b2 100644 --- a/src/tools/percy-mcp/v2/get-snapshot.ts +++ b/src/tools/percy-mcp/v2/get-snapshot.ts @@ -1,4 +1,5 @@ import { percyGet } from "../../../lib/percy-api/percy-auth.js"; +import { formatDiffPercent } from "../../../lib/percy-api/build-items.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -29,7 +30,7 @@ export async function percyGetSnapshot( output += `| Field | Value |\n|---|---|\n`; output += `| **Review** | ${attrs["review-state"] || "—"} (${attrs["review-state-reason"] || "—"}) |\n`; - output += `| **Diff ratio** | ${attrs["diff-ratio"] != null ? (attrs["diff-ratio"] * 100).toFixed(2) + "%" : "—"} |\n`; + output += `| **Diff ratio** | ${formatDiffPercent(attrs["diff-ratio"])} |\n`; output += `| **Test case** | ${attrs["test-case-name"] || "none"} |\n`; output += `| **Comments** | ${attrs["total-open-comments"] ?? 0} |\n`; output += `| **Layout** | ${attrs["enable-layout"] ? "enabled" : "disabled"} |\n`; @@ -65,14 +66,8 @@ export async function percyGetSnapshot( const ca = c.attributes || {}; const browserId = c.relationships?.browser?.data?.id; const browserName = browsers.get(browserId) || "?"; - const diff = - ca["diff-ratio"] != null - ? (ca["diff-ratio"] * 100).toFixed(1) + "%" - : "—"; - const aiDiff = - ca["ai-diff-ratio"] != null - ? (ca["ai-diff-ratio"] * 100).toFixed(1) + "%" - : "—"; + const diff = formatDiffPercent(ca["diff-ratio"]); + const aiDiff = formatDiffPercent(ca["ai-diff-ratio"]); const aiState = ca["ai-processing-state"] || "—"; const bugs = ca["ai-details"]?.["total-potential-bugs"] ?? "—"; output += `| ${browserName} | ${ca.width || "?"}px | ${diff} | ${aiDiff} | ${aiState} | ${bugs} |\n`; diff --git a/src/tools/percy-mcp/v2/search-build-items.ts b/src/tools/percy-mcp/v2/search-build-items.ts index bbba54a..6585690 100644 --- a/src/tools/percy-mcp/v2/search-build-items.ts +++ b/src/tools/percy-mcp/v2/search-build-items.ts @@ -1,4 +1,9 @@ import { percyGet } from "../../../lib/percy-api/percy-auth.js"; +import { + fetchAllBuildItems, + formatDiffPercent, + CHANGED_CATEGORY_PARAMS, +} from "../../../lib/percy-api/build-items.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -15,25 +20,37 @@ export async function percySearchBuildItems( }, config: BrowserStackConfig, ): Promise { - const params: Record = { "filter[build-id]": args.build_id }; - if (args.category) params["filter[category]"] = args.category; + const params: Record = { + "filter[build-id]": args.build_id, + }; + if (args.category === "changed") { + // The API resolves the changed category through subcategories; without + // them filter[category]=changed matches nothing. + Object.assign(params, CHANGED_CATEGORY_PARAMS); + } else if (args.category) { + params["filter[category]"] = args.category; + } if (args.sort_by) params["filter[sort_by]"] = args.sort_by; - if (args.limit) params["page[limit]"] = String(args.limit); // Array filters if (args.browser_ids) - args.browser_ids.split(",").forEach((id) => { - params[`filter[browser_ids][]`] = id.trim(); - }); + params["filter[browser_ids][]"] = args.browser_ids + .split(",") + .map((id) => id.trim()); if (args.widths) - args.widths.split(",").forEach((w) => { - params[`filter[widths][]`] = w.trim(); - }); + params["filter[widths][]"] = args.widths.split(",").map((w) => w.trim()); if (args.os) params["filter[os]"] = args.os; if (args.device_name) params["filter[device_name]"] = args.device_name; - const response = await percyGet("/build-items", config, params); - const items = response?.data || []; + let items: any[]; + let truncated = false; + if (args.limit) { + params["page[limit]"] = String(args.limit); + const response = await percyGet("/build-items", config, params); + items = response?.data || []; + } else { + ({ items, truncated } = await fetchAllBuildItems(config, params)); + } if (!items.length) { return { @@ -48,14 +65,17 @@ export async function percySearchBuildItems( items.forEach((item: any, i: number) => { const attrs = item.attributes || item; const name = attrs.coverSnapshotName || attrs["cover-snapshot-name"] || "?"; - const diff = - attrs.maxDiffRatio != null - ? `${(attrs.maxDiffRatio * 100).toFixed(1)}%` - : "—"; + const diff = formatDiffPercent( + attrs.maxDiffRatio ?? attrs["max-diff-ratio"], + ); const review = attrs.reviewState || attrs["review-state"] || "?"; const count = attrs.itemCount || attrs["item-count"] || 1; output += `| ${i + 1} | ${name} | ${diff} | ${review} | ${count} |\n`; }); + if (truncated) { + output += `\n⚠️ Result may be incomplete — pagination could not be fully exhausted.\n`; + } + return { content: [{ type: "text", text: output }] }; } diff --git a/src/tools/percy-mcp/workflows/auto-triage.ts b/src/tools/percy-mcp/workflows/auto-triage.ts index 54e2064..ff48562 100644 --- a/src/tools/percy-mcp/workflows/auto-triage.ts +++ b/src/tools/percy-mcp/workflows/auto-triage.ts @@ -1,4 +1,5 @@ import { PercyClient } from "../../../lib/percy-api/client.js"; +import { formatDiffPercent } from "../../../lib/percy-api/build-items.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -56,14 +57,14 @@ export async function percyAutoTriage( if (critical.length > 0) { output += `### CRITICAL — Potential Bugs (${critical.length})\n`; critical.forEach((e, i) => { - output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff, ${e.potentialBugs} bug(s)\n`; + output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff, ${e.potentialBugs} bug(s)\n`; }); output += "\n"; } if (reviewRequired.length > 0) { output += `### REVIEW REQUIRED (${reviewRequired.length})\n`; reviewRequired.forEach((e, i) => { - output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff\n`; + output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff\n`; }); output += "\n"; } diff --git a/src/tools/percy-mcp/workflows/diff-explain.ts b/src/tools/percy-mcp/workflows/diff-explain.ts index f42b524..9e54d20 100644 --- a/src/tools/percy-mcp/workflows/diff-explain.ts +++ b/src/tools/percy-mcp/workflows/diff-explain.ts @@ -1,4 +1,5 @@ import { PercyClient } from "../../../lib/percy-api/client.js"; +import { formatDiffPercent } from "../../../lib/percy-api/build-items.js"; import { pollUntil } from "../../../lib/percy-api/polling.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; @@ -36,9 +37,9 @@ export async function percyDiffExplain( // Basic diff info const diffRatio = comparison.diffRatio ?? 0; const aiDiffRatio = comparison.aiDiffRatio; - output += `**Diff:** ${(diffRatio * 100).toFixed(1)}%`; + output += `**Diff:** ${formatDiffPercent(diffRatio)}`; if (aiDiffRatio !== null && aiDiffRatio !== undefined) { - output += ` | **AI Diff:** ${(aiDiffRatio * 100).toFixed(1)}%`; + output += ` | **AI Diff:** ${formatDiffPercent(aiDiffRatio)}`; const reduction = diffRatio > 0 ? ((1 - aiDiffRatio / diffRatio) * 100).toFixed(0) : "0"; output += ` (${reduction}% noise filtered)`; diff --git a/src/tools/percy-mcp/workflows/pr-visual-report.ts b/src/tools/percy-mcp/workflows/pr-visual-report.ts index 06ca432..c062c02 100644 --- a/src/tools/percy-mcp/workflows/pr-visual-report.ts +++ b/src/tools/percy-mcp/workflows/pr-visual-report.ts @@ -1,4 +1,5 @@ import { PercyClient } from "../../../lib/percy-api/client.js"; +import { formatDiffPercent } from "../../../lib/percy-api/build-items.js"; import { percyCache } from "../../../lib/percy-api/cache.js"; import { formatBuild } from "../../../lib/percy-api/formatter.js"; import { BrowserStackConfig } from "../../../lib/types.js"; @@ -162,7 +163,7 @@ export async function percyPrVisualReport( if (critical.length > 0) { output += `**CRITICAL — Potential Bugs (${critical.length}):**\n`; critical.forEach((e, i) => { - output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff, ${e.potentialBugs} bug(s) flagged\n`; + output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff, ${e.potentialBugs} bug(s) flagged\n`; }); output += "\n"; } @@ -170,7 +171,7 @@ export async function percyPrVisualReport( if (review.length > 0) { output += `**REVIEW REQUIRED (${review.length}):**\n`; review.forEach((e, i) => { - output += `${i + 1}. **${e.name}** — ${(e.diffRatio * 100).toFixed(1)}% diff\n`; + output += `${i + 1}. **${e.name}** — ${formatDiffPercent(e.diffRatio)} diff\n`; }); output += "\n"; } @@ -178,7 +179,7 @@ export async function percyPrVisualReport( if (expected.length > 0) { output += `**EXPECTED CHANGES (${expected.length}):**\n`; expected.forEach((e, i) => { - output += `${i + 1}. ${e.name} — ${(e.diffRatio * 100).toFixed(1)}% diff\n`; + output += `${i + 1}. ${e.name} — ${formatDiffPercent(e.diffRatio)} diff\n`; }); output += "\n"; }