diff --git a/packages/cli/src/commands/issue/list.ts b/packages/cli/src/commands/issue/list.ts index 1069122c0..4131309bd 100644 --- a/packages/cli/src/commands/issue/list.ts +++ b/packages/cli/src/commands/issue/list.ts @@ -7,6 +7,10 @@ import type { SentryContext } from "../../context.js"; import { buildProjectAliasMap } from "../../lib/alias.js"; +import { + fetchScopedCounts, + type ScopedCount, +} from "../../lib/api/issue-scoped-counts.js"; import { API_MAX_PER_PAGE, buildIssueListCollapse, @@ -87,6 +91,7 @@ import { import { SEARCH_SYNTAX_REFERENCE, sanitizeQuery, + unscopedCountFilterKeys, } from "../../lib/search-query.js"; import { isSaaS } from "../../lib/sentry-web-urls.js"; import { @@ -261,6 +266,166 @@ function buildListApiOptions(json: boolean, fields?: string[]): ListApiOptions { }; } +/** + * Whether the seen-stats counts (`count`/`userCount`) are displayed for the + * given flags. Human output always shows them; JSON only omits them when + * `--fields` collapses lifetime fields away. Used to skip the scoped-count + * refetch when the counts aren't shown anyway. See #1518. + */ +function countsShown(json: boolean, fields?: string[]): boolean { + return !(json && shouldCollapseForFields(fields, LIFETIME_FIELDS)); +} + +/** Whether the issue ordering depends on `count`/`userCount`. */ +function sortUsesCounts(sort: ListFlags["sort"]): boolean { + return sort === "freq" || sort === "user"; +} + +/** + * Whether a scoped-count refetch applies to the current flags: the query must + * filter on a count-unscoped key, and the counts must either be displayed or + * drive the `freq`/`user` ordering. + */ +function shouldScopeCounts( + flags: Pick, +): boolean { + return Boolean( + flags.query && + (countsShown(flags.json, flags.fields) || sortUsesCounts(flags.sort)) && + unscopedCountFilterKeys(flags.query).length > 0, + ); +} + +/** + * Group display rows by org slug so each events query runs against the right + * organization (org-all / multi-project modes mix orgs). Rows without a short + * ID are skipped — they can't be scoped via `issue:`. + */ +function groupRowsByOrg(rows: IssueTableRow[]): Map { + const byOrg = new Map(); + for (const row of rows) { + if (!row.issue.shortId) { + continue; + } + const group = byOrg.get(row.orgSlug); + if (group) { + group.push(row); + } else { + byOrg.set(row.orgSlug, [row]); + } + } + return byOrg; +} + +/** + * Overwrite each row's issue counts from the per-org scoped-count maps. + * Returns true when at least one count changed. + */ +function overwriteScopedCounts( + groups: { orgRows: IssueTableRow[]; scoped: Map }[], +): boolean { + let changed = false; + for (const { orgRows, scoped } of groups) { + for (const row of orgRows) { + const s = scoped.get(row.issue.shortId); + if (s) { + row.issue.count = String(s.count); + row.issue.userCount = s.userCount; + changed = true; + } + } + } + return changed; +} + +/** + * Replace each issue's unscoped list-endpoint counts (`count`/`userCount`) with + * filter-scoped counts from the events endpoint when the query filters on a key + * the issues-list seen-stats ignore (e.g. `release:`). The list endpoint reports + * period-wide totals across every value of that key, so a release-scoped triage + * silently over-reports; the events endpoint returns counts that match the + * filter. See getsentry/cli#1518. + * + * Best-effort: any failure leaves the original counts untouched. When the sort + * depends on counts (`freq`/`user`), the display is re-ranked to follow the + * corrected numbers. Mutates `result` in place. + */ +async function applyScopedCounts( + result: IssueListResult, + flags: Pick, + timeRange: TimeRange, +): Promise { + try { + const rows = result.displayRows; + if (!(shouldScopeCounts(flags) && rows && rows.length > 0)) { + return; + } + const byOrg = groupRowsByOrg(rows); + if (byOrg.size === 0) { + return; + } + + const timeParams = timeRangeToApiParams(timeRange); + const groups = await Promise.all( + [...byOrg.entries()].map(async ([orgSlug, orgRows]) => { + const scoped = await fetchScopedCounts( + orgSlug, + orgRows.map((row) => ({ + shortId: row.issue.shortId, + projectId: parseProjectId(row.issue.project?.id), + })), + { + query: flags.query ?? "", + statsPeriod: timeParams.statsPeriod, + start: timeParams.start, + end: timeParams.end, + }, + ); + return { orgRows, scoped }; + }), + ); + + // Re-rank when the ordering follows the counts we just corrected. + if (overwriteScopedCounts(groups) && sortUsesCounts(flags.sort)) { + const cmp = getComparator(flags.sort); + result.items.sort(cmp); + rows.sort((a, b) => cmp(a.issue, b.issue)); + } + } catch (error) { + logger + .withTag("issue.list") + .debug(`Scoped-count refetch skipped: ${String(error)}`); + } +} + +/** Coerce an optional project ID (string from the API) to a number. */ +function parseProjectId(id: string | undefined): number | undefined { + if (id === undefined) { + return; + } + const n = Number(id); + return Number.isFinite(n) ? n : undefined; +} + +/** + * Combine the framework footer hint from a result's more/footer parts. Only + * forwards a hint when items exist — empty results render their hint text + * inside {@link formatIssueListHuman}. + */ +function buildCombinedHint(result: IssueListResult): string | undefined { + if (result.items.length === 0) { + return; + } + const hintParts: string[] = []; + if (result.moreHint) { + hintParts.push(result.moreHint); + } + if (result.footer) { + hintParts.push(result.footer); + } + return hintParts.length > 0 ? hintParts.join("\n") : result.hint; +} + /** * Resolve the effective compact mode from the flag tri-state and issue count. * @@ -1721,19 +1886,13 @@ export const listCommand = buildListCommand("issue", { }, })) as IssueListResult; - // Only forward hints to the framework footer when items exist — empty - // results already render hint text inside formatIssueListHuman. - let combinedHint: string | undefined; - if (result.items.length > 0) { - const hintParts: string[] = []; - if (result.moreHint) { - hintParts.push(result.moreHint); - } - if (result.footer) { - hintParts.push(result.footer); - } - combinedHint = hintParts.length > 0 ? hintParts.join("\n") : result.hint; - } + // The issues-list endpoint's seen-stats (count/userCount) are period-wide + // totals that ignore query filters like release:. When such a filter is + // active, re-fetch filter-scoped counts from the events endpoint so the + // displayed numbers (and freq/user ordering) match the query. See #1518. + await applyScopedCounts(result, flags, timeRange); + + const combinedHint = buildCombinedHint(result); yield new CommandOutput(result); return { hint: combinedHint }; diff --git a/packages/cli/src/lib/api/explore.ts b/packages/cli/src/lib/api/explore.ts index b5533e8a7..2ec786fab 100644 --- a/packages/cli/src/lib/api/explore.ts +++ b/packages/cli/src/lib/api/explore.ts @@ -42,6 +42,12 @@ export type ExploreQueryOptions = { start?: string; /** Absolute end datetime (ISO-8601). Mutually exclusive with statsPeriod. */ end?: string; + /** + * Numeric project IDs to scope the query to. Passed as repeated `project` + * params. Avoids the "project not actively selected" error when querying a + * specific project's events. + */ + project?: string[]; }; /** @@ -79,6 +85,7 @@ async function fetchEventsPage( start: options.start, end: options.end, cursor: options.cursor, + project: options.project, }, schema: EventsTableResponseSchema, }, diff --git a/packages/cli/src/lib/api/issue-scoped-counts.ts b/packages/cli/src/lib/api/issue-scoped-counts.ts new file mode 100644 index 000000000..4a153c000 --- /dev/null +++ b/packages/cli/src/lib/api/issue-scoped-counts.ts @@ -0,0 +1,113 @@ +/** + * Scoped issue counts via the Sentry events endpoint. + * + * The issues-list endpoint's per-group seen-stats (`count`, `userCount`) honor + * the `statsPeriod` but are NOT scoped to arbitrary query filters like + * `release:` — they report period-wide totals across every value of the key. + * The events endpoint, in contrast, returns counts that actually match the + * filter. We re-query it per issue with `issue:` AND the user's query + * to replace the over-reported numbers. See getsentry/cli#1518. + */ + +import pLimit from "p-limit"; +import { logger } from "../logger.js"; +import { queryEvents } from "./explore.js"; +import { ORG_FANOUT_CONCURRENCY } from "./infrastructure.js"; + +const log = logger.withTag("issue-scoped-counts"); + +/** Filter-scoped event and affected-user counts for a single issue. */ +export type ScopedCount = { + count: number; + userCount: number; +}; + +/** Time range + project scope shared by scoped-count queries. */ +type ScopedCountTimeOptions = { + statsPeriod?: string; + start?: string; + end?: string; +}; + +/** + * Fetch filter-scoped event & user counts for a single issue via the events + * endpoint. Returns `undefined` on any failure so the caller keeps the + * (unscoped) list-endpoint numbers rather than breaking the listing. + * + * @param orgSlug - Organization slug + * @param shortId - Issue short ID (scopes to the one group via `issue:`) + * @param options - The user's query plus time range and optional project ID + */ +export async function fetchScopedCount( + orgSlug: string, + shortId: string, + options: ScopedCountTimeOptions & { + /** The user's --query (already sanitized). */ + query: string; + /** Numeric project ID to scope the events query to. */ + projectId?: number; + }, +): Promise { + const query = `issue:${shortId} ${options.query}`.trim(); + try { + const response = await queryEvents(orgSlug, { + fields: ["count()", "count_unique(user)"], + dataset: "errors", + query, + statsPeriod: options.statsPeriod, + start: options.start, + end: options.end, + project: + options.projectId === undefined + ? undefined + : [String(options.projectId)], + limit: 1, + }); + const row = response.data.data[0]; + if (!row) { + return; + } + return { + count: Number(row["count()"] ?? 0), + userCount: Number(row["count_unique(user)"] ?? 0), + }; + } catch (error) { + log.debug(`Scoped count refetch failed for ${shortId}: ${String(error)}`); + return; + } +} + +/** + * Fetch scoped counts for many issues in parallel (bounded concurrency), + * returning a Map keyed by issue short ID. Issues whose refetch fails are + * absent from the map so the caller keeps the unscoped fallback. + */ +export async function fetchScopedCounts( + orgSlug: string, + issues: { shortId: string; projectId?: number }[], + options: ScopedCountTimeOptions & { query: string }, +): Promise> { + const limit = pLimit(ORG_FANOUT_CONCURRENCY); + const results = await Promise.all( + issues.map((issue) => + limit(async () => { + const scoped = await fetchScopedCount(orgSlug, issue.shortId, { + query: options.query, + statsPeriod: options.statsPeriod, + start: options.start, + end: options.end, + projectId: issue.projectId, + }); + return { shortId: issue.shortId, scoped }; + }), + ), + ); + + const map = new Map(); + for (const { shortId, scoped } of results) { + if (scoped) { + map.set(shortId, scoped); + } + } + return map; +} diff --git a/packages/cli/src/lib/search-query.ts b/packages/cli/src/lib/search-query.ts index 5bd734711..5d6985ee3 100644 --- a/packages/cli/src/lib/search-query.ts +++ b/packages/cli/src/lib/search-query.ts @@ -39,6 +39,19 @@ const log = logger.withTag("search-query"); */ const INVALID_INLIST_KEYS = new Set(["is", "has"]); +/** + * Filter keys whose values scope *which* issues match but which the Sentry + * issues-list endpoint does NOT apply to its per-group seen-stats + * (`count`, `userCount`, `firstSeen`, `lastSeen`). + * + * A query filtering on one of these returns the groups matching the filter, + * but the displayed counts remain period-wide totals across every value of the + * key — e.g. `release:"X"` restricts the result set yet the counts still cover + * all releases. Callers must be told so they don't attribute the numbers to the + * filter. See getsentry/cli#1518. + */ +const COUNT_UNSCOPED_FILTER_KEYS = new Set(["release"]); + // --------------------------------------------------------------------------- // AST → string serialization // --------------------------------------------------------------------------- @@ -474,6 +487,65 @@ function handleOr( ); } +/** + * Collect the count-unscoped filter keys (see {@link COUNT_UNSCOPED_FILTER_KEYS}) + * present in a query, recursing into parenthesized groups. + */ +function collectUnscopedCountKeys( + nodes: SearchNode[], + found: Set, +): void { + for (const node of nodes) { + if (node.type === "paren_group") { + collectUnscopedCountKeys(node.inner, found); + continue; + } + if ( + node.type === "text_filter" || + node.type === "text_in_filter" || + node.type === "comparison_filter" || + node.type === "regex_filter" + ) { + const key = node.key.toLowerCase(); + if (COUNT_UNSCOPED_FILTER_KEYS.has(key)) { + found.add(key); + } + } + } +} + +/** + * Return the filter keys in `query` whose values restrict which issues match + * but are NOT reflected in the issues-list endpoint's seen-stats + * (`count`, `userCount`, `firstSeen`, `lastSeen`). See getsentry/cli#1518. + * + * Used to warn callers that a release-scoped `issue list` reports period-wide + * counts, not counts matching the filter. Returns an empty array when the + * query is absent, unparseable, or contains no such keys. + * + * @param query - Raw `--query` value, or `undefined` + * @returns Lower-cased matching keys (e.g. `["release"]`), deduplicated + */ +export function unscopedCountFilterKeys(query: string | undefined): string[] { + if (!query) { + return []; + } + let nodes: SearchNode[]; + try { + nodes = parse(normalizeQuery(query)); + } catch (error) { + // Unparseable query has no keys we can detect; the API will reject it with + // a proper 400. Nothing actionable here — just skip the count-scope check. + log.debug( + `Skipping count-scope check for unparseable query: ${String(error)}`, + ); + return []; + } + const found = new Set(); + collectUnscopedCountKeys(nodes, found); + return [...found]; +} + // --------------------------------------------------------------------------- // Search syntax reference // --------------------------------------------------------------------------- diff --git a/packages/cli/test/commands/issue/list.test.ts b/packages/cli/test/commands/issue/list.test.ts index 1fb9c3f61..99602bb4e 100644 --- a/packages/cli/test/commands/issue/list.test.ts +++ b/packages/cli/test/commands/issue/list.test.ts @@ -782,6 +782,178 @@ describe("issue list: server sort order preservation", () => { }); }); +describe("issue list: scoped counts via events endpoint (#1518)", () => { + /** + * Mock a single issue from the list endpoint and (optionally) a scoped-count + * response from the events endpoint. Tracks the events-endpoint URLs seen so + * tests can assert what was queried. + */ + function mockScopedCounts(options?: { + events?: { count: number; userCount: number } | "error"; + issues?: Record[]; + }) { + const eventsUrls: string[] = []; + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + const projectResp = mockDefaultProject(req.url); + if (projectResp) return projectResp; + if (req.url.includes("/events/")) { + eventsUrls.push(req.url); + if (options?.events === "error") { + return new Response(JSON.stringify({ detail: "boom" }), { + status: 500, + headers: { "Content-Type": "application/json" }, + }); + } + const ev = options?.events ?? { count: 0, userCount: 0 }; + return Response.json({ + data: [ + { + "count()": ev.count, + "count_unique(user)": ev.userCount, + }, + ], + meta: {}, + }); + } + if (req.url.includes("/issues/")) { + const issues = options?.issues ?? [mockIssue()]; + return new Response(JSON.stringify(issues), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + return { eventsUrls }; + } + + test("replaces counts with scoped values from the events endpoint", async () => { + const { eventsUrls } = mockScopedCounts({ + events: { count: 238, userCount: 189 }, + }); + const { context, stdout } = createContext(); + + await func.call(context, { + limit: 10, + sort: "date", + period: parsePeriod("90d"), + json: true, + query: 'release:"1.0.0"', + }); + + // The events endpoint is queried with issue: AND the release filter. + expect(eventsUrls.length).toBe(1); + const decoded = decodeURIComponent(eventsUrls[0] ?? ""); + expect(decoded).toContain("issue:TEST-PROJECT-1"); + expect(decoded).toContain('release:"1.0.0"'); + + const output = JSON.parse(stdout.output); + expect(output.data[0].count).toBe("238"); + expect(output.data[0].userCount).toBe(189); + }); + + test("does not call the events endpoint without an unscoped filter", async () => { + const { eventsUrls } = mockScopedCounts({ + events: { count: 238, userCount: 189 }, + }); + const { context, stdout } = createContext(); + + await func.call(context, { + limit: 10, + sort: "date", + period: parsePeriod("90d"), + json: true, + query: "is:unresolved", + }); + + expect(eventsUrls.length).toBe(0); + const output = JSON.parse(stdout.output); + // Original list-endpoint count is preserved. + expect(output.data[0].count).toBe("10"); + expect(output.data[0].userCount).toBe(5); + }); + + test("falls back to unscoped counts when the events refetch fails", async () => { + mockScopedCounts({ events: "error" }); + const { context, stdout } = createContext(); + + await func.call(context, { + limit: 10, + sort: "date", + period: parsePeriod("90d"), + json: true, + query: 'release:"1.0.0"', + }); + + const output = JSON.parse(stdout.output); + // Listing still renders with the original list-endpoint counts. + expect(output.data[0].count).toBe("10"); + expect(output.data[0].userCount).toBe(5); + }); + + test.each([ + { label: "with counts shown", fields: undefined }, + { label: "when --fields omits counts", fields: ["id", "title"] }, + ])("--sort freq re-ranks by scoped counts $label", async ({ fields }) => { + // List endpoint orders by unscoped counts: A (100) before B (50). + const issues = [ + mockIssue({ id: "A", shortId: "AAA-1", count: "100", userCount: 100 }), + mockIssue({ id: "B", shortId: "BBB-2", count: "50", userCount: 50 }), + ]; + // Scoped counts invert the ranking: B (90) should outrank A (5). + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + const projectResp = mockDefaultProject(req.url); + if (projectResp) return projectResp; + if (req.url.includes("/events/")) { + const decoded = decodeURIComponent(req.url); + const scoped = decoded.includes("issue:AAA-1") + ? { count: 5, userCount: 5 } + : { count: 90, userCount: 90 }; + return Response.json({ + data: [ + { + "count()": scoped.count, + "count_unique(user)": scoped.userCount, + }, + ], + meta: {}, + }); + } + if (req.url.includes("/issues/")) { + return new Response(JSON.stringify(issues), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + const { context, stdout } = createContext(); + + await func.call(context, { + limit: 10, + sort: "freq", + period: parsePeriod("90d"), + json: true, + fields, + query: 'release:"1.0.0"', + }); + + const output = JSON.parse(stdout.output); + expect(output.data.map((issue: { id: string }) => issue.id)).toEqual([ + "B", + "A", + ]); + }); +}); + /** Shared mock references — vi.mocked() returns the same mock object each time */ const listIssuesPaginatedMock = vi.mocked(issuesApi.listIssuesPaginated); const listIssuesAllPagesMock = vi.mocked(issuesApi.listIssuesAllPages); diff --git a/packages/cli/test/lib/search-query.test.ts b/packages/cli/test/lib/search-query.test.ts index 5dc9e9a86..98c025414 100644 --- a/packages/cli/test/lib/search-query.test.ts +++ b/packages/cli/test/lib/search-query.test.ts @@ -12,7 +12,11 @@ import { describe, expect, test } from "vitest"; import { ValidationError } from "../../src/lib/errors.js"; -import { __testing, sanitizeQuery } from "../../src/lib/search-query.js"; +import { + __testing, + sanitizeQuery, + unscopedCountFilterKeys, +} from "../../src/lib/search-query.js"; const { normalizeQuery, transformUnquoted } = __testing; @@ -684,3 +688,62 @@ describe("sanitizeQuery: normalization integration", () => { expect(result).toBe("((( broken"); }); }); + +// --------------------------------------------------------------------------- +// unscopedCountFilterKeys (#1518) +// --------------------------------------------------------------------------- + +describe("unscopedCountFilterKeys", () => { + test("returns empty for undefined or empty query", () => { + expect(unscopedCountFilterKeys(undefined)).toEqual([]); + expect(unscopedCountFilterKeys("")).toEqual([]); + }); + + test("returns empty when no count-unscoped filter is present", () => { + expect(unscopedCountFilterKeys("is:unresolved level:error")).toEqual([]); + }); + + test("detects a release: filter", () => { + expect(unscopedCountFilterKeys('release:"1.0.0"')).toEqual(["release"]); + }); + + test("detects release: alongside other terms", () => { + expect(unscopedCountFilterKeys('release:"1.0.0" is:unresolved')).toEqual([ + "release", + ]); + }); + + test("detects release: in an in-list filter", () => { + expect(unscopedCountFilterKeys("release:[1.0.0,2.0.0]")).toEqual([ + "release", + ]); + }); + + test("detects release: in a regex filter", () => { + expect(unscopedCountFilterKeys("release://1\\.0\\..*//")).toEqual([ + "release", + ]); + }); + + test("is case-insensitive on the key", () => { + expect(unscopedCountFilterKeys("Release:1.0.0")).toEqual(["release"]); + }); + + test("detects release: inside a parenthesized group", () => { + expect(unscopedCountFilterKeys("(release:1.0.0)")).toEqual(["release"]); + }); + + test("deduplicates repeated keys", () => { + expect(unscopedCountFilterKeys("release:1.0.0 release:2.0.0")).toEqual([ + "release", + ]); + }); + + test("does not match free text that merely contains 'release'", () => { + expect(unscopedCountFilterKeys("release notes")).toEqual([]); + }); + + test("returns empty for an unparseable query", () => { + expect(unscopedCountFilterKeys("((( broken")).toEqual([]); + }); +});