fix: address PR review feedback for AI Query Builder#205
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 7 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds an AI Query Builder: a Python FastAPI microservice (LangChain/Groq) and verification, a Dashboard API controller and signed forwarding client, web UI components to submit prompts and apply generated filters/sort, tests for signing/verification, and supporting config/workflow updates. ChangesAI Query Builder Feature Implementation
Sequence Diagram(s)sequenceDiagram
participant User as User
participant WebDashboard as Web Dashboard
participant DashboardAPI as Dashboard API
participant PythonService as Python Service
participant Redis as Redis
User->>WebDashboard: Enter prompt in AiQueryBar and submit
WebDashboard->>DashboardAPI: POST /api/projects/:projectId/ai/query-builder (prompt, collectionName)
DashboardAPI->>DashboardAPI: Stringify payload & compute timestamp
DashboardAPI->>DashboardAPI: HMAC-SHA256("timestamp.payload")
DashboardAPI->>PythonService: POST /ai/query-builder with X-Internal-Signature & X-Timestamp
PythonService->>PythonService: Verify GROQ_API_KEY present
PythonService->>PythonService: Verify timestamp freshness (30s) and recompute HMAC
PythonService->>Redis: GET nonce(timestamp, signature)
Redis-->>PythonService: null
PythonService->>Redis: SET nonce with 30s TTL
PythonService->>PythonService: Invoke LangChain ChatGroq -> structured filters & sort
PythonService-->>DashboardAPI: 200 OK with filters[], sort
DashboardAPI->>DashboardAPI: Normalize filters and validate fields/operators
DashboardAPI-->>WebDashboard: 200 OK with validated filters & sort
WebDashboard->>WebDashboard: Update queryParams, reset page to 1 and refetch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 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 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds/finishes the AI Query Builder feature across Dashboard UI + Dashboard API + a new internal FastAPI microservice, with HMAC-signed internal calls and some repo hygiene/doc updates.
Changes:
- Dashboard UI: add an AI query bar to generate filters/sort from natural language (blocked for
users). - Dashboard API: add
/api/projects/:projectId/ai/query-builder, forward to Python service with HMAC signature, and normalize the returned filter payload. - Python microservice: introduce FastAPI app with HMAC verification + replay protection and an AI router that uses Groq/LangChain structured output.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/ISSUES/project-rate-limiting.md | Adds an issue doc describing per-project/API-key rate limiting gaps and proposed fixes. |
| apps/web-dashboard/src/pages/Database.jsx | Wires AI-generated filters/sort into existing query param state. |
| apps/web-dashboard/src/components/Database/DatabaseHeader.jsx | Renders the new AI query bar for non-users collections. |
| apps/web-dashboard/src/components/Database/DatabaseFilter.jsx | Adds _id/createdAt/updatedAt as selectable filter fields. |
| apps/web-dashboard/src/components/Database/AiQueryBar.jsx | New UI component to submit prompts and apply returned filters/sort. |
| apps/python-service/tests/test_auth.py | New tests for internal signature verification + replay protection. |
| apps/python-service/routers/ai.py | New AI endpoint using Groq/LangChain structured output. |
| apps/python-service/requirements.txt | Pins Python service dependencies (FastAPI, LangChain, Redis, etc.). |
| apps/python-service/main.py | FastAPI app entrypoint + router inclusion. |
| apps/python-service/dependencies.py | Implements HMAC verification and Redis-backed replay protection. |
| apps/python-service/config.py | Adds pydantic-settings config for Python service env. |
| apps/python-service/.env.example | Adds example env vars for the Python service. |
| apps/dashboard-api/test_python.js | Adds a local script to manually test HMAC calls to Python service. |
| apps/dashboard-api/src/utils/internalPythonClient.js | Adds HMAC-signed forwarding client to the Python service. |
| apps/dashboard-api/src/routes/ai.routes.js | Adds authenticated AI route wiring under project scope. |
| apps/dashboard-api/src/controllers/ai.controller.js | Adds controller to build schema payload, call Python, and return filters/sort. |
| apps/dashboard-api/src/app.js | Registers the new AI routes under /api/projects/:projectId/ai. |
| apps/dashboard-api/src/tests/internalPythonClient.test.js | Adds unit tests for HMAC signature generation and env handling. |
| .vscode/settings.json | Suppresses Git ignore limit warning in VS Code workspace. |
| .gitignore | Adds Python ignores (__pycache__, venv, pytest cache, etc.). |
| .github/workflows/weekly-changelog.yml | Adjusts frontmatter file creation logic for changelog generation. |
| .env.example | Documents Python service URL + INTERNAL_SECRET + GROQ key placeholders. |
| .aiignore | Adds ignore patterns for env/secrets files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+53
to
+56
| // 4. Return the structured JSON to the frontend | ||
| // Ensure filters is always an array to prevent frontend crash | ||
| const rawFilters = Array.isArray(aiResponse.filters) ? aiResponse.filters : []; | ||
| const safeFilters = rawFilters.filter(f => f && typeof f.field === 'string' && typeof f.operator === 'string' && f.value !== undefined); |
Comment on lines
+16
to
+19
| class QueryResult(BaseModel): | ||
| filters: List[FilterItem] = Field(description="List of MongoDB filters to apply to the frontend") | ||
| sort: str = Field(description="MongoDB sort string, e.g. '-createdAt' or 'name'. Default to '-createdAt'") | ||
|
|
Comment on lines
+66
to
+67
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=str(e)) |
Comment on lines
+1
to
+12
| import pytest | ||
| import hmac | ||
| import hashlib | ||
| import time | ||
| from fastapi import FastAPI, Request | ||
| from fastapi.testclient import TestClient | ||
| from unittest.mock import AsyncMock, patch | ||
|
|
||
| from dependencies import verify_signature | ||
| from config import settings | ||
| from fastapi import Depends | ||
|
|
Comment on lines
+10
to
+25
| const handleSubmit = async (e) => { | ||
| e.preventDefault(); | ||
| if (!prompt.trim()) return; | ||
|
|
||
| setIsLoading(true); | ||
| try { | ||
| const res = await api.post(`/api/projects/${projectId}/ai/query-builder`, { | ||
| collectionName: activeCollection?.name, | ||
| prompt: prompt.trim() | ||
| }); | ||
|
|
||
| if (res.data?.success && res.data?.data) { | ||
| const { filters, sort } = res.data.data; | ||
| onFiltersGenerated(filters, sort); | ||
| toast.success('AI query applied!'); | ||
| setPrompt(''); |
Comment on lines
+56
to
+62
| <input | ||
| type="text" | ||
| value={prompt} | ||
| onChange={(e) => setPrompt(e.target.value)} | ||
| placeholder="Ask AI to filter data..." | ||
| disabled={isLoading || !activeCollection} | ||
| style={{ |
… from user-controlled sources' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/a5a8938b-e932-4e90-ab1f-ef1546aaf66e Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
apps/python-service/routers/ai.py (2)
59-62: ⚡ Quick winConsider adding a timeout to the LLM invocation.
The
chain.ainvokecall has no timeout, so if the Groq API is slow or unresponsive, the request could hang indefinitely. This could exhaust connection pools or cause cascading timeouts upstream.♻️ Suggested improvement using asyncio.wait_for
+import asyncio + # Invoke the chain - result = await chain.ainvoke({ - "schema": str(request.schema_fields), - "user_prompt": request.prompt - }) + try: + result = await asyncio.wait_for( + chain.ainvoke({ + "schema": str(request.schema_fields), + "user_prompt": request.prompt + }), + timeout=30.0 # 30 second timeout + ) + except asyncio.TimeoutError: + raise HTTPException(status_code=504, detail="LLM request timed out")🤖 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 `@apps/python-service/routers/ai.py` around lines 59 - 62, The chain.ainvoke call can hang indefinitely; wrap the call in an asyncio.wait_for with a sensible timeout (e.g., configurable constant) and handle asyncio.TimeoutError to return/raise an appropriate error response; update the code around chain.ainvoke in routers/ai.py (the async handler that calls chain.ainvoke) to import asyncio, call await asyncio.wait_for(chain.ainvoke({...}), TIMEOUT_SECONDS), and add a try/except to catch TimeoutError and log/return a controlled error instead of letting the request hang.
20-22: 💤 Low valueAdd type annotation for
schema_fields.
schema_fields: listlacks a specific type annotation, reducing API documentation clarity and validation. Consider specifying the expected element type.♻️ Suggested improvement
+from typing import List, Union, Any + class QueryBuilderRequest(BaseModel): prompt: str - schema_fields: list + schema_fields: List[str] # or List[dict] if complex schema objects🤖 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 `@apps/python-service/routers/ai.py` around lines 20 - 22, The QueryBuilderRequest model's schema_fields is declared as an untyped list; change it to a typed sequence (e.g., List[str] or list[str]) to improve validation and OpenAPI docs: update the QueryBuilderRequest class to use a concrete element type for schema_fields (and add the necessary typing import such as List from typing if using Python <3.9) so Pydantic knows the expected item type.apps/web-dashboard/src/pages/Database.jsx (1)
159-166: ⚡ Quick winAdd defensive validation for
aiFiltersparameter.While the backend controller is documented to enforce that
filtersis always an Array, the frontend should defensively validate the parameter to prevent potential runtime errors if the API contract changes or returns unexpected data. Line 107 callsqueryParams.filters.forEach, which will throw iffiltersis not an array.🛡️ Proposed validation
const handleFiltersGenerated = (aiFilters, aiSort) => { + if (!Array.isArray(aiFilters)) { + console.warn('AI filters returned non-array value:', aiFilters); + return; + } setQueryParams(prev => ({ ...prev, page: 1, filters: aiFilters, sort: aiSort || prev.sort })); };🤖 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 `@apps/web-dashboard/src/pages/Database.jsx` around lines 159 - 166, handleFiltersGenerated should defensively ensure aiFilters is an Array before writing it into state to avoid runtime errors where queryParams.filters.forEach is called; update handleFiltersGenerated to set filters to Array.isArray(aiFilters) ? aiFilters : (prev.filters || []) when calling setQueryParams and leave sort logic unchanged (refer to handleFiltersGenerated and the setQueryParams call and the later usage of queryParams.filters.forEach).apps/web-dashboard/src/components/Database/AiQueryBar.jsx (1)
72-81: 💤 Low valueConsider extracting the inline
<style>block to avoid DOM duplication.The inline
<style>tag will be duplicated in the DOM for each instance ofAiQueryBar, which can increase memory footprint if multiple instances are rendered. React 19 allows inline styles, but using a module-level CSS file, CSS modules, or a single global stylesheet would be more efficient.♻️ Suggested approach using a separate CSS module
Create
AiQueryBar.module.css:`@keyframes` shimmer { 0% { background-position: -200% 0; } 100% { background-position: 200% 0; } } .aiQueryBar:focus-within { border-color: var(--color-primary); box-shadow: 0 0 0 2px rgba(16, 185, 129, 0.2); }Then import and apply:
+import styles from './AiQueryBar.module.css'; <form onSubmit={handleSubmit} - className="ai-query-bar" + className={styles.aiQueryBar} ... > ... - <style>{`...`}</style> </form>🤖 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 `@apps/web-dashboard/src/components/Database/AiQueryBar.jsx` around lines 72 - 81, Extract the inline <style> block out of the AiQueryBar component into a separate CSS module (e.g., AiQueryBar.module.css) to avoid DOM duplication; move the `@keyframes` shimmer and the focus-within rule there, rename the selector to match a module-safe class (e.g., .aiQueryBar) and then import that module into AiQueryBar.jsx and apply the class via className (replace the literal "ai-query-bar" usage with the imported module class) so the animation and focus styling are provided from the module stylesheet instead of being injected inline for each instance.apps/dashboard-api/test_python.js (1)
5-13: ⚡ Quick winUse environment variables for secret and URL in the test script.
Hardcoded values make local verification brittle and can encourage unsafe secret handling.
♻️ Proposed refactor
const axios = require('axios'); const crypto = require('crypto'); const run = async () => { const timestamp = Date.now().toString(); const payload = JSON.stringify({ prompt: "test", schema_fields: [] }); - const secret = "test-secret"; + const secret = process.env.INTERNAL_SECRET; + const pythonUrl = process.env.PYTHON_SERVICE_URL || 'http://127.0.0.1:8000'; + + if (!secret) { + throw new Error('INTERNAL_SECRET is required'); + } @@ - const res = await axios.post('http://127.0.0.1:8000/ai/query-builder', payload, { + const res = await axios.post(`${pythonUrl}/ai/query-builder`, payload, {🤖 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 `@apps/dashboard-api/test_python.js` around lines 5 - 13, Replace hardcoded secret and URL with environment variables: read the secret into the existing secret variable (e.g., process.env.TEST_SECRET or process.env.AI_TEST_SECRET) and read the request URL to use in axios.post (e.g., process.env.API_URL or process.env.AI_API_URL), validate they exist (throw or exit with a clear message if missing), then keep using those variables when computing signature (signature = crypto.createHmac(..., secret)...) and when calling axios.post (use the URL variable instead of 'http://127.0.0.1:8000/ai/query-builder').
🤖 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 `@apps/dashboard-api/src/controllers/ai.controller.js`:
- Around line 13-24: Validate and normalize inputs before calling
Project.findOne: ensure collectionName and prompt are strings and non-empty
after trimming (e.g., const collectionNameTrim = String(collectionName).trim();
const promptTrim = String(prompt).trim()), throwing AppError(400, ...) if
invalid; validate projectId using mongoose.Types.ObjectId.isValid (or
equivalent) and throw AppError(400, ...) if not a valid ObjectId; then use the
normalized collectionNameTrim and validated projectId in the Project.findOne
selector (replace "collections.name": collectionName with "collections.name":
collectionNameTrim and _id: projectId with the validated ObjectId) and keep the
users check against the trimmed value.
In `@apps/python-service/config.py`:
- Around line 4-5: The INTERNAL_SECRET field currently allows any string; update
the settings validation in the config (the BaseSettings/Settings class that
defines INTERNAL_SECRET) to require a minimum length of at least 32 characters
(e.g., use pydantic.constr(min_length=32) or add a `@validator` for
INTERNAL_SECRET) so weak HMAC keys are rejected; ensure the validation produces
a clear error message and update any tests or instantiation sites that expect a
shorter default.
In `@apps/python-service/dependencies.py`:
- Around line 20-21: Update the except block to preserve the original traceback
by capturing the ValueError and re-raising HTTPException with exception
chaining: change the handler to "except ValueError as e:" and then "raise
HTTPException(status_code=403, detail='Invalid timestamp format') from e" so the
original error context is retained (modify the except ValueError block in
apps/python-service/dependencies.py).
- Around line 41-48: Replace the non-atomic check-then-set nonce logic using
redis_client.get(...) and redis_client.set(...) with a single atomic Redis SET
using NX and EX options (use redis_client.set with nx=True, ex=30 or equivalent)
to ensure only the first request succeeds when creating nonce_key; if the SET
returns a falsy result, raise HTTPException(status_code=403, detail="Replay
attack detected"). Also update the ValueError handler to chain the original
exception when re-raising the HTTPException (e.g., in the except ValueError as
err: block raise HTTPException(...) from err) so ruff B904 is addressed; look
for references to nonce_key, redis_client, and the except ValueError handler to
locate the code.
In `@apps/python-service/requirements.txt`:
- Around line 3-4: The requirements.txt pins vulnerable LangChain packages;
update langchain and langchain-groq to non-vulnerable versions (or remove exact
pins) and re-run a dependency audit; specifically edit
apps/python-service/requirements.txt to replace langchain==0.1.16 and
langchain-groq==0.1.3 with safe versions (or use >= constraints) determined by
running pip-audit, then run project tests and the provided pip_audit command to
confirm advisories are resolved before merging.
In `@apps/python-service/routers/ai.py`:
- Around line 66-67: Replace the bare except block that does "except Exception
as e: raise HTTPException(status_code=500, detail=str(e))" so you preserve the
exception chain and avoid leaking internals: log the caught exception (e.g.,
using the module logger with logger.exception or logger.error(...,
exc_info=True)) and raise a generic HTTPException like
HTTPException(status_code=500, detail="Internal server error") from e to keep
the original traceback. Update the except block in the ai router where
HTTPException is raised to use logging + "from e" and remove str(e) from the
client-facing message.
In `@apps/python-service/tests/test_auth.py`:
- Around line 87-89: In test_replay_attack remove the unused mock_set unpacking
to fix RUF059: change the unpacking of mock_redis in test_replay_attack so only
the used mock_get is extracted (e.g., assign mock_get, _ = mock_redis or just
mock_get = mock_redis[0]) or otherwise replace mock_set with an underscore;
update references to mock_get accordingly in the test_replay_attack function.
In `@apps/web-dashboard/src/components/Database/AiQueryBar.jsx`:
- Around line 56-70: The input in AiQueryBar.jsx (the text field bound to prompt
with onChange calling setPrompt and disabled by isLoading || !activeCollection)
lacks an accessible label; add one by giving the input a unique id and either
adding a <label htmlFor="...">Ask AI to filter data</label> (styled
visually-hidden) or adding an aria-label/aria-labelledby that conveys "Ask AI to
filter data", ensuring the label is present for screen readers and remains
visually hidden if desired.
---
Nitpick comments:
In `@apps/dashboard-api/test_python.js`:
- Around line 5-13: Replace hardcoded secret and URL with environment variables:
read the secret into the existing secret variable (e.g., process.env.TEST_SECRET
or process.env.AI_TEST_SECRET) and read the request URL to use in axios.post
(e.g., process.env.API_URL or process.env.AI_API_URL), validate they exist
(throw or exit with a clear message if missing), then keep using those variables
when computing signature (signature = crypto.createHmac(..., secret)...) and
when calling axios.post (use the URL variable instead of
'http://127.0.0.1:8000/ai/query-builder').
In `@apps/python-service/routers/ai.py`:
- Around line 59-62: The chain.ainvoke call can hang indefinitely; wrap the call
in an asyncio.wait_for with a sensible timeout (e.g., configurable constant) and
handle asyncio.TimeoutError to return/raise an appropriate error response;
update the code around chain.ainvoke in routers/ai.py (the async handler that
calls chain.ainvoke) to import asyncio, call await
asyncio.wait_for(chain.ainvoke({...}), TIMEOUT_SECONDS), and add a try/except to
catch TimeoutError and log/return a controlled error instead of letting the
request hang.
- Around line 20-22: The QueryBuilderRequest model's schema_fields is declared
as an untyped list; change it to a typed sequence (e.g., List[str] or list[str])
to improve validation and OpenAPI docs: update the QueryBuilderRequest class to
use a concrete element type for schema_fields (and add the necessary typing
import such as List from typing if using Python <3.9) so Pydantic knows the
expected item type.
In `@apps/web-dashboard/src/components/Database/AiQueryBar.jsx`:
- Around line 72-81: Extract the inline <style> block out of the AiQueryBar
component into a separate CSS module (e.g., AiQueryBar.module.css) to avoid DOM
duplication; move the `@keyframes` shimmer and the focus-within rule there, rename
the selector to match a module-safe class (e.g., .aiQueryBar) and then import
that module into AiQueryBar.jsx and apply the class via className (replace the
literal "ai-query-bar" usage with the imported module class) so the animation
and focus styling are provided from the module stylesheet instead of being
injected inline for each instance.
In `@apps/web-dashboard/src/pages/Database.jsx`:
- Around line 159-166: handleFiltersGenerated should defensively ensure
aiFilters is an Array before writing it into state to avoid runtime errors where
queryParams.filters.forEach is called; update handleFiltersGenerated to set
filters to Array.isArray(aiFilters) ? aiFilters : (prev.filters || []) when
calling setQueryParams and leave sort logic unchanged (refer to
handleFiltersGenerated and the setQueryParams call and the later usage of
queryParams.filters.forEach).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 064f7a65-498d-4d96-a203-fe4fe8748d04
📒 Files selected for processing (23)
.aiignore.env.example.github/workflows/weekly-changelog.yml.gitignore.vscode/settings.jsonapps/dashboard-api/src/__tests__/internalPythonClient.test.jsapps/dashboard-api/src/app.jsapps/dashboard-api/src/controllers/ai.controller.jsapps/dashboard-api/src/routes/ai.routes.jsapps/dashboard-api/src/utils/internalPythonClient.jsapps/dashboard-api/test_python.jsapps/python-service/.env.exampleapps/python-service/config.pyapps/python-service/dependencies.pyapps/python-service/main.pyapps/python-service/requirements.txtapps/python-service/routers/ai.pyapps/python-service/tests/test_auth.pyapps/web-dashboard/src/components/Database/AiQueryBar.jsxapps/web-dashboard/src/components/Database/DatabaseFilter.jsxapps/web-dashboard/src/components/Database/DatabaseHeader.jsxapps/web-dashboard/src/pages/Database.jsxdocs/ISSUES/project-rate-limiting.md
Agent-Logs-Url: https://github.com/geturbackend/urBackend/sessions/a5a8938b-e932-4e90-ab1f-ef1546aaf66e Co-authored-by: yash-pouranik <172860064+yash-pouranik@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
apps/dashboard-api/src/controllers/ai.controller.js (2)
24-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the
userscollection guard case-insensitive.The guard is case-sensitive; normalizing before compare ensures blocked access consistently returns 403 for
usersvariants.🔧 Proposed fix
- if (safeCollectionName === 'users') { + if (safeCollectionName.toLowerCase() === 'users') { throw new AppError(403, "Cannot query the users collection via AI"); }🤖 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 `@apps/dashboard-api/src/controllers/ai.controller.js` around lines 24 - 25, Normalize safeCollectionName to a consistent case before comparing to 'users' so the guard is case-insensitive: update the conditional that throws AppError(403, "Cannot query the users collection via AI") (referencing safeCollectionName and the AppError throw site in ai.controller.js) to compare a lowercase version of safeCollectionName (e.g., (safeCollectionName || '').toLowerCase()) against 'users'.
10-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
projectIdbeforeProject.findOneto avoid cast-path failures.
projectIdis still used directly in the Mongo selector. Invalid IDs can fall into Mongoose cast/error flow and currently degrade into a generic 500 instead of a clear 400.🔧 Proposed fix
+const mongoose = require('mongoose'); const { Project } = require('`@urbackend/common/src/models`'); const { forwardToPythonService } = require('../utils/internalPythonClient'); const { AppError } = require('`@urbackend/common`'); @@ const { projectId } = req.params; const { collectionName, prompt } = req.body; + + if (!mongoose.Types.ObjectId.isValid(projectId)) { + throw new AppError(400, "Invalid project id"); + }#!/bin/bash # Verify whether ObjectId validation exists before Project.findOne usage. rg -n -C3 "Types\.ObjectId\.isValid|Project\.findOne\(" apps/dashboard-api/src/controllers/ai.controller.jsAs per coding guidelines, “Use AppError class for errors — never raw throw, never expose MongoDB errors to client.”
🤖 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 `@apps/dashboard-api/src/controllers/ai.controller.js` around lines 10 - 31, Validate projectId before calling Project.findOne: check that req.params.projectId is a valid Mongo ObjectId (e.g., using mongoose.Types.ObjectId.isValid) and if not throw new AppError(400, "Invalid projectId") instead of letting Mongoose cast; then proceed to use the validated projectId in Project.findOne({ _id: projectId, owner: req.user._id, "collections.name": safeCollectionName }, { "collections.$": 1 }) so Mongo errors never surface as a 500 and the AppError pattern is preserved.apps/web-dashboard/src/components/Database/AiQueryBar.jsx (1)
59-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd an explicit accessible label for the prompt input.
The input still relies on placeholder text only; add a programmatic label (
aria-labelor<label htmlFor>).♿ Proposed fix
<input + aria-label="Ask AI to filter data" type="text" value={prompt} onChange={(e) => setPrompt(e.target.value)} placeholder="Ask AI to filter data..." disabled={isLoading || !activeCollection || !projectId}🤖 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 `@apps/web-dashboard/src/components/Database/AiQueryBar.jsx` around lines 59 - 64, Add an explicit accessible label for the prompt input in AiQueryBar.jsx: update the input that uses the prompt state and setPrompt handler to include a programmatic label (for example add aria-label="AI query input" or create a <label htmlFor="ai-prompt"> and give the input id="ai-prompt"). Ensure the label is descriptive and remains present when the input is disabled; keep existing props (value={prompt}, onChange={(e) => setPrompt(e.target.value)}, disabled={isLoading || !activeCollection || !projectId}) unchanged.apps/python-service/routers/ai.py (1)
66-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve the original exception chain.
This still rethrows a new
HTTPExceptionwithoutfrom, so the underlying LangChain/Groq traceback is lost when debugging failures. Catch the exception asexcand raise from it.Proposed fix
- except Exception: - raise HTTPException(status_code=500, detail="Failed to generate query") + except Exception as exc: + raise HTTPException(status_code=500, detail="Failed to generate query") from exc🤖 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 `@apps/python-service/routers/ai.py` around lines 66 - 67, The except block that currently does "except Exception: raise HTTPException(status_code=500, detail='Failed to generate query')" should capture the original exception and re-raise the HTTPException with exception chaining so the original traceback is preserved; change the handler to "except Exception as exc" and raise the HTTPException(...) from exc (preserve status_code and detail) so LangChain/Groq errors remain visible in traces (look for the except Exception and HTTPException in the ai router function that generates the query).
🤖 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.
Duplicate comments:
In `@apps/dashboard-api/src/controllers/ai.controller.js`:
- Around line 24-25: Normalize safeCollectionName to a consistent case before
comparing to 'users' so the guard is case-insensitive: update the conditional
that throws AppError(403, "Cannot query the users collection via AI")
(referencing safeCollectionName and the AppError throw site in ai.controller.js)
to compare a lowercase version of safeCollectionName (e.g., (safeCollectionName
|| '').toLowerCase()) against 'users'.
- Around line 10-31: Validate projectId before calling Project.findOne: check
that req.params.projectId is a valid Mongo ObjectId (e.g., using
mongoose.Types.ObjectId.isValid) and if not throw new AppError(400, "Invalid
projectId") instead of letting Mongoose cast; then proceed to use the validated
projectId in Project.findOne({ _id: projectId, owner: req.user._id,
"collections.name": safeCollectionName }, { "collections.$": 1 }) so Mongo
errors never surface as a 500 and the AppError pattern is preserved.
In `@apps/python-service/routers/ai.py`:
- Around line 66-67: The except block that currently does "except Exception:
raise HTTPException(status_code=500, detail='Failed to generate query')" should
capture the original exception and re-raise the HTTPException with exception
chaining so the original traceback is preserved; change the handler to "except
Exception as exc" and raise the HTTPException(...) from exc (preserve
status_code and detail) so LangChain/Groq errors remain visible in traces (look
for the except Exception and HTTPException in the ai router function that
generates the query).
In `@apps/web-dashboard/src/components/Database/AiQueryBar.jsx`:
- Around line 59-64: Add an explicit accessible label for the prompt input in
AiQueryBar.jsx: update the input that uses the prompt state and setPrompt
handler to include a programmatic label (for example add aria-label="AI query
input" or create a <label htmlFor="ai-prompt"> and give the input
id="ai-prompt"). Ensure the label is descriptive and remains present when the
input is disabled; keep existing props (value={prompt}, onChange={(e) =>
setPrompt(e.target.value)}, disabled={isLoading || !activeCollection ||
!projectId}) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 973fdb24-ce3e-4cff-a88b-d6dad838ba1e
📒 Files selected for processing (4)
apps/dashboard-api/src/controllers/ai.controller.jsapps/python-service/routers/ai.pyapps/python-service/tests/test_auth.pyapps/web-dashboard/src/components/Database/AiQueryBar.jsx
Collaborator
Author
|
HOw many change remaining?? @coderabbitai |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
|
yash-pouranik
temporarily deployed
to
fix/ai-query-builder-review - urBackend-frankfrut PR #205
Render
Destroyed
Collaborator
Author
|
everything done? |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
|
Collaborator
Author
|
verify them against code please, I think they are outdated? |
Contributor
|
All previously flagged review comments are now resolved. 🎉 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== ai.controller.js ==="
cat apps/dashboard-api/src/controllers/ai.controller.js
echo ""
echo "=== config.py ==="
cat apps/python-service/config.py
echo ""
echo "=== dependencies.py ==="
cat apps/python-service/dependencies.py
echo ""
echo "=== requirements.txt ==="
cat apps/python-service/requirements.txt
echo ""
echo "=== routers/ai.py ==="
cat apps/python-service/routers/ai.py
echo ""
echo "=== tests/test_auth.py ==="
cat apps/python-service/tests/test_auth.py
echo ""
echo "=== AiQueryBar.jsx ==="
cat apps/web-dashboard/src/components/Database/AiQueryBar.jsxLength of output: 17993 ✅ Actions performedComments resolved. Approval is disabled; enable |
Collaborator
Author
|
OK to merge? @coderabbitai |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
All CI checks are green, there are no open review concerns, and all previously flagged issues have been addressed. You're clear to merge! 🐇 🎉 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== PR status ==="
gh pr view 205 --repo geturbackend/urBackend --json state,mergeable,mergeStateStatus,isDraft,reviewDecision,statusCheckRollup \
--jq '{state,mergeable,mergeStateStatus,isDraft,reviewDecision,statusCheckRollup}'
echo ""
echo "=== Open/unresolved review threads ==="
gh pr view 205 --repo geturbackend/urBackend --json reviewThreads \
--jq '[.reviewThreads[] | select(.isResolved == false) | {path: .path, body: (.comments[0].body | .[0:200])}]'Length of output: 4046 |
Collaborator
Author
|
for prod |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
1. Dashboard API — root
|
| Variable | Value |
|---|---|
PYTHON_SERVICE_URL |
URL of your deployed Python service, e.g. https://python.yourserver.com |
INTERNAL_SECRET |
A random 32+ character secret (generate with openssl rand -hex 32) |
GROQ_API_KEY |
Your Groq API key from console.groq.com |
2. Python Microservice — apps/python-service/.env (new server)
Copy apps/python-service/.env.example → .env and fill in:
| Variable | Value |
|---|---|
INTERNAL_SECRET |
Must be identical to the one set in the Dashboard API |
GROQ_API_KEY |
Same Groq API key as above |
REDIS_URL |
Your production Redis URL (e.g. redis://your-redis-host:6379) — don't leave it as localhost |
3. Deploy & wire the Python service
The Python microservice is a new FastAPI server that needs to be running separately:
# Install deps
pip install -r apps/python-service/requirements.txt
# Start the service
uvicorn main:app --host 0.0.0.0 --port 8000Make sure PYTHON_SERVICE_URL in your Dashboard API .env points to this server's address.
🔑 Key constraint
INTERNAL_SECRET must match exactly on both sides — the Node.js Dashboard API uses it to sign requests and the Python service uses it to verify them. If they differ, all AI Query Builder calls will get 403 Forbidden.
ʕ•ᴥ•ʔ 🐇
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Root .env.example ==="
cat .env.example
echo ""
echo "=== apps/python-service/.env.example ==="
cat apps/python-service/.env.example
echo ""
echo "=== apps/python-service/config.py ==="
cat apps/python-service/config.py
echo ""
echo "=== apps/dashboard-api/src/utils/internalPythonClient.js ==="
cat apps/dashboard-api/src/utils/internalPythonClient.jsLength of output: 5621
Collaborator
Author
|
groq api is only needed in python service not in dashboard api |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Here's the corrected production env summary: Dashboard API — root
|
| Variable | Value |
|---|---|
PYTHON_SERVICE_URL |
URL of your deployed Python service |
INTERNAL_SECRET |
A random 32+ character secret (openssl rand -hex 32) |
Python Microservice — apps/python-service/.env (new server)
| Variable | Value |
|---|---|
INTERNAL_SECRET |
Must match exactly the one in the Dashboard API |
GROQ_API_KEY |
Your Groq API key from console.groq.com |
REDIS_URL |
Your production Redis URL |
✏️ Learnings added
Learnt from: yash-pouranik
URL: https://github.com/geturbackend/urBackend/pull/205
Timestamp: 2026-05-25T20:42:16.076Z
Learning: In the urBackend project (geturbackend/urBackend), GROQ_API_KEY is only needed in the Python microservice (`apps/python-service/.env`), NOT in the Dashboard API root `.env`. The Dashboard API (`apps/dashboard-api`) only needs `PYTHON_SERVICE_URL` and `INTERNAL_SECRET` for communicating with the Python service. The Python service handles all direct Groq API calls.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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
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.
Description
This PR introduces the AI Query Builder feature, allowing users to filter their database collections using natural language (powered by
llama-3.1-8b-instant). It includes the full stack implementation across the React frontend, Node.js Dashboard API, and a new Python microservice, along with strict security measures.✨ Key Features
🛠️ Review Feedback Addressed (Fixes)
apps/python-service/.envwith local dummy placeholders to prevent production secret exposure.userscollection in the AI endpoint to prevent exposing sensitive auth fields (like passwords) to the LLM context.ai.controller.jsto guaranteefiltersis always an Array with properly shaped objects, preventing frontend runtime crashes.test_auth.py) to correctly mock and utilizedependencies.pyinstead of non-existent middleware.__pycache__/,venv/,.pytest_cache/) to the root.gitignore.🧪 Verification Steps
users) in the Dashboard.userscollection explicitly throws a403error.pytest apps/python-service/teststo verify internal security HMAC validations pass successfully.Summary by CodeRabbit
New Features
User Experience
Infrastructure