Skip to content

Commit 6096cba

Browse files
authored
fix(issue): conditionally collapse lifetime to preserve count/userCount/firstSeen/lastSeen (#985)
## Summary Fixes #969 — `sentry issue list --json --fields count,firstSeen,lastSeen,userCount` was silently omitting those fields because `collapse=lifetime` was always sent to the Sentry API. ## Root Cause `buildIssueListCollapse()` always included `"lifetime"` in the collapse array. Despite the JSDoc claiming top-level `count`/`userCount`/`firstSeen`/`lastSeen` are unaffected by collapsing, `collapse=lifetime` actually strips these fields from the list endpoint response. This caused: - `EVENTS: ?` in human output (`count` undefined) - `SEEN: —` / `AGE: —` (`lastSeen`/`firstSeen` undefined) - `USERS: 0` (`userCount` undefined) - Empty values in `--json --fields` output ## Fix Makes the `lifetime` collapse conditional via a new `shouldCollapseLifetime` parameter on `buildIssueListCollapse()` (defaults to `false`). **Lifetime is NOT collapsed (fields available) when:** - Human output — always needs these for EVENTS, USERS, SEEN, AGE columns - JSON mode without `--fields` — all fields returned - JSON mode with `--fields` that include any of: `count`, `userCount`, `firstSeen`, `lastSeen` **Lifetime IS collapsed (perf optimization) when:** - JSON mode with explicit `--fields` that don't include any lifetime-dependent field ## Changes - `src/lib/api/issues.ts` — Added `shouldCollapseLifetime` option to `buildIssueListCollapse()`, fixed misleading JSDoc comments - `src/commands/issue/list.ts` — Added `LIFETIME_FIELDS` set, updated `buildListApiOptions()` to compute lifetime collapse condition from `flags.fields` - `test/lib/issue-collapse.property.test.ts` — Updated property tests for new parameter (11 tests, 407 expect calls) - `test/commands/issue/list.test.ts` — Added 4 integration tests covering all lifetime collapse scenarios
1 parent 472f381 commit 6096cba

4 files changed

Lines changed: 263 additions & 47 deletions

File tree

src/commands/issue/list.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,17 +175,45 @@ function shouldCollapseStats(json: boolean): boolean {
175175
return !willShowTrend();
176176
}
177177

178+
/**
179+
* Fields that depend on the `lifetime` API data. When `collapse=lifetime`
180+
* is sent, the server omits these from the list response. See #969.
181+
*/
182+
const LIFETIME_FIELDS = new Set([
183+
"count",
184+
"userCount",
185+
"firstSeen",
186+
"lastSeen",
187+
]);
188+
178189
/**
179190
* Build the collapse and groupStatsPeriod options for issue list API calls.
180191
*
181192
* When stats are collapsed, groupStatsPeriod is omitted (undefined) since
182193
* the server won't compute stats anyway. This avoids wasted server-side
183194
* processing and makes the request intent explicit.
195+
*
196+
* Lifetime is only collapsed in JSON mode when explicit `--fields` are
197+
* provided and none of them are lifetime-dependent (`count`, `userCount`,
198+
* `firstSeen`, `lastSeen`). Human output always needs these for the
199+
* EVENTS, USERS, SEEN, and AGE columns.
184200
*/
185-
function buildListApiOptions(json: boolean): ListApiOptions {
201+
function buildListApiOptions(json: boolean, fields?: string[]): ListApiOptions {
186202
const collapseStats = shouldCollapseStats(json);
203+
// Collapse lifetime only when in JSON mode with explicit --fields that
204+
// don't include any lifetime-dependent field. Human output always needs
205+
// these (EVENTS, USERS, SEEN, AGE columns), and JSON without --fields
206+
// returns all fields.
207+
const collapseLifetime =
208+
json &&
209+
fields !== undefined &&
210+
fields.length > 0 &&
211+
!fields.some((f) => LIFETIME_FIELDS.has(f));
187212
return {
188-
collapse: buildIssueListCollapse({ shouldCollapseStats: collapseStats }),
213+
collapse: buildIssueListCollapse({
214+
shouldCollapseStats: collapseStats,
215+
shouldCollapseLifetime: collapseLifetime,
216+
}),
189217
groupStatsPeriod: collapseStats ? undefined : "auto",
190218
};
191219
}
@@ -870,14 +898,14 @@ function prevPageHint(org: string, flags: ListFlags): string {
870898
*/
871899
async function fetchOrgAllIssues(
872900
org: string,
873-
flags: Pick<ListFlags, "query" | "limit" | "sort" | "json">,
901+
flags: Pick<ListFlags, "query" | "limit" | "sort" | "json" | "fields">,
874902
timeRange: TimeRange,
875903
options: {
876904
cursor?: string;
877905
onPage?: (fetched: number, limit: number) => void;
878906
}
879907
): Promise<IssuesPage> {
880-
const apiOpts = buildListApiOptions(flags.json);
908+
const apiOpts = buildListApiOptions(flags.json, flags.fields);
881909
const timeParams = timeRangeToApiParams(timeRange);
882910
const { cursor, onPage } = options;
883911

@@ -1257,7 +1285,7 @@ async function handleResolvedTargets(
12571285
? `Fetching issues from ${targetCount} projects`
12581286
: "Fetching issues";
12591287

1260-
const apiOpts = buildListApiOptions(flags.json);
1288+
const apiOpts = buildListApiOptions(flags.json, flags.fields);
12611289

12621290
const { results, hasMore } = await withProgress(
12631291
{ message: `${baseMessage} (up to ${flags.limit})...`, json: flags.json },

src/lib/api/issues.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export type IssueSort = NonNullable<
4747
* expensive Snuba/ClickHouse queries on the backend.
4848
*
4949
* - `'stats'` — time-series event counts (sparkline data)
50-
* - `'lifetime'` — lifetime aggregate counts (count, userCount, firstSeen)
50+
* - `'lifetime'` — lifetime aggregate sub-object AND top-level count/userCount/firstSeen/lastSeen on list endpoints
5151
* - `'filtered'` — filtered aggregate counts
5252
* - `'unhandled'` — unhandled event flag computation
5353
* - `'base'` — base group fields (rarely useful to collapse)
@@ -59,9 +59,17 @@ export type IssueCollapseField = NonNullable<
5959
/**
6060
* Build the `collapse` parameter for issue list API calls.
6161
*
62-
* Always collapses fields the CLI never consumes in issue list:
63-
* `filtered`, `lifetime`, `unhandled`. Conditionally collapses `stats`
64-
* when sparklines won't be rendered (narrow terminal, non-TTY, or JSON).
62+
* Always collapses `filtered` and `unhandled` — the CLI never consumes
63+
* these in issue list views. Conditionally collapses `stats` when
64+
* sparklines won't be rendered (narrow terminal, non-TTY, or JSON),
65+
* and `lifetime` when the caller confirms the lifetime-dependent
66+
* top-level fields (`count`, `userCount`, `firstSeen`, `lastSeen`)
67+
* aren't needed.
68+
*
69+
* **Important:** Despite being documented as removing only the `lifetime`
70+
* sub-object, `collapse=lifetime` also strips the top-level `count`,
71+
* `userCount`, `firstSeen`, and `lastSeen` fields from list responses.
72+
* Only collapse it when those fields are confirmed unnecessary. See #969.
6573
*
6674
* Matches the Sentry web UI's optimization: the initial page load sends
6775
* `collapse=stats,unhandled` to skip expensive Snuba queries, fetching
@@ -70,12 +78,20 @@ export type IssueCollapseField = NonNullable<
7078
* @param options - Context for determining what to collapse
7179
* @param options.shouldCollapseStats - Whether stats data can be skipped
7280
* (true when sparklines won't be shown: narrow terminal, non-TTY, --json)
81+
* @param options.shouldCollapseLifetime - Whether lifetime data can be skipped.
82+
* Defaults to `false` because most output paths need `count`/`userCount`/
83+
* `firstSeen`/`lastSeen`. Only set to `true` when `--json --fields` omits
84+
* all lifetime-dependent fields.
7385
* @returns Array of fields to collapse
7486
*/
7587
export function buildIssueListCollapse(options: {
7688
shouldCollapseStats: boolean;
89+
shouldCollapseLifetime?: boolean;
7790
}): IssueCollapseField[] {
78-
const collapse: IssueCollapseField[] = ["filtered", "lifetime", "unhandled"];
91+
const collapse: IssueCollapseField[] = ["filtered", "unhandled"];
92+
if (options.shouldCollapseLifetime) {
93+
collapse.push("lifetime");
94+
}
7995
if (options.shouldCollapseStats) {
8096
collapse.push("stats");
8197
}
@@ -90,8 +106,10 @@ export function buildIssueListCollapse(options: {
90106
* in detail views (`issue view`, `issue explain`, `issue plan`).
91107
* Collapsing these skips expensive Snuba queries, saving 100-300ms per request.
92108
*
93-
* Note: `count`, `userCount`, `firstSeen`, `lastSeen` are top-level fields
94-
* and remain unaffected by collapsing.
109+
* Note: On the **list** endpoint, `collapse=lifetime` strips top-level
110+
* `count`, `userCount`, `firstSeen`, `lastSeen` (see #969). The detail
111+
* endpoint preserves these fields regardless of collapse — safe to include
112+
* `lifetime` here.
95113
*/
96114
export const ISSUE_DETAIL_COLLAPSE: IssueCollapseField[] = [
97115
"stats",

test/commands/issue/list.test.ts

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ type ListFlags = {
4141
readonly period: TimeRange;
4242
readonly json: boolean;
4343
readonly cursor?: string;
44+
readonly fields?: string[];
45+
readonly fresh?: boolean;
46+
readonly compact?: boolean;
4447
};
4548

4649
/** Command function type extracted from loader result */
@@ -1110,7 +1113,7 @@ describe("issue list: collapse parameter optimization", () => {
11101113
advancePaginationStateSpy.mockRestore();
11111114
});
11121115

1113-
test("always collapses filtered, lifetime, unhandled in org-all mode", async () => {
1116+
test("always collapses filtered and unhandled in org-all mode", async () => {
11141117
listIssuesPaginatedSpy.mockResolvedValue({
11151118
data: [sampleIssue],
11161119
nextCursor: undefined,
@@ -1134,10 +1137,125 @@ describe("issue list: collapse parameter optimization", () => {
11341137
const options = callArgs?.[2] as Record<string, unknown> | undefined;
11351138
const collapse = options?.collapse as string[];
11361139
expect(collapse).toContain("filtered");
1137-
expect(collapse).toContain("lifetime");
11381140
expect(collapse).toContain("unhandled");
11391141
});
11401142

1143+
test("does not collapse lifetime in human mode (needed for EVENTS/USERS/SEEN/AGE)", async () => {
1144+
listIssuesPaginatedSpy.mockResolvedValue({
1145+
data: [sampleIssue],
1146+
nextCursor: undefined,
1147+
});
1148+
1149+
const orgAllFunc = (await listCommand.loader()) as unknown as (
1150+
this: unknown,
1151+
flags: Record<string, unknown>,
1152+
target?: string
1153+
) => Promise<void>;
1154+
1155+
const { context } = createOrgAllContext();
1156+
await orgAllFunc.call(
1157+
context,
1158+
{ limit: 10, sort: "date", period: parsePeriod("90d"), json: false },
1159+
"my-org/"
1160+
);
1161+
1162+
expect(listIssuesPaginatedSpy).toHaveBeenCalled();
1163+
const callArgs = listIssuesPaginatedSpy.mock.calls[0];
1164+
const options = callArgs?.[2] as Record<string, unknown> | undefined;
1165+
const collapse = options?.collapse as string[];
1166+
expect(collapse).not.toContain("lifetime");
1167+
});
1168+
1169+
test("does not collapse lifetime in JSON mode without --fields", async () => {
1170+
listIssuesPaginatedSpy.mockResolvedValue({
1171+
data: [sampleIssue],
1172+
nextCursor: undefined,
1173+
});
1174+
1175+
const orgAllFunc = (await listCommand.loader()) as unknown as (
1176+
this: unknown,
1177+
flags: Record<string, unknown>,
1178+
target?: string
1179+
) => Promise<void>;
1180+
1181+
const { context } = createOrgAllContext();
1182+
await orgAllFunc.call(
1183+
context,
1184+
{ limit: 10, sort: "date", period: parsePeriod("90d"), json: true },
1185+
"my-org/"
1186+
);
1187+
1188+
expect(listIssuesPaginatedSpy).toHaveBeenCalled();
1189+
const callArgs = listIssuesPaginatedSpy.mock.calls[0];
1190+
const options = callArgs?.[2] as Record<string, unknown> | undefined;
1191+
const collapse = options?.collapse as string[];
1192+
expect(collapse).not.toContain("lifetime");
1193+
});
1194+
1195+
test("does not collapse lifetime in JSON mode when --fields includes lifetime-dependent field", async () => {
1196+
listIssuesPaginatedSpy.mockResolvedValue({
1197+
data: [sampleIssue],
1198+
nextCursor: undefined,
1199+
});
1200+
1201+
const orgAllFunc = (await listCommand.loader()) as unknown as (
1202+
this: unknown,
1203+
flags: Record<string, unknown>,
1204+
target?: string
1205+
) => Promise<void>;
1206+
1207+
const { context } = createOrgAllContext();
1208+
await orgAllFunc.call(
1209+
context,
1210+
{
1211+
limit: 10,
1212+
sort: "date",
1213+
period: parsePeriod("90d"),
1214+
json: true,
1215+
fields: ["shortId", "title", "count"],
1216+
},
1217+
"my-org/"
1218+
);
1219+
1220+
expect(listIssuesPaginatedSpy).toHaveBeenCalled();
1221+
const callArgs = listIssuesPaginatedSpy.mock.calls[0];
1222+
const options = callArgs?.[2] as Record<string, unknown> | undefined;
1223+
const collapse = options?.collapse as string[];
1224+
expect(collapse).not.toContain("lifetime");
1225+
});
1226+
1227+
test("collapses lifetime in JSON mode when --fields omits all lifetime-dependent fields", async () => {
1228+
listIssuesPaginatedSpy.mockResolvedValue({
1229+
data: [sampleIssue],
1230+
nextCursor: undefined,
1231+
});
1232+
1233+
const orgAllFunc = (await listCommand.loader()) as unknown as (
1234+
this: unknown,
1235+
flags: Record<string, unknown>,
1236+
target?: string
1237+
) => Promise<void>;
1238+
1239+
const { context } = createOrgAllContext();
1240+
await orgAllFunc.call(
1241+
context,
1242+
{
1243+
limit: 10,
1244+
sort: "date",
1245+
period: parsePeriod("90d"),
1246+
json: true,
1247+
fields: ["shortId", "title"],
1248+
},
1249+
"my-org/"
1250+
);
1251+
1252+
expect(listIssuesPaginatedSpy).toHaveBeenCalled();
1253+
const callArgs = listIssuesPaginatedSpy.mock.calls[0];
1254+
const options = callArgs?.[2] as Record<string, unknown> | undefined;
1255+
const collapse = options?.collapse as string[];
1256+
expect(collapse).toContain("lifetime");
1257+
});
1258+
11411259
test("collapses stats in JSON mode", async () => {
11421260
listIssuesPaginatedSpy.mockResolvedValue({
11431261
data: [sampleIssue],

0 commit comments

Comments
 (0)