fix(initiatives): paginate projects in initiatives get#39
Conversation
The `initiative(id:) { projects { nodes } }` selection relies on the
Linear API's default connection page size of 50 and requests no
`pageInfo`, so `initiatives get` silently truncates an initiative's
project list to its first 50 members. An initiative with more than 50
projects reports a partial page as if it were the full set.
Fetch the initiative's scalar fields and its projects in two steps:
scalars via the original single query, then the `projects` connection
via `paginate_nodes` with `first`/`after` cursors and `pageInfo`. A
single-initiative detail view should return the full membership, so
default to fetching every page while still honoring an explicit
`--limit` / `--all`. The emitted JSON shape (`projects.nodes`) is
unchanged.
Verified against a real initiative with 162 projects: previously
returned 50, now returns all 162.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Hey, thanks for the PR. I will review it ASAP. |
Extract the "default to all pages unless --limit/--all is set" decision into a pure `projects_pagination` helper and unit-test its equivalence classes: unbounded upgrades to all, explicit --limit is preserved, explicit --all passes through, and cursor/page-size fields carry along. The API round-trip and paginate_nodes wiring aren't unit-tested — the crate has no HTTP-mock harness and LINEAR_API_URL is a hardcoded const — so this covers the one piece of new branching logic the fix introduces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Finesssee
left a comment
There was a problem hiding this comment.
Thermo-nuclear review — PR #39
Scope: full diff vs base (src/commands/initiatives.rs only, +119/−10).
Verdict: approve with one small fix before merge
Correct root-cause fix. Reuses paginate_nodes (right layer). Detail-view default-to-all is the right product call. Not spaghetti. File stays healthy.
One contract gap vs the existing nested-pagination pattern should be closed first.
1. Structural regressions
None.
- Nested connection was unpaginated → silent 50-cap. Splitting scalars vs
projectsand driving cursors throughpaginate_nodesis the architecture this codebase already uses (commentsunder issue, cycles under team, etc.). - Emitted JSON shape
projects.nodespreserved — good boundary stability. - No file crossed ~1k; initiatives stays well under.
2. Code judo
Landed well
- Reuse
paginate_nodesinstead of hand-rolled cursor loops. - Pure
projects_pagination+ unit tests over the policy equivalence classes (unbounded → all, explicit limit, explicit all, field carry-through). That's the right place to lock the behavior.
Prefer this small reframe (non-blocking if you keep the local helper)
projects_paginationis reallyPaginationOptions::with_default_all()— the dual of existingwith_default_limit:
// pagination.rs
pub fn with_default_all(&self) -> Self {
let mut options = self.clone();
if !options.all && options.limit.is_none() {
options.all = true;
}
options
}Then get becomes paginate_nodes(..., &pagination.with_default_all(), 100) and the initiatives-local helper + four tests move next to with_default_limit tests (or stay as thin wrappers). Same behavior, one less bespoke concept when the next detail-view hits the same trap.
Not required: merging the two GraphQL round-trips. Nested multi-page isn't one-shot in GraphQL; always-2-queries is simpler than “first page co-fetched then continue.” Latency judo is optional.
3. Spaghetti / branching
No growth. One focused policy function; no feature checks scattered into shared paths.
4. Boundary / contract — fix before merge
Incomplete pagination surface on the projects query
PR query:
query($id: String!, $first: Int, $after: String) {
initiative(id: $id) {
projects(first: $first, after: $after) {
nodes { ... }
pageInfo { hasNextPage endCursor }
}
}
}Canonical nested pattern in this repo (comments.rs / most list commands):
query(..., $first: Int, $after: String, $last: Int, $before: String) {
...
pageInfo { hasNextPage endCursor hasPreviousPage startCursor }
}paginate_nodes will send last/before when the global CLI flags are set (PaginationOptions.before). Global pagination flags are wired for all commands, including initiatives get. As written, --before / reverse page walks are broken or GraphQL-error.
Required for merge: match the comments nested query variable + pageInfo shape (four cursor args + four pageInfo fields). Cheap, deletes a footgun.
Also note (pre-existing, out of scope as you said): initiatives list project counts still silently cap at 50 via bare nested projects { nodes { id } }. Fine to defer; please file/track so it doesn't rot. Prefer projects { nodes { id } totalCount } if the API exposes totalCount — one field, no N+1.
5. File size / modularity
Fine. +119 in one command file is justified; helper + tests earn their keep. No extract-to-new-file needed.
6. Tests
Policy unit tests are high-value and deterministic — good. Agree HTTP mock harness is out of scope for this PR; not a blocker given real 162-project verification in the description.
Approval bar
| Criterion | Status |
|---|---|
| No structural regression | pass |
Obvious simpler framing used (paginate_nodes) |
pass |
| No 1k-line explosion | pass |
| No spaghetti growth | pass |
| Canonical helper reuse | pass |
Nested query complete vs paginate_nodes contract |
fail — add last/before + full pageInfo |
Optional judo: with_default_all on PaginationOptions |
preferred follow-up / nice-in-PR |
Merge after the GraphQL variable/pageInfo parity fix. Everything else is solid.
@oliverbarnes — thanks for the focused PR and the honest out-of-scope note on list counts.
|
Hey @oliverbarnes — thanks again for this. The approach is right and we're ready to merge once this one gap is closed. Please fix before mergeThe nested Currently in the PR: query($id: String!, $first: Int, $after: String) {
initiative(id: $id) {
projects(first: $first, after: $after) {
nodes { id name state progress }
pageInfo { hasNextPage endCursor }
}
}
}Please change to: query($id: String!, $first: Int, $after: String, $last: Int, $before: String) {
initiative(id: $id) {
projects(first: $first, after: $after, last: $last, before: $before) {
nodes { id name state progress }
pageInfo {
hasNextPage
endCursor
hasPreviousPage
startCursor
}
}
}
}Why: global CLI flags ( Optional (nice-to-have, not required to merge)If you want, lift Out of scope (as you noted)
Once the GraphQL parity fix is pushed, ping us and we'll merge. |
…default_all Address review on PR nesszer#39: - Match the canonical nested-connection query shape (as in comments.rs): add `$last`/`$before` variables and `hasPreviousPage`/`startCursor` to pageInfo so `paginate_nodes`' reverse walks (`--before`) work instead of erroring. - Replace the initiatives-local `projects_pagination` helper with a reusable `PaginationOptions::with_default_all()` — the dual of `with_default_limit` — and move its tests alongside the existing pagination tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pushed the parity fix in 306b3b7.
|
|
Thanks as well for accepting the fix @Finesssee! We've ran into this issue in practice, and this fix has been smoke tested with our linear instance in fact |
Problem
initiatives get <id>silently truncates an initiative's project list to 50.The query selects the nested
projectsconnection as a bareprojects { nodes { ... } }, with nofirst:argument and nopageInfo. Linear's GraphQL API defaults connections to a page size of 50, so an initiative with more than 50 projects returns only the first page — and because nopageInfois requested, there's no signal that the result was truncated.--limitis ignored for the nested connection.This makes
initiatives getunreliable for reading full project membership. On a real initiative with 162 projects it returned 50.Fix
Split the fetch into two steps:
projectsconnection via the existingpaginate_nodeshelper, drivingfirst/aftercursors and readingpageInfo { hasNextPage endCursor }.Because a single-initiative detail view should return the full membership, it defaults to fetching every page, while still honoring an explicit
--limit/--allif the caller sets one. The emitted JSON shape (projects.nodes) is unchanged, so existing consumers keep working — they just get the complete list.Tests
The "default to all pages unless
--limit/--allis set" decision is extracted into a pureprojects_paginationhelper with unit tests over its equivalence classes (unbounded → all, explicit--limitpreserved, explicit--allpassed through, cursor/page-size fields carried along).The API round-trip and
paginate_nodeswiring aren't unit-tested: the crate has no HTTP-mock harness (no dev-dependencies,LINEAR_API_URLis a hardcoded const with no override), so there's no in-repo way to assert against a fake GraphQL response without introducing that infrastructure. Happy to add a mock-server harness in a follow-up if you'd like that coverage.Verification
cargo build,cargo clippy, andcargo test(347 + 203 tests) all pass.Notes / out of scope
initiatives listcomputes each row'sProjectscount from the same un-paginated nestedprojects { nodes }selection, so that count is also capped at 50. It's left out of this PR to keep the change focused (an accurate per-row count would add an N+1 pagination pass); happy to follow up if wanted.