Added cap for limit #486
Added cap for limit #486
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a configurable upper limit for function search results. The hardcoded upper bound on the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server
participant Config
participant CRUD
Client->>API_Server: search_functions(limit=N)
API_Server->>Config: Get MAX_FUNCTIONS_SEARCH_LIMIT
API_Server->>API_Server: limit = min(N, MAX_FUNCTIONS_SEARCH_LIMIT)
API_Server->>CRUD: search_functions(limit)
CRUD-->>API_Server: functions[]
API_Server-->>Client: functions[] (length ≤ MAX_FUNCTIONS_SEARCH_LIMIT)
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/aci/server/config.py (1)
84-86: Consider making the cap configurable via env var instead of a hard-coded literalA static
MAX_FUNCTIONS_SEARCH_LIMIT = 1000works, but it forces a code change + deploy for any future tuning.
Expose the value throughcheck_and_get_env_variable("SERVER_MAX_FUNCTIONS_SEARCH_LIMIT", default=1000)(or analogous helper) and default to1000when the env var is absent.-# Search limits -MAX_FUNCTIONS_SEARCH_LIMIT = 1000 +# Search limits +MAX_FUNCTIONS_SEARCH_LIMIT = int( + check_and_get_env_variable("SERVER_MAX_FUNCTIONS_SEARCH_LIMIT", default="1000") +)This stays backward-compatible while providing runtime flexibility.
backend/aci/common/schemas/function.py (1)
127-128: Schema now advertises “unbounded” limit—consider aligning docs with runtime capThe
le=1000constraint was removed, but the route still caps results atconfig.MAX_FUNCTIONS_SEARCH_LIMIT. Without an upper bound in the schema, OpenAPI docs will suggest any integer ≥ 1 is acceptable, which might confuse API consumers.Two lightweight options:
- Re-add an upper bound tied to the config value (preferred for self-documentation):
-limit: int = Field(default=100, ge=1, description="Maximum number of Functions per response.") +limit: int = Field( + default=100, + ge=1, + le=config.MAX_FUNCTIONS_SEARCH_LIMIT, + description=f"Maximum number of Functions per response " + f"(<= {config.MAX_FUNCTIONS_SEARCH_LIMIT}).", +)
- Update the field’s
descriptionto explicitly mention the 1000-item cap.Either keeps validation + docs in sync with runtime behaviour.
backend/aci/server/tests/routes/functions/test_functions_search.py (1)
568-589: Nice coverage for the new capThe test correctly patches
config.MAX_FUNCTIONS_SEARCH_LIMITand asserts the cap is enforced across all formats. Solid addition. One nit: if any downstream module imported the constant withfrom ...config import MAX_FUNCTIONS_SEARCH_LIMIT, the patch would not affect it. That isn’t the case today, but consider patching those symbols explicitly or switching tomonkeypatch.setattrif such imports appear later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/aci/common/schemas/function.py(1 hunks)backend/aci/server/config.py(1 hunks)backend/aci/server/routes/functions.py(1 hunks)backend/aci/server/tests/routes/functions/test_functions_search.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/aci/server/tests/routes/functions/test_functions_search.py (3)
backend/aci/common/enums.py (1)
FunctionDefinitionFormat(73-81)backend/aci/server/tests/conftest.py (3)
test_client(119-124)dummy_functions(277-281)dummy_api_key_1(162-163)backend/aci/common/schemas/function.py (1)
FunctionsSearch(110-142)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Format & Lint
- GitHub Check: Compose Tests
- GitHub Check: Format, Lint, and Test
🔇 Additional comments (1)
backend/aci/server/routes/functions.py (1)
107-115: Capping logic looks correctUsing
min(query_params.limit, config.MAX_FUNCTIONS_SEARCH_LIMIT)cleanly enforces the server-side cap and keeps the caller’s requestedoffset.
No further issues spotted here.
|
✨ No issues found! Your code is sparkling clean! ✨ Need help? Join our Discord for support! |
🏷️ Ticket
notion
📝 Description
[Describe your changes in detail (optional if the issue you linked already contains a
detail description of the change)]
🎥 Demo (if applicable)
📸 Screenshots (if applicable)
✅ Checklist
Summary by CodeRabbit