docs: rate limits table with search#2460
Conversation
unatasha8
changed the title
|
|
||
| - **Burst limit**: Maximum requests per second (rps), allowing for short traffic spikes. | ||
| - **Sustained limit**: Maximum requests per minute (rpm), ensuring consistent performance over time. | ||
|
|
There was a problem hiding this comment.
Should we split the technical below into another document? It feels odd to get into the "how" when we are explaining the why above.
There was a problem hiding this comment.
I think if we split it, they won't see/read it... the above is 'what' project rate limits are and below is 'how' they need to implement them
Update bucket naming 'threshold' definitions to match new definitions
|
Hey Una, Here's the banner text, along with a proposal for a small restructuring that I think makes the overall navigation cleaner. Proposed structure Three pages:
The key benefit: since New visitors coming via search will also land on the overview first, which means everyone gets context on old vs. new before diving into the details. The banners on the detail pages are then a secondary safety net for anyone who arrives there directly. Banner text for
Banner text for
Both banners: One open item on the parent page: I included a note that customers can check which system applies to them in the Ory Console under Settings → Rate Limits. That's just an idea for now — would actually be quite simple to implement and a nice touch, but it's not there yet. I've marked it as a placeholder so we can either build it or drop the reference before publishing. Let me know if the restructuring works for you! |
…allout to top of files
tricky42
requested review from
aeneasr,
piotrmsc,
vinckr and
zepatrik
as code owners
There was a problem hiding this comment.
Pull request overview
This PR updates the Ory Network rate limits documentation to support a new “bucket-based” rate limiting policy rollout, and introduces an interactive rate limits table (with filtering/search) powered by CSV → JSON generation during the Docusaurus build.
Changes:
- Add a build-time plugin + CSV-backed data pipeline that generates
src/static/rate-limits.jsonfor client-side consumption. - Introduce a
RateLimitsTableReact component with tier/env selection and API path search/highlighting. - Restructure rate limit docs into “new/legacy/project/endpoint” pages and update sidebars accordingly.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/static/rate-limits.json | Generated/static rate limits dataset consumed by the table UI. |
| src/sidebar-old.ts | Adds nested “Rate limits” sidebar structure in the legacy sidebar config. |
| src/plugins/docusaurus-rate-limits-data/index.ts | New Docusaurus plugin to generate rate-limits.json from CSV inputs. |
| src/plugins/docusaurus-polyfill/index.js | Updates webpack polyfills/fallbacks (currently has a config bug). |
| src/lib/rate-limits/types.ts | Adds typed domain model + CSV normalization maps. |
| src/lib/rate-limits/provider.ts | Adds provider indirection (default CSV provider). |
| src/lib/rate-limits/index.ts | Barrel exports for rate limits lib. |
| src/lib/rate-limits/data/bucket-to-threshold.csv | Adds/updates rate limit thresholds per bucket/tier/env. |
| src/lib/rate-limits/data/bucket-to-endpoints.csv | Adds/updates endpoint → bucket mappings. |
| src/lib/rate-limits/csv-provider.ts | Implements the CSV provider for endpoints/thresholds. |
| src/components/RateLimitsTable/types.ts | UI-side types for the table data. |
| src/components/RateLimitsTable/index.tsx | New interactive table component with tier/env selection + search. |
| sidebars.ts | Adds nested “Rate limits” sidebar structure in the main sidebar config. |
| package.json | Adds @docusaurus/plugin-svgr and updates axios version. |
| package-lock.json | Lockfile updates from dependency changes. |
| docusaurus.config.ts | Registers the new data plugin and enables svgr plugin usage. |
| docs/guides/rate-limits.mdx | Rewrites the top-level rate limits page for the migration and new structure. |
| docs/guides/rate-limits-project.mdx | Adds “Project rate limits” page embedding the interactive table. |
| docs/guides/rate-limits-new.mdx | Adds “new policy” page and guidance (headers, 429 handling, load testing). |
| docs/guides/rate-limits-legacy.mdx | Restores legacy policy details as a separate page. |
| docs/guides/rate-limit-endpoint.mdx | Adds endpoint-based rate limits page (inflight/volumetric overview). |
| docs/guides/load-performance-testing.mdx | Adds load/performance testing policy page and sidebar entry. |
| .prettierignore | Ignores rate-limits.json for formatting. |
Comments suppressed due to low confidence (1)
src/plugins/docusaurus-polyfill/index.js:32
configureWebpackreturns an object with tworesolvekeys; in JavaScript the latter overwrites the former, so thebufferfallback (and any other settings added earlier) are dropped. Merge these into a singleresolveobject (e.g., oneresolve.fallbackcontaining bothbufferandpath) to ensure both polyfills are applied.
resolve: {
fallback: {
path: require.resolve("path-browserify"),
},
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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 ignored due to path filters (1)
📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThis PR adds a rate-limits display system: domain types and provider, a CSV-backed provider, a Docusaurus plugin that emits src/static/rate-limits.json at build time, a React RateLimitsTable component that reads that JSON, and site/config changes to integrate the feature. ChangesRate Limits Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/plugins/docusaurus-polyfill/index.js (1)
8-33:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMerge duplicate
resolveblocks.The webpack configuration contains two
resolveblocks (lines 13-17 and lines 28-32) at the same level of the returned object. JavaScript objects cannot have duplicate keys; the second declaration overwrites the first, causing thebufferfallback to be lost.🔧 Proposed fix to merge resolve.fallback entries
return { mergeStrategy: { "module.rules": "prepend" }, plugins: [ new NodePolyfillPlugin(), ], resolve: { fallback: { buffer: require.resolve("buffer/"), + path: require.resolve("path-browserify"), }, }, module: { rules: [ { test: /\.m?js$/, resolve: { fullySpecified: false, }, }, ], }, - resolve: { - fallback: { - path: require.resolve("path-browserify"), - }, - }, }🤖 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/plugins/docusaurus-polyfill/index.js` around lines 8 - 33, The returned webpack config currently declares two duplicate resolve blocks causing the first fallback (buffer) to be overwritten by the second (path); merge them into a single resolve object so resolve.fallback contains both entries. Edit the object returned in the function that builds the config (the block with mergeStrategy, plugins: [new NodePolyfillPlugin()], module.rules) and replace the duplicate resolve declarations with one resolve: { fallback: { buffer: require.resolve("buffer/"), path: require.resolve("path-browserify") } } so both fallbacks are preserved.
🧹 Nitpick comments (2)
src/components/RateLimitsTable/types.ts (1)
10-25: ⚡ Quick winUse shared
Tier/Envcontracts instead of broadstringfields.At Line 12–13,
tierandenvare typed asstring, which drops compile-time guarantees from the source contract and makes drift easier.Proposed refactor
+import type { Env, Tier } from "../../lib/rate-limits/types" + export interface EndpointRow { method: string path: string bucket: string } export interface ThresholdRow { bucket: string - tier: string - env: string + tier: Tier + env: Env rpm: number rps: number } @@ -export type Tier = "Developer" | "Production" | "Growth" | "Enterprise" -export type Env = "Development" | "Staging" | "Production"🤖 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/components/RateLimitsTable/types.ts` around lines 10 - 25, ThresholdRow currently uses broad string types for tier and env which breaks compile-time guarantees; update ThresholdRow to use the shared Tier and Env contract types instead of string by importing the canonical Tier and Env types (or re-exported aliases) and changing the properties to use those types (ThresholdRow.tier and ThresholdRow.env). Make the same substitution anywhere ThresholdRow is consumed (e.g., RateLimitsData) so the module consistently uses the shared Tier/Env contracts.src/static/rate-limits.json (1)
1-1: ⚡ Quick winConsider excluding generated rate-limits.json from version control.
According to the PR context, this file is generated by the
docusaurus-rate-limits-dataplugin at build time. Checking in generated artifacts can lead to:
- Sync issues if CSV sources change but this JSON isn't regenerated
- Merge conflicts when multiple developers work on rate-limits
- Confusion about the source of truth
If this file is indeed generated at build time, consider adding it to
.gitignore:# .gitignore +src/static/rate-limits.jsonIf the file needs to be checked in as a fallback for development, document this decision clearly in the README or plugin documentation.
🤖 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/static/rate-limits.json` at line 1, The checked-in generated artifact src/static/rate-limits.json should be excluded from version control; remove it from the repo index (git rm --cached src/static/rate-limits.json), add src/static/rate-limits.json to .gitignore, and if you must keep a fallback copy, document that exception in the README or the docusaurus-rate-limits-data plugin docs so contributors know when to regenerate it.
🤖 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/components/RateLimitsTable/index.tsx`:
- Around line 23-35: The effect in the RateLimitsTable component uses fetch
without aborting, so create an AbortController inside the useEffect, pass
controller.signal to fetch (the call in the effect that uses baseUrl), and in
the cleanup function call controller.abort() to cancel in-flight requests on
unmount or re-run; update the .catch handler to ignore AbortError (or check
error.name === 'AbortError') before calling setError, and ensure
setLoading(false) still runs appropriately (e.g., in finally but guarded against
aborted requests) so state setters are not invoked for aborted/stale responses.
In `@src/lib/rate-limits/csv-provider.ts`:
- Around line 97-107: The CSV parsing currently silently coerces unknown
tiers/envs and malformed rpm/rps to defaults; update the mapping logic (where
TIER_FROM_CSV and ENV_FROM_CSV are used and parseInt is applied to rpm/rps) to
validate inputs and fail fast: if TIER_FROM_CSV[r[1]] or ENV_FROM_CSV[r[2]] are
undefined (and the raw value is not a valid Tier/Env), throw a descriptive error
identifying the bucket and invalid value, and for rpm/rps use Number parsing and
reject NaN/negative values (throw an error) instead of falling back to 0 so bad
CSV rows surface immediately. Ensure the thrown errors include the bucket and
the offending CSV cell to aid debugging.
---
Outside diff comments:
In `@src/plugins/docusaurus-polyfill/index.js`:
- Around line 8-33: The returned webpack config currently declares two duplicate
resolve blocks causing the first fallback (buffer) to be overwritten by the
second (path); merge them into a single resolve object so resolve.fallback
contains both entries. Edit the object returned in the function that builds the
config (the block with mergeStrategy, plugins: [new NodePolyfillPlugin()],
module.rules) and replace the duplicate resolve declarations with one resolve: {
fallback: { buffer: require.resolve("buffer/"), path:
require.resolve("path-browserify") } } so both fallbacks are preserved.
---
Nitpick comments:
In `@src/components/RateLimitsTable/types.ts`:
- Around line 10-25: ThresholdRow currently uses broad string types for tier and
env which breaks compile-time guarantees; update ThresholdRow to use the shared
Tier and Env contract types instead of string by importing the canonical Tier
and Env types (or re-exported aliases) and changing the properties to use those
types (ThresholdRow.tier and ThresholdRow.env). Make the same substitution
anywhere ThresholdRow is consumed (e.g., RateLimitsData) so the module
consistently uses the shared Tier/Env contracts.
In `@src/static/rate-limits.json`:
- Line 1: The checked-in generated artifact src/static/rate-limits.json should
be excluded from version control; remove it from the repo index (git rm --cached
src/static/rate-limits.json), add src/static/rate-limits.json to .gitignore, and
if you must keep a fallback copy, document that exception in the README or the
docusaurus-rate-limits-data plugin docs so contributors know when to regenerate
it.
🪄 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: 3e5fad81-bb9c-43ef-813d-a999bc806f6d
⛔ Files ignored due to path filters (9)
docs/guides/load-performance-testing.mdxis excluded by!**/*.mdxdocs/guides/rate-limit-endpoint.mdxis excluded by!**/*.mdxdocs/guides/rate-limits-legacy.mdxis excluded by!**/*.mdxdocs/guides/rate-limits-new.mdxis excluded by!**/*.mdxdocs/guides/rate-limits-project.mdxis excluded by!**/*.mdxdocs/guides/rate-limits.mdxis excluded by!**/*.mdxpackage-lock.jsonis excluded by!**/package-lock.jsonsrc/lib/rate-limits/data/bucket-to-endpoints.csvis excluded by!**/*.csvsrc/lib/rate-limits/data/bucket-to-threshold.csvis excluded by!**/*.csv
📒 Files selected for processing (14)
.prettierignoredocusaurus.config.tspackage.jsonsidebars.tssrc/components/RateLimitsTable/index.tsxsrc/components/RateLimitsTable/types.tssrc/lib/rate-limits/csv-provider.tssrc/lib/rate-limits/index.tssrc/lib/rate-limits/provider.tssrc/lib/rate-limits/types.tssrc/plugins/docusaurus-polyfill/index.jssrc/plugins/docusaurus-rate-limits-data/index.tssrc/sidebar-old.tssrc/static/rate-limits.json
|
Picked up the Copilot review. Two items applied in this PR (the The remaining three feel out of scope for this PR; want your call on how to handle them:
@wassimoo @unatasha8 what do you prefer? |
@tricky42 Thanks, I have removed the trailing comments, although these copilot comments seemed like nitpicking to me. |
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.com) from the maintainers to push the changes.
Further comments
Summary by CodeRabbit
New Features
Documentation
Chores