Skip to content

[#232] Durable rate limiting for view counts#237

Merged
realproject7 merged 3 commits intomainfrom
task/232-durable-view-rate-limit
Mar 17, 2026
Merged

[#232] Durable rate limiting for view counts#237
realproject7 merged 3 commits intomainfrom
task/232-durable-view-rate-limit

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Replaced volatile in-memory IP-based rate limiter with durable Supabase-based session rate limit
  • Counts page_views entries per session_id in the last hour — max 10 per session
  • Survives serverless cold starts (no more in-memory Map)
  • Removes x-forwarded-for dependency (not spoofable)
  • Net -26 lines (removed ~40 lines of in-memory rate limit code)

Test plan

  • First 10 views from a session within an hour → succeed
  • 11th view from same session within an hour → returns 429
  • After cold restart, rate limit still enforced
  • Session dedup still works (same page within 1hr → deduplicated: true)

Fixes #232

🤖 Generated with Claude Code

Replace volatile in-memory IP rate limiter with a durable session-based
rate limit that queries page_views count per session per hour. Survives
serverless cold starts and doesn't rely on x-forwarded-for.

Fixes #232

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The durable rate-limit approach is reasonable, but the new count query currently ignores Supabase errors. If that lookup fails, the endpoint silently skips throttling for the request instead of surfacing a server error.

Findings

  • [medium] The page_views count query used for rate limiting does not check its error result, so database failures disable the limiter instead of returning a 500.
    • File: src/app/api/views/route.ts:79
    • Suggestion: capture error from the select(..., { count: "exact", head: true }) call and return error(Database error: ${dbError.message}, 500) before evaluating recentCount.

Decision

Requesting changes because the new durable limiter currently fails open when the backing count query errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The DB-error handling fix landed, but the durable limiter still introduces a behavior regression and a schema-performance gap. As written, it throttles normal browsing across unrelated stories and backs the new query with no useful index.

Findings

  • [medium] The new rate-limit query counts all page_views for a session_id globally, which is broader than the previous per-storyline/per-page limiter and will 429 normal browsing after 10 distinct views in an hour.
    • File: src/app/api/views/route.ts:79
    • Suggestion: scope the count to at least .eq("storyline_id", storylineId) to preserve prior behavior, and include plot_index if you want to stay aligned with the original per-page key. If the global cap is intentional, it needs to be explicitly documented and the threshold likely needs to be much higher.
  • [medium] The new count query filters on (session_id, viewed_at) but the existing indexes are on storyline_id, (storyline_id, plot_index), (storyline_id, plot_index, session_id, viewed_at), and contract_address. None of those support the new query shape efficiently, so this will degrade into table scans as page_views grows.
    • File: src/app/api/views/route.ts:79
    • File: supabase/migrations/00007_page_views.sql:14
    • Suggestion: add a migration with an index tailored to the new lookup, such as CREATE INDEX idx_page_views_session_rate ON page_views(session_id, viewed_at); or a more selective composite that matches the final scoped query.

Decision

Requesting changes because the durable limiter still regresses request scope and lacks the DB index needed for the new query pattern.

- Add storyline_id filter to rate limit query (prevents blocking users
  who browse many storylines)
- Add index on (session_id, viewed_at) for efficient rate limit counts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

Re-review complete. The durable limiter now scopes counts per session_id and storyline_id, and the new migration adds the missing index for the session-based lookup.

Findings

  • None. The follow-up addresses the previously requested scope and performance issues.

Decision

Approving because the updated rate-limit query now preserves the intended behavior more closely and is backed by a matching database index. CI was still pending at the time of review.

@realproject7 realproject7 merged commit e94a7dc into main Mar 17, 2026
1 check passed
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.

[Tech Debt] Durable rate limiting for view counts (optional)

2 participants