Master #1
Master #1
Conversation
Features: - Convert create user inline form to proper popup modal - Fix notification banner readability with light theme colors - Add user dropdown menu in sidebar for Settings/Logout - Remove notifications tab from Settings completely - Remove Two-Factor Authentication from Settings > Security - Fix login page sidebar visibility issue with conditional layout Components: - Add CreateUserModal with form validation and error handling - Add LayoutWrapper for conditional sidebar display - Update admin page to use popup modal instead of inline form - Fix yellow warning banners contrast across all pages - Make sidebar user section collapsible with dropdown UI fixes: - Yellow banners: bg-yellow-900/20 → bg-yellow-50, text-yellow-300 → text-yellow-800 - Login page no longer shows sidebar or left margin - User menu now toggles Settings/Logout options on click - Improved accessibility and light theme consistency
…agement This commit adds critical missing functionality to EvalWise: Backend (API): - Complete playground testing endpoint with real LLM integration - Password change endpoint with validation - API key generation endpoint with secure tokens - Full team members CRUD operations (list, invite, update, remove) - Proper authorization and role-based access control - Comprehensive error handling with detailed responses Frontend (Web): - Password change form with validation in Settings - API key generation interface - Complete team members management UI with: - Member listing with avatars and details - Invite new members with role selection - Update member roles (admin/member/viewer) - Remove members with confirmation - Fixed TypeScript interface mismatches - Connected all Settings actions to backend APIs Security improvements: - Password strength validation - Self-action prevention (can't remove/deactivate yourself) - Admin-only operations for team management - Soft deletes for audit trail 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add comprehensive LLM providers page with CRUD operations - Support multiple provider types (OpenAI, Anthropic, Azure, Cohere, Google, Ollama, Custom) - Add providers navigation item to sidebar - Fix ScenarioFactory import error in run execution - Implement evaluators and scenarios management pages - Add API key management for evaluators - Improve authentication flow and error handling - Add database migrations for LLM providers table - Fix responsive layout issues across pages - Remove transition effects for better performance - Add proper error display for API responses 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Performance improvements: - Implement sessionStorage caching for auth state to eliminate unnecessary API calls - Split playground page into server and client components with cached data fetching - Add 30-second in-memory cache for evaluators to speed up repeat visits Technical changes: - Auth context: Cache user and token in sessionStorage, skip refresh if valid cache exists - Auth context: Eliminate 2 API calls (/auth/refresh, /auth/me) on every navigation - Playground: Extract client logic into playground-client.tsx for better code organization - Playground: Add evaluators caching with stale-while-revalidate pattern - Playground: Prevent double-fetch in development mode with useRef guard Performance gains: - First page load: ~550ms (unchanged, necessary API calls) - Subsequent navigations: ~150ms (73% faster, no auth refresh calls) - Return to playground: ~0ms for evaluators (using cache) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Issue: - Server rendered with null user/token, but client immediately read from sessionStorage - This caused React hydration mismatch error Solution: - Initialize state with null on both server and client - Restore from sessionStorage in useEffect after hydration completes - Wait for hydration flag before running auth initialization This ensures server and client render the same initial HTML, preventing hydration errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughThis pull request introduces comprehensive organizational and LLM provider management capabilities to the EvalWise platform. Changes include new database migrations and ORM models for LLM providers, expanded API endpoints for user/organization/provider lifecycle management, frontend pages for managing evaluator keys and scenarios, refactored layout with consistent authentication guards, and supporting utilities and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant DB as Database
participant Encrypt as Encryption
rect rgb(200, 220, 240)
Note over User,Encrypt: Organization API Key Retrieval Flow (New)
end
User->>Frontend: Request to use org-scoped provider
Frontend->>Backend: POST /orgs/{org_id}/llm-providers (with api_key)
Backend->>Backend: create_adapter(provider, org_id=org_id, db=db)
alt API key provided
Backend->>Backend: Use provided api_key
else No API key provided
Backend->>Backend: get_organization_api_key(provider, org_id, db)
Backend->>DB: Query EncryptedApiKey for org
DB-->>Backend: Return encrypted key
Backend->>Encrypt: Decrypt key
Encrypt-->>Backend: Return plaintext key
Backend->>Backend: Use decrypted key
end
Backend->>Backend: Initialize adapter with api_key
Backend-->>Frontend: Adapter ready
Frontend-->>User: Success
rect rgb(220, 240, 220)
Note over User,Frontend: Frontend Scenario Management (New)
end
User->>Frontend: Navigate to /scenarios
Frontend->>Frontend: Load with ProtectedRoute check
Frontend->>Backend: GET /scenarios
Backend->>DB: Query scenarios
DB-->>Backend: Return list
Backend-->>Frontend: Scenario list
Frontend->>Frontend: Render ScenariosPage with cards
User->>Frontend: Click "Create Scenario"
Frontend->>Frontend: Show CreateScenarioModal
User->>Frontend: Fill form & submit
Frontend->>Backend: POST /scenarios (name, type, params_json, tags)
Backend->>DB: Insert scenario
DB-->>Backend: Confirm
Backend-->>Frontend: Success with scenario id
Frontend->>Frontend: Close modal, refresh list, show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/main_v2.py (1)
995-1024:create_organizationpasses unknownjoined_atfield toUserOrganization
UserOrganizationinauth.modelsdefinescreated_at/updated_atbut notjoined_at. The constructor call here:user_org = UserOrganization( id=uuid.uuid4(), user_id=current_user.id, organization_id=org.id, role="admin", joined_at=datetime.utcnow() )will fail with
TypeError: 'joined_at' is an invalid keyword argumentfor this mapped class.Either:
- Drop
joined_atand rely oncreated_at, or- Add a
joined_atcolumn toUserOrganizationand corresponding migration.Given the current model snippet, the safe change is to remove
joined_atfrom this constructor.
🧹 Nitpick comments (27)
api/middleware/rate_limiting.py (1)
25-31: Consider externalizing rate limits to configuration.The rate limits are currently hardcoded, making it difficult to adjust them per environment without code changes. Since
config.settingsis already imported, consider moving these values to configuration to enable environment-specific tuning.Example implementation in
config.pyor settings file:# In config.py or settings RATE_LIMIT_UNAUTHENTICATED = int(os.getenv("RATE_LIMIT_UNAUTHENTICATED", "20")) RATE_LIMIT_AUTHENTICATED = int(os.getenv("RATE_LIMIT_AUTHENTICATED", "100")) RATE_LIMIT_ADMIN = int(os.getenv("RATE_LIMIT_ADMIN", "500")) RATE_LIMIT_WINDOW_SIZE = int(os.getenv("RATE_LIMIT_WINDOW_SIZE", "60"))Then update the middleware:
- # Rate limits (requests per minute) - Increased 10x for testing - self.unauthenticated_limit = 200 # 20 * 10 for easier testing - self.authenticated_limit = 1000 # 100 * 10 for authenticated users - self.admin_limit = 5000 # 500 * 10 for admin users - - # Window size in seconds - self.window_size = 60 + # Rate limits (requests per minute) + self.unauthenticated_limit = settings.RATE_LIMIT_UNAUTHENTICATED + self.authenticated_limit = settings.RATE_LIMIT_AUTHENTICATED + self.admin_limit = settings.RATE_LIMIT_ADMIN + + # Window size in seconds + self.window_size = settings.RATE_LIMIT_WINDOW_SIZEapi/alembic/versions/c9d4e8f7a6b5_add_llm_providers_table.py (1)
19-39: Tighten nullability and defaults for timestamps and flagsThe table definition works as-is, but a few tweaks would improve data integrity and keep the DB in sync with typical ORM conventions:
created_at/updated_atare nullable and usesa.DateTime()without timezone or server defaults. ConsiderDateTime(timezone=True)withserver_default/onupdate(or matching whatever is used in the ORM) so rows always have consistent audit timestamps.is_defaultandis_activeare nullable booleans; in practice these are usually non-null with explicit defaults (False/Trueas appropriate) to avoid tri‑state logic.- If IDs are usually generated DB‑side in other migrations, you may also want to add a
server_defaultforidto keep behavior consistent across tables.Not mandatory for correctness, but tightening these now will avoid subtle null/tri‑state handling downstream.
Please confirm that these column definitions match your SQLAlchemy
LLMProvidermodel and adjust either the migration or model so they stay aligned.create_admin.py (1)
1-7: Address minor script hygiene issues (shebang + f‑string)Two small cleanups:
- Line 1: the shebang (
#!/usr/bin/env python3) triggers the Ruff EXE001 warning because the file isn’t executable. Either make the file executable in git or drop the shebang if it’s intended to be imported/used as a module.- Line 60:
print(f" Password: admin123")is an f‑string with no interpolation; drop thefprefix to avoid the F541 warning.Neither affects behavior, but they’ll keep static analysis clean.
After adjusting, please re-run your linter/formatter to confirm the warnings are cleared.
Also applies to: 58-60
web/src/app/playground/playground-client.tsx (2)
242-267: Use stable keys and functional updates for evaluator selectionTwo small React best practices to reduce potential UI glitches:
- In the evaluations list you use
key={index}. If the evaluations array ever changes order or length, this can cause odd re-renders. Prefer a stable key such asevaluation.id:- {result.evaluations.map((evaluation, index) => { + {result.evaluations.map((evaluation) => { const evaluator = evaluators.find(e => e.id === evaluation.evaluator_id) return ( - <div - key={index} + <div + key={evaluation.id} className={`border-2 rounded-lg p-4 ${getEvaluationColor(evaluation)}`} >
- When toggling selected evaluators, use functional updates to avoid relying on a potentially stale
selectedEvaluatorIdsarray:onChange={(e) => { if (e.target.checked) { - setSelectedEvaluatorIds([...selectedEvaluatorIds, evaluator.id]) + setSelectedEvaluatorIds(prev => [...prev, evaluator.id]) } else { - setSelectedEvaluatorIds(selectedEvaluatorIds.filter(id => id !== evaluator.id)) + setSelectedEvaluatorIds(prev => prev.filter(id => id !== evaluator.id)) } }}These are non-breaking refinements that make the component more robust to changes.
After applying, please verify that evaluator selection and evaluation rendering still behave as expected when toggling checkboxes and re-running tests.
Also applies to: 328-333
298-320: Render numeric stats even when they are zeroRight now the conditional rendering uses truthiness:
{result.token_input && ( ... )} {result.token_output && ( ... )} {result.cost_usd && ( ... )}If any of these happen to be
0, the row won’t render. For counts/costs,0is still meaningful.Consider switching to explicit null/undefined checks, e.g.:
- {result.token_input && ( + {result.token_input != null && ( ... )}(and similarly for
token_output/cost_usd).Please confirm with a test run that you see these fields rendered correctly when values are
0or very small (e.g., cost rounded to 0.0000).web/src/contexts/auth-context.tsx (2)
31-32: ConfirmAPI_BASE_URLand trim noisy logging for productionA few small operational points:
API_BASE_URLnow defaults to'http://localhost:8003'. Make sure this matches the actual backend port in your dev environment, and thatNEXT_PUBLIC_API_URLis set appropriately in non-local environments to avoid hard‑coding localhost.- There are quite a few
console.logcalls in the auth flow (Initializing auth…, refresh attempts, statuses, etc.). They’re very helpful during development but can clutter production logs and potentially leak implementation details.Suggested refinements:
- Gate verbose logging behind
process.env.NODE_ENV !== 'production'or a custom debug flag.- Double‑check your
.envfiles so all deployed environments override the localhost default.These won’t change behavior but will make auth behavior cleaner in production.
Please verify your
.env/ deployment configs forNEXT_PUBLIC_API_URL, and consider running a production build locally to see whether the current logging volume is acceptable.Also applies to: 100-105, 262-270
134-169: Remove or repurpose unusedcheckAuthand reconcile comment with global token usageMinor cleanups:
- The
checkAuthfunction is defined but not used anywhere in this file. If it’s truly unused, consider removing it to reduce surface area; if you intend to keep it, wiring it into a specific flow (or marking it as a utility) would clarify its purpose.- The comment at line 134 says “Token managed via secure httpOnly cookies - no global window token needed”, but the code still sets
window.__authTokenin bothloginandrefreshToken. Either:
- Update the comment to reflect the current design (cookies + in-memory/global token), or
- Remove
window.__authTokenusage if it’s no longer needed by your API client.Aligning comments and removing dead code will make future maintenance of the auth system less error-prone.
Please search for usages of
checkAuthandwindow.__authTokenin the codebase to decide whether they’re still needed and adjust accordingly.Also applies to: 171-237, 262-321
web/src/app/runs/page.tsx (1)
111-115: Consider migrating provider storage from localStorage to API.Loading providers from localStorage is a temporary solution that has several drawbacks:
- Data is not synced across devices/browsers
- No server-side validation or consistency
- Can become stale or corrupted
- Not suitable for multi-user organizations
Consider adding an API endpoint to fetch providers, similar to how datasets, scenarios, and evaluators are loaded:
// Add to the Promise.allSettled array const providersResponse = await api.get('/providers').catch(err => { console.error('Failed to load providers:', err) return { data: [] } })web/src/app/evaluators/page.tsx (1)
104-107: Strengthen validation for optional API keys.The current validation logic may allow empty strings for optional fields. For local providers like Ollama, an empty API key might be acceptable, but the validation should explicitly handle this.
Consider making the validation more explicit:
- // For cloud providers, API key is required - if (requiredFields.includes('api_key') && ['openai', 'azure_openai'].includes(llmKeyForm.provider) && !llmKeyForm.api_key.trim()) { - alert('Please enter an API key') - return - } + // Validate API key based on provider + if (requiredFields.includes('api_key')) { + // Cloud providers require non-empty API keys + if (['openai', 'azure_openai'].includes(llmKeyForm.provider)) { + if (!llmKeyForm.api_key?.trim()) { + alert('Please enter an API key') + return + } + } + // Local providers can have empty API keys (will use default/dummy) + }web/src/components/create-user-modal.tsx (1)
47-67: Consider validation before submission.The validation at line 49 only checks for truthy values, which could allow whitespace-only inputs. Also consider validating email format on the client side.
Enhance the validation:
const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() - if (!newUser.email || !newUser.username || !newUser.password) return + + // Validate required fields + if (!newUser.email?.trim() || !newUser.username?.trim() || !newUser.password?.trim()) { + alert('Please fill in all required fields') + return + } + + // Validate email format + if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(newUser.email)) { + alert('Please enter a valid email address') + return + } + + // Validate password length + if (newUser.password.length < 8) { + alert('Password must be at least 8 characters') + return + } try { await onSubmit(newUser)web/src/components/layout-wrapper.tsx (1)
13-27: Consider centralizing auth-route detection with Sidebar
isAuthPageduplicates the auth-page check already inSidebar. If a new auth route is added and only one of these is updated, layout and sidebar behavior will diverge. Consider extracting a sharedisAuthPath(pathname)helper or colocating this logic to avoid drift.web/src/components/providers/providers-content.tsx (1)
8-29: Reuse shared LLM provider types instead of re-declaring shape
ProviderandProviderFormDatalargely mirror the backend/publicLLMProvidershape described elsewhere in the PR. Duplicating this structure here risks drift if the API changes (e.g., additional fields inmodel_defaults).If you already expose a shared provider type (e.g., from
web/src/app/providers/page.tsxor a centraltypesmodule), consider importing and reusing it instead of redefining, and derivingProviderFormDatafrom it where possible.web/src/components/scenarios/create-scenario-modal.tsx (1)
165-177: Align naming of safety probe “Approach” field with underlying paramFor the
safety_probebranch, the select is labeled “Approach” but is bound to thedirectnessfield (value={params.directness}/setParams({ ...params, directness: ... })). This works functionally but mixes terminology between UI and data.Consider renaming the label to “Directness” or renaming the field in
ScenarioParams(if feasible) for clearer intent and easier future maintenance.web/src/app/datasets/page.tsx (1)
52-64: Remove or use the returned dataset fromcreateDataset
createDatasetnow returnsresponse.data, but when used as a formonSubmithandler that return value is ignored. This can be misleading to readers who might expect the caller to consume the dataset.Either drop the
return response.dataor, if you intend to reuse this helper for flows like “Create with Table”, refactorhandleCreateWithTableto callcreateDatasetinstead of duplicating the create logic.web/src/app/scenarios/page.tsx (1)
67-81: TightenfetchScenarioseffect dependencies and cleanup logicThe data-loading effect closes over
fetchScenariosbut doesn’t list it in the dependency array, and theisCancelledflag only protects the outersetError, not the internal state updates insidefetchScenarios. This is safe-ish today but will likely tripreact-hooks/exhaustive-depsand makes unmount behavior harder to reason about.Consider updating the effect to depend on
fetchScenariosand to use a simpleisMountedguard for the outer error path:- useEffect(() => { - let isCancelled = false - - if (isAuthenticated) { - fetchScenarios().catch(() => { - if (!isCancelled) { - setError('Failed to load scenarios') - } - }) - } - - return () => { - isCancelled = true - } - }, [isAuthenticated]) + useEffect(() => { + if (!isAuthenticated) return + + let isMounted = true + + const load = async () => { + try { + await fetchScenarios() + } catch { + if (isMounted) { + setError('Failed to load scenarios') + } + } + } + + load() + + return () => { + isMounted = false + } + }, [isAuthenticated, fetchScenarios])If you want to fully prevent any state updates after unmount, you could also pass an
isMounted/AbortSignalintofetchScenariosand guard itssetStatecalls there.Also applies to: 83-101
web/src/components/scenarios/scenario-preview-modal.tsx (1)
3-27: Reuse shared Scenario/GeneratedAttack types and stabilize preview hook depsThis component redefines
ScenarioandGeneratedAttacklocally and uses auseEffectthat depends onscenario.idbut calls a non-memoizedgeneratePreviews. Functionally this works, but:
- Duplicated types can drift from
web/src/types/scenarios.ts.react-hooks/exhaustive-depswill likely complain about the missinggeneratePreviewsdependency.You can address both by importing the shared types and memoizing
generatePreviews:-import { useState, useEffect } from 'react' +import { useState, useEffect, useCallback } from 'react' import { Button } from '@/components/ui' import { api } from '@/lib/api' import { X, RefreshCw } from 'lucide-react' - -interface Scenario { - id: string - name: string - type: string - params_json: any - tags: string[] -} +import type { Scenario, GeneratedAttack } from '@/types/scenarios' @@ -interface ScenarioPreviewModalProps { - scenario: Scenario - onClose: () => void -} - -interface GeneratedAttack { - id: number - prompt: string - technique: string - category?: string -} +interface ScenarioPreviewModalProps { + scenario: Scenario + onClose: () => void +} @@ - const generatePreviews = async () => { + const generatePreviews = useCallback(async () => { @@ - } + }, [scenario.id]) @@ - useEffect(() => { - generatePreviews() - }, [scenario.id]) + useEffect(() => { + generatePreviews() + }, [generatePreviews])This keeps the modal aligned with the shared data model and quiets hook-deps linting.
Also applies to: 33-55, 169-171
web/src/app/evaluators/keys/page.tsx (1)
11-24: Evaluator keys flow looks correct; consider tightening types and UX laterThe evaluator key management flow (org selection, provider-specific validation, redacted logging, and CRUD calls to
/organizations/{orgId}/llm-keys) looks consistent and functionally sound. No blocking issues from this file.For future polish (non-blocking):
- Replace
any[]fororganizations/llmKeyswith small interfaces matching the API shape to catch drift at compile time.- Swap
alert/confirmfor the shared toast system or in-page banners to align UX with the rest of the app.These can be deferred without risking correctness.
Also applies to: 36-68, 70-146, 167-190, 223-351, 355-399
web/src/app/admin/page.tsx (2)
180-194: ProtectedRoute already enforces admin access; inline admin gate is now redundantWith the default export wrapping
AdminPageContentin<ProtectedRoute requireAdmin={true}>, the internalif (!user?.is_superuser)branch that renders the “Admin Access Required” card becomes effectively unreachable—ProtectedRoute will redirect and rendernullfor non-admins.If you’re happy with the redirect behavior, you can simplify by removing the inner admin gate; or, if you prefer the in-page message, consider dropping
requireAdminand relying on this UI instead. Right now both are present, but only the ProtectedRoute behavior is observable.Also applies to: 372-377
68-69:showPasswordstoggle currently has no effect beyond the icon
showPasswordsis toggled by the “eye” button in the User Management search area, but it isn’t used to conditionally render anything (no password field or extra details depend on it). The only visible change is the icon itself.If this is leftover from a previous inline form, you could either wire it up to show/hide additional sensitive columns, or remove the state and button to avoid confusing users with a control that doesn’t do anything functional.
Also applies to: 289-303
api/adapters/factory.py (2)
20-42: Hardenget_organization_api_keyerror handling and loggingCatching bare
Exceptionand printing to stdout makes failures hard to monitor and masks whether the problem is “no key found” vs. “DB/encryption error”.Consider:
- Narrowing the
exceptto the specific exceptions you expect (e.g., SQLAlchemy errors,ValueErrorfrom decryption).- Using your structured logger instead of
print(...).- Returning
Noneonly when the key truly doesn’t exist; for unexpected failures, either re‑raise or wrap in a more specific exception so caller can distinguish misconfiguration from absence.A sketch:
- encrypted_key_record = db.query(EncryptedApiKey).filter( - EncryptedApiKey.provider == provider, - EncryptedApiKey.organization_id == organization_id, - EncryptedApiKey.is_active == True - ).first() + encrypted_key_record = ( + db.query(EncryptedApiKey) + .filter( + EncryptedApiKey.provider == provider, + EncryptedApiKey.organization_id == organization_id, + EncryptedApiKey.is_active, # let SQLAlchemy build the predicate + ) + .first() + ) @@ - except Exception as e: - print(f"Failed to retrieve API key for {provider}: {str(e)}") - return None + except Exception as e: + # TODO: replace with proper logger + print(f"[ModelAdapterFactory] Error retrieving API key for {provider}: {e!s}") + raise
55-66: Be explicit about fallback behavior when organization API key lookup failsThe
create_adapterflow silently falls back to environment variables / defaults ifget_organization_api_keyreturnsNone. That’s convenient, but it can hide configuration issues (e.g., key exists in DB but failed decryption).If the intent is:
- “Use org‑scoped key if present, otherwise env var/user‑supplied key”, this is fine but should be documented, and the error path in
get_organization_api_keyshould not silently swallow real failures.- Alternatively, for certain deployments you might want a hard requirement that an org‑scoped key exists and is valid, and surface a clear error when it’s missing.
Also, logging via
print(f"Using stored API key...")leaks internal IDs to stdout; consider switching to your logger with appropriate level and message.web/src/app/settings/page.tsx (2)
35-50: Avoidany[]formembersand introduce a typed Member interface
membersis currentlyany[], but the code assumes a specific shape (id,full_name,username,joined_at,last_login,role,is_current_user).Defining a
Memberinterface and usinguseState<Member[]>([])will give you compile‑time safety when accessing those fields and make future changes to the members API less error‑prone.Example:
interface Member { id: string email: string full_name?: string username?: string joined_at: string last_login?: string | null role: 'viewer' | 'member' | 'admin' is_current_user?: boolean } const [members, setMembers] = useState<Member[]>([])
145-300: Make “first organization only” behavior explicit or future‑proof
loadMembers,handleInviteMember,handleCreateUser,handleUpdateMember, andhandleRemoveMemberall implicitly operate onorganizations[0]after checking thatorganizations.length > 0. That’s fine as an initial “single‑org” assumption, but it bakes the convention into several places.To make this easier to evolve later:
- Either centralize the “current organization” selection (e.g. a
currentOrgIdstate, possibly with a dropdown), or- Add a short comment noting that using
organizations[0]is intentional and scoped to the current MVP.This will help avoid subtle bugs if/when multi‑organization support is expanded.
web/src/types/scenarios.ts (1)
37-42: Consider narrowingCreateScenarioRequest.typeto the known scenario typesYou already model
Scenario.typeas:type: 'jailbreak_basic' | 'safety_probe' | 'privacy_probe'but
CreateScenarioRequest.typeis a plainstring. Tightening this to the same union will give you stronger compile‑time checks when constructing create requests:-export interface CreateScenarioRequest { - name: string - type: string +export interface CreateScenarioRequest { + name: string + type: Scenario['type'] params_json: ScenarioParams tags: string[] }This keeps the definitions in sync and prevents typos in new scenario creation flows.
web/src/app/providers/page.tsx (1)
62-88: Modal CRUD flow and payload shape look consistent with backend; a couple of minor UX considerationsThe create/edit modal wiring, payload construction in
handleSubmit, and list rendering all align well with the LLM provider API (no API keys leaked back to the UI, and model defaults mapped correctly).Two small follow‑ups you might consider (non-blocking):
- When the initial
loadProviderscall fails, the error is stored inerrorbut only ever displayed inside the modal. If the first fetch fails before any modal is opened, there is no visible error on the page. A separatelistErrorfor the table (or reusingerrorin the main content area) would improve debuggability.base_urlis only sent when non-empty, so an existingbase_urlcannot be cleared via the UI. If clearing is a real use case, you’d need to explicitly sendbase_url: nullor a dedicated “reset to default” action.Otherwise, the new LLMProvider interface and modal-driven CRUD flow look solid.
Also applies to: 96-146, 190-255, 258-391
api/main_v2.py (2)
1111-1161: User API key generation is placeholder-only and doesn’t persist keys
/users/me/api-keyscurrently generates a key and hash, builds anapi_key_datadict, logs, and returns the key once, but does not persist anything to the database.list_api_keysthen returns an empty list unconditionally.This is fine as a temporary stub, but behavior might surprise API consumers who expect keys to be listable/revocable. Once you add a proper
APIKeymodel, this endpoint will need to:
- Insert the key metadata (hash, preview, name, user_id, timestamps) into the database.
- Implement
list_api_keysto read from that table and return summaries.Until then, you may want to clearly label this as experimental/temporary in API docs or protect it behind a feature flag.
1231-1398: LLM org-key endpoint: metadata unused and debug prints left inIn
create_llm_provider_key:
- You build a
metadatadict withendpoint_url,model_deployment_name, andapi_version, butEncryptedApiKeyhas no corresponding column, and this metadata is never persisted or returned. That’s confusing for callers who provide these fields.- There are several
print(f"DEBUG: ...")statements that should be removed or switched to structured logging before production.If you intend to support provider-specific settings at the key level, consider either:
- Extending
EncryptedApiKey(and migration) with a JSON metadata column, storing this dict, or- Removing these extra Body parameters until they’re actually used.
Also, you can replace the
logger.debugincludingrequest_idfor consistency with the rest of the module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
.gitignore(1 hunks)AGENTS.md(1 hunks)CONTRIBUTING.md(1 hunks)api/adapters/factory.py(2 hunks)api/alembic/versions/b8f5c9a3d2e1_remove_usage_tracking_from_api_keys.py(1 hunks)api/alembic/versions/c9d4e8f7a6b5_add_llm_providers_table.py(1 hunks)api/auth/models.py(3 hunks)api/main_v2.py(5 hunks)api/middleware/rate_limiting.py(1 hunks)api/tasks/simple_evaluation.py(2 hunks)api/utils/encryption.py(1 hunks)cookies.txt(1 hunks)create_admin.py(1 hunks)docker-compose.yml(3 hunks)web/src/app/admin/page.tsx(7 hunks)web/src/app/datasets/page.tsx(6 hunks)web/src/app/evaluators/keys/page.tsx(1 hunks)web/src/app/evaluators/page.tsx(1 hunks)web/src/app/globals.css(1 hunks)web/src/app/layout.tsx(2 hunks)web/src/app/login/page.tsx(2 hunks)web/src/app/page.tsx(1 hunks)web/src/app/playground/page.tsx(1 hunks)web/src/app/playground/playground-client.tsx(1 hunks)web/src/app/providers/page.tsx(1 hunks)web/src/app/providers/page.tsx.backup(1 hunks)web/src/app/runs/page.tsx(5 hunks)web/src/app/scenarios/page.tsx(1 hunks)web/src/app/settings/page.tsx(5 hunks)web/src/components/create-user-modal.tsx(1 hunks)web/src/components/layout-wrapper.tsx(1 hunks)web/src/components/protected-route.tsx(3 hunks)web/src/components/providers/providers-content.tsx(1 hunks)web/src/components/scenarios/create-scenario-modal.tsx(1 hunks)web/src/components/scenarios/edit-scenario-modal.tsx(1 hunks)web/src/components/scenarios/loading-skeleton.tsx(1 hunks)web/src/components/scenarios/scenario-preview-modal.tsx(1 hunks)web/src/components/scenarios/toast-notification.tsx(1 hunks)web/src/components/sidebar.tsx(5 hunks)web/src/contexts/auth-context.tsx(5 hunks)web/src/hooks/use-debounced-value.ts(1 hunks)web/src/types/scenarios.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (25)
web/src/app/playground/playground-client.tsx (2)
api/models.py (1)
Evaluator(47-56)api/evaluators/base.py (1)
EvaluationResult(6-11)
web/src/app/evaluators/page.tsx (2)
web/src/contexts/auth-context.tsx (1)
useAuth(340-346)web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
web/src/components/layout-wrapper.tsx (1)
web/src/components/sidebar.tsx (1)
Sidebar(68-232)
web/src/components/create-user-modal.tsx (1)
api/auth/models.py (1)
User(33-64)
api/tasks/simple_evaluation.py (1)
api/scenarios/factory.py (2)
ScenarioGeneratorFactory(7-54)create_generator(17-24)
web/src/contexts/auth-context.tsx (1)
api/auth/models.py (1)
User(33-64)
web/src/components/providers/providers-content.tsx (1)
web/src/contexts/auth-context.tsx (1)
useAuth(340-346)
web/src/components/scenarios/edit-scenario-modal.tsx (1)
web/src/types/scenarios.ts (2)
Scenario(12-20)ScenarioParams(3-10)
web/src/app/scenarios/page.tsx (9)
web/src/types/scenarios.ts (2)
ScenarioTypeConfig(22-28)Scenario(12-20)web/src/contexts/auth-context.tsx (1)
useAuth(340-346)web/src/hooks/use-debounced-value.ts (1)
useDebouncedValue(3-17)web/src/components/scenarios/toast-notification.tsx (2)
useToast(128-140)ToastProvider(99-126)web/src/components/scenarios/loading-skeleton.tsx (1)
ScenarioListSkeleton(43-51)web/src/components/scenarios/create-scenario-modal.tsx (1)
CreateScenarioModal(51-332)web/src/components/scenarios/edit-scenario-modal.tsx (1)
EditScenarioModal(16-295)web/src/components/scenarios/scenario-preview-modal.tsx (1)
ScenarioPreviewModal(28-315)web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
web/src/app/evaluators/keys/page.tsx (2)
web/src/contexts/auth-context.tsx (1)
useAuth(340-346)web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
web/src/app/playground/page.tsx (2)
web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)web/src/app/playground/playground-client.tsx (1)
PlaygroundClient(36-389)
web/src/app/runs/page.tsx (1)
web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
web/src/app/layout.tsx (1)
web/src/components/layout-wrapper.tsx (1)
LayoutWrapper(10-37)
web/src/components/scenarios/create-scenario-modal.tsx (1)
web/src/types/scenarios.ts (1)
ScenarioParams(3-10)
web/src/components/scenarios/scenario-preview-modal.tsx (1)
web/src/types/scenarios.ts (2)
Scenario(12-20)GeneratedAttack(30-35)
web/src/app/admin/page.tsx (3)
web/src/contexts/auth-context.tsx (1)
useAuth(340-346)web/src/components/create-user-modal.tsx (1)
CreateUserModal(27-207)web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
web/src/app/settings/page.tsx (1)
web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
web/src/app/providers/page.tsx (2)
api/auth/models.py (1)
LLMProvider(108-135)web/src/contexts/auth-context.tsx (1)
useAuth(340-346)
api/auth/models.py (1)
api/utils/logging.py (1)
default(11-14)
create_admin.py (2)
api/auth/models.py (3)
User(33-64)Organization(8-30)UserOrganization(67-84)api/auth/security.py (1)
get_password_hash(39-41)
api/alembic/versions/c9d4e8f7a6b5_add_llm_providers_table.py (1)
api/alembic/versions/b8f5c9a3d2e1_remove_usage_tracking_from_api_keys.py (2)
upgrade(19-22)downgrade(25-28)
web/src/app/login/page.tsx (1)
web/src/contexts/auth-context.tsx (1)
useAuth(340-346)
api/adapters/factory.py (3)
api/database.py (1)
get_db(20-25)api/auth/models.py (1)
EncryptedApiKey(87-105)api/utils/encryption.py (1)
decrypt_api_key(40-50)
api/main_v2.py (8)
api/auth/models.py (5)
User(33-64)Organization(8-30)UserOrganization(67-84)LLMProvider(108-135)EncryptedApiKey(87-105)api/auth/security.py (4)
get_current_user_flexible(247-262)get_current_user(239-244)verify_password(35-37)get_password_hash(39-41)api/utils/errors.py (4)
NotFoundError(59-82)ValidationError(42-57)ErrorDetail(7-12)InternalServerError(156-170)api/database.py (1)
get_db(20-25)api/adapters/factory.py (2)
ModelAdapterFactory(10-102)create_adapter(45-92)api/evaluators/factory.py (1)
EvaluatorFactory(8-64)api/models.py (1)
Evaluator(47-56)api/utils/encryption.py (1)
encrypt_api_key(32-38)
web/src/app/datasets/page.tsx (1)
web/src/components/protected-route.tsx (1)
ProtectedRoute(13-58)
🪛 Gitleaks (8.29.0)
cookies.txt
[high] 5-5: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
docker-compose.yml
[high] 41-41: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.14.5)
create_admin.py
1-1: Shebang is present but file is not executable
(EXE001)
60-60: f-string without any placeholders
Remove extraneous f prefix
(F541)
api/adapters/factory.py
31-31: Avoid equality comparisons to True; use EncryptedApiKey.is_active: for truth checks
Replace with EncryptedApiKey.is_active
(E712)
40-40: Do not catch blind exception: Exception
(BLE001)
41-41: Use explicit conversion flag
Replace with conversion flag
(RUF010)
56-56: Avoid specifying long messages outside the exception class
(TRY003)
api/main_v2.py
866-866: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
867-867: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
884-884: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
897-897: Do not catch blind exception: Exception
(BLE001)
898-898: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
898-898: Use explicit conversion flag
Replace with conversion flag
(RUF010)
899-907: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
917-917: Do not catch blind exception: Exception
(BLE001)
918-918: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
918-918: Use explicit conversion flag
Replace with conversion flag
(RUF010)
919-927: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
962-962: Do not catch blind exception: Exception
(BLE001)
963-963: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
963-963: Use explicit conversion flag
Replace with conversion flag
(RUF010)
969-969: Use explicit conversion flag
Replace with conversion flag
(RUF010)
972-979: Consider moving this statement to an else block
(TRY300)
983-983: Do not catch blind exception: Exception
(BLE001)
984-984: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
984-984: Use explicit conversion flag
Replace with conversion flag
(RUF010)
985-993: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1067-1067: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1068-1068: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1117-1117: Unused function argument: db
(ARG001)
1117-1117: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1118-1118: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1124-1124: Local variable request_id is assigned to but never used
Remove assignment to unused variable request_id
(F841)
1164-1164: Unused function argument: request
(ARG001)
1165-1165: Unused function argument: db
(ARG001)
1165-1165: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1166-1166: Unused function argument: current_user
(ARG001)
1166-1166: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1177-1177: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1178-1178: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1188-1196: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1202-1202: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1216-1216: Avoid equality comparisons to True; use EncryptedApiKey.is_active: for truth checks
Replace with EncryptedApiKey.is_active
(E712)
1241-1241: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1242-1242: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1257-1265: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1287-1287: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1343-1343: Avoid equality comparisons to True; use EncryptedApiKey.is_active: for truth checks
Replace with EncryptedApiKey.is_active
(E712)
1360-1360: Do not catch blind exception: Exception
(BLE001)
1361-1361: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1361-1361: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1362-1365: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1405-1405: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1406-1406: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1418-1426: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1433-1433: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1448-1448: Avoid equality comparisons to True; use EncryptedApiKey.is_active: for truth checks
Replace with EncryptedApiKey.is_active
(E712)
1476-1476: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1477-1477: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1486-1494: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1500-1500: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1514-1514: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1515-1515: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
1540-1540: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1541-1541: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1550-1558: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1578-1578: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1590-1590: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
1663-1663: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1664-1664: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1675-1683: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1690-1690: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1804-1804: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1805-1805: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1815-1823: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1842-1842: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1868-1868: Avoid equality comparisons to False; use not is_active: for false checks
Replace with not is_active
(E712)
1904-1904: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1905-1905: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1915-1923: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1930-1930: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
1987-1987: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1988-1988: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1997-2001: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2006-2006: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
2009-2009: Avoid specifying long messages outside the exception class
(TRY003)
2014-2022: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2027-2027: Avoid equality comparisons to True; use LLMProvider.is_active: for truth checks
Replace with LLMProvider.is_active
(E712)
2056-2056: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2058-2058: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2059-2059: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2068-2072: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2077-2077: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
2080-2080: Avoid specifying long messages outside the exception class
(TRY003)
2085-2093: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2188-2188: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2190-2190: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2191-2191: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2197-2197: Undefined name get_organization_membership
(F821)
2199-2199: Avoid specifying long messages outside the exception class
(TRY003)
2205-2213: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2219-2219: Avoid equality comparisons to True; use LLMProvider.is_active: for truth checks
Replace with LLMProvider.is_active
(E712)
2276-2276: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2277-2277: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
2283-2283: Undefined name get_organization_membership
(F821)
2285-2285: Avoid specifying long messages outside the exception class
(TRY003)
2291-2299: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2305-2305: Avoid equality comparisons to True; use LLMProvider.is_active: for truth checks
Replace with LLMProvider.is_active
(E712)
| node_modules/ | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log*web/.next/ |
There was a problem hiding this comment.
Fix malformed .gitignore entry at line 80.
Line 80 concatenates web/.next/ directly to yarn-error.log* without a newline separator, creating an invalid pattern. This breaks .gitignore parsing for both entries.
Apply this diff to fix the formatting:
-yarn-error.log*web/.next/
-CLAUDE.md
+yarn-error.log*
+web/.next/
+CLAUDE.mdCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .gitignore around line 80, the entry "yarn-error.log*web/.next/" is malformed
because two patterns were concatenated without a newline; separate them into two
lines so "yarn-error.log*" and "web/.next/" are each on their own line to
restore correct .gitignore parsing.
| # Playground endpoint | ||
| @app.post("/playground/test") | ||
| def playground_test( | ||
| async def playground_test( | ||
| request: PlaygroundRequest, | ||
| db: Session = Depends(get_db), | ||
| current_user = Depends(get_current_user_flexible) | ||
| ): | ||
| """Test a single prompt (placeholder implementation)""" | ||
| return { | ||
| "output": f"Mock response to: {request.prompt[:50]}...", | ||
| "latency_ms": 1000, | ||
| "token_input": 10, | ||
| "token_output": 20, | ||
| "cost_usd": 0.001, | ||
| "evaluations": [ | ||
| { | ||
| "evaluator_id": eid, | ||
| "score_float": 0.8, | ||
| "pass_bool": True, | ||
| "notes_text": "Mock evaluation result" | ||
| } | ||
| for eid in request.evaluator_ids[:2] # Limit to first 2 | ||
| ] | ||
| } | ||
| """Test a single prompt with real LLM and evaluators""" | ||
| request_id = str(uuid.uuid4()) | ||
|
|
||
| try: | ||
| # Import adapters and evaluators | ||
| from adapters.factory import ModelAdapterFactory | ||
| from evaluators.factory import EvaluatorFactory | ||
|
|
||
| # Get model configuration from request | ||
| model_provider = getattr(request, 'model_provider', 'openai') | ||
| model_name = getattr(request, 'model_name', 'gpt-3.5-turbo') | ||
|
|
||
| # Get user's organization for API key lookup | ||
| user_org = db.query(UserOrganization).filter( | ||
| UserOrganization.user_id == current_user.id, | ||
| UserOrganization.is_active == True | ||
| ).first() | ||
|
|
||
| organization_id = str(user_org.organization_id) if user_org else None | ||
|
|
||
| # Create model adapter | ||
| try: | ||
| adapter = ModelAdapterFactory.create_adapter( | ||
| provider=model_provider, | ||
| api_key=None, # Will try stored keys first, then environment variables | ||
| organization_id=organization_id, | ||
| db=db | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Failed to create adapter for {model_provider}: {str(e)}") | ||
| raise ValidationError( | ||
| message=f"Failed to configure model provider: {model_provider}", | ||
| details=[ErrorDetail( | ||
| code="MODEL_CONFIGURATION_ERROR", | ||
| message=str(e), | ||
| field="model_provider" | ||
| )], | ||
| request_id=request_id | ||
| ) | ||
|
|
||
| # Generate response from LLM | ||
| try: | ||
| response = await adapter.generate( | ||
| prompt=request.prompt, | ||
| model_name=model_name, | ||
| temperature=0.7, | ||
| max_tokens=1000 | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Failed to generate response: {str(e)}") | ||
| raise ValidationError( | ||
| message="Failed to generate response from model", | ||
| details=[ErrorDetail( | ||
| code="MODEL_GENERATION_ERROR", | ||
| message=str(e), | ||
| field="prompt" | ||
| )], | ||
| request_id=request_id | ||
| ) | ||
|
|
||
| # Run evaluators if any are specified | ||
| evaluations = [] | ||
| if request.evaluator_ids: | ||
| for evaluator_id in request.evaluator_ids: | ||
| try: | ||
| # Get evaluator from database | ||
| evaluator = db.query(Evaluator).filter(Evaluator.id == evaluator_id).first() | ||
| if not evaluator: | ||
| logger.warning(f"Evaluator {evaluator_id} not found") | ||
| continue | ||
|
|
||
| # Create evaluator instance | ||
| evaluator_instance = EvaluatorFactory.create_evaluator( | ||
| evaluator.type, | ||
| evaluator.config or {} | ||
| ) | ||
|
|
||
| # Run evaluation | ||
| eval_result = await evaluator_instance.evaluate( | ||
| input_text=request.prompt, | ||
| output_text=response.content, | ||
| expected_output=None, | ||
| metadata=None | ||
| ) | ||
|
|
||
| evaluations.append({ | ||
| "evaluator_id": str(evaluator_id), | ||
| "evaluator_name": evaluator.name, | ||
| "score_float": eval_result.score, | ||
| "pass_bool": eval_result.pass_fail, | ||
| "notes_text": eval_result.notes or "" | ||
| }) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to run evaluator {evaluator_id}: {str(e)}") | ||
| evaluations.append({ | ||
| "evaluator_id": str(evaluator_id), | ||
| "evaluator_name": f"Evaluator {evaluator_id}", | ||
| "score_float": None, | ||
| "pass_bool": None, | ||
| "notes_text": f"Evaluation failed: {str(e)}" | ||
| }) | ||
|
|
||
| return { | ||
| "output": response.content, | ||
| "latency_ms": response.latency_ms, | ||
| "token_input": response.token_input, | ||
| "token_output": response.token_output, | ||
| "cost_usd": response.cost_usd, | ||
| "evaluations": evaluations | ||
| } |
There was a problem hiding this comment.
Playground evaluator integration uses wrong Evaluator fields
Inside /playground/test, the evaluator wiring doesn’t match the Evaluator model or factory:
- You call
EvaluatorFactory.create_evaluator(evaluator.type, evaluator.config or {}), butEvaluatorhaskindandconfig_jsonfields (seeapi/models.py), nottype/config. This will raise at runtime whenrequest.evaluator_idsis non-empty. - The config passed to the factory should be
evaluator.config_json(or whatever field is actually used), notevaluator.config.
A minimal fix would be:
- evaluator_instance = EvaluatorFactory.create_evaluator(
- evaluator.type,
- evaluator.config or {}
- )
+ evaluator_instance = EvaluatorFactory.create_evaluator(
+ evaluator.kind,
+ evaluator.config_json or {}
+ )You may also want to normalize evaluator_id to a UUID (as done in other endpoints) for consistency, but the field mismatch above is the blocker.
🧰 Tools
🪛 Ruff (0.14.5)
866-866: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
867-867: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
884-884: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
897-897: Do not catch blind exception: Exception
(BLE001)
898-898: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
898-898: Use explicit conversion flag
Replace with conversion flag
(RUF010)
899-907: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
917-917: Do not catch blind exception: Exception
(BLE001)
918-918: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
918-918: Use explicit conversion flag
Replace with conversion flag
(RUF010)
919-927: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
962-962: Do not catch blind exception: Exception
(BLE001)
963-963: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
963-963: Use explicit conversion flag
Replace with conversion flag
(RUF010)
969-969: Use explicit conversion flag
Replace with conversion flag
(RUF010)
972-979: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
In api/main_v2.py around lines 862-979, the playground evaluator wiring uses
non-existent Evaluator fields (calls evaluator.type and evaluator.config) which
will raise at runtime; update uses to the real model fields (evaluator.kind and
evaluator.config_json or whichever field holds JSON config) when calling
EvaluatorFactory.create_evaluator and when building response metadata, e.g. pass
evaluator.kind as the first arg and parse/convert evaluator.config_json into a
dict for the second arg; also normalize evaluator_id to a UUID string (as in
other endpoints) before using it in logs and returned evaluations for
consistency.
| if not user_org: | ||
| raise NotFoundError( | ||
| message="Organization not found or access denied", | ||
| resource_type="organization", | ||
| resource_id=org_id, | ||
| request_id=request_id | ||
| ) |
There was a problem hiding this comment.
NotFoundError is being constructed with unsupported keyword arguments
Across the new LLM key and member-management endpoints you call NotFoundError like:
raise NotFoundError(
message="Organization not found or access denied",
resource_type="organization",
resource_id=org_id,
request_id=request_id
)But NotFoundError is defined as:
def __init__(self, resource: str, resource_id: Optional[str] = None, request_id: Optional[str] = None)It does not accept message or resource_type kwargs. These calls will raise TypeError before your handler runs.
You should instead pass the resource label and ID positionally, e.g.:
- raise NotFoundError(
- message="Organization not found or access denied",
- resource_type="organization",
- resource_id=org_id,
- request_id=request_id
- )
+ raise NotFoundError(
+ "Organization",
+ org_id,
+ request_id=request_id,
+ )Apply the same pattern to the other NotFoundError callsites in these new endpoints (organizations, users, memberships, encrypted_api_keys) so they align with the existing usage earlier in this module.
Also applies to: 1291-1297, 1437-1442, 1503-1509, 1581-1587, 1693-1699, 1845-1851, 1933-1939, 1959-1965
🤖 Prompt for AI Agents
In api/main_v2.py around lines 1205-1211 (and also update the other listed
callsites at 1291-1297, 1437-1442, 1503-1509, 1581-1587, 1693-1699, 1845-1851,
1933-1939, 1959-1965), you are constructing NotFoundError using unsupported
keyword args (message, resource_type); change those calls to use the
NotFoundError signature: pass the resource label string as the first positional
arg (e.g., "organization", "user", "membership", or "encrypted_api_key"), the
resource id as the second positional arg if available, and request_id as the
third positional arg (or omit if None). Ensure all NotFoundError instantiations
in these ranges follow that positional order to match the defined __init__.
| @app.get("/organizations/{org_id}/llm-providers") | ||
| async def list_llm_providers( | ||
| org_id: str, | ||
| current_user: User = Depends(get_current_user), | ||
| db: Session = Depends(get_db) | ||
| ): | ||
| """List all LLM providers for an organization""" | ||
| request_id = str(uuid.uuid4()) | ||
|
|
||
| # Verify user belongs to organization | ||
| try: | ||
| org_uuid = uuid.UUID(org_id) | ||
| except ValueError: | ||
| raise ValidationError( | ||
| message="Invalid organization ID format", | ||
| details=[ErrorDetail(code="INVALID_UUID", message=f"'{org_id}' is not a valid UUID")], | ||
| request_id=request_id | ||
| ) | ||
|
|
||
| membership = db.query(UserOrganization).filter( | ||
| UserOrganization.user_id == current_user.id, | ||
| UserOrganization.organization_id == org_uuid, | ||
| UserOrganization.is_active == True | ||
| ).first() | ||
| if not membership: | ||
| raise PermissionError("User does not belong to this organization") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use APIException-based errors instead of bare PermissionError for auth failures
The new LLM provider endpoints raise built‑in PermissionError when membership checks fail:
if not membership:
raise PermissionError("User does not belong to this organization")
...
if not membership or membership.role not in ["owner", "admin"]:
raise PermissionError("User does not have permission to create/update/delete providers")Because PermissionError doesn’t subclass your APIException, these won’t go through your structured error handling and may surface as generic 500s instead of consistent JSON error payloads.
Prefer one of:
- A 403
HTTPException, or - A dedicated
ValidationError/customAuthorizationErrorbuilt onAPIException.
Example:
- if not membership or membership.role not in ["owner", "admin"]:
- raise PermissionError("User does not have permission to create providers")
+ if not membership or membership.role not in ["owner", "admin"]:
+ raise ValidationError(
+ message="User does not have permission to create providers",
+ details=[ErrorDetail(
+ code="FORBIDDEN",
+ message="Admin privileges are required to manage providers",
+ field="organization_id",
+ )],
+ request_id=request_id,
+ )Aligning all auth failures with your existing error types will keep responses predictable for clients.
Also applies to: 2059-2081, 2197-2200, 2276-2285
🧰 Tools
🪛 Ruff (0.14.5)
1987-1987: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1988-1988: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
1997-2001: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2006-2006: Avoid equality comparisons to True; use UserOrganization.is_active: for truth checks
Replace with UserOrganization.is_active
(E712)
2009-2009: Avoid specifying long messages outside the exception class
(TRY003)
| membership = get_organization_membership(current_user.id, org_id, db) | ||
| if not membership or membership.role not in ["owner", "admin"]: | ||
| raise PermissionError("User does not have permission to update providers") | ||
|
|
There was a problem hiding this comment.
get_organization_membership is undefined in provider update/delete endpoints
Both update_llm_provider and delete_llm_provider call get_organization_membership(current_user.id, org_id, db), but there is no such function defined or imported in this module. This will immediately crash on any update/delete attempt.
Either define a helper in this module or inline the membership query (as you do elsewhere), for example:
- # Verify user belongs to organization with admin rights
- membership = get_organization_membership(current_user.id, org_id, db)
- if not membership or membership.role not in ["owner", "admin"]:
- raise PermissionError("User does not have permission to update providers")
+ # Verify user belongs to organization with admin rights
+ try:
+ org_uuid = uuid.UUID(org_id)
+ except ValueError:
+ raise ValidationError(
+ message="Invalid organization ID format",
+ details=[ErrorDetail(code="INVALID_UUID", message=f"'{org_id}' is not a valid UUID")],
+ request_id=request_id,
+ )
+
+ membership = db.query(UserOrganization).filter(
+ UserOrganization.user_id == current_user.id,
+ UserOrganization.organization_id == org_uuid,
+ UserOrganization.is_active == True,
+ ).first()
+ if not membership or membership.role not in ["owner", "admin"]:
+ raise ValidationError(
+ message="User does not have permission to update providers",
+ details=[ErrorDetail(
+ code="FORBIDDEN",
+ message="Admin privileges are required to manage providers",
+ field="organization_id",
+ )],
+ request_id=request_id,
+ )(Adjust to your preferred error type, but you need a real implementation in place of the undefined helper.)
Also applies to: 2283-2285
🧰 Tools
🪛 Ruff (0.14.5)
2197-2197: Undefined name get_organization_membership
(F821)
2199-2199: Avoid specifying long messages outside the exception class
(TRY003)
| const organizationId = user?.organizations?.[0]?.id | ||
|
|
||
| useEffect(() => { | ||
| if (organizationId) { | ||
| fetchProviders() | ||
| } | ||
| }, [organizationId]) | ||
|
|
||
| const fetchProviders = async () => { | ||
| if (!organizationId) { | ||
| setError('No organization found') | ||
| setLoading(false) | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| setLoading(true) | ||
| setError(null) | ||
| const response = await api.get(`/organizations/${organizationId}/llm-providers`) | ||
| setProviders(response.data) | ||
| } catch (err: any) { | ||
| console.error('Failed to fetch providers:', err) | ||
| const errorMessage = err.response?.data?.message || err.response?.data?.detail || err.message || 'Failed to load providers' | ||
| setError(errorMessage) | ||
| } finally { | ||
| setLoading(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid infinite loading when user has no organization
loading is initialized to true, and useEffect only calls fetchProviders() when organizationId is truthy. If user.organizations is empty or missing, fetchProviders is never invoked, so loading never flips to false and the spinner is shown indefinitely, even though fetchProviders already has a guard for the no-org case.
Consider calling fetchProviders() unconditionally and letting it handle the !organizationId branch, e.g.:
- useEffect(() => {
- if (organizationId) {
- fetchProviders()
- }
- }, [organizationId])
+ useEffect(() => {
+ fetchProviders()
+ }, [organizationId])The early return in fetchProviders will then set an appropriate error and clear loading.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const organizationId = user?.organizations?.[0]?.id | |
| useEffect(() => { | |
| if (organizationId) { | |
| fetchProviders() | |
| } | |
| }, [organizationId]) | |
| const fetchProviders = async () => { | |
| if (!organizationId) { | |
| setError('No organization found') | |
| setLoading(false) | |
| return | |
| } | |
| try { | |
| setLoading(true) | |
| setError(null) | |
| const response = await api.get(`/organizations/${organizationId}/llm-providers`) | |
| setProviders(response.data) | |
| } catch (err: any) { | |
| console.error('Failed to fetch providers:', err) | |
| const errorMessage = err.response?.data?.message || err.response?.data?.detail || err.message || 'Failed to load providers' | |
| setError(errorMessage) | |
| } finally { | |
| setLoading(false) | |
| } | |
| } | |
| const organizationId = user?.organizations?.[0]?.id | |
| useEffect(() => { | |
| fetchProviders() | |
| }, [organizationId]) | |
| const fetchProviders = async () => { | |
| if (!organizationId) { | |
| setError('No organization found') | |
| setLoading(false) | |
| return | |
| } | |
| try { | |
| setLoading(true) | |
| setError(null) | |
| const response = await api.get(`/organizations/${organizationId}/llm-providers`) | |
| setProviders(response.data) | |
| } catch (err: any) { | |
| console.error('Failed to fetch providers:', err) | |
| const errorMessage = err.response?.data?.message || err.response?.data?.detail || err.message || 'Failed to load providers' | |
| setError(errorMessage) | |
| } finally { | |
| setLoading(false) | |
| } | |
| } |
🤖 Prompt for AI Agents
In web/src/components/providers/providers-content.tsx around lines 49 to 76, the
component initializes loading=true but only calls fetchProviders when
organizationId is truthy, which causes an infinite spinner if the user has no
organizations; change the useEffect to call fetchProviders() unconditionally
(i.e., remove the if (organizationId) guard) so the fetchProviders early-return
branch handles the no-organization case, sets an error, and flips loading to
false.
| import { useState, useEffect } from 'react' | ||
| import { Button } from '@/components/ui' | ||
| import { api } from '@/lib/api' | ||
| import { X } from 'lucide-react' | ||
| import type { Scenario, ScenarioParams } from '@/types/scenarios' | ||
|
|
There was a problem hiding this comment.
Import React types or use FormEvent instead of bare React.FormEvent
handleSubmit is typed as React.FormEvent, but this module doesn’t import the React namespace, only hooks:
import { useState, useEffect } from 'react'
const handleSubmit = async (e: React.FormEvent) => {This will cause a TypeScript error (Cannot find name 'React') under typical Next/TS configs.
A minimal fix is to import the event type explicitly and update the signature:
-import { useState, useEffect } from 'react'
+import { useState, useEffect, type FormEvent } from 'react'
@@
- const handleSubmit = async (e: React.FormEvent) => {
+ const handleSubmit = async (e: FormEvent) => {(or import type FormEvent separately if you prefer).
Also applies to: 23-24
🤖 Prompt for AI Agents
In web/src/components/scenarios/edit-scenario-modal.tsx around lines 3-8 and
23-24, the handler is typed as React.FormEvent but the React namespace isn't
imported; update the imports to include the event type (for example: import type
{ FormEvent } from 'react') and change the handler signature to use that type
(e: FormEvent), or alternatively import the React namespace (import * as React
from 'react') so React.FormEvent is available.
| return ( | ||
| <div | ||
| className={`fixed top-4 right-4 max-w-sm w-full transform transition-all duration-300 z-50 ${ | ||
| isVisible ? 'translate-x-0 opacity-100' : 'translate-x-full opacity-0' | ||
| }`} | ||
| > | ||
| <div className={`rounded-lg border p-4 shadow-lg ${getStyles()}`}> | ||
| <div className="flex items-start"> |
There was a problem hiding this comment.
🧩 Analysis chain
Fix toast positioning to avoid overlapping toasts
Both Toast and ToastProvider use fixed top-4 right-4, so multiple toasts will overlap instead of stacking (the provider’s space-y-2 won’t apply).
Let the provider own the fixed positioning and keep each toast relatively positioned:
- return (
- <div
- className={`fixed top-4 right-4 max-w-sm w-full transform transition-all duration-300 z-50 ${
- isVisible ? 'translate-x-0 opacity-100' : 'translate-x-full opacity-0'
- }`}
- >
+ return (
+ <div
+ className={`max-w-sm w-full transform transition-all duration-300 ${
+ isVisible ? 'translate-x-0 opacity-100' : 'translate-x-full opacity-0'
+ }`}
+ >
<div className={`rounded-lg border p-4 shadow-lg ${getStyles()}`}>With this, the provider’s outer fixed container will handle screen placement and vertical spacing as intended.
Also applies to: 119-123
I'll verify the review comment by inspecting the actual Toast and ToastProvider code to confirm the positioning overlap issue.
🏁 Script executed:
#!/bin/bash
# Find and examine the toast-notification.tsx file
fd -t f "toast-notification.tsx" --exec cat {} \;Length of output: 3929
Toast positioning overlap issue confirmed—fix as suggested.
The code confirms the problem: both Toast (lines 61-73) and ToastProvider (lines 115-119) use fixed top-4 right-4 z-50. Nested fixed positioning causes all toasts to render at the same viewport coordinates, making space-y-2 ineffective. Multiple toasts will stack only via z-index, not vertical spacing.
The proposed fix correctly removes fixed positioning from individual toasts, letting the provider's fixed container and space-y-2 handle placement and vertical stacking.
🤖 Prompt for AI Agents
In web/src/components/scenarios/toast-notification.tsx around lines 61 to 68,
the Toast component currently applies fixed positioning and top/right/z-50 which
conflicts with the ToastProvider's fixed container and prevents vertical
stacking via space-y-2; remove the fixed/top-4/right-4/z-50 classes from the
individual Toast wrapper so it becomes positioned by the provider (keep
transition/transform/opacity classes for enter/exit animations) and let the
provider’s fixed container and space-y-2 manage placement and spacing of
multiple toasts.
| {/* Collapsed state - show icon only */} | ||
| {collapsed && ( | ||
| <button | ||
| onClick={() => setUserMenuOpen(!userMenuOpen)} | ||
| className="w-full flex items-center justify-center p-2 text-gray-500 hover:bg-gray-100 rounded-md transition-colors" | ||
| title={user.full_name || user.username} | ||
| > | ||
| <LogOut className="h-4 w-4 flex-shrink-0" /> | ||
| {!collapsed && ( | ||
| <span className="ml-3">Logout</span> | ||
| )} | ||
| </Button> | ||
| </div> | ||
| <User className="h-4 w-4" /> | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
Collapsed user icon toggles state but renders no menu
In the collapsed sidebar branch, the user icon button toggles userMenuOpen but there is no corresponding dropdown rendered in this state. From a UX perspective, the button appears interactive but has no visible effect, and settings/logout remain accessible only when the sidebar is expanded.
Either reuse the dropdown markup for the collapsed case (e.g., position it beside/above the icon) or avoid toggling userMenuOpen here to prevent a no-op control.
🤖 Prompt for AI Agents
In web/src/components/sidebar.tsx around lines 219 to 228, the collapsed branch
renders a user icon button that toggles userMenuOpen but never renders the
dropdown, producing a no-op control; fix by either (A) reusing the existing
dropdown markup for the collapsed case — render the same menu when userMenuOpen
is true but position it absolutely beside/above the icon and ensure
accessibility attributes and click-away handling are preserved, or (B) stop
toggling userMenuOpen here and make the button a non-toggle (e.g., remove the
onClick or open a dedicated collapsed-menu state) so the control does not appear
interactive unless a visible menu is shown. Ensure styling, focus management,
and the logout/settings actions remain reachable in the collapsed view if you
choose option A.
| export interface ScenarioTypeConfig { | ||
| label: string | ||
| icon: React.ComponentType<{ className?: string }> | ||
| color: string | ||
| bgColor: string | ||
| description: string | ||
| } |
There was a problem hiding this comment.
Import React types for ScenarioTypeConfig.icon
ScenarioTypeConfig.icon is typed as React.ComponentType<{ className?: string }>, but this file doesn’t import React at all:
export interface ScenarioTypeConfig {
label: string
icon: React.ComponentType<{ className?: string }>
// ...
}Under normal TS settings, React is not a global, so this will fail to compile.
A cleaner approach is to import the component type explicitly and drop the React. prefix:
-// Proper TypeScript interfaces for scenarios
+// Proper TypeScript interfaces for scenarios
+import type { ComponentType } from 'react'
@@
-export interface ScenarioTypeConfig {
- label: string
- icon: React.ComponentType<{ className?: string }>
+export interface ScenarioTypeConfig {
+ label: string
+ icon: ComponentType<{ className?: string }>
color: string
bgColor: string
description: string
}🤖 Prompt for AI Agents
In web/src/types/scenarios.ts around lines 22 to 28, the icon property uses
React.ComponentType but React is not imported; import the ComponentType type
from 'react' (using an `import type { ComponentType } from 'react'`) and update
the icon signature to use ComponentType<{ className?: string }> so the file
compiles under strict TS settings.
No description provided.