Repository navigation
perf(github): cache repository catalog - #207
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe pull request replaces separate GitHub user, organization, and repository loading with a cached repository catalog. It adds shared types and configuration, centralizes token handling, introduces catalog caching and pagination, updates the picker integration, and adds tests for loading, errors, caching, retries, and token safety. ChangesRepository catalog flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AddRepositoryCombobox
participant useRepositoryCatalog
participant getAuthenticatedRepositoryCatalog
participant getCachedGitHubRepositoryCatalog
participant GitHub
User->>AddRepositoryCombobox: Open repository picker
AddRepositoryCombobox->>useRepositoryCatalog: Load catalog
useRepositoryCatalog->>getAuthenticatedRepositoryCatalog: Request catalog
getAuthenticatedRepositoryCatalog->>getCachedGitHubRepositoryCatalog: Validate token and load pages
getCachedGitHubRepositoryCatalog->>GitHub: Fetch /user and missing catalog pages
GitHub-->>getCachedGitHubRepositoryCatalog: User, organization, and repository data
getCachedGitHubRepositoryCatalog-->>getAuthenticatedRepositoryCatalog: Merged catalog
getAuthenticatedRepositoryCatalog-->>useRepositoryCatalog: ActionResult
useRepositoryCatalog-->>AddRepositoryCombobox: Filtered picker data
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #207 +/- ##
==========================================
+ Coverage 70.39% 70.68% +0.29%
==========================================
Files 171 172 +1
Lines 4732 4824 +92
Branches 1265 1279 +14
==========================================
+ Hits 3331 3410 +79
- Misses 1382 1395 +13
Partials 19 19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/github/repository-catalog.ts (1)
409-419: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKnown-status GitHub errors are silently dropped with zero telemetry. Both sites intentionally skip logging raw Axios/catalog errors to avoid leaking the
Authorizationheader (per the comment "Axios config contains Authorization, so response-less errors must never be logged raw" atsrc/lib/actions/github.tsline 100), but end up adding no logging at all for very common failure classes (403/429/500/network) — only genuinely unrecognized errors are logged/reported to Sentry. A sanitized, status-only log line preserves the security goal while restoring observability.
src/lib/github/repository-catalog.ts#L409-L419: when an organization's repository fetch is rejected with a non-401 status, add a token-freelog.warn({ status: getGitHubCatalogErrorStatus(organizationResult.reason) }, 'Skipping organization repositories')(or similar) beforecontinue, instead of silently dropping it.src/lib/actions/github.ts#L86-L121: in theisAxiosErrorand catalog-status branches ofhandleGitHubError, add alog.warn({ status: errorStatus, context }, ...)/ lightweightSentry.captureMessagecall before returning the mapped message, without ever loggingerror.configorerror.response?.dataraw.🤖 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 `@src/lib/github/repository-catalog.ts` around lines 409 - 419, Add sanitized status-only warning telemetry for known GitHub failures without logging raw Axios error objects or response data. In src/lib/github/repository-catalog.ts lines 409-419, update the non-401 rejection path in the organizationRepositoryResults loop to log the derived status before skipping; in src/lib/actions/github.ts lines 86-121, update handleGitHubError’s Axios-error and catalog-status branches to warn or capture a lightweight message with errorStatus and context before returning the mapped message.
🤖 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 `@src/lib/github/repository-catalog.ts`:
- Around line 313-325: Update fetchOrganizationRepositoryCatalog to isolate
failures from fetchAllOrganizations: catch non-401 organization-list errors and
return an empty organization list with no repository results, allowing the
top-level catalog flow to preserve already-fetched user repositories. Preserve
existing 401 propagation and per-organization Promise.allSettled behavior.
---
Nitpick comments:
In `@src/lib/github/repository-catalog.ts`:
- Around line 409-419: Add sanitized status-only warning telemetry for known
GitHub failures without logging raw Axios error objects or response data. In
src/lib/github/repository-catalog.ts lines 409-419, update the non-401 rejection
path in the organizationRepositoryResults loop to log the derived status before
skipping; in src/lib/actions/github.ts lines 86-121, update handleGitHubError’s
Axios-error and catalog-status branches to warn or capture a lightweight message
with errorStatus and context before returning the mapped message.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 175e51ad-0296-4894-af1d-f23fe2dc8bc6
📒 Files selected for processing (19)
src/components/Board/AddRepositoryCombobox.tsxsrc/hooks/board/index.tssrc/hooks/board/useOrganizationData.tssrc/hooks/board/useRepositoryCatalog.tssrc/hooks/board/useRepositoryData.tssrc/lib/actions/github.tssrc/lib/axios-github.tssrc/lib/constants/github.tssrc/lib/github/repository-catalog.tssrc/lib/types/github.tssrc/lib/utils/handle-github-token-missing.tssrc/tests/unit/hooks/board/useOrganizationData.test.tsxsrc/tests/unit/hooks/board/useRepositoryCatalog.test.tsxsrc/tests/unit/hooks/board/useRepositoryData.test.tsxsrc/tests/unit/lib/actions/github-error-code.test.tssrc/tests/unit/lib/actions/github-network-error.test.tssrc/tests/unit/lib/actions/github-pagination.test.tssrc/tests/unit/lib/axios-github.test.tssrc/tests/unit/lib/github/repository-catalog.test.ts
💤 Files with no reviewable changes (5)
- src/tests/unit/hooks/board/useRepositoryData.test.tsx
- src/tests/unit/lib/actions/github-pagination.test.ts
- src/tests/unit/hooks/board/useOrganizationData.test.tsx
- src/hooks/board/useOrganizationData.ts
- src/hooks/board/useRepositoryData.ts
🧪 E2E Coverage Report (Sharded: 12 parallel jobs)
📊 Full report available in workflow artifacts |
- isolate non-auth organization list failures\n- add sanitized GitHub failure telemetry\n- cover partial failure and revoked-token behavior
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/tests/unit/lib/actions/github-network-error.test.ts`:
- Around line 92-94: Update the assertion in the github network error test to
inspect the relevant mock calls directly rather than
JSON.stringify(actionHarness). Serialize the .mock.calls arrays for logWarn and
captureException (and any other relevant mocks) so the rawToken leak check
includes function-invocation arguments.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c50ab3d-d46c-4730-a0fc-493a09bb3b90
📒 Files selected for processing (4)
src/lib/actions/github.tssrc/lib/github/repository-catalog.tssrc/tests/unit/lib/actions/github-network-error.test.tssrc/tests/unit/lib/github/repository-catalog.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/actions/github.ts
- src/lib/github/repository-catalog.ts
- src/tests/unit/lib/github/repository-catalog.test.ts
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Architecture
Verification
Summary by CodeRabbit