fix: page through issues instead of relying on an over-cap limit#351
fix: page through issues instead of relying on an over-cap limit#351cavidelizade wants to merge 1 commit into
Conversation
The issues list endpoint reset any limit above 100 back to 50 instead of clamping to 100, so the many callers that passed 200 to 2000 expecting to fetch a whole project silently got only 50 issues. That showed up as incomplete data on cycle, module and epic detail, saved views, intake, analytics and exports for any project with more than 50 issues. Two parts: - Backend: clamp limit to 100 instead of dropping to 50 when it's over the cap (issue list, drafts, archived). - Web: add issueService.listAll, which pages through in chunks of 100, and switch every "fetch all" caller (and the export modal loop) to it. Closes Devlaner#335 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe API now clamps oversized issue-list limits to 100. A new web-service helper aggregates paginated results, and multiple pages and components use it instead of fixed issue limits. Export pagination also uses batches of 100. ChangesIssue pagination
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Page as Web page
participant Service as issueService.listAll
participant API as Issues API
Page->>Service: request project issues
Service->>API: request 100 issues at offset
API-->>Service: return issue page
Service->>API: request next page when full
API-->>Service: return final page
Service-->>Page: return aggregated issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/pages/IssueDetailPage.tsx (1)
1596-1599: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid expensive full-project refetches after creating a single issue.
Calling
issueService.listAlltriggers a cascade of sequential network requests. While necessary for initial page loads, using it to refresh the list after creating a single work item is highly inefficient and (when awaited) blocks the UI for several seconds on large projects.Replace these post-creation refetches with local state mutations:
apps/web/src/pages/IssueDetailPage.tsx#L1596-L1599: Replace the awaitedlistAllcall with a local state update (setAllIssues), appending the newly created issue after assigning anycycle_idsormodule_ids.apps/web/src/pages/ModuleDetailPage.tsx#L275-L282: InhandleCreateSave, avoid callingrefetchIssues()and instead append the created issue (withresolvedModuleIdadded to itsmodule_ids) directly to theissuesstate.apps/web/src/pages/ViewDetailPage.tsx#L197-L203: InhandleCreateSave, avoid callingrefetchIssues()and instead append the created issue directly to theissuesstate.⚡ Proposed local mutation for IssueDetailPage.tsx
- const refreshedAll = await issueService - .listAll(workspaceSlug, project.id) - .catch(() => null); - if (refreshedAll) setAllIssues(refreshedAll); + const newIssue = { ...created }; + if (data.cycleId) newIssue.cycle_ids = [data.cycleId]; + if (data.moduleId) newIssue.module_ids = [data.moduleId]; + setAllIssues((prev) => [...prev, newIssue]); setSubCreateOpen(false);⚡ Suggested adjustments outside the changed lines
For
ModuleDetailPage.tsx(inhandleCreateSave):if (created?.id) { await moduleService.addIssue(workspaceSlug, data.projectId, resolvedModuleId, created.id); const newIssue = { ...created, module_ids: [...(created.module_ids || []), resolvedModuleId] }; setIssues((prev) => [...prev, newIssue]); } handleCloseCreate(); // Remove the refetchIssues() callFor
ViewDetailPage.tsx(inhandleCreateSave):if (created?.id) { const newIssue = { ...created }; if (data.cycleId) { await cycleService.addIssue(workspaceSlug, data.projectId, data.cycleId, created.id); newIssue.cycle_ids = [data.cycleId]; } if (data.moduleId) { await moduleService.addIssue(workspaceSlug, data.projectId, data.moduleId, created.id); newIssue.module_ids = [data.moduleId]; } setIssues((prev) => [...prev, newIssue]); } handleCloseCreate(); // Remove the refetchIssues() call🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/IssueDetailPage.tsx` around lines 1596 - 1599, Replace post-creation full-project refetches with local issue-state mutations. In apps/web/src/pages/IssueDetailPage.tsx#L1596-L1599, update setAllIssues to append the created issue after applying its cycle_ids and module_ids; in apps/web/src/pages/ModuleDetailPage.tsx#L275-L282, update handleCreateSave to append the created issue with resolvedModuleId in module_ids and remove refetchIssues(); in apps/web/src/pages/ViewDetailPage.tsx#L197-L203, append the created issue after applying any cycleId and moduleId associations and remove refetchIssues().apps/web/src/components/settings/modals/ExportModal.tsx (1)
89-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
issueService.listAllto avoid duplicating pagination logic.Since you just introduced
issueService.listAll, you can reuse it here to replace the manualwhileloop and hardcoded limit, keeping the pagination logic centralized in the service.♻️ Proposed refactor
- // The server caps `limit` at 100, so page through in 100s. - const limit = 100; for (const pid of projectIds) { const proj = projects.find((p) => p.id === pid); - let offset = 0; - while (true) { - const issues = await issueService.list(workspaceSlug, pid, { limit, offset }); - if (!issues.length) break; - for (const i of issues) { - allIssues.push({ - ...i, - project_id: pid, - project_name: proj?.name, - }); - } - if (issues.length < limit) break; - offset += issues.length; - } + const issues = await issueService.listAll(workspaceSlug, pid); + for (const i of issues) { + allIssues.push({ + ...i, + project_id: pid, + project_name: proj?.name, + }); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/settings/modals/ExportModal.tsx` around lines 89 - 107, Replace the manual pagination loop in the export flow with `issueService.listAll`, passing `workspaceSlug` and each project ID. Preserve the existing enrichment of returned issues with `project_id` and `project_name`, and remove the local limit/offset pagination logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/services/issueService.ts`:
- Around line 66-84: Replace the sequential full-dataset retrieval with
backend-scoped or aggregated endpoints. In
apps/web/src/services/issueService.ts:66-84, remove or stop using listAll and
provide bulk/filtered retrieval with cancellation support; update
apps/web/src/components/layout/PageHeader.tsx:95-95 to request only issue_count,
apps/web/src/pages/AnalyticsOverviewPage.tsx:59-59 and
apps/web/src/pages/AnalyticsWorkItemsPage.tsx:129-129 to request backend
analytics aggregates, apps/web/src/pages/CycleDetailPage.tsx:158-162 and 309-314
to fetch only current-cycle or affected issues, and
apps/web/src/pages/CyclesPage.tsx:372-379 and 471-477 to request backend cycle
stats/progress.
---
Nitpick comments:
In `@apps/web/src/components/settings/modals/ExportModal.tsx`:
- Around line 89-107: Replace the manual pagination loop in the export flow with
`issueService.listAll`, passing `workspaceSlug` and each project ID. Preserve
the existing enrichment of returned issues with `project_id` and `project_name`,
and remove the local limit/offset pagination logic.
In `@apps/web/src/pages/IssueDetailPage.tsx`:
- Around line 1596-1599: Replace post-creation full-project refetches with local
issue-state mutations. In apps/web/src/pages/IssueDetailPage.tsx#L1596-L1599,
update setAllIssues to append the created issue after applying its cycle_ids and
module_ids; in apps/web/src/pages/ModuleDetailPage.tsx#L275-L282, update
handleCreateSave to append the created issue with resolvedModuleId in module_ids
and remove refetchIssues(); in apps/web/src/pages/ViewDetailPage.tsx#L197-L203,
append the created issue after applying any cycleId and moduleId associations
and remove refetchIssues().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dfb63589-585d-409d-8e5a-63d70ea39447
📒 Files selected for processing (15)
apps/api/internal/handler/issue.goapps/api/internal/handler/issue_archive.goapps/web/src/components/AddExistingWorkItemModal.tsxapps/web/src/components/layout/PageHeader.tsxapps/web/src/components/settings/modals/ExportModal.tsxapps/web/src/pages/AnalyticsOverviewPage.tsxapps/web/src/pages/AnalyticsWorkItemsPage.tsxapps/web/src/pages/CycleDetailPage.tsxapps/web/src/pages/CyclesPage.tsxapps/web/src/pages/EpicDetailPage.tsxapps/web/src/pages/IntakePage.tsxapps/web/src/pages/IssueDetailPage.tsxapps/web/src/pages/ModuleDetailPage.tsxapps/web/src/pages/ViewDetailPage.tsxapps/web/src/services/issueService.ts
| /** | ||
| * Fetch every issue for a project by paging through `list` in chunks of 100 | ||
| * (the server caps `limit` at 100). Use this instead of passing a large | ||
| * `limit`, which the server silently caps and truncates. | ||
| */ | ||
| async listAll(workspaceSlug: string, projectId: string): Promise<IssueApiResponse[]> { | ||
| const pageSize = 100; | ||
| const all: IssueApiResponse[] = []; | ||
| let offset = 0; | ||
| // Bound the loop so a misbehaving backend can't spin forever. | ||
| for (let page = 0; page < 500; page++) { | ||
| const batch = await this.list(workspaceSlug, projectId, { limit: pageSize, offset }); | ||
| all.push(...batch); | ||
| if (batch.length < pageSize) break; | ||
| offset += pageSize; | ||
| } | ||
| return all; | ||
| }, | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Sequential fetching of the entire issue dataset causes severe latency and connection starvation.
Replacing single bounded requests with a client-side sequential loop (listAll) means projects with thousands of issues will trigger dozens of sequential round-trips. Furthermore, listAll lacks cancellation support (AbortSignal), meaning rapid navigation or unmounting will leave orphaned loops running in the background, starving the browser's connection pool. When triggered simultaneously across multiple projects (e.g., in Analytics), this risks immediate API rate-limiting.
apps/web/src/services/issueService.ts#L66-L84: Provide a bulk retrieval backend endpoint, or parallelize chunk fetches if the total count can be pre-fetched, instead of forcing a sequential client-side loop.apps/web/src/components/layout/PageHeader.tsx#L95-L95: Request an endpoint that returns just theissue_countinstead of downloading the entire issue dataset sequentially.apps/web/src/pages/AnalyticsOverviewPage.tsx#L59-L59: Compute aggregated analytics metrics on the backend rather than pulling all issues for all projects into the client.apps/web/src/pages/AnalyticsWorkItemsPage.tsx#L129-L129: Retrieve aggregated metrics directly from the backend to avoid overwhelming the client with data.apps/web/src/pages/CycleDetailPage.tsx#L158-L162: Fetch only the issues associated with the current cycle (via backend filtering) rather than the complete project dataset.apps/web/src/pages/CycleDetailPage.tsx#L309-L314: Limit the refresh request to issues affected by the cycle completion.apps/web/src/pages/CyclesPage.tsx#L372-L379: Fetch cycle stats/progress from the backend instead of retrieving all project issues for client-side aggregations.apps/web/src/pages/CyclesPage.tsx#L471-L477: Fetch cycle stats/progress from the backend instead of retrieving all project issues for client-side aggregations.
📍 Affects 6 files
apps/web/src/services/issueService.ts#L66-L84(this comment)apps/web/src/components/layout/PageHeader.tsx#L95-L95apps/web/src/pages/AnalyticsOverviewPage.tsx#L59-L59apps/web/src/pages/AnalyticsWorkItemsPage.tsx#L129-L129apps/web/src/pages/CycleDetailPage.tsx#L158-L162apps/web/src/pages/CycleDetailPage.tsx#L309-L314apps/web/src/pages/CyclesPage.tsx#L372-L379apps/web/src/pages/CyclesPage.tsx#L471-L477
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/services/issueService.ts` around lines 66 - 84, Replace the
sequential full-dataset retrieval with backend-scoped or aggregated endpoints.
In apps/web/src/services/issueService.ts:66-84, remove or stop using listAll and
provide bulk/filtered retrieval with cancellation support; update
apps/web/src/components/layout/PageHeader.tsx:95-95 to request only issue_count,
apps/web/src/pages/AnalyticsOverviewPage.tsx:59-59 and
apps/web/src/pages/AnalyticsWorkItemsPage.tsx:129-129 to request backend
analytics aggregates, apps/web/src/pages/CycleDetailPage.tsx:158-162 and 309-314
to fetch only current-cycle or affected issues, and
apps/web/src/pages/CyclesPage.tsx:372-379 and 471-477 to request backend cycle
stats/progress.
Summary
The issues list endpoint reset any
limitabove 100 back to 50 instead of clamping to 100. A lot of the app passes 200 to 2000 expecting to fetch a whole project and filter client-side, so those callers silently got only 50 issues. Cycle, module and epic detail, saved views, intake, analytics and exports all showed incomplete data once a project passed 50 issues.Linked issues
Closes #335
Type of change
fix:)Surface
apps/api/)apps/web/)What changed
limitto 100 instead of dropping to 50 when it's over the cap, in the issue list, drafts and archived handlers.issueService.listAll, which pages through in chunks of 100 until it gets a short page, and switch every "fetch all" caller to it (cycle/module/epic detail, saved views, intake, analytics, add-existing-item, issue detail, page header). Fixed the export modal's own loop, which had the same over-caplimit.Why this approach
The 100 cap looks deliberate (one big unbounded query would be worse), so rather than raise it I kept it and made the callers page.
listAllbounds its loop so a misbehaving backend can't spin forever.Test plan
go build,go vet,go test ./...greennpm run typecheck+npm run lintgreenRollout notes
The page header still fetches all issues just to show a count badge (now correct via pagination). A dedicated count endpoint would be lighter, tracked separately in the performance issues.
AI assistance
Claude Code (Opus 4.8), commits carry aCo-Authored-By:trailerSummary by CodeRabbit