fix(cli): scope checks list summary bar to account-wide activated state#1291
fix(cli): scope checks list summary bar to account-wide activated state#1291MichaelHogers wants to merge 3 commits intomainfrom
Conversation
Before: the summary bar on `checks list` was computed from an unfiltered `/v1/check-statuses` response combined with a filtered `paginated.total`. The two came from different universes, producing nonsensical output like `6 passing 1 failing (1 total checks)` with `--status=failing`. It also silently counted deactivated checks (which the status endpoint returns despite their inactive state) as "passing", and reacted oddly to pagination. After: the bar is account-wide and never reacts to filters or pagination, matching the webapp's top-level summary cards. Deactivated checks are explicit in their own bucket rather than leaking into passing. Scope is signalled with a dim `Account:` prefix. - Drop the `activeCheckIds` parameter and `(N total checks)` label from `formatSummaryBar`; new signature is `(checks, statuses)`. - Extract `getActivatedStatuses(checks, statuses)` as a pure helper. - In `list.ts` and `stats.ts`, add a parallel `api.checks.fetchAll()` to drive the bar independently of the filtered/paginated table fetch. - Remove the `(N total)` label — total lives in the pagination line only. - Commit sanitized API-response fixtures under `rest/__tests__/__fixtures__/api/` with a scenario-slicing helper and a README documenting the empirical API quirks (status endpoint ignores pagination params, includes inactive checks, per-type shape varies for HEARTBEAT).
- Merge the Account: prefix assertion into the main bucket-counts test — same scenario, no reason to split. - Use .every() instead of a for loop in getActivatedStatuses. - Single regex assertion for zero-hiding instead of three .not.toContain calls. - Rename "shows only inactive bucket" to reflect its actual intent: the regression being guarded is stale passing statuses on deactivated checks leaking into the passing count. - Drop the scenario helper for the empty-input case; [], [] reads clearer.
|
@stefanjudis @thebiglabasky this should fix the awkwardness/confusion that the status bar caused, we now apply the same pattern/behavior as our web-app: always show the account wide status, regardless of filters applied 
|
| search: flags.search, | ||
| status: flags.status, | ||
| }), | ||
| api.checks.fetchAll(), |
There was a problem hiding this comment.
If we fetch all, why also fetching paginated? That looks like quite a waste. I get we're skipping the pagination logic that'd need to be replicated locally, but also 😩
Idk, that feels a bit much.
There was a problem hiding this comment.
Yeah, I need to dig a bit deeper tomorrow
Basically; the status bar requires data that is not efficiently returned via our public api, i'll check tomorrow if there's an easy workaround, extending the public api or simplifying the status bar further
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0848a09f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| search: flags.search, | ||
| status: flags.status, | ||
| }), | ||
| api.checks.fetchAll(), |
There was a problem hiding this comment.
checks list now always calls api.checks.fetchAll() before branching on --output, but JSON/Markdown paths never render the summary bar that needs account-wide data. On large accounts this turns a single paginated request into a full-account scan (many extra API calls), which can noticeably slow scripted --output json usage and increase failure risk from rate limits/timeouts even though the returned payload is unchanged.
Useful? React with 👍 / 👎.
| checkType: flags.type, | ||
| search: flags.search, | ||
| }), | ||
| api.checks.fetchAll(), |
There was a problem hiding this comment.
In the non-explicit-ID path, checks stats now always does api.checks.fetchAll() even when --output json or --output md is used, but those formats never print the terminal summary bar. This introduces unnecessary full-account pagination for common machine-readable workflows, increasing runtime and API load and making otherwise valid filtered stats calls more likely to fail under large-account or rate-limited conditions.
Useful? React with 👍 / 👎.
|
@thebiglabasky on second consideration; i am looking at adding an aggregation endpoint in our public API we can also skip the account wide summary in the CLI let me know what you think/prefer |
I think the endpoint is a good way to go! I assume this would take filtering options too in case one wants a specific kind of summary then? |
yes exactly note that the webapp's home dashboard top bar's card-per-status with counts is completely non-reactive to filters (for a complexity reason i think) we could make the CLI status bar responsive only to the use of |
2 participants
Problem
checks list's summary bar combined unfiltered statuses with the filteredpaginated.total, producing nonsense like6 passing 1 failing (1 total checks)on--status=failing. Same bug instats.ts. Secondary symptom: deactivated checks' stale statuses were silently counted as passing.Fix
Account-wide summary bar, never reacts to filters/pagination — matches the webapp's top cards.
getActivatedStatuses(checks, statuses)excludes stale statuses from deactivated checks.formatSummaryBar(checks, statuses)— new signature, drops the misleading(N total)label and the misusedactiveCheckIdsparam.list.tsandstats.tsadd a parallelchecks.fetchAll()to drive the bar independently of the filtered table.⊘ N inactivebucket because the CLI mixes deactivated into the table (webapp has separate tabs for them).Account wide:prefix makes scope unambiguous when a filter is applied below.Tests
6 new unit tests using committed, sanitized fixtures under
rest/__tests__/__fixtures__/api/(scenario helper + README documenting API quirks we verified on prod).Test plan
./bin/run checks list—Account wide:bar with 4 buckets, no total./bin/run checks list --status failing— bar numbers identical to unfiltered./bin/run checks list --output json— shape unchanged./bin/run checks stats— sameAccount wide:barnpm run testpasses