diff --git a/AGENTS.md b/AGENTS.md index 78f4998c..9694d746 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -814,71 +814,70 @@ mock.module("./some-module", () => ({ ### Architecture - -* **API client wraps all errors as CliError subclasses — no raw exceptions escape**: The API client (src/lib/api-client.ts) wraps ALL errors as CliError subclasses (ApiError or AuthError) — no raw exceptions escape. Commands don't need try-catch for error display; the central handler in app.ts formats CliError cleanly. Only add try-catch when a command needs to handle errors specially (e.g., login continuing despite user-info fetch failure). + +* **defaultCommand:help blocks Stricli fuzzy matching for top-level typos**: Stricli's \`defaultCommand: "help"\` in \`app.ts\` routes unrecognized top-level words to the help command, bypassing Stricli's built-in Damerau-Levenshtein fuzzy matching. Fixed: \`resolveCommandPath()\` in \`introspect.ts\` now returns an \`UnresolvedPath\` (with \`kind: "unresolved"\`, \`input\`, and \`suggestions\`) when a path segment doesn't match. It calls \`fuzzyMatch()\` from \`fuzzy.ts\` to produce up to 3 suggestions. \`introspectCommand()\` and \`formatHelpHuman()\` in \`help.ts\` surface these as "Did you mean: X?" messages. Both top-level (\`sentry isseu\`) and subcommand (\`sentry help issue lis\`) typos now get suggestions. JSON output includes a \`suggestions\` array in the error variant. - -* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: Shell completions (\`\_\_complete\`) set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before any imports, which causes \`db/index.ts\` to skip the \`createTracedDatabase\` wrapper (lazy \`require\` of telemetry.ts). This avoids loading \`@sentry/node-core/light\` (~85ms). Completion timing is recorded to \`completion\_telemetry\_queue\` SQLite table via \`queueCompletionTelemetry()\` (~1ms overhead). During normal CLI runs, \`withTelemetry()\` calls \`drainCompletionTelemetry()\` which uses \`DELETE FROM ... RETURNING\` for atomic read+delete, then emits each entry as \`Sentry.metrics.distribution("completion.duration\_ms", ...)\`. Schema version 11 added this table. The fast-path achieves ~60ms dev / ~140ms CI, with a 200ms e2e test budget. + +* **DSN org prefix normalization in arg-parsing.ts**: Sentry DSN hosts encode org IDs as \`oNNNNN\` (e.g., \`o1081365.ingest.us.sentry.io\`). The Sentry API rejects the \`o\`-prefixed form. \`stripDsnOrgPrefix()\` in \`src/lib/arg-parsing.ts\` uses \`/^o(\d+)$/\` to strip the prefix — safe for slugs like \`organic\`. Applied in \`parseOrgProjectArg()\` and \`parseWithSlash()\`, covering all API call paths consuming \`parsed.org\`. - -* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping: Events require org+project in URL path (\`/projects/{org}/{project}/events/{id}/\`). Issues use legacy global endpoint (\`/api/0/issues/{id}/\`) without org context. Traces need only org (\`/organizations/{org}/trace/{traceId}/\`). Two-step lookup for events: fetch issue → extract org/project from response → fetch event. Cross-project event search possible via Discover endpoint \`/organizations/{org}/events/\` with \`query=id:{eventId}\`. + +* **GHCR versioned nightly tags for delta upgrade support**: GHCR nightly distribution uses three tag types: \`:nightly\` (rolling), \`:nightly-\\` (immutable), \`:patch-\\` (delta manifest). Delta patches use zig-bsdiff TRDIFF10 (zstd-compressed), ~50KB vs ~29MB full. Client bspatch via \`Bun.zstdDecompressSync()\`. N-1 patches only, full download fallback, SHA-256 verify, 60% size threshold. npm/Node excluded. Test mocks: use \`mockGhcrNightlyVersion()\` helper. - -* **Sentry CLI authenticated fetch architecture with response caching**: \`createAuthenticatedFetch()\` wraps fetch with auth, 30s timeout, retry (max 2), 401 refresh, and span tracing. Response caching integrates BEFORE auth/retry via \`http-cache-semantics\` (RFC 7234) with filesystem storage at \`~/.sentry/cache/responses/\`. URL-based fallback TTL tiers: immutable (24hr), stable (5min), volatile (60s), no-cache (0). Only GET 2xx cached. \`--fresh\` and \`SENTRY\_NO\_CACHE=1\` bypass cache. Cache cleared on login/logout. \`hasServerCacheDirectives(policy)\` distinguishes \`max-age=0\` from missing headers. + +* **Issue list auto-pagination beyond API's 100-item cap**: Sentry API silently caps \`limit\` at 100 per request. \`listIssuesAllPages()\` auto-paginates using Link headers, bounded by MAX\_PAGINATION\_PAGES (50). \`API\_MAX\_PER\_PAGE\` constant is shared across all paginated consumers. \`--limit\` means total results everywhere (max 1000, default 25). Org-all mode uses \`fetchOrgAllIssues()\`; explicit \`--cursor\` does single-page fetch to preserve cursor chain. - -* **Sentry CLI has two distribution channels with different runtimes**: Sentry CLI ships two ways: (1) Standalone binary via \`Bun.build()\` with \`compile: true\`. (2) npm package via esbuild producing CJS \`dist/bin.cjs\` for Node 22+, with Bun API polyfills from \`script/node-polyfills.ts\`. \`Bun.$\` has NO polyfill — use \`execSync\` instead. \`require()\` in ESM is safe (Bun native, esbuild resolves at bundle time). As of PR #474, SDK is \`@sentry/node-core/light\` (not \`@sentry/bun\`), reducing import cost from ~218ms to ~85ms. + +* **resolveProjectBySlug carries full projectData to avoid redundant getProject calls**: \`resolveProjectBySlug()\` returns \`{ org, project, projectData: SentryProject }\` — the full project object from \`findProjectsBySlug()\`. \`ResolvedOrgProject\` and \`ResolvedTarget\` have optional \`projectData?\` (populated only in project-search path, not explicit/auto-detect). Downstream commands (\`project/view\`, \`project/delete\`, \`dashboard/create\`) use \`projectData\` when available to skip redundant \`getProject()\` API calls (~500-800ms savings). Pattern: \`resolved.projectData ?? await getProject(org, project)\` for callers that need both paths. - -* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: Resolve-target cascade (src/lib/resolve-target.ts) has 5 priority levels: (1) Explicit CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite config defaults, (4) DSN auto-detection, (5) Directory name inference. SENTRY\_PROJECT supports combo notation \`org/project\` — when used, SENTRY\_ORG is ignored. If combo parse fails (e.g. \`org/\`), the entire value is discarded. The \`resolveFromEnvVars()\` helper is injected into all four resolution functions. + +* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import. -### Decision + +* **Sentry CLI fuzzy matching coverage map across subsystems**: Fuzzy matching exists in: (1) Stricli built-in Damerau-Levenshtein for subcommand/flag typos within known route groups, (2) custom \`fuzzyMatch()\` in \`complete.ts\` for dynamic tab-completion using Levenshtein+prefix+contains scoring, (3) custom \`levenshtein()\` in \`platforms.ts\` for platform name suggestions, (4) plural alias detection in \`app.ts\`, (5) \`resolveCommandPath()\` in \`introspect.ts\` uses \`fuzzyMatch()\` from \`fuzzy.ts\` for top-level and subcommand typos — covering both \`sentry \\` and \`sentry help \\`. Static shell tab-completion uses shell-native prefix matching (compgen/\`\_describe\`/fish \`-a\`). - -* **Issue list global limit with fair per-project distribution and representation guarantees**: \`issue list --limit\` is a global total across all detected projects. \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus via cursor resume. \`trimWithProjectGuarantee\` ensures at least 1 issue per project before filling remaining slots. JSON output wraps in \`{ data, hasMore }\` with optional \`errors\` array. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination, keyed by sorted target fingerprint. + +* **Sentry CLI markdown-first formatting pipeline replaces ad-hoc ANSI**: Formatters build CommonMark strings; \`renderMarkdown()\` renders to ANSI for TTY or raw markdown for non-TTY. Key helpers: \`colorTag()\`, \`mdKvTable()\`, \`mdRow()\`, \`mdTableHeader()\` (\`:\` suffix = right-aligned), \`renderTextTable()\`. \`isPlainOutput()\` checks \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`!isTTY\`. Batch path: \`formatXxxTable()\`. Streaming path: \`StreamingTable\` (TTY) or raw markdown rows (plain). Both share \`buildXxxRowCells()\`. - -* **Sentry CLI config dir should stay at ~/.sentry/, not move to XDG**: Config dir stays at \`~/.sentry/\` (not XDG). The readonly DB errors on macOS are from \`sudo brew install\` creating root-owned files. Fixes: (1) bestEffort() makes setup steps non-fatal, (2) tryRepairReadonly() detects root-owned files and prints \`sudo chown\` instructions, (3) \`sentry cli fix\` handles ownership repair. Ownership must be checked BEFORE permissions — root-owned files cause chmod to EPERM. + +* **Sentry issue stats field: time-series controlled by groupStatsPeriod**: The \`stats\` field on issues is \`{ '24h': \[\[ts, count], ...] }\`. Key depends on \`groupStatsPeriod\` param (\`""\`, \`"14d"\`, \`"24h"\`, \`"auto"\`). \`statsPeriod\` controls time window; \`groupStatsPeriod\` controls stats key. \*\*Critical\*\*: \`count\` is period-scoped — \`lifetime.count\` is the true lifetime total. Issue list table uses \`groupStatsPeriod: 'auto'\` for sparkline data. Column order: SHORT ID, ISSUE, SEEN, AGE, TREND, EVENTS, USERS, TRIAGE. TREND auto-hidden when terminal < 100 cols. \`--compact\` tri-state: explicit overrides; \`undefined\` triggers \`shouldAutoCompact(rowCount)\` — compact if \`3N + 3 > termHeight\`, false for non-TTY. Height is \`3N + 3\` (not \`3N + 4\`) because last data row has no trailing separator. -### Gotcha - - -* **brew is not in VALID\_METHODS but Homebrew formula passes --method brew**: Homebrew install: \`isHomebrewInstall()\` detects via Cellar realpath (checked before stored install info). Upgrade command tells users \`brew upgrade getsentry/tools/sentry\`. Formula runs \`sentry cli setup --method brew --no-modify-path\` as post\_install. Version pinning throws 'unsupported\_operation'. Uses .gz artifacts. Tap at getsentry/tools. + +* **Sentry trace-logs API is org-scoped, not project-scoped**: The Sentry trace-logs endpoint (\`/organizations/{org}/trace-logs/\`) is org-scoped, so \`trace logs\` uses \`resolveOrg()\` not \`resolveOrgAndProject()\`. The endpoint is PRIVATE in Sentry source, excluded from the public OpenAPI schema — \`@sentry/api\` has no generated types. The hand-written \`TraceLogSchema\` in \`src/types/sentry.ts\` is required until Sentry makes it public. - -* **Bun mock.module() leaks globally across test files in same process**: Bun's mock.module() replaces modules globally and leaks across test files in the same process. Solution: tests using mock.module() must run in a separate \`bun test\` invocation. In package.json, use \`bun run test:unit && bun run test:isolated\` instead of \`bun test\`. The \`test/isolated/\` directory exists for these tests. This was the root cause of ~100 test failures (getsentry/cli#258). + +* **withAuthGuard returns discriminated Result type, not fallback+onError**: \`withAuthGuard\(fn)\` in \`src/lib/errors.ts\` returns a discriminated Result: \`{ ok: true, value: T } | { ok: false, error: unknown }\`. AuthErrors always re-throw (triggers bin.ts auto-login). All other errors are captured. Callers inspect \`result.ok\` to degrade gracefully. Used across 12+ files. - -* **Making clearAuth() async breaks model-based tests — use non-async Promise\ return instead**: Making \`clearAuth()\` \`async\` breaks fast-check model-based tests — real async yields (macrotasks) during \`asyncModelRun\` cause \`createIsolatedDbContext\` cleanup to interleave. Fix: keep non-async, return \`clearResponseCache().catch(...)\` directly. Model-based tests should NOT await it. Also: model-based tests need explicit timeouts (e.g., \`30\_000\`) — Bun's default 5s causes false failures during shrinking. +### Gotcha - -* **Multiregion mock must include all control silo API routes**: When changing which Sentry API endpoint a function uses, mock routes must be updated in BOTH \`test/mocks/routes.ts\` (single-region) AND \`test/mocks/multiregion.ts\` \`createControlSiloRoutes()\`. Missing the multiregion mock causes 404s in multi-region test scenarios. + +* **Biome lint: Response.redirect() required, nested ternaries forbidden**: Biome lint rules that frequently trip up this codebase: (1) \`useResponseRedirect\`: use \`Response.redirect(url, status)\` not \`new Response\`. (2) \`noNestedTernary\`: use \`if/else\`. (3) \`noComputedPropertyAccess\`: use \`obj.property\` not \`obj\["property"]\`. (4) Max cognitive complexity 15 per function — extract helpers to stay under. - -* **Sentry /users/me/ endpoint returns 403 for OAuth tokens — use /auth/ instead**: The Sentry \`/users/me/\` endpoint returns 403 for OAuth tokens. Use \`/auth/\` instead — it works with ALL token types and lives on the control silo. In the CLI, \`getControlSiloUrl()\` handles routing correctly. \`SentryUserSchema\` (with \`.passthrough()\`) handles the \`/auth/\` response since it only requires \`id\`. + +* **Bugbot flags defensive null-checks as dead code — keep them with JSDoc justification**: Cursor Bugbot and Sentry Seer repeatedly flag two false positives: (1) defensive null-checks as "dead code" — keep them with JSDoc explaining why the guard exists for future safety, especially when removing would require \`!\` assertions banned by \`noNonNullAssertion\`. (2) stderr spinner output during \`--json\` mode — always a false positive since progress goes to stderr, JSON to stdout. Reply explaining the rationale and resolve. - -* **Stricli command context uses this.stdout not this.process.stdout**: In Stricli command \`func()\` handlers, use \`this.stdout\` and \`this.stderr\` directly — NOT \`this.process.stdout\`. The \`SentryContext\` interface has both \`process\` and \`stdout\`/\`stderr\` as separate top-level properties. Test mock contexts typically provide \`stdout\` but not a full \`process\` object, so \`this.process.stdout\` causes \`TypeError: undefined is not an object\` at runtime in tests even though TypeScript doesn't flag it. + +* **Bun mock.module for node:tty requires default export and class stubs**: Bun testing gotchas: (1) \`mock.module()\` for CJS built-ins requires a \`default\` re-export plus all named exports. Missing any causes \`SyntaxError: Export named 'X' not found\`. Always check the real module's full export list. (2) \`Bun.mmap()\` always opens with PROT\_WRITE — macOS SIGKILL on signed Mach-O, Linux ETXTBSY. Fix: use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` in bspatch.ts. (3) Wrap \`Bun.which()\` with optional \`pathEnv\` param for deterministic testing without mocks. - -* **Stricli defaultCommand blends default command flags into route completions**: When a Stricli route map has \`defaultCommand\` set, requesting completions for that route (e.g. \`\["issues", ""]\`) returns both the subcommand names AND the default command's flags/positional completions. This means completion tests that compare against \`extractCommandTree()\` subcommand lists will fail for groups with defaultCommand, since the actual completions include extra entries like \`--limit\`, \`--query\`, etc. Solution: track \`hasDefaultCommand\` in the command tree and skip strict subcommand-matching assertions for those groups. + +* **Use toMatchObject not toEqual when testing resolution results with optional fields**: When \`resolveProjectBySlug()\` or \`resolveOrgProjectTarget()\` adds optional fields (like \`projectData\`) to the return type, tests using \`expect(result).toEqual({ org, project })\` fail because \`toEqual\` requires exact match. Use \`toMatchObject({ org, project })\` instead — it checks the specified subset without failing on extra properties. This affects tests across \`event/view\`, \`log/view\`, \`trace/view\`, and \`trace/list\` test files. ### Pattern - -* **Extract logic from Stricli func() handlers into standalone functions for testability**: Stricli command \`func()\` handlers are hard to unit test because they require full command context setup. To boost coverage, extract flag validation and body-building logic into standalone exported functions (e.g., \`resolveBody()\` extracted from the \`api\` command's \`func()\`). This moved ~20 lines of mutual-exclusivity checks and flag routing from an untestable handler into a directly testable pure function. Property-based tests on the extracted function drove patch coverage from 78% to 97%. The general pattern: keep \`func()\` as a thin orchestrator that calls exported helpers. This also keeps biome complexity under the limit (max 15). + +* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\\` or \`fix/\-\\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message. - -* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\` in whoami.ts and login.ts) must be wrapped in try-catch. If the DB is broken, the cache write shouldn't crash the command when its primary operation already succeeded. In login.ts specifically, \`getCurrentUser()\` failure after token save must not block authentication — wrap in try-catch, log warning to stderr, let login succeed. This differs from \`getUserRegions()\` failure which should \`clearAuth()\` and fail hard (indicates invalid token). + +* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function. - -* **Stricli buildCommand output config injects json flag into func params**: When a Stricli command uses \`output: { json: true, human: formatFn }\`, the framework injects \`--json\` and \`--fields\` flags automatically. The \`func\` handler receives these as its first parameter. Type it explicitly (e.g., \`flags: { json?: boolean }\`) rather than \`\_flags: unknown\` to access the json flag for conditional behavior (e.g., skipping interactive output in JSON mode). The \`human\` formatter runs on the returned \`data\` for non-JSON output. Commands that produce interactive side effects (browser prompts, QR codes) should check \`flags.json\` and skip them when true. + +* **Pagination contextKey must include all query-varying parameters with escaping**: Pagination \`contextKey\` must encode every query-varying parameter (sort, query, period) with \`escapeContextKeyValue()\` (replaces \`|\` with \`%7C\`). Always provide a fallback before escaping since \`flags.period\` may be \`undefined\` in tests despite having a default: \`flags.period ? escapeContextKeyValue(flags.period) : "90d"\`. -### Preference + +* **PR review workflow: reply, resolve, amend, force-push**: PR review workflow: (1) Read unresolved threads via GraphQL, (2) make code changes, (3) run lint+typecheck+tests, (4) create a SEPARATE commit per review round (not amend) for incremental review, (5) push normally, (6) reply to comments via REST API, (7) resolve threads via GraphQL \`resolveReviewThread\`. Only amend+force-push when user explicitly asks or pre-commit hook modified files. - -* **Code style: Array.from() over spread for iterators, allowlist not whitelist**: User prefers \`Array.from(map.keys())\` over \`\[...map.keys()]\` for converting iterators to arrays (avoids intermediate spread). Use "allowlist" terminology instead of "whitelist" in comments and variable names. When a reviewer asks "Why not .filter() here?" — it may be a question, not a change request; the \`for..of\` loop may be intentionally more efficient. Confirm intent before changing. + +* **Stricli optional boolean flags produce tri-state (true/false/undefined)**: Stricli boolean flags with \`optional: true\` (no \`default\`) produce \`boolean | undefined\` in the flags type. \`--flag\` → \`true\`, \`--no-flag\` → \`false\`, omitted → \`undefined\`. This enables auto-detect patterns: explicit user choice overrides, \`undefined\` triggers heuristic. Used by \`--compact\` on issue list. The flag type must be \`readonly field?: boolean\` (not \`readonly field: boolean\`). This differs from \`default: false\` which always produces a defined boolean. - -* **PR workflow: address review comments, resolve threads, wait for CI**: User's PR workflow after creation: (1) Wait for CI checks to pass, (2) Check for unresolved review comments via \`gh api\` for PR review comments, (3) Fix issues in follow-up commits (not amends), (4) Reply to the comment thread explaining the fix, (5) Resolve the thread programmatically via \`gh api graphql\` with \`resolveReviewThread\` mutation, (6) Push and wait for CI again, (7) Final sweep for any remaining unresolved comments. Use \`git notes add\` to attach implementation plans to commits. Branch naming: \`fix/descriptive-slug\` or \`feat/descriptive-slug\`. + +* **Testing Stricli command func() bodies via spyOn mocking**: Stricli/Bun test patterns: (1) Command func tests: \`const func = await cmd.loader()\`, then \`func.call(mockContext, flags, ...args)\`. \`loader()\` return type union causes LSP errors — false positives that pass \`tsc\`. File naming: \`\*.func.test.ts\`. (2) ESM prevents \`vi.spyOn\` on Node built-in exports. Workaround: test subclass that overrides the method calling the built-in. (3) Follow-mode uses \`setTimeout\`-based scheduling; test with \`interceptSigint()\` helper. \`Bun.sleep()\` has no AbortSignal so \`setTimeout\`/\`clearTimeout\` required. diff --git a/src/commands/issue/list.ts b/src/commands/issue/list.ts index 31f1ceb9..0e707a6a 100644 --- a/src/commands/issue/list.ts +++ b/src/commands/issue/list.ts @@ -9,9 +9,11 @@ import type { SentryContext } from "../../context.js"; import { buildOrgAwareAliases } from "../../lib/alias.js"; import { API_MAX_PER_PAGE, + buildIssueListCollapse, findProjectsByPattern, findProjectsBySlug, getProject, + type IssueCollapseField, type IssuesPage, listIssuesAllPages, listIssuesPaginated, @@ -44,6 +46,7 @@ import { import { type IssueTableRow, shouldAutoCompact, + willShowTrend, writeIssueTable, } from "../../lib/formatters/index.js"; import { @@ -133,6 +136,48 @@ const USAGE_HINT = "sentry issue list /"; */ const MAX_LIMIT = 1000; +/** Options returned by {@link buildListApiOptions}. */ +type ListApiOptions = { + /** Fields to collapse (omit) from the API response for performance. */ + collapse: IssueCollapseField[]; + /** Stats period resolution — undefined when stats are collapsed. */ + groupStatsPeriod: "" | "14d" | "24h" | "auto" | undefined; +}; + +/** + * Determine whether stats data should be collapsed (skipped) in the API request. + * + * Stats power the TREND sparkline column, which is only shown when: + * 1. Output is human (not `--json`) — JSON consumers don't render sparklines + * 2. Terminal is wide enough — narrow terminals and non-TTY hide TREND + * + * Collapsing stats avoids expensive Snuba/ClickHouse aggregation queries, + * saving 200-500ms per API request. + * + * @see {@link willShowTrend} for the terminal width threshold logic + */ +function shouldCollapseStats(json: boolean): boolean { + if (json) { + return true; + } + return !willShowTrend(); +} + +/** + * Build the collapse and groupStatsPeriod options for issue list API calls. + * + * When stats are collapsed, groupStatsPeriod is omitted (undefined) since + * the server won't compute stats anyway. This avoids wasted server-side + * processing and makes the request intent explicit. + */ +function buildListApiOptions(json: boolean): ListApiOptions { + const collapseStats = shouldCollapseStats(json); + return { + collapse: buildIssueListCollapse({ shouldCollapseStats: collapseStats }), + groupStatsPeriod: collapseStats ? undefined : "auto", + }; +} + /** * Resolve the effective compact mode from the flag tri-state and issue count. * @@ -502,13 +547,21 @@ async function fetchIssuesForTarget( /** Resume from this cursor (Phase 2 redistribution or next-page resume). */ startCursor?: string; onPage?: (fetched: number, limit: number) => void; + /** Pre-computed API performance options. @see {@link buildListApiOptions} */ + collapse?: IssueCollapseField[]; + /** Stats period resolution — undefined when stats are collapsed. */ + groupStatsPeriod?: "" | "14d" | "24h" | "auto"; } ): Promise { const result = await withAuthGuard(async () => { const { issues, nextCursor } = await listIssuesAllPages( target.org, target.project, - { ...options, projectId: target.projectId, groupStatsPeriod: "auto" } + { + ...options, + projectId: target.projectId, + groupStatsPeriod: options.groupStatsPeriod, + } ); return { target, issues, hasMore: !!nextCursor, nextCursor }; }); @@ -578,6 +631,10 @@ type BudgetFetchOptions = { statsPeriod?: string; /** Per-target cursors from a previous page (compound cursor resume). */ startCursors?: Map; + /** Pre-computed collapse fields for API performance. @see {@link buildListApiOptions} */ + collapse?: IssueCollapseField[]; + /** Stats period resolution — undefined when stats are collapsed. */ + groupStatsPeriod?: "" | "14d" | "24h" | "auto"; }; /** @@ -792,10 +849,12 @@ function nextPageHint(org: string, flags: ListFlags): string { */ async function fetchOrgAllIssues( org: string, - flags: Pick, + flags: Pick, cursor: string | undefined, onPage?: (fetched: number, limit: number) => void ): Promise { + const apiOpts = buildListApiOptions(flags.json); + // When resuming with --cursor, fetch a single page so the cursor chain stays intact. if (cursor) { const perPage = Math.min(flags.limit, API_MAX_PER_PAGE); @@ -805,7 +864,8 @@ async function fetchOrgAllIssues( perPage, sort: flags.sort, statsPeriod: flags.period, - groupStatsPeriod: "auto", + groupStatsPeriod: apiOpts.groupStatsPeriod, + collapse: apiOpts.collapse, }); return { issues: response.data, nextCursor: response.nextCursor }; } @@ -816,7 +876,8 @@ async function fetchOrgAllIssues( limit: flags.limit, sort: flags.sort, statsPeriod: flags.period, - groupStatsPeriod: "auto", + groupStatsPeriod: apiOpts.groupStatsPeriod, + collapse: apiOpts.collapse, onPage, }); return { issues, nextCursor }; @@ -1110,6 +1171,8 @@ async function handleResolvedTargets( ? `Fetching issues from ${targetCount} projects` : "Fetching issues"; + const apiOpts = buildListApiOptions(flags.json); + const { results, hasMore } = await withProgress( { message: `${baseMessage} (up to ${flags.limit})...`, json: flags.json }, (setMessage) => @@ -1121,6 +1184,8 @@ async function handleResolvedTargets( sort: flags.sort, statsPeriod: flags.period, startCursors, + collapse: apiOpts.collapse, + groupStatsPeriod: apiOpts.groupStatsPeriod, }, (fetched) => { setMessage( diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index 1d6b1ce8..c966653a 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -44,9 +44,11 @@ export { rawApiRequest, } from "./api/infrastructure.js"; export { + buildIssueListCollapse, getIssue, getIssueByShortId, getIssueInOrg, + type IssueCollapseField, type IssueSort, type IssuesPage, listIssuesAllPages, diff --git a/src/lib/api/issues.ts b/src/lib/api/issues.ts index e7e96075..2cd75076 100644 --- a/src/lib/api/issues.ts +++ b/src/lib/api/issues.ts @@ -33,6 +33,47 @@ export type IssueSort = NonNullable< NonNullable["sort"] >; +/** + * Collapse options for issue listing, derived from the @sentry/api SDK types. + * Each value tells the server to skip computing that data field, avoiding + * expensive Snuba/ClickHouse queries on the backend. + * + * - `'stats'` — time-series event counts (sparkline data) + * - `'lifetime'` — lifetime aggregate counts (count, userCount, firstSeen) + * - `'filtered'` — filtered aggregate counts + * - `'unhandled'` — unhandled event flag computation + * - `'base'` — base group fields (rarely useful to collapse) + */ +export type IssueCollapseField = NonNullable< + NonNullable["collapse"] +>[number]; + +/** + * Build the `collapse` parameter for issue list API calls. + * + * Always collapses fields the CLI never consumes in issue list: + * `filtered`, `lifetime`, `unhandled`. Conditionally collapses `stats` + * when sparklines won't be rendered (narrow terminal, non-TTY, or JSON). + * + * Matches the Sentry web UI's optimization: the initial page load sends + * `collapse=stats,unhandled` to skip expensive Snuba queries, fetching + * stats in a follow-up request only when needed. + * + * @param options - Context for determining what to collapse + * @param options.shouldCollapseStats - Whether stats data can be skipped + * (true when sparklines won't be shown: narrow terminal, non-TTY, --json) + * @returns Array of fields to collapse + */ +export function buildIssueListCollapse(options: { + shouldCollapseStats: boolean; +}): IssueCollapseField[] { + const collapse: IssueCollapseField[] = ["filtered", "lifetime", "unhandled"]; + if (options.shouldCollapseStats) { + collapse.push("stats"); + } + return collapse; +} + /** * List issues for a project with pagination control. * @@ -59,6 +100,9 @@ export async function listIssuesPaginated( projectId?: number; /** Controls the time resolution of inline stats data. "auto" adapts to statsPeriod. */ groupStatsPeriod?: "" | "14d" | "24h" | "auto"; + /** Fields to collapse (omit) from the response for performance. + * @see {@link buildIssueListCollapse} */ + collapse?: IssueCollapseField[]; } = {} ): Promise> { // When we have a numeric project ID, use the `project` query param (Array) @@ -86,6 +130,7 @@ export async function listIssuesPaginated( sort: options.sort, statsPeriod: options.statsPeriod, groupStatsPeriod: options.groupStatsPeriod, + collapse: options.collapse, }, }); @@ -138,6 +183,9 @@ export async function listIssuesAllPages( startCursor?: string; /** Called after each page is fetched. Useful for progress indicators. */ onPage?: (fetched: number, limit: number) => void; + /** Fields to collapse (omit) from the response for performance. + * @see {@link buildIssueListCollapse} */ + collapse?: IssueCollapseField[]; } ): Promise { if (options.limit < 1) { @@ -161,6 +209,7 @@ export async function listIssuesAllPages( statsPeriod: options.statsPeriod, projectId: options.projectId, groupStatsPeriod: options.groupStatsPeriod, + collapse: options.collapse, }); allResults.push(...response.data); diff --git a/src/lib/formatters/human.ts b/src/lib/formatters/human.ts index 8f4ffaa6..bdab6975 100644 --- a/src/lib/formatters/human.ts +++ b/src/lib/formatters/human.ts @@ -360,7 +360,22 @@ function computeAliasShorthand(shortId: string, projectAlias?: string): string { // Issue Table Helpers /** Minimum terminal width to show the TREND sparkline column. */ -const TREND_MIN_TERM_WIDTH = 100; +export const TREND_MIN_TERM_WIDTH = 100; + +/** + * Whether the TREND sparkline column will be rendered in the issue table. + * + * Returns `true` when the terminal is wide enough (≥ {@link TREND_MIN_TERM_WIDTH}). + * Non-TTY output defaults to 80 columns, which is below the threshold. + * + * Used by the issue list command to decide whether to request stats data + * from the API — when TREND won't be shown, stats can be collapsed to + * save 200-500ms per request. + */ +export function willShowTrend(): boolean { + const termWidth = process.stdout.columns || 80; + return termWidth >= TREND_MIN_TERM_WIDTH; +} /** Lines per issue row in non-compact mode (2-line content + separator). */ const LINES_PER_DEFAULT_ROW = 3; @@ -592,8 +607,7 @@ export function writeIssueTable( options?: { compact?: boolean } ): void { const compact = options?.compact ?? false; - const termWidth = process.stdout.columns || 80; - const showTrend = termWidth >= TREND_MIN_TERM_WIDTH; + const showTrend = willShowTrend(); const columns: Column[] = [ // SHORT ID — primary identifier (+ alias), never shrink diff --git a/test/commands/issue/list.test.ts b/test/commands/issue/list.test.ts index 0cb8edc0..ec156f93 100644 --- a/test/commands/issue/list.test.ts +++ b/test/commands/issue/list.test.ts @@ -979,3 +979,140 @@ describe("issue list: compound cursor resume", () => { expect(output.data.length).toBeGreaterThanOrEqual(1); }); }); + +// --------------------------------------------------------------------------- +// Collapse parameter optimization tests +// --------------------------------------------------------------------------- + +describe("issue list: collapse parameter optimization", () => { + let listIssuesPaginatedSpy: ReturnType; + let setPaginationCursorSpy: ReturnType; + let clearPaginationCursorSpy: ReturnType; + + const sampleIssue = { + id: "1", + shortId: "PROJ-1", + title: "Test Error", + status: "unresolved", + platform: "javascript", + type: "error", + count: "5", + userCount: 2, + lastSeen: "2025-01-01T00:00:00Z", + firstSeen: "2025-01-01T00:00:00Z", + level: "error", + project: { slug: "test-proj" }, + }; + + function createOrgAllContext() { + const stdoutWrite = mock(() => true); + const stderrWrite = mock(() => true); + return { + context: { + stdout: { write: stdoutWrite }, + stderr: { write: stderrWrite }, + cwd: "/tmp", + }, + stdoutWrite, + }; + } + + beforeEach(async () => { + listIssuesPaginatedSpy = spyOn(apiClient, "listIssuesPaginated"); + setPaginationCursorSpy = spyOn( + paginationDb, + "setPaginationCursor" + ).mockReturnValue(undefined); + clearPaginationCursorSpy = spyOn( + paginationDb, + "clearPaginationCursor" + ).mockReturnValue(undefined); + + await setOrgRegion("my-org", DEFAULT_SENTRY_URL); + }); + + afterEach(() => { + listIssuesPaginatedSpy.mockRestore(); + setPaginationCursorSpy.mockRestore(); + clearPaginationCursorSpy.mockRestore(); + }); + + test("always collapses filtered, lifetime, unhandled in org-all mode", async () => { + listIssuesPaginatedSpy.mockResolvedValue({ + data: [sampleIssue], + nextCursor: undefined, + }); + + const orgAllFunc = (await listCommand.loader()) as unknown as ( + this: unknown, + flags: Record, + target?: string + ) => Promise; + + const { context } = createOrgAllContext(); + await orgAllFunc.call( + context, + { limit: 10, sort: "date", json: false }, + "my-org/" + ); + + expect(listIssuesPaginatedSpy).toHaveBeenCalled(); + const callArgs = listIssuesPaginatedSpy.mock.calls[0]; + const options = callArgs?.[2] as Record | undefined; + const collapse = options?.collapse as string[]; + expect(collapse).toContain("filtered"); + expect(collapse).toContain("lifetime"); + expect(collapse).toContain("unhandled"); + }); + + test("collapses stats in JSON mode", async () => { + listIssuesPaginatedSpy.mockResolvedValue({ + data: [sampleIssue], + nextCursor: undefined, + }); + + const orgAllFunc = (await listCommand.loader()) as unknown as ( + this: unknown, + flags: Record, + target?: string + ) => Promise; + + const { context } = createOrgAllContext(); + await orgAllFunc.call( + context, + { limit: 10, sort: "date", json: true }, + "my-org/" + ); + + expect(listIssuesPaginatedSpy).toHaveBeenCalled(); + const callArgs = listIssuesPaginatedSpy.mock.calls[0]; + const options = callArgs?.[2] as Record | undefined; + const collapse = options?.collapse as string[]; + expect(collapse).toContain("stats"); + }); + + test("omits groupStatsPeriod when stats are collapsed (JSON mode)", async () => { + listIssuesPaginatedSpy.mockResolvedValue({ + data: [sampleIssue], + nextCursor: undefined, + }); + + const orgAllFunc = (await listCommand.loader()) as unknown as ( + this: unknown, + flags: Record, + target?: string + ) => Promise; + + const { context } = createOrgAllContext(); + await orgAllFunc.call( + context, + { limit: 10, sort: "date", json: true }, + "my-org/" + ); + + expect(listIssuesPaginatedSpy).toHaveBeenCalled(); + const callArgs = listIssuesPaginatedSpy.mock.calls[0]; + const options = callArgs?.[2] as Record | undefined; + expect(options?.groupStatsPeriod).toBeUndefined(); + }); +}); diff --git a/test/lib/issue-collapse.property.test.ts b/test/lib/issue-collapse.property.test.ts new file mode 100644 index 00000000..eb2136b9 --- /dev/null +++ b/test/lib/issue-collapse.property.test.ts @@ -0,0 +1,76 @@ +/** + * Property-based tests for buildIssueListCollapse. + * + * Verifies invariants that must hold for any configuration of the collapse + * parameter: always-collapsed fields, stats control, and safety constraints. + */ + +import { describe, expect, test } from "bun:test"; +import { boolean, assert as fcAssert, property } from "fast-check"; + +import { buildIssueListCollapse } from "../../src/lib/api/issues.js"; +import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; + +describe("property: buildIssueListCollapse", () => { + test("always collapses filtered, lifetime, unhandled regardless of stats flag", () => { + fcAssert( + property(boolean(), (collapseStats) => { + const result = buildIssueListCollapse({ + shouldCollapseStats: collapseStats, + }); + expect(result).toContain("filtered"); + expect(result).toContain("lifetime"); + expect(result).toContain("unhandled"); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("stats presence is exactly controlled by shouldCollapseStats", () => { + fcAssert( + property(boolean(), (collapseStats) => { + const result = buildIssueListCollapse({ + shouldCollapseStats: collapseStats, + }); + expect(result.includes("stats")).toBe(collapseStats); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("never collapses base (would break all rendering)", () => { + fcAssert( + property(boolean(), (collapseStats) => { + const result = buildIssueListCollapse({ + shouldCollapseStats: collapseStats, + }); + expect(result).not.toContain("base"); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("returns no duplicates", () => { + fcAssert( + property(boolean(), (collapseStats) => { + const result = buildIssueListCollapse({ + shouldCollapseStats: collapseStats, + }); + expect(new Set(result).size).toBe(result.length); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("length is 3 without stats, 4 with stats", () => { + fcAssert( + property(boolean(), (collapseStats) => { + const result = buildIssueListCollapse({ + shouldCollapseStats: collapseStats, + }); + expect(result.length).toBe(collapseStats ? 4 : 3); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +});