From e49c6232a771fa2abab03cc37d176d3c4ef3a487 Mon Sep 17 00:00:00 2001 From: Herve Labas Date: Wed, 17 Jun 2026 16:50:57 +0200 Subject: [PATCH 1/2] perf(cli): project recent-results fields in checks get The default `checks get ` screen only renders a narrow recent-results table, but it was fetching full result bodies (metadata, assets, apiCheckResult, browserCheckResult, ...) from /v2/check-results, which can trigger slow paths for result-heavy checks. Now that the backend supports a `fields` query parameter (checkly/monorepo#2384), request only the columns the table needs. - rest: add CheckResultField union, extend ListCheckResultsParams with `fields?`, and normalize the array to a comma-separated query string in getAll() rather than relying on Axios array serialization. - checks get: pass a fixed internal projection (id, startedAt, runLocation, hasErrors, hasFailures, isDegraded, responseTime) for detail/markdown output. No new public flag is added; the projection is internal and fixed. - JSON output intentionally omits `fields` to preserve full `results` entries for existing consumers. - --result drilldown and the --include-attempts window query are unchanged and still request full payloads. Co-Authored-By: Claude Opus 4.8 --- .../checks-get-results-projection.spec.ts | 160 ++++++++++++++++++ packages/cli/src/commands/checks/get.ts | 31 +++- .../src/rest/__tests__/check-results.spec.ts | 41 +++++ packages/cli/src/rest/check-results.ts | 39 ++++- 4 files changed, 265 insertions(+), 6 deletions(-) create mode 100644 packages/cli/src/commands/__tests__/checks-get-results-projection.spec.ts create mode 100644 packages/cli/src/rest/__tests__/check-results.spec.ts diff --git a/packages/cli/src/commands/__tests__/checks-get-results-projection.spec.ts b/packages/cli/src/commands/__tests__/checks-get-results-projection.spec.ts new file mode 100644 index 000000000..d17c1f39d --- /dev/null +++ b/packages/cli/src/commands/__tests__/checks-get-results-projection.spec.ts @@ -0,0 +1,160 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { CheckResult } from '../../rest/check-results.js' + +vi.mock('../../rest/api.js', () => ({ + checks: { get: vi.fn() }, + checkStatuses: { get: vi.fn() }, + checkResults: { get: vi.fn(), getAll: vi.fn() }, + errorGroups: { getByCheckId: vi.fn(), get: vi.fn() }, + analytics: { get: vi.fn() }, +})) + +import * as api from '../../rest/api.js' +import ChecksGet from '../checks/get.js' + +// The fixed, internal projection the recent-results table needs. Exactly the +// fields formatResults() and resolveResultStatus() read — kept in sync with +// RECENT_RESULTS_FIELDS in checks/get.ts. +const EXPECTED_FIELDS = ['id', 'startedAt', 'runLocation', 'hasErrors', 'hasFailures', 'isDegraded', 'responseTime'] + +function makeCheck () { + return { + id: 'check-1', + name: 'My API Check', + description: null, + checkType: 'API', + activated: true, + muted: false, + frequency: 10, + locations: ['eu-west-1'], + privateLocations: [], + tags: [], + groupId: null, + scriptPath: null, + request: { url: 'https://example.com', method: 'GET' }, + created_at: '2026-01-01T00:00:00.000Z', + updated_at: '2026-01-01T00:00:00.000Z', + } +} + +function makeResult (overrides: Partial = {}): CheckResult { + return { + id: 'r1', + checkId: 'check-1', + name: 'My API Check', + hasFailures: false, + hasErrors: false, + isDegraded: false, + overMaxResponseTime: false, + runLocation: 'eu-west-1', + startedAt: '2026-05-20T08:00:00.000Z', + stoppedAt: '2026-05-20T08:00:04.000Z', + created_at: '2026-05-20T08:00:04.000Z', + responseTime: 4000, + checkRunId: 1, + attempts: 1, + resultType: 'FINAL', + sequenceId: 'seq-1', + ...overrides, + } +} + +function createCommandContext (parsed: unknown) { + const logged: string[] = [] + return Object.assign(Object.create(ChecksGet.prototype), { + parse: vi.fn().mockResolvedValue(parsed), + log: vi.fn((msg?: string) => { + if (msg) logged.push(msg) + }), + style: { outputFormat: undefined, longError: vi.fn() }, + logged, + }) +} + +describe('checks get recent-results projection', () => { + beforeEach(() => { + vi.clearAllMocks() + process.exitCode = undefined + vi.mocked(api.checks.get).mockResolvedValue({ data: makeCheck() } as any) + vi.mocked(api.checkStatuses.get).mockResolvedValue({ data: undefined } as any) + vi.mocked(api.errorGroups.getByCheckId).mockResolvedValue({ data: [] } as any) + vi.mocked(api.analytics.get).mockResolvedValue({ data: undefined } as any) + vi.mocked(api.checkResults.getAll).mockResolvedValue({ + data: { entries: [makeResult()], nextId: null, length: 1 }, + } as any) + }) + + it('requests only the narrow projection for default (detail) output', async () => { + const ctx = createCommandContext({ + args: { id: 'check-1' }, + flags: { 'results-limit': 10, 'output': 'detail', 'stats-range': 'last24Hours' }, + }) + + await ChecksGet.prototype.run.call(ctx as any) + + expect(api.checkResults.getAll).toHaveBeenCalledWith('check-1', { + limit: 10, + nextId: undefined, + fields: EXPECTED_FIELDS, + }) + }) + + it('forwards a results cursor unchanged alongside the projection', async () => { + const ctx = createCommandContext({ + args: { id: 'check-1' }, + flags: { 'results-limit': 25, 'results-cursor': 'cursor-abc', 'output': 'detail', 'stats-range': 'last24Hours' }, + }) + + await ChecksGet.prototype.run.call(ctx as any) + + expect(api.checkResults.getAll).toHaveBeenCalledWith('check-1', { + limit: 25, + nextId: 'cursor-abc', + fields: EXPECTED_FIELDS, + }) + }) + + it('requests the narrow projection for markdown output', async () => { + const ctx = createCommandContext({ + args: { id: 'check-1' }, + flags: { 'results-limit': 10, 'output': 'md', 'stats-range': 'last24Hours' }, + }) + + await ChecksGet.prototype.run.call(ctx as any) + + expect(api.checkResults.getAll).toHaveBeenCalledWith('check-1', { + limit: 10, + nextId: undefined, + fields: EXPECTED_FIELDS, + }) + }) + + it('does not project fields for json output (preserves full results entries)', async () => { + const ctx = createCommandContext({ + args: { id: 'check-1' }, + flags: { 'results-limit': 10, 'output': 'json', 'stats-range': 'last24Hours' }, + }) + + await ChecksGet.prototype.run.call(ctx as any) + + expect(api.checkResults.getAll).toHaveBeenCalledTimes(1) + const callArgs = vi.mocked(api.checkResults.getAll).mock.calls[0][1] + expect(callArgs).not.toHaveProperty('fields') + expect(callArgs).toMatchObject({ limit: 10, nextId: undefined }) + }) + + it('--result drilldown uses the detail endpoint and never lists with projection', async () => { + vi.mocked(api.checkResults.get).mockResolvedValue({ + data: makeResult({ id: 'res-42', resultType: 'FINAL', attempts: 1 }), + } as any) + const ctx = createCommandContext({ + args: { id: 'check-1' }, + flags: { result: 'res-42', output: 'detail' }, + }) + + await ChecksGet.prototype.run.call(ctx as any) + + expect(api.checkResults.get).toHaveBeenCalledWith('check-1', 'res-42') + expect(api.checkResults.getAll).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cli/src/commands/checks/get.ts b/packages/cli/src/commands/checks/get.ts index ff2d71c6e..8cc4d982e 100644 --- a/packages/cli/src/commands/checks/get.ts +++ b/packages/cli/src/commands/checks/get.ts @@ -21,11 +21,22 @@ import { formatAttemptsSection, groupAttemptsBySequence, } from '../../formatters/check-result-detail.js' -import type { CheckResult } from '../../rest/check-results.js' +import type { CheckResult, CheckResultField, ListCheckResultsParams } from '../../rest/check-results.js' import { formatRcaDetail, formatRcaHint, transformErrorGroupForJson } from '../../formatters/rca.js' import { quickRangeValues, type QuickRange, type GroupBy } from '../../rest/analytics.js' import { formatAnalyticsSection } from '../../formatters/analytics.js' +// Internal, fixed projection for the embedded recent-results table. These are +// exactly the fields formatResults() and resolveResultStatus() read; requesting +// the wide result bodies (apiCheckResult, browserCheckResult, metadata, assets, +// …) would make the backend select and decorate payloads this view never +// renders. This is intentionally not a user-facing flag: `checks get` aggregates +// check details, status, analytics, error groups, and results, so a generic +// `--fields` would be ambiguous. +const RECENT_RESULTS_FIELDS: CheckResultField[] = [ + 'id', 'startedAt', 'runLocation', 'hasErrors', 'hasFailures', 'isDegraded', 'responseTime', +] + export default class ChecksGet extends AuthCommand { static hidden = false static readOnly = true @@ -96,13 +107,23 @@ export default class ChecksGet extends AuthCommand { // Fetch check first (need checkType for analytics) const { data: check } = await api.checks.get(args.id) + // The recent-results table only needs a narrow column set, so project to + // those fields and let the backend skip wide result payloads. JSON output + // is the exception: it exposes full `results` entries, so we omit `fields` + // there to preserve backwards compatibility for existing consumers. + const resultsParams: ListCheckResultsParams = { + limit: flags['results-limit'], + nextId: flags['results-cursor'], + } + if (flags.output !== 'json') { + resultsParams.fields = RECENT_RESULTS_FIELDS + } + // Fetch remaining data in parallel const [statusResp, resultsResp, errorGroupsResp, analyticsResp] = await Promise.all([ api.checkStatuses.get(args.id).catch(() => ({ data: undefined })), - api.checkResults.getAll(args.id, { - limit: flags['results-limit'], - nextId: flags['results-cursor'], - }).catch(() => ({ data: { entries: [], nextId: null, length: 0 } })), + api.checkResults.getAll(args.id, resultsParams) + .catch(() => ({ data: { entries: [], nextId: null, length: 0 } })), api.errorGroups.getByCheckId(args.id).catch(() => ({ data: [] })), api.analytics.get(args.id, check.checkType, { quickRange: (flags['stats-range'] ?? 'last24Hours') as QuickRange, diff --git a/packages/cli/src/rest/__tests__/check-results.spec.ts b/packages/cli/src/rest/__tests__/check-results.spec.ts new file mode 100644 index 000000000..7d1bfcade --- /dev/null +++ b/packages/cli/src/rest/__tests__/check-results.spec.ts @@ -0,0 +1,41 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { AxiosInstance } from 'axios' +import CheckResults from '../check-results.js' + +function makeAxiosMock (): AxiosInstance { + return { + get: vi.fn().mockResolvedValue({ data: { entries: [], nextId: null, length: 0 } }), + } as unknown as AxiosInstance +} + +describe('CheckResults.getAll()', () => { + let api: AxiosInstance + let checkResults: CheckResults + + beforeEach(() => { + api = makeAxiosMock() + checkResults = new CheckResults(api) + }) + + it('serializes the fields array as a comma-separated query string', async () => { + await checkResults.getAll('check-1', { limit: 10, fields: ['id', 'startedAt', 'responseTime'] }) + + expect(api.get).toHaveBeenCalledWith('/v2/check-results/check-1', { + params: { limit: 10, fields: 'id,startedAt,responseTime' }, + }) + }) + + it('does not send a fields param when fields is omitted', async () => { + await checkResults.getAll('check-1', { limit: 5 }) + + const [, config] = vi.mocked(api.get).mock.calls[0] + expect((config as any).params.fields).toBeUndefined() + }) + + it('passes other params through untouched', async () => { + await checkResults.getAll('check-1', { from: 100, to: 200, resultType: 'ALL', nextId: 'cursor' }) + + const [, config] = vi.mocked(api.get).mock.calls[0] + expect((config as any).params).toMatchObject({ from: 100, to: 200, resultType: 'ALL', nextId: 'cursor' }) + }) +}) diff --git a/packages/cli/src/rest/check-results.ts b/packages/cli/src/rest/check-results.ts index 3074a4ed4..744dc25eb 100644 --- a/packages/cli/src/rest/check-results.ts +++ b/packages/cli/src/rest/check-results.ts @@ -174,6 +174,37 @@ export interface CheckResultsPage { nextId: string | null } +// Result fields the backend `GET /v2/check-results/{checkId}` endpoint accepts +// for projection via the `fields` query parameter. Requesting a narrow subset +// lets the backend skip selecting and decorating wide payloads (metadata, +// assets, apiCheckResult, browserCheckResult, …) that a given view never needs. +export type CheckResultField = + | 'id' + | 'name' + | 'checkId' + | 'hasFailures' + | 'hasErrors' + | 'isDegraded' + | 'isCancelled' + | 'overMaxResponseTime' + | 'runLocation' + | 'startedAt' + | 'stoppedAt' + | 'created_at' + | 'createdAt' + | 'responseTime' + | 'apiCheckResult' + | 'browserCheckResult' + | 'multiStepCheckResult' + | 'agenticCheckResult' + | 'playwrightCheckResult' + | 'checkRunId' + | 'attempts' + | 'resultType' + | 'sequenceId' + | 'traceId' + | 'errorGroupIds' + export interface ListCheckResultsParams { limit?: number nextId?: string @@ -181,6 +212,7 @@ export interface ListCheckResultsParams { to?: number hasFailures?: boolean resultType?: 'FINAL' | 'ATTEMPT' | 'ALL' + fields?: CheckResultField[] } class CheckResults { @@ -190,7 +222,12 @@ class CheckResults { } getAll (checkId: string, params: ListCheckResultsParams = {}) { - return this.api.get(`/v2/check-results/${checkId}`, { params }) + // Serialize `fields` as a single comma-separated value rather than relying on + // Axios array serialization. The backend accepts both `fields=id,startedAt` + // and repeated `fields=id&fields=startedAt`, and the comma form is the + // safest, most predictable shape across HTTP clients. + const requestParams = { ...params, fields: params.fields?.join(',') } + return this.api.get(`/v2/check-results/${checkId}`, { params: requestParams }) } get (checkId: string, checkResultId: string) { From 8c532cbe6d3d12a383c167c2391af683e46c1a9d Mon Sep 17 00:00:00 2001 From: Herve Labas Date: Thu, 18 Jun 2026 10:29:16 +0200 Subject: [PATCH 2/2] test(cli): give playwright global-files tests the standard timeout The two `test-global-files-bundling-*/should include global files` tests omitted the `DEFAULT_TEST_TIMEOUT` (180s) that every sibling test in playwright-check.spec.ts passes, so they fell back to vitest's 5000ms default. `parseProject()` does real bundling + tar work that exceeds 5s on the slower windows-latest-x64 CI runner, timing out reliably there (it stays just under 5s on Linux, which is why main looked green). Pass the timeout like the rest of the file. Unrelated to the recent-results projection change, but folded in here because it was blocking this PR's required `test - windows-latest-x64` check and a rerun did not clear it. Co-Authored-By: Claude Opus 4.8 --- .../cli/src/constructs/__tests__/playwright-check.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/constructs/__tests__/playwright-check.spec.ts b/packages/cli/src/constructs/__tests__/playwright-check.spec.ts index ac27a74d3..14175921a 100644 --- a/packages/cli/src/constructs/__tests__/playwright-check.spec.ts +++ b/packages/cli/src/constructs/__tests__/playwright-check.spec.ts @@ -358,7 +358,7 @@ describe('PlaywrightCheck', () => { 'teardown.ts', 'tsconfig.playwright.json', ]) - }) + }, DEFAULT_TEST_TIMEOUT) }) describe('test-global-files-bundling-without-projects', () => { @@ -411,7 +411,7 @@ describe('PlaywrightCheck', () => { 'teardown.ts', 'tsconfig.playwright.json', ]) - }) + }, DEFAULT_TEST_TIMEOUT) }) describe('headless', () => {