Skip to content

Fix rate-limit dialog firing on 200 responses + skip Details enrichment when anon#508

Merged
rainxchzed merged 2 commits into
mainfrom
fix/rate-limit-dialog-and-details-enrichment
May 4, 2026
Merged

Fix rate-limit dialog firing on 200 responses + skip Details enrichment when anon#508
rainxchzed merged 2 commits into
mainfrom
fix/rate-limit-dialog-and-details-enrichment

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented May 4, 2026

Two related fixes for the residual rate-limit dialog on Details screen.

1. `RateLimitInterceptor` was firing global dialog on successful responses

The interceptor parses `X-RateLimit-Remaining` from every direct GitHub response. When the value reaches 0 (anon tipped over the 60/hr boundary), `rateLimitRepository.updateRateLimit` was emitting `rateLimitExhaustedEvent` even on 200 responses. `MainViewModel` collects that event and pops the global `RateLimitDialog` ("Resets in 31 minutes").

Result: user sees the dialog after a successful Details load, because the secondary GitHub enrichment call tipped `remaining` to 0 on its way back with valid data.

Fix: `updateRateLimit` now takes an explicit `notifyExhausted` flag. Interceptor only sets it true on actual error responses (403/429). State tracking (the `StateFlow` that powers the rate-limit indicator chip) still updates on every response. Domain interface signature backward-compatible — default value preserves existing behavior at the type level for any other callers.

2. Details `getRepoStats` enrichment burns anon's 60/hr for openIssues/license

`DetailsRepositoryImpl.getRepoStats` calls backend first (provides stars/forks/downloads), then makes a direct GitHub call to enrich with `openIssues` and `license` fields that backend doesn't expose. For anon users, this single enrichment call is 1/60 of their hourly budget — for a non-critical UI field. Cached stale value falls through gracefully on absence.

Fix: gate the enrichment on `tokenStore.currentToken()`. Signed-in users (5000/hr quota) keep getting enriched stats. Anon users skip it entirely — backend stats only. `openIssues` falls back to stale cache or 0; `license` falls back to stale cache or null. Same UX as if the GitHub call had failed silently.

Better long-term: ask backend to add `openIssues` and `license` to `/v1/repo` so signed-in users don't need this either. Tracked separately.

Test plan

  • Anon, open Details on a fresh repo: backend-only stats render. No global rate-limit dialog. No 60/hr drain on this open.
  • Signed-in, open Details: enrichment fires, openIssues + license populate.
  • Anon, hit GitHub direct rate limit elsewhere (e.g. via existing direct path that's still anon): 403 + exhausted → dialog fires correctly. Bug fix doesn't suppress legit dialog.
  • State chip / `rateLimitState` flow still updates on every response (verify via existing UI affordance if any reads it).

Independent of #505/#506/#507 in scope. Branches off main.

Summary by CodeRabbit

  • Bug Fixes
    • Repository details now display correctly when unauthenticated, using cached information where available
    • Improved rate limit detection and error notification handling

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c51f0754-27f1-4f58-8938-884e800ce1e4

📥 Commits

Reviewing files that changed from the base of the PR and between 3c7e3f2 and 37ab42f.

📒 Files selected for processing (5)
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/interceptor/RateLimitInterceptor.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/RateLimitRepositoryImpl.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/RateLimitRepository.kt
  • feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/di/SharedModule.kt
  • feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt

Walkthrough

This PR enhances rate-limit detection to recognize HTTP 429 (in addition to 403) as rate-limited responses, adds a notifyExhausted parameter to control exhaustion notifications, and gates GitHub repository enrichment in the Details repository on token availability.

Changes

Rate Limit Handling Enhancement

Layer / File(s) Summary
Interface Contract
core/domain/.../RateLimitRepository.kt
updateRateLimit signature expands to accept notifyExhausted: Boolean parameter, defaulting to true only when rateLimitInfo?.isExhausted == true.
Repository Implementation
core/data/.../RateLimitRepositoryImpl.kt
updateRateLimit respects the new notifyExhausted flag; event emission is now conditional on both the flag and non-null rateLimitInfo.
Error Detection & Propagation
core/data/.../RateLimitInterceptor.kt
HTTP 403 or 429 responses are now identified as rate-limited; notifyExhausted is set based on both error status and exhaustion state; exception is thrown only when both conditions hold.

Details Repository Token-Gated GitHub Enrichment

Layer / File(s) Summary
Constructor Dependency
feature/details/data/.../DetailsRepositoryImpl.kt
Adds tokenStore dependency to enable token-aware decision logic.
Conditional Enrichment Logic
feature/details/data/.../DetailsRepositoryImpl.kt
GitHub repository enrichment (fetching openIssues, license) is skipped when tokenStore.currentToken()?.accessToken is absent or blank; cached values are used instead.
DI Wiring
feature/details/data/di/SharedModule.kt
DetailsRepositoryImpl is instantiated with tokenStore = get() injected from the container.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Rate limits tamed with flags so fine,
GitHub calls now check for signs—
Is the token there? Then enrich we shall!
Exhaustion whispers, but we heed the call. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely captures both primary changes: fixing rate-limit dialog on successful responses and skipping Details enrichment for anonymous users.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rate-limit-dialog-and-details-enrichment

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.

❤️ Share
Review rate limit: 6/8 reviews remaining, refill in 8 minutes and 26 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@rainxchzed rainxchzed merged commit 4b5ca92 into main May 4, 2026
1 check passed
@rainxchzed rainxchzed deleted the fix/rate-limit-dialog-and-details-enrichment branch May 4, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant