Feat: Integrate AI Price Optimisation for Smarter Pricing Strategies #126
Conversation
|
Caution Review failedFailed to post review comments WalkthroughThis pull request introduces comprehensive backend APIs for brand dashboards, contract management, and AI-powered features, alongside corresponding frontend components. Changes include new ORM models, route modules integrating Groq AI and Supabase, Redis session management, database schema expansion, and UI pages with modals for contract and dashboard interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Frontend<br/>(BrandDashboard)
participant AIQuery as /api/ai/query
participant AIRouter as AIRouter<br/>(Service)
participant DashboardAPI as /api/brand<br/>(Endpoints)
participant Redis as Redis<br/>(Session)
participant Supabase as Supabase<br/>(DB)
User->>Frontend: Enters AI query
Frontend->>AIQuery: POST /api/ai/query<br/>(query, session_id)
AIQuery->>Redis: Load session state
Redis-->>AIQuery: Prior context
AIQuery->>AIRouter: process_query()
AIRouter->>AIRouter: Call Groq LLM
AIRouter-->>AIQuery: intent, route, parameters
AIQuery->>DashboardAPI: Conditional route invocation<br/>(e.g., search_creators)
DashboardAPI->>Supabase: Query aggregated data
Supabase-->>DashboardAPI: Results
DashboardAPI-->>AIQuery: Route response
AIQuery->>Redis: Save enriched<br/>session state
AIQuery-->>Frontend: AIQueryResponse<br/>(intent, route, result)
Frontend->>User: Display results
sequenceDiagram
participant User
participant Frontend as SmartContractGenerator<br/>(Modal)
participant GenAPI as /api/contracts/<br/>generation/generate
participant PricingAPI as /api/pricing/<br/>recommendation
participant Groq as Groq AI
participant Supabase as Supabase
User->>Frontend: Fill form (creator, brand,<br/>budget range, duration, etc.)
Frontend->>PricingAPI: GET pricing recommendation<br/>(creator metrics, content type)
PricingAPI->>Supabase: Find similar contracts
Supabase-->>PricingAPI: Similar contracts
PricingAPI-->>Frontend: Adjusted price, confidence
Frontend->>User: Show pricing estimate
User->>Frontend: Confirm and generate
Frontend->>GenAPI: POST generate contract<br/>(all form data)
GenAPI->>Supabase: Fetch creator/brand,<br/>similar contracts
GenAPI->>Groq: Build prompt + call AI
Groq-->>GenAPI: Generated contract JSON
GenAPI-->>Frontend: GeneratedContract payload
Frontend->>User: Display generated contract
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Specific areas requiring attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 69
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Backend/sql.txt (1)
1-1: Enable pgcrypto for gen_random_uuid()Ensure extension is installed before usage.
+CREATE EXTENSION IF NOT EXISTS pgcrypto;
🧹 Nitpick comments (81)
Frontend/src/pages/BasicDetails.tsx (1)
173-173: Consider consistency in icon approach.The Globe icon still uses the lucide-react icon component while all social platform icons have been changed to image elements. This creates an inconsistent visual approach.
If intentional (to distinguish personal websites from social platforms), this is fine. Otherwise, consider using an image element for consistency, or converting all icons back to components for a unified approach.
Backend/app/services/db_service.py (1)
10-11: LGTM! Consider extracting to a custom exception.The environment variable validation is a good defensive practice and prevents cryptic errors downstream.
If you prefer stricter adherence to exception handling best practices, consider extracting the message to a custom exception class:
+class ConfigurationError(ValueError): + """Raised when required configuration is missing.""" + pass + if not url or not key: - raise ValueError("SUPABASE_URL and SUPABASE_KEY must be set in environment variables") + raise ConfigurationError("SUPABASE_URL and SUPABASE_KEY must be set in environment variables")Based on static analysis hints.
Backend/app/models/models.py (1)
20-21: Refactor duplicategenerate_uuidfunction.The
generate_uuidfunction is duplicated across multiple files in the codebase. This violates the DRY principle and makes maintenance harder.Consider extracting this utility to a shared module:
Step 1: Create a new utility file
Backend/app/utils/uuid_utils.py:import uuid def generate_uuid() -> str: """Generate a UUID string for database primary keys.""" return str(uuid.uuid4())Step 2: Update imports across the codebase:
In
Backend/app/models/models.py:-import uuid - - -def generate_uuid(): - return str(uuid.uuid4()) +from app.utils.uuid_utils import generate_uuidRepeat similar changes in:
Backend/app/routes/post.py(lines 32-33)Backend/app/models/chat.py(lines 8-9)Frontend/src/components/analytics/metrics-chart.tsx (2)
67-81: Type the CustomTooltip props to Recharts’ shape instead ofany.Prevents runtime/TS errors and aids refactors. Define a TooltipProps type and use it.
- const CustomTooltip = ({ active, payload, label }: any) => { + import type { TooltipProps } from 'recharts'; + const CustomTooltip: React.FC<TooltipProps<number, string>> = ({ active, payload, label }) => {
87-92: Harden date parsing in tick formatters.
new Date(value)is locale/format sensitive; parse only ISO strings or preformat upstream to avoid NaN/TZ drift. Consider a safe parser (e.g., date-fnsparseISO) or pass preformattedlabelfields.Also applies to: 115-120
Backend/app/services/data_collectors.py (4)
58-61: Network calls lack timeouts and robust error handling.All
requests.get(...)should settimeoutand use consistent logging (avoid- media_response = requests.get(media_url, params=media_params) + media_response = requests.get(media_url, params=media_params, timeout=10) ... - insights_response = requests.get(insights_url, params=insights_params) + insights_response = requests.get(insights_url, params=insights_params, timeout=10) ... - response = requests.get(analytics_url, params=analytics_params) + response = requests.get(analytics_url, params=analytics_params, timeout=10) ... - response = requests.get(demo_url, params=demo_params) + response = requests.get(demo_url, params=demo_params, timeout=10) ... - geo_response = requests.get(demo_url, params=geo_params) + geo_response = requests.get(demo_url, params=geo_params, timeout=10)Also applies to: 65-70, 129-131, 173-176, 238-241, 280-283, 307-311
182-183: Remove unusedsnippet.It’s assigned but never used.
- snippet = video_info.get('snippet', {})
11-19: Prefer timezone-aware datetimes and explicit unions.
- Use
datetime.now(timezone.utc)instead ofutcnow()to avoid naive timestamps.- Type
demographics/collected_atasdict[str, Any] | None(PEP 604) per Ruff.-from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone ... - def __init__(..., demographics: Dict[str, Any] = None, collected_at: datetime = None): + def __init__(..., demographics: Dict[str, Any] | None = None, collected_at: datetime | None = None): ... - self.collected_at = collected_at or datetime.utcnow() + self.collected_at = collected_at or datetime.now(timezone.utc) ... - collected_at=datetime.utcnow() + collected_at=datetime.now(timezone.utc) ... - collected_at=datetime.utcnow() + collected_at=datetime.now(timezone.utc)Also applies to: 88-93, 205-210
255-257: Avoid bareexcept ExceptionandReplace
error_handling_service.log_error(...)and catch specific exceptions where possible.Also applies to: 323-325, 99-103, 216-219
Backend/app/services/roi_service.py (1)
163-165: Replace silenttry/except/ passwith debug logging.At least log at debug to aid ops; avoid swallowing cache failures entirely.
Also applies to: 173-175, 438-439, 460-461
Frontend/src/components/collaboration-hub/CreatorMatchGrid.tsx (2)
21-23: Prefer a stable ID for React keys; fall back to index only if necessary.Using index harms reconciliation on reorders. If
creator.idexists, use it; else fallback.- {currentCreators.map((creator, index) => ( - <CreatorMatchCard key={`${creator.name}-${index}`} {...creator} /> + {currentCreators.map((creator, index) => ( + <CreatorMatchCard key={creator.id ?? `${creator.name}-${index}`} {...creator} /> ))}
12-12: Guard pagination for empty lists to avoid “Page 1 of 0”.Ensure
totalPages >= 1and clamp page accordingly.- const totalPages = Math.ceil(creators.length / PAGE_SIZE); + const totalPages = Math.max(1, Math.ceil(creators.length / PAGE_SIZE));Also applies to: 25-41
Frontend/src/index.css (1)
1-1: Consider self-hosting or using<link rel="preconnect">for Google Fonts.
@importcan block rendering and complicate CSP. Prefer<link>in HTML with preconnect or self-host the font for performance/privacy.Backend/app/routes/post.py (1)
25-26: Avoid raising at import time; validate during startup or first useRaising here crashes the app at import if envs are missing. Prefer validating in app startup (lifespan) or a dependency that returns 503 with a clear message.
Example:
- if not url or not key: - raise ValueError("SUPABASE_URL and SUPABASE_KEY must be set in environment variables") + if not url or not key: + # Defer hard failure to runtime with a clear API error + # or log and continue for non-dependent routes. + import logging + logging.error("Missing SUPABASE_URL/SUPABASE_KEY; user routes will 503")And gate usage inside handlers:
if not url or not key: raise HTTPException(status_code=503, detail="Supabase is not configured")As per static analysis hints (TRY003).
Backend/.env-example (2)
6-6: Add a model selector to avoid code edits when switching modelsInclude
GROQ_MODELso deployments can switch models without code changes.GROQ_API_KEY=your_groq_api_key_here +GROQ_MODEL=llama3-8b-8192
10-15: Secrets hygiene and quoting noteDocument that values with special chars (e.g., Redis passwords) should be quoted, and never commit real keys.
Add:
# Note: wrap values containing special characters in quotes, e.g. # REDIS_PASSWORD="p@ss:word" # Do NOT commit real secrets.Frontend/src/App.tsx (1)
73-84: Nice additions; consider lazy loading to reduce initial bundleWrap heavy pages in React.lazy/Suspense (e.g., Contracts, DashboardOverview) to improve TTI.
Example:
-import DashboardOverview from "./pages/Brand/DashboardOverview"; -import ErrorBoundary from "./components/ErrorBoundary"; +const DashboardOverview = React.lazy(() => import("./pages/Brand/DashboardOverview")); +const ErrorBoundary = React.lazy(() => import("./components/ErrorBoundary"));And wrap routes with a .
Frontend/src/components/user-nav.tsx (2)
17-21: Prop addition LGTMOptional: document
showDashboardin JSDoc for autocomplete and clarity.
67-71: Make dashboard link role‑aware (brand vs. generic)Linking to
/dashboardmay be wrong for brands now using/brand/dashboard. Derive target from user role.- <Link to="/dashboard">Dashboard</Link> + <Link to={user.user_metadata?.role === "brand" ? "/brand/dashboard" : "/dashboard"}> + Dashboard + </Link>Frontend/README-INTEGRATION.md (1)
47-60: Add note for the new Overview route and backend /api prefix
- Include
/brand/dashboard/overviewas an entry point.- Mention that backend routes are now under
/api/*so proxies/env must match.Proposed additions:
5. Visit: http://localhost:5173/brand/dashboard/overview Note: Backend endpoints are prefixed with /api (e.g., /api/brand/overview). Ensure vite proxy maps ^/api to http://localhost:8000.Backend/test_smart_contract_features.py (5)
19-27: Add timeouts and catchrequests.RequestExceptionNetwork calls without timeouts can hang CI; use explicit exceptions.
- try: - response = requests.get(f"{base_url}/docs") + try: + response = requests.get(f"{base_url}/docs", timeout=10) @@ - except Exception as e: + except requests.RequestException as e: print(f"❌ Cannot connect to backend: {e}") returnAs per static analysis (S113, BLE001).
42-56: Timeouts for POST and remove extraneous f‑strings
requests.post(json=...)already sets Content‑Type; keep it simple, add a timeout, and removefwhere unused.- response = requests.post( - f"{base_url}/api/pricing/recommendation", - json=pricing_data, - headers={"Content-Type": "application/json"} - ) + response = requests.post( + f"{base_url}/api/pricing/recommendation", + json=pricing_data, + timeout=15 + ) @@ - print(f"✅ Pricing recommendation received") + print("✅ Pricing recommendation received")As per static analysis (S113, F541).
85-102: Same for contract generation: add timeout and fix f‑string- response = requests.post( + response = requests.post( f"{base_url}/api/contracts/generation/generate", json=contract_data, - headers={"Content-Type": "application/json"} - ) + timeout=30 + ) @@ - print(f"✅ Contract generation successful") + print("✅ Contract generation successful")As per static analysis (S113, F541).
60-62: Preferrequests.RequestExceptionover bareExceptionBroader except hides real failures.
- except Exception as e: + except requests.RequestException as e: print(f"❌ Pricing recommendation test failed: {e}")As per static analysis (BLE001).
106-107: Narrow exception type- except Exception as e: + except requests.RequestException as e: print(f"❌ Contract generation test failed: {e}")As per static analysis (BLE001).
Frontend/src/components/contracts/governing-laws-selector.tsx (1)
28-78: Consider externalizing legal data for maintainability.The hardcoded jurisdiction laws (lines 29-78) could become outdated or legally inaccurate. Consider moving this data to a separate configuration file or fetching it from a backend service maintained by legal experts.
Create a separate file
Frontend/src/config/jurisdictions.ts:export interface Jurisdiction { value: string; label: string; laws: string[]; description: string; } export const jurisdictions: Jurisdiction[] = [ // ... jurisdiction data ];Then import in the component:
+import { jurisdictions } from "@/config/jurisdictions" export function GoverningLawsSelector({ // ... }: GoverningLawsSelectorProps) { - const jurisdictions = [ - // ... (remove hardcoded data) - ]This makes it easier to update legal information without touching component code.
Frontend/src/components/contracts/AdvancedFilters.tsx (2)
38-40: Remove redundant wrapper function.The
handleClearFiltersfunction (lines 38-40) is an unnecessary wrapper aroundonClearFilters. You can callonClearFiltersdirectly.- const handleClearFilters = () => { - onClearFilters(); - }; // In the JSX: <button onClick={(e) => { e.stopPropagation(); - handleClearFilters(); + onClearFilters(); }}
341-350: Budget display shows misleading values for empty filters.When
min_budgetormax_budgetare empty, the display shows "$0 - $∞" (lines 341-350), which implies a range was selected when none was. Consider showing "Any" or omitting the badge entirely if both are empty.{(filters.min_budget || filters.max_budget) && ( <span style={{...}}> - Budget: ${filters.min_budget || '0'} - ${filters.max_budget || '∞'} + Budget: {filters.min_budget ? `$${filters.min_budget}` : 'Any'} - {filters.max_budget ? `$${filters.max_budget}` : 'Any'} </span> )}Or only show the badge if at least one value is set:
- {(filters.min_budget || filters.max_budget) && ( + {(filters.min_budget && filters.max_budget) && ( <span style={{...}}> Budget: ${filters.min_budget} - ${filters.max_budget} </span> )} + {filters.min_budget && !filters.max_budget && ( + <span style={{...}}> + Budget: ${filters.min_budget}+ + </span> + )} + {!filters.min_budget && filters.max_budget && ( + <span style={{...}}> + Budget: Up to ${filters.max_budget} + </span> + )}Frontend/src/components/contracts/contract-generator.tsx (1)
227-241: Hardcoded section numbers will break if sections are reordered.The generated contract uses hardcoded section numbers (4, 5) for governing law and dispute resolution. If you add or reorder sections above, these numbers become incorrect.
Consider using dynamic section numbering:
const sections = [ { condition: true, title: "SCOPE OF SERVICES", content: "..." }, { condition: true, title: "COMPENSATION", content: "..." }, { condition: true, title: "TERM", content: "..." }, { condition: selectedJurisdiction, title: "GOVERNING LAW", content: "..." }, { condition: disputeResolution, title: "DISPUTE RESOLUTION", content: "..." }, ]; // In the render: {sections.filter(s => s.condition).map((section, index) => ( <div key={index}> <p className="text-muted-foreground mt-4">{index + 1}. {section.title}</p> <p>{section.content}</p> </div> ))}Frontend/src/components/contracts/ContractAIAssistant.tsx (2)
4-11: Message interface conflicts with existing chatSlice.ts Message type.A different
Messageinterface already exists inFrontend/src/redux/chatSlice.ts(lines 4-11). Using the same name for a different structure could cause confusion and import errors if both are used in the same scope.Rename to be more specific:
-interface Message { +interface AIAssistantMessage { id: string; type: 'user' | 'ai'; content: string; timestamp: Date; analysis?: any; suggestions?: string[]; } -const [messages, setMessages] = useState<Message[]>([ +const [messages, setMessages] = useState<AIAssistantMessage[]>([
44-98: Consider adding retry logic for transient failures.The API call (lines 58-74) doesn't retry on failure. Network issues or temporary backend problems will immediately show an error to the user. Adding retry logic would improve reliability.
const MAX_RETRIES = 2; const handleSendMessage = async () => { if (!inputValue.trim() || isLoading) return; const userMessage: AIAssistantMessage = { /* ... */ }; setMessages(prev => [...prev, userMessage]); setInputValue(''); setIsLoading(true); let lastError: Error | null = null; for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { try { const response = await fetch(`${API_BASE_URL}/api/contracts/ai/chat`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ query: inputValue, contract_id: selectedContractId }), }); if (!response.ok) { throw new Error(`HTTP ${response.status}: ${response.statusText}`); } const data = await response.json(); const aiMessage: AIAssistantMessage = { /* ... */ }; setMessages(prev => [...prev, aiMessage]); return; // Success } catch (error) { lastError = error as Error; if (attempt < MAX_RETRIES) { await new Promise(resolve => setTimeout(resolve, 1000 * (attempt + 1))); // Exponential backoff } } } // All retries failed console.error('Error sending message after retries:', lastError); const errorMessage: AIAssistantMessage = { /* ... */ }; setMessages(prev => [...prev, errorMessage]); setIsLoading(false); };Backend/migrate_existing_data.py (2)
40-40: Simplify key checks using dict.get().Lines 40 and 49 check if a key exists before accessing it. You can use
dict.get()to simplify.-if "comments" in terms_and_conditions and terms_and_conditions["comments"]: - comments_data = terms_and_conditions["comments"] +comments_data = terms_and_conditions.get("comments") +if comments_data: update_payload["comments"] = comments_data-if "update_history" in terms_and_conditions and terms_and_conditions["update_history"]: - history_data = terms_and_conditions["update_history"] +history_data = terms_and_conditions.get("update_history") +if history_data: update_payload["update_history"] = history_dataAlso applies to: 49-49
65-66: Replace blind exception catching with specific exception types.Lines 65 and 81 catch all exceptions, which can hide bugs and make debugging difficult. Catch specific exceptions or at least log the exception type.
try: supabase.table("contracts").update(update_payload).eq("id", contract_id).execute() migrated_count += 1 print(f"✅ Successfully migrated contract {contract_id}") -except Exception as e: - print(f"❌ Error migrating contract {contract_id}: {str(e)}") +except Exception as e: + import traceback + print(f"❌ Error migrating contract {contract_id}: {type(e).__name__}: {e}") + print(traceback.format_exc())-except Exception as e: - print(f"❌ Error during migration: {str(e)}") +except Exception as e: + import traceback + print(f"❌ Error during migration: {type(e).__name__}: {e}") + print(traceback.format_exc()) + raise # Re-raise to ensure migration failures are visibleAlso applies to: 81-82
Frontend/src/components/contracts/CreateContractModal.tsx (1)
819-819: Inconsistent styling: Tailwind class in inline-styled component.Line 819 uses Tailwind's
className="animate-spin"while the rest of the component uses inline styles. This creates an inconsistent approach and could fail if Tailwind isn't properly configured.Use inline style for consistency:
{loading ? ( <> - <Loader2 size={16} className="animate-spin" /> + <Loader2 size={16} style={{ animation: 'spin 1s linear infinite' }} /> Creating... </>And add the keyframes if needed (in a global stylesheet or styled component):
@keyframes spin { from { transform: rotate(0deg); } to { transform: rotate(360deg); } }Backend/test_roi_integration.py (3)
129-131: Restore dependency overrides after testsGlobal
app.dependency_overridespersists across modules. Clear it in teardown.# Cleanup Base.metadata.drop_all(bind=engine) + app.dependency_overrides.clear()
213-214: Remove debug prints from testsPrinting in tests is noisy in CI.
- print(f"Response status: {response.status_code}") - print(f"Response content: {response.content}")
142-146: Silence “unused fixture arg” while keeping pytest fixture injectionRename
setup_databaseparam to_setup_databasein test signatures.- def test_get_campaign_roi(self, client, setup_database): + def test_get_campaign_roi(self, client, _setup_database):Repeat for the other test methods in this file. Based on static analysis hints.
Also applies to: 168-171, 186-191, 206-211, 227-231, 248-253, 270-275, 286-291, 302-307, 310-315, 316-325
Frontend/src/pages/Brand/Dashboard.tsx (4)
464-468: Replace deprecated onKeyPress with onKeyDownReact marks onKeyPress as deprecated. Use onKeyDown.
- onKeyPress={(e) => { - if (e.key === 'Enter' && searchQuery.trim()) { + onKeyDown={(e) => { + if (e.key === 'Enter' && searchQuery.trim()) { handleAISearch(); } }}
128-151: Hook up “New Campaign” buttonAdd navigation so the button performs an action.
- <button style={{ + <button + onClick={() => navigate('/brand/campaigns/new')} + style={{ width: "100%", background: PRIMARY,
289-321: A11Y: Toggle should expose stateExpose collapsed state to assistive tech.
- <button + <button + aria-label="Toggle sidebar" + aria-expanded={!sidebarCollapsed} + aria-controls="brand-sidebar" onClick={() => setSidebarCollapsed(!sidebarCollapsed)}Also add
id="brand-sidebar"to the sidebar container div.
246-251: Bind profile to real dataReplace placeholders with
brandProfilefromuseBrandDashboardif available.- <span style={{ fontSize: "14px", fontWeight: 500 }}>John Doe</span> - <span style={{ fontSize: "12px", color: "#808080" }}>[email protected]</span> + <span style={{ fontSize: "14px", fontWeight: 500 }}> + {brandProfile?.name ?? 'Brand'} + </span> + <span style={{ fontSize: "12px", color: "#808080" }}> + {brandProfile?.email ?? ''} + </span>Frontend/src/components/contracts/UpdateContractModal.tsx (3)
57-58: Default to a valid section id
activeSectiondefaults tocomments, which isn’t insections; the initial tab highlight is inconsistent.- const [activeSection, setActiveSection] = useState('comments'); + const [activeSection, setActiveSection] = useState<'status'|'budget'|'deliverables'|'timeline'>('status');Also applies to: 93-99, 404-411
269-285: Avoid in-place mutation of state arraysClone before update to keep state immutable.
- onChange={(e) => { - const currentUpdates = updateData.deliverable_status_updates || []; - const existingIndex = currentUpdates.findIndex(u => u.deliverable_id === `deliverable_${index}`); - - if (existingIndex >= 0) { - currentUpdates[existingIndex].new_status = e.target.value; - } else { - currentUpdates.push({ + onChange={(e) => { + const currentUpdates = updateData.deliverable_status_updates || []; + const next = currentUpdates.map(u => ({ ...u })); + const existingIndex = next.findIndex(u => u.deliverable_id === `deliverable_${index}`); + if (existingIndex >= 0) { + next[existingIndex].new_status = e.target.value; + } else { + next.push({ deliverable_id: `deliverable_${index}`, new_status: e.target.value, notes: '' }); } - - handleInputChange('deliverable_status_updates', currentUpdates); + handleInputChange('deliverable_status_updates', next); }}Apply the same pattern to the textarea handler below.
Also applies to: 305-333
414-466: A11Y: Modal semanticsAdd dialog semantics and associate title.
- return ( - <div style={{ + return ( + <div role="dialog" aria-modal="true" aria-labelledby="update-contract-title" style={{ position: 'fixed',- <h2 style={{ fontSize: '24px', fontWeight: '700', marginBottom: '4px' }}>Update Contract</h2> + <h2 id="update-contract-title" style={{ fontSize: '24px', fontWeight: '700', marginBottom: '4px' }}>Update Contract</h2>Also applies to: 448-451
Frontend/src/components/chat/BrandChatAssistant.tsx (2)
71-75: Guard initial send and improve session handlingSkip initial request for empty queries; always accept updated session id.
- useEffect(() => { - if (messages.length === 1) { + useEffect(() => { + if (messages.length === 1 && initialQuery.trim()) { setLoading(true); sendMessageToBackend(initialQuery) .then((response) => { + if (response.session_id) setSessionId(response.session_id);- const response = await sendMessageToBackend(input, sessionId || undefined); + const response = await sendMessageToBackend(input, sessionId || undefined); + if (response.session_id) setSessionId(response.session_id);Also applies to: 97-105
209-219: Render structured results as preformattedImproves readability for JSON payloads.
- {msg.result && ( - <div style={{ + {msg.result && ( + <pre style={{ marginTop: "8px", padding: "8px", background: "rgba(59, 130, 246, 0.1)", borderRadius: "8px", fontSize: "14px", border: "1px solid rgba(59, 130, 246, 0.3)", }}> - <strong>Result:</strong> {JSON.stringify(msg.result, null, 2)} - </div> + <strong>Result:</strong> {`\n`}{JSON.stringify(msg.result, null, 2)} + </pre> )}Frontend/src/components/contracts/ContractDetailsModal.tsx (2)
828-833: Replace deprecated onKeyPress with onKeyDown- onKeyPress={(e) => { + onKeyDown={(e) => { if (e.key === 'Enter' && !e.shiftKey) { e.preventDefault(); handleSubmitComment(); } }}
894-907: A11Y: Modal semanticsAdd dialog semantics and label.
- return ( - <div style={{ + return ( + <div role="dialog" aria-modal="true" aria-labelledby="contract-details-title" style={{ position: 'fixed',- <h2 style={{ fontSize: '24px', fontWeight: '700', marginBottom: '4px' }}>{contract.contract_title || `Contract ${contract.id.slice(0, 8)}`}</h2> + <h2 id="contract-details-title" style={{ fontSize: '24px', fontWeight: '700', marginBottom: '4px' }}> + {contract.contract_title || `Contract ${contract.id.slice(0, 8)}`} + </h2>Frontend/src/pages/Brand/DashboardOverview.tsx (2)
120-169: Parallelize requests and add AbortController to avoid blocking/unmounted updates.Current sequential fetches slow the page and may update state after unmount.
useEffect(() => { - const fetchDashboardData = async () => { + const fetchDashboardData = async () => { + const controller = new AbortController(); try { setLoading(true); setError(null); - const kpisResponse = await fetch(`${API_BASE}/brand/dashboard/kpis?brand_id=${brandId}`); - ... - const notificationsResponse = await fetch(`${API_BASE}/brand/dashboard/notifications?brand_id=${brandId}`); + const [kpisResponse, campaignsResponse, analyticsResponse, notificationsResponse] = await Promise.all([ + fetch(`${API_BASE}/brand/dashboard/kpis?brand_id=${brandId}`, { signal: controller.signal }), + fetch(`${API_BASE}/brand/dashboard/campaigns/overview?brand_id=${brandId}`, { signal: controller.signal }), + fetch(`${API_BASE}/brand/dashboard/analytics?brand_id=${brandId}`, { signal: controller.signal }), + fetch(`${API_BASE}/brand/dashboard/notifications?brand_id=${brandId}`, { signal: controller.signal }), + ]); if (!kpisResponse.ok) throw new Error('Failed to fetch KPIs data'); if (!campaignsResponse.ok) throw new Error('Failed to fetch campaigns data'); if (!analyticsResponse.ok) throw new Error('Failed to fetch analytics data'); if (!notificationsResponse.ok) throw new Error('Failed to fetch notifications data'); const kpisData = await kpisResponse.json(); const campaignsData = await campaignsResponse.json(); const analyticsData = await analyticsResponse.json(); const notificationsData = await notificationsResponse.json(); setDashboardData({ kpis: kpisData.kpis, creators: kpisData.creators, financial: kpisData.financial, analytics: analyticsData.analytics, campaigns: campaignsData.campaigns, notifications: notificationsData.notifications }); } catch (err) { console.error('Error fetching dashboard data:', err); setError(err instanceof Error ? err.message : 'Failed to load dashboard data'); setDashboardData(mockData); } finally { setLoading(false); } + return () => controller.abort(); }; fetchDashboardData(); }, [brandId]);
187-214: Show fallback data with an inline error banner instead of halting the page.You set fallback data but early-return the error view, so users never see it.
- Render the error banner at the top and continue rendering with
data = dashboardData || mockData.Backend/app/routes/pricing.py (2)
178-187: Remove unused DI param or underscore it; switch to Annotated.
pricing_serviceisn’t used in market-analysis, and Ruff flags B008. Either remove it or underscore + Annotated.-async def get_market_analysis( - content_type: str, - platform: str, - pricing_service: PricingService = Depends(get_pricing_service) -): +async def get_market_analysis( + content_type: str, + platform: str, + _pricing_service: "Annotated[PricingService, Depends(get_pricing_service)]", +):Also applies to: 182-183
217-236: Use statistics.median for correct median (even-sized lists).Current median picks the upper middle only.
+import statistics @@ - "median": sorted(prices)[len(prices)//2] if prices else 0, + "median": statistics.median(prices) if prices else 0, @@ - "median": sorted(followers)[len(followers)//2] if followers else 0, + "median": statistics.median(followers) if followers else 0, @@ - "median": sorted(engagement_rates)[len(engagement_rates)//2] if engagement_rates else 0, + "median": statistics.median(engagement_rates) if engagement_rates else 0,Backend/app/services/ai_router.py (2)
131-131: PEP 484 typing: make brand_id Optional.-async def process_query(self, query: str, brand_id: str = None) -> Dict[str, Any]: +async def process_query(self, query: str, brand_id: Optional[str] = None) -> Dict[str, Any]:
167-170: Use logger.exception and chain the HTTPException.Better diagnostics; don’t drop the original traceback.
- except Exception as e: - logger.error(f"Error processing query with AI Router: {e}") - raise HTTPException(status_code=500, detail="AI processing error") + except Exception as e: + logger.exception("Error processing query with AI Router") + raise HTTPException(status_code=500, detail="AI processing error") from eBackend/app/routes/ai_query.py (2)
147-152: Parenthesize boolean logic for follow_up_needed.Clarifies precedence and silences Ruff RUF021.
- follow_up_needed=not all_params_present and not only_optional_params or api_error is not None, + follow_up_needed=(not all_params_present and not only_optional_params) or (api_error is not None),
138-141: Use logger.exception and chain raised errors.Improves observability and preserves tracebacks.
- except Exception as api_exc: - logger.error(f"API call failed for intent '{intent}': {api_exc}") - api_error = str(api_exc) + except Exception as api_exc: + logger.exception("API call failed for intent '%s'", intent) + api_error = str(api_exc) @@ - except Exception as e: - logger.error(f"Error processing AI query: {e}") - raise HTTPException(status_code=500, detail="Failed to process AI query") + except Exception as e: + logger.exception("Error processing AI query") + raise HTTPException(status_code=500, detail="Failed to process AI query") from e @@ - except Exception as e: - logger.error(f"Error fetching available routes: {e}") + except Exception: + logger.exception("Error fetching available routes") @@ - except Exception as e: - logger.error(f"Error fetching route info: {e}") + except Exception: + logger.exception("Error fetching route info") @@ - except Exception as e: - logger.error(f"Error in test AI query: {e}") + except Exception: + logger.exception("Error in test AI query")Also applies to: 186-187, 200-202, 221-222, 239-239
Frontend/src/components/contracts/SmartContractGenerator.tsx (2)
539-546: Clamp duration_weeks to [1, 52] and prevent NaN on empty input.Prevents sending invalid values to backend.
- onChange={(e) => handleInputChange('duration_weeks', parseInt(e.target.value))} + onChange={(e) => { + const v = Math.max(1, Math.min(52, Number(e.target.value) || 1)); + handleInputChange('duration_weeks', v); + }}
912-913: Tailwind z-60 may be undefined; use arbitrary value or z-50.[z utilities default to 0..50]. Use
z-[60]orz-50.- <div className="fixed inset-0 bg-black/80 backdrop-blur-sm z-60 flex items-center justify-center p-4"> + <div className="fixed inset-0 bg-black/80 backdrop-blur-sm z-[60] flex items-center justify-center p-4">Frontend/src/components/contracts/EditContractModal.tsx (2)
246-250: Remove debug logs before releaseConsole logs include potentially sensitive payloads. Remove to keep console clean.
- console.log('Updating contract with data:', finalUpdateData); - console.log('Jurisdiction data:', cleanJurisdictionData); - console.log('Terms and conditions:', termsAndConditions); - console.log('JSON stringified data:', JSON.stringify(finalUpdateData, null, 2));
820-835: Optional: use numeric inputs for numeric fieldsQuantity/advance/final payment look numeric; using type="number" improves UX and validation.
- <input - type="text" + <input + type="number" value={deliverablesData.quantity} @@ - <input - type="text" + <input + type="number" value={paymentData.advance_payment} @@ - <input - type="text" + <input + type="number" value={paymentData.final_payment}Also applies to: 748-766, 772-786
Backend/app/services/pricing_service.py (3)
52-54: Replace prints and blind excepts with structured loggingUse logging and include context; avoid bare
except Exception.+import logging +logger = logging.getLogger(__name__) @@ - print(f"Found {len(contracts)} contracts in database") - print(f"Query filters: content_type={content_type}, platform={platform}") + logger.debug("Found %d contracts (content_type=%s, platform=%s)", len(contracts), content_type, platform) @@ - if not contracts: - print("No contracts found with current filters") + if not contracts: + logger.info("No contracts found with current filters") return [] @@ - except Exception as e: - print(f"Error finding similar contracts: {e}") + except Exception as e: + logger.exception("Error finding similar contracts") return []Also applies to: 85-87
189-199: Remove stray docstring blockThis triple‑quoted string is a no‑op and confuses readers.
- """ - Generate price recommendation based on similar contracts - """
412-434: Minor: unused parameter in_calculate_confidence_score
similar_contractsisn’t used. Either remove it or factor it into confidence. Up to you; keeping the signature consistent may be fine, but Ruff flags it.Would you like me to adjust the signature and all call sites?
Backend/app/routes/contracts_ai.py (1)
43-49: Reduce payload: limit and narrow Supabase selectsPulling all contracts is expensive. Limit rows and fields; compute stats via dedicated queries.
- contracts_response = supabase.table("contracts").select("*").execute() + contracts_response = supabase.table("contracts").select( + "id,contract_title,total_budget,status,start_date,end_date,contract_type,brand_id,creator_id,updated_at" + ).limit(200).execute() @@ - stats_response = supabase.table("contracts").select("status, total_budget").execute() + stats_response = supabase.table("contracts").select("status,total_budget").execute()Backend/app/services/data_ingestion_service.py (4)
46-53: Use SQLAlchemy boolean expressions, not== TrueAvoid E712 and ensure proper SQL semantics.
@@ - content_mapping = db.query(ContractContentMapping).filter( - ContractContentMapping.id == content_mapping_id, - ContractContentMapping.is_active == True - ).first() + content_mapping = db.query(ContractContentMapping).filter( + ContractContentMapping.id == content_mapping_id, + ContractContentMapping.is_active.is_(True) + ).first() @@ - user_token = db.query(UserSocialToken).filter( + user_token = db.query(UserSocialToken).filter( UserSocialToken.user_id == content_mapping.user_id, UserSocialToken.platform == content_mapping.platform, - UserSocialToken.is_active == True + UserSocialToken.is_active.is_(True) ).first() @@ - content_mappings = db.query(ContractContentMapping).filter( + content_mappings = db.query(ContractContentMapping).filter( ContractContentMapping.contract_id == contract_id, - ContractContentMapping.is_active == True + ContractContentMapping.is_active.is_(True) ).all() @@ - content_mappings = db.query(ContractContentMapping).filter( + content_mappings = db.query(ContractContentMapping).filter( ContractContentMapping.user_id == user_id, - ContractContentMapping.is_active == True + ContractContentMapping.is_active.is_(True) ).all() @@ - user_token = db.query(UserSocialToken).filter( + user_token = db.query(UserSocialToken).filter( UserSocialToken.user_id == user_id, UserSocialToken.platform == platform, - UserSocialToken.is_active == True + UserSocialToken.is_active.is_(True) ).first()Also applies to: 59-66, 129-136, 181-189, 371-379
96-107: Handle asyncio task properly; remove unused loop varStore task handle; drop unused variable.
- loop = asyncio.get_running_loop() - # Run cache invalidation in background - asyncio.create_task( + asyncio.get_running_loop() + # Run cache invalidation in background + _invalidate_task = asyncio.create_task( cache_invalidation_service.invalidate_related_data( db, 'content', content_mapping_id ) )
104-107: Replace prints with loggingPromote observability and avoid stdout noise.
+import logging +logger = logging.getLogger(__name__) @@ - print("Warning: No running event loop for cache invalidation") + logger.warning("No running event loop for cache invalidation") @@ - return False, f"Error syncing content data: {str(e)}" + return False, f"Error syncing content data: {e!s}" @@ - return False, f"Error syncing contract content: {str(e)}", {} + return False, f"Error syncing contract content: {e!s}", {} @@ - print(f"Error getting content analytics: {e}") + logger.exception("Error getting content analytics") @@ - print(f"Error checking rate limit: {e}") + logger.exception("Error checking rate limit") @@ - print(f"Error updating usage tracker: {e}") + logger.exception("Error updating usage tracker") @@ - print(f"Error storing content analytics: {e}") + logger.exception("Error storing content analytics")Also applies to: 114-114, 166-167, 219-221, 271-273, 323-325, 336-337, 365-365
389-391: Minor: remove unused local
collectoris unused; remove or prefix with underscore.- collector = DataCollectorFactory.get_collector(platform) + DataCollectorFactory.get_collector(platform) # validate platform supportBackend/app/schemas/schema.py (3)
21-28: Tighten required_audience typingUse Dict[str, List[str]] for clarity and better validation.
Apply:
- required_audience: Dict[str, list] + required_audience: Dict[str, List[str]]
150-157: Strongly type recent_activityPrefer List[Dict] (or a concrete model) over bare list.
- recent_activity: list + recent_activity: List[Dict]
197-199: Constrain status fields with LiteralsLimit allowed values to avoid invalid states.
-from typing import Optional, Dict, List +from typing import Optional, Dict, List, Literal @@ -class ApplicationUpdateRequest(BaseModel): - status: str # "accepted", "rejected", "pending" +class ApplicationUpdateRequest(BaseModel): + status: Literal["accepted", "rejected", "pending"] @@ -class PaymentStatusUpdate(BaseModel): - status: str # "pending", "completed", "failed", "cancelled" +class PaymentStatusUpdate(BaseModel): + status: Literal["pending", "completed", "failed", "cancelled"]Also applies to: 224-226
Backend/app/routes/contracts_generation.py (3)
84-105: Avoid printing PII; use logger and redactReplace prints with logger.debug/info and don’t log full user rows.
- print(f"Looking up user with email: {email}") + # logger.debug("Looking up user by email") @@ - print(f"User response: {user_response.data}") + # logger.debug("User lookup succeeded") @@ - print(f"Error in get_user_by_email: {str(e)}") + # logger.exception("Error in get_user_by_email")
356-358: Clarify and/or precedenceMake the expression explicit for readability.
-{jurisdiction_info and f"- {jurisdiction_info}" or ""} -{dispute_info and f"- {dispute_info}" or ""} +{f"- {jurisdiction_info}" if jurisdiction_info else ""} +{f"- {dispute_info}" if dispute_info else ""}
271-279: Model name should be configurableRead from env (e.g., GROQ_MODEL) with a sensible default.
- payload = { - "model": "moonshotai/kimi-k2-instruct", + payload = { + "model": os.environ.get("GROQ_MODEL", "llama3-70b-8192"),Also applies to: 419-427
Backend/app/routes/contracts.py (4)
349-363: Drop noisy prints in update_contractThese leak internal data to logs. Remove or guard behind debug logger.
- print(f"Updating contract {contract_id} with data: {contract_update.dict()}") - print(f"Raw request data: {contract_update}") + # Optionally log at debug with redaction @@ - print(f"Final update data: {update_data}") - print(f"JSON stringified update data: {json.dumps(update_data, default=str)}") @@ - print(f"Field {key}: {type(value)} = {value}") + pass
481-483: Remove f-prefix with no placeholdersMinor nit; avoid f-strings without interpolation.
- print(f"Added update history entry") + print("Added update history entry") @@ - print(f"Added comment entry") + print("Added comment entry")Also applies to: 523-526
1216-1221: Avoid bare excepts in text generationCatch specific exceptions to prevent masking errors.
- try: + try: dt = datetime.fromisoformat(timestamp.replace('Z', '+00:00')) formatted_time = dt.strftime("%B %d, %Y at %I:%M %p") - except: + except (ValueError, TypeError): formatted_time = timestamp(Same change for Lines 1254-1260.)
Also applies to: 1219-1221, 1254-1262
282-328: Search endpoint loads all contracts then filters in PythonConsider Supabase ilike/text search to offload filtering and reduce transfer.
Backend/sql.txt (1)
71-80: Add indexes on FK/filter columnsAdd btree indexes to match frequent filters in routes (brand_id, creator_id, campaign_id, contract_id, created_at).
Example:
CREATE INDEX IF NOT EXISTS idx_contracts_brand_id ON contracts(brand_id); CREATE INDEX IF NOT EXISTS idx_contracts_creator_id ON contracts(creator_id); CREATE INDEX IF NOT EXISTS idx_campaign_metrics_campaign_id ON campaign_metrics(campaign_id); CREATE INDEX IF NOT EXISTS idx_contract_payments_contract_id ON contract_payments(contract_id);Also applies to: 512-570, 576-586, 591-616
Backend/app/routes/brand_dashboard.py (2)
81-95: Use window_seconds in rate limiter keyCurrently ignored; fixes accuracy and reduces unbounded keys.
- key = f"{user_id}:{current_time.minute}" + window_start = int(current_time.timestamp()) // window_seconds + key = f"{user_id}:{window_start}"
74-76: Use logger.exception in except blocksKeeps stack traces without prints.
- logger.error(f"Supabase error in {error_message}: {e}") + logger.exception(f"Supabase error in {error_message}")(Apply similarly in other except blocks where logger.error is used.)
Also applies to: 159-161, 283-284, 354-355, 461-462, 529-530
| class AIQueryResponse(BaseModel): | ||
| intent: str | ||
| route: Optional[str] = None | ||
| parameters: Dict[str, Any] = {} | ||
| follow_up_needed: bool = False | ||
| follow_up_question: Optional[str] = None | ||
| explanation: str | ||
| original_query: str | ||
| timestamp: str | ||
|
|
There was a problem hiding this comment.
Response model is dropping session_id/result/error. Include them in schema and avoid mutable default.
FastAPI filters unknown fields; your extras won’t reach clients. Also avoid {} as default.
-from pydantic import BaseModel
+from pydantic import BaseModel, Field
@@
class AIQueryResponse(BaseModel):
intent: str
route: Optional[str] = None
- parameters: Dict[str, Any] = {}
+ parameters: Dict[str, Any] = Field(default_factory=dict)
follow_up_needed: bool = False
follow_up_question: Optional[str] = None
explanation: str
original_query: str
timestamp: str
+ session_id: str
+ result: Optional[Any] = None
+ error: Optional[str] = None🤖 Prompt for AI Agents
In Backend/app/routes/ai_query.py around lines 22 to 31, the AIQueryResponse
Pydantic model is missing session_id, result, and error fields and uses a
mutable default for parameters; update the schema to include session_id:
Optional[str], result: Optional[Any] (or a more specific type), and error:
Optional[str] (or Optional[Dict[str, Any]] depending on needs), and change
parameters to use a non-mutable default (e.g., Optional[Dict[str, Any]] = None
or use Field(default_factory=dict)) so FastAPI returns those fields and you
avoid mutable default pitfalls.
| @router.get("/users/{user_id}") | ||
| async def get_user(user_id: str): | ||
| try: | ||
| result = supabase.table("users").select("*").eq("id", user_id).execute() | ||
|
|
||
| if not result.data: | ||
| raise HTTPException(status_code=404, detail="User not found") | ||
|
|
||
| return result.data[0] | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"Error fetching user: {str(e)}") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent response shape and broad exception; tighten and standardize
- Other endpoints return the Supabase response object; this returns a raw record. Pick one shape and keep it consistent.
- Avoid catching bare
Exception; chain withfrom e. - Don’t leak internal error details in API responses.
@router.get("/users/{user_id}")
async def get_user(user_id: str):
- try:
- result = supabase.table("users").select("*").eq("id", user_id).execute()
-
- if not result.data:
- raise HTTPException(status_code=404, detail="User not found")
-
- return result.data[0]
- except HTTPException:
- raise
- except Exception as e:
- raise HTTPException(status_code=500, detail=f"Error fetching user: {str(e)}")
+ try:
+ if not url or not key:
+ raise HTTPException(status_code=503, detail="Supabase is not configured")
+ result = supabase.table("users").select("*").eq("id", user_id).execute()
+ if not getattr(result, "data", None):
+ raise HTTPException(status_code=404, detail="User not found")
+ # Option A: return consistent wrapper
+ return {"data": result.data[:1], "count": 1}
+ # Option B (if other routes are updated): return result
+ except HTTPException:
+ raise
+ except Exception as e:
+ raise HTTPException(status_code=500, detail="Error fetching user") from eAs per static analysis hints (BLE001, B904, RUF010).
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.1)
68-68: Abstract raise to an inner function
(TRY301)
73-73: Do not catch blind exception: Exception
(BLE001)
74-74: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
74-74: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In Backend/app/routes/post.py around lines 62 to 74, standardize the response
shape to match other endpoints by returning the full Supabase response object
(not a raw record), avoid catching a bare Exception by catching specific
exceptions or re-raising with exception chaining, and stop leaking internal
error text to clients: after calling
supabase.table(...).select(...).eq(...).execute(), if result.data is empty raise
HTTPException(status_code=404, detail="User not found"); otherwise return result
(the Supabase response); for unexpected errors catch Exception as e and raise
HTTPException(status_code=500, detail="Internal server error") from e (and log e
internally rather than including it in the response).
| @router.post("/recommendation", response_model=PricingRecommendation) | ||
| async def get_pricing_recommendation( | ||
| request: PricingRequest, | ||
| pricing_service: PricingService = Depends(get_pricing_service) | ||
| ): | ||
| """ | ||
| Get AI-powered pricing recommendation based on similar contracts | ||
| """ | ||
| try: | ||
| # Validate input parameters | ||
| if request.creator_followers <= 0: | ||
| raise HTTPException(status_code=400, detail="Creator followers must be positive") | ||
|
|
||
| if request.creator_engagement_rate < 0 or request.creator_engagement_rate > 100: | ||
| raise HTTPException(status_code=400, detail="Engagement rate must be between 0 and 100") | ||
|
|
||
| if request.duration_weeks <= 0: | ||
| raise HTTPException(status_code=400, detail="Duration must be positive") | ||
|
|
||
| # Find similar contracts | ||
| similar_contracts = pricing_service.find_similar_contracts( | ||
| creator_followers=request.creator_followers, | ||
| creator_engagement_rate=request.creator_engagement_rate, | ||
| content_type=request.content_type, | ||
| campaign_type=request.campaign_type, | ||
| platform=request.platform, | ||
| duration_weeks=request.duration_weeks, | ||
| exclusivity_level=request.exclusivity_level | ||
| ) | ||
|
|
||
| # Generate price recommendation | ||
| recommendation = pricing_service.generate_price_recommendation( | ||
| similar_contracts=similar_contracts, | ||
| creator_followers=request.creator_followers, | ||
| creator_engagement_rate=request.creator_engagement_rate, | ||
| content_type=request.content_type, | ||
| campaign_type=request.campaign_type, | ||
| platform=request.platform, | ||
| duration_weeks=request.duration_weeks, | ||
| exclusivity_level=request.exclusivity_level | ||
| ) | ||
|
|
||
| return recommendation | ||
|
|
||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"Error generating pricing recommendation: {str(e)}") | ||
|
|
There was a problem hiding this comment.
Preserve HTTPException status; avoid masking 4xx as 500.
The broad except Exception converts validation errors to 500.
@router.post("/recommendation", response_model=PricingRecommendation)
async def get_pricing_recommendation(
- request: PricingRequest,
- pricing_service: PricingService = Depends(get_pricing_service)
+ request: PricingRequest,
+ pricing_service: "Annotated[PricingService, Depends(get_pricing_service)]"
):
@@
- except Exception as e:
- raise HTTPException(status_code=500, detail=f"Error generating pricing recommendation: {str(e)}")
+ except HTTPException as e:
+ raise e
+ except Exception as e:
+ raise HTTPException(status_code=500, detail="Error generating pricing recommendation") from eAdd at top:
-from typing import Optional, List, Dict
+from typing import Optional, List, Dict, AnnotatedCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.1)
49-49: 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)
57-57: Abstract raise to an inner function
(TRY301)
60-60: Abstract raise to an inner function
(TRY301)
63-63: Abstract raise to an inner function
(TRY301)
88-88: Consider moving this statement to an else block
(TRY300)
90-90: Do not catch blind exception: Exception
(BLE001)
91-91: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
91-91: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In Backend/app/routes/pricing.py around lines 46 to 92, the broad `except
Exception` is converting existing HTTPException (4xx) errors into 500 responses;
update the error handling to preserve HTTPException status codes by re-raising
them and only wrap non-HTTPException errors as 500. Concretely, add an `except
HTTPException: raise` clause before the generic exception handler (or check
isinstance(e, HTTPException) and re-raise), then keep the generic `except
Exception as e:` to convert unexpected errors into a 500 with the original error
message.
| @router.post("/feedback") | ||
| async def submit_pricing_feedback( | ||
| feedback: PricingFeedback, | ||
| pricing_service: PricingService = Depends(get_pricing_service) | ||
| ): | ||
| """ | ||
| Submit feedback on pricing recommendation accuracy | ||
| """ | ||
| try: | ||
| # Validate feedback | ||
| if feedback.satisfaction_score < 1 or feedback.satisfaction_score > 10: | ||
| raise HTTPException(status_code=400, detail="Satisfaction score must be between 1 and 10") | ||
|
|
||
| if feedback.roi_achieved < 0 or feedback.roi_achieved > 1000: | ||
| raise HTTPException(status_code=400, detail="ROI achieved must be between 0 and 1000") | ||
|
|
||
| # Learn from the outcome | ||
| success = pricing_service.learn_from_outcome( | ||
| contract_id=feedback.contract_id, | ||
| recommended_price=feedback.recommended_price, | ||
| actual_price=feedback.actual_price, | ||
| satisfaction_score=feedback.satisfaction_score, | ||
| roi_achieved=feedback.roi_achieved, | ||
| repeat_business=feedback.repeat_business | ||
| ) | ||
|
|
||
| if not success: | ||
| raise HTTPException(status_code=500, detail="Failed to process feedback") | ||
|
|
||
| return { | ||
| "message": "Feedback submitted successfully", | ||
| "contract_id": feedback.contract_id, | ||
| "accuracy_score": pricing_service._calculate_accuracy_score( | ||
| feedback.recommended_price, | ||
| feedback.actual_price | ||
| ) | ||
| } | ||
|
|
||
| except Exception as e: | ||
| raise HTTPException(status_code=500, detail=f"Error submitting feedback: {str(e)}") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Apply the same HTTPException preservation pattern across all endpoints.
Repeat the except HTTPException: raise + chained 500 in /similar-contracts, /test-db, and /learning-stats for consistent status semantics.
Also applies to: 134-177, 248-274, 275-336
🧰 Tools
🪛 Ruff (0.14.1)
96-96: 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)
104-104: Abstract raise to an inner function
(TRY301)
107-107: Abstract raise to an inner function
(TRY301)
120-120: Abstract raise to an inner function
(TRY301)
131-131: Do not catch blind exception: Exception
(BLE001)
132-132: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
132-132: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In Backend/app/routes/pricing.py around lines 93 to 133 (and also apply to
ranges 134-177, 248-274, 275-336), the generic except Exception block swallows
HTTPException status codes; update each endpoint to preserve HTTPException by
adding an except HTTPException: raise clause before the generic except, and in
the generic except block re-raise a new HTTPException(status_code=500,
detail=f"Error ...: {str(e)}") using exception chaining (raise
HTTPException(...) from e) so non-HTTP errors map to 500 while original
HTTPExceptions pass through unchanged.
🛠️ Refactor suggestion | 🟠 Major
Same exception handling fix for feedback endpoint.
Avoid swallowing 4xx and use exception chaining.
- except Exception as e:
- raise HTTPException(status_code=500, detail=f"Error submitting feedback: {str(e)}")
+ except HTTPException as e:
+ raise e
+ except Exception as e:
+ raise HTTPException(status_code=500, detail="Error submitting feedback") from e🧰 Tools
🪛 Ruff (0.14.1)
96-96: 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)
104-104: Abstract raise to an inner function
(TRY301)
107-107: Abstract raise to an inner function
(TRY301)
120-120: Abstract raise to an inner function
(TRY301)
131-131: Do not catch blind exception: Exception
(BLE001)
132-132: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
132-132: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In Backend/app/routes/pricing.py around lines 93 to 133, the current broad
except block swallows HTTPException (4xx) responses and loses original
traceback; change exception handling to re-raise HTTPException instances
unchanged and only convert unexpected exceptions into a 500 while preserving
chaining. Specifically, catch exceptions, if isinstance(e, HTTPException):
raise, otherwise raise a new HTTPException(status_code=500, detail=f"Error
submitting feedback: {e}") from e so you keep the original exception context;
remove the blanket re-raise of everything as a 500 and ensure any custom
validation raises HTTPException directly.
| def __init__(self): | ||
| """Initialize AI Router with Groq client""" | ||
| self.groq_api_key = os.getenv("GROQ_API_KEY") | ||
| if not self.groq_api_key: | ||
| raise ValueError("GROQ_API_KEY environment variable is required") | ||
|
|
||
| self.client = Groq(api_key=self.groq_api_key) | ||
|
|
There was a problem hiding this comment.
Avoid hard failing at import when GROQ_API_KEY is missing; degrade gracefully.
Creating the global instance requires the key and can crash app startup. Initialize without raising and fall back when missing.
class AIRouter:
def __init__(self):
"""Initialize AI Router with Groq client"""
self.groq_api_key = os.getenv("GROQ_API_KEY")
- if not self.groq_api_key:
- raise ValueError("GROQ_API_KEY environment variable is required")
-
- self.client = Groq(api_key=self.groq_api_key)
+ self.client = Groq(api_key=self.groq_api_key) if self.groq_api_key else None
@@
- # Call Groq LLM with lower temperature for more consistent responses
- response = self.client.chat.completions.create(
+ # Call Groq LLM only if client available, else use fallback
+ if not self.client:
+ parsed_response = self._create_fallback_response(query)
+ return self._enhance_response(parsed_response, brand_id, query)
+ response = await asyncio.to_thread(
+ self.client.chat.completions.create,
model="moonshotai/kimi-k2-instruct",
messages=messages,
temperature=0.1,
max_tokens=1024
- )
+ )Also applies to: 131-170
🧰 Tools
🪛 Ruff (0.14.1)
22-22: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In Backend/app/services/ai_router.py around lines 18 to 25 (and similarly for
the block around 131 to 170), the constructor currently raises ValueError if
GROQ_API_KEY is missing which crashes startup; change the init to read the env
var but do not raise—set self.client to None when key is absent and log a
warning; ensure any methods that use self.client check for None and either raise
a clear runtime error at call time or return an appropriate error response, so
the app degrades gracefully instead of failing at import.
| useEffect(() => { | ||
| if (brandId) { | ||
| Promise.all([ | ||
| loadDashboardOverview(), | ||
| loadBrandProfile(), | ||
| loadCampaigns(), | ||
| loadCreatorMatches(), | ||
| loadApplications(), | ||
| loadPayments(), | ||
| ]).catch(err => { | ||
| console.error('Error loading dashboard data:', err); | ||
| }); | ||
| } | ||
| }, [brandId, loadDashboardOverview, loadBrandProfile, loadCampaigns, loadCreatorMatches, loadApplications, loadPayments]); |
There was a problem hiding this comment.
Promise.all silently swallows errors for most loaders.
The concurrent data loading catches all errors but only logs them (line 235). Individual loaders also catch and log errors (lines 48, 60, 72, 84, 96), but these don't update the component's error state. Users won't see feedback if profile, campaigns, or other data fails to load.
Consider aggregating errors:
useEffect(() => {
if (brandId) {
- Promise.all([
+ const loadAllData = async () => {
+ const errors: string[] = [];
+ await Promise.allSettled([
loadDashboardOverview(),
loadBrandProfile(),
loadCampaigns(),
loadCreatorMatches(),
loadApplications(),
loadPayments(),
- ]).catch(err => {
- console.error('Error loading dashboard data:', err);
- });
+ ]).then(results => {
+ results.forEach((result, index) => {
+ if (result.status === 'rejected') {
+ const names = ['overview', 'profile', 'campaigns', 'matches', 'applications', 'payments'];
+ errors.push(`${names[index]}: ${result.reason}`);
+ }
+ });
+ if (errors.length > 0) {
+ setError(`Failed to load: ${errors.join(', ')}`);
+ }
+ });
+ };
+ loadAllData();
}
}, [brandId, loadDashboardOverview, loadBrandProfile, loadCampaigns, loadCreatorMatches, loadApplications, loadPayments]);📝 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.
| useEffect(() => { | |
| if (brandId) { | |
| Promise.all([ | |
| loadDashboardOverview(), | |
| loadBrandProfile(), | |
| loadCampaigns(), | |
| loadCreatorMatches(), | |
| loadApplications(), | |
| loadPayments(), | |
| ]).catch(err => { | |
| console.error('Error loading dashboard data:', err); | |
| }); | |
| } | |
| }, [brandId, loadDashboardOverview, loadBrandProfile, loadCampaigns, loadCreatorMatches, loadApplications, loadPayments]); | |
| useEffect(() => { | |
| if (brandId) { | |
| const loadAllData = async () => { | |
| const errors: string[] = []; | |
| await Promise.allSettled([ | |
| loadDashboardOverview(), | |
| loadBrandProfile(), | |
| loadCampaigns(), | |
| loadCreatorMatches(), | |
| loadApplications(), | |
| loadPayments(), | |
| ]).then(results => { | |
| results.forEach((result, index) => { | |
| if (result.status === 'rejected') { | |
| const names = ['overview', 'profile', 'campaigns', 'matches', 'applications', 'payments']; | |
| errors.push(`${names[index]}: ${result.reason}`); | |
| } | |
| }); | |
| if (errors.length > 0) { | |
| setError(`Failed to load: ${errors.join(', ')}`); | |
| } | |
| }); | |
| }; | |
| loadAllData(); | |
| } | |
| }, [brandId, loadDashboardOverview, loadBrandProfile, loadCampaigns, loadCreatorMatches, loadApplications, loadPayments]); |
🤖 Prompt for AI Agents
In Frontend/src/hooks/useBrandDashboard.ts around lines 225 to 238, the
Promise.all call swallows loader failures by only logging the error and the
individual loaders swallow their own errors without updating the hook's error
state; change the approach so failures are surfaced to the hook state: either
make each loader throw/reject on failure (remove internal catches or rethrow
after logging) and keep Promise.all so a rejection propagates to a single catch
that calls setError with a meaningful combined message, or use
Promise.allSettled to inspect results and if any are rejected, aggregate their
errors and call setError (and still set successful data where applicable);
ensure the UI error state is set and returned so components can show feedback
instead of silently logging.
| export const useContentLinkingIntegration = (contractId: string) => { | ||
| const integration = useIntegration(); | ||
|
|
||
| const linkContent = useCallback(async (contentUrl: string, userId: string) => { | ||
| const params: ContentLinkingWorkflow = { | ||
| contractId, | ||
| contentUrl, | ||
| userId, | ||
| platform: contentUrl.includes('instagram') ? 'instagram' : 'youtube', | ||
| contentId: '' // Will be extracted by the service | ||
| }; | ||
|
|
||
| return integration.executeContentLinking(params); | ||
| }, [contractId, integration]); | ||
|
|
||
| return { | ||
| ...integration, | ||
| linkContent | ||
| }; | ||
| }; | ||
|
|
||
| export const useAnalyticsExportIntegration = () => { | ||
| const integration = useIntegration(); | ||
|
|
||
| const exportAnalytics = useCallback(async ( | ||
| contractIds: string[], | ||
| metrics: string[], | ||
| dateRange: { start: string; end: string }, | ||
| format: 'csv' | 'pdf' = 'csv' | ||
| ) => { | ||
| const params: ExportWorkflow = { | ||
| format, | ||
| dateRange, | ||
| metrics, | ||
| contractIds | ||
| }; | ||
|
|
||
| return integration.executeExport(params); | ||
| }, [integration]); | ||
|
|
||
| return { | ||
| ...integration, | ||
| exportAnalytics | ||
| }; | ||
| }; | ||
|
|
||
| export const useAlertIntegration = (contractId: string) => { | ||
| const integration = useIntegration(); | ||
|
|
||
| const setupAlerts = useCallback(async ( | ||
| thresholds: Array<{ | ||
| metric: string; | ||
| operator: 'gt' | 'lt' | 'eq'; | ||
| value: number; | ||
| }>, | ||
| notificationChannels: ('email' | 'in_app')[] = ['email', 'in_app'] | ||
| ) => { | ||
| const params: AlertIntegration = { | ||
| contractId, | ||
| thresholds, | ||
| notificationChannels | ||
| }; | ||
|
|
||
| return integration.executeAlertSetup(params); | ||
| }, [contractId, integration]); | ||
|
|
||
| return { | ||
| ...integration, | ||
| setupAlerts | ||
| }; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
Critical: Broken memoization in specialized hooks.
All three specialized hooks include integration in their dependency arrays (lines 197, 222, 248). Since integration is the object returned from useIntegration() and is not memoized, it's a new object reference on every render. This causes the callbacks (linkContent, exportAnalytics, setupAlerts) to be recreated on every render, defeating the purpose of useCallback and causing unnecessary re-renders in consuming components.
Apply this pattern to fix the issue:
export const useContentLinkingIntegration = (contractId: string) => {
const integration = useIntegration();
const linkContent = useCallback(async (contentUrl: string, userId: string) => {
const params: ContentLinkingWorkflow = {
contractId,
contentUrl,
userId,
platform: contentUrl.includes('instagram') ? 'instagram' : 'youtube',
contentId: '' // Will be extracted by the service
};
return integration.executeContentLinking(params);
- }, [contractId, integration]);
+ }, [contractId, integration.executeContentLinking]);
return {
...integration,
linkContent
};
};Apply the same fix to useAnalyticsExportIntegration and useAlertIntegration, extracting only the specific method used from integration in the dependency array.
🤖 Prompt for AI Agents
In Frontend/src/hooks/useIntegration.ts around lines 184 to 254, the specialized
hooks include the whole non-memoized integration object in useCallback deps
causing callbacks to be recreated; fix by destructuring the specific method you
call from integration (e.g. const { executeContentLinking } = integration) and
use that function reference in the useCallback dependency array instead of the
full integration object, and apply the same pattern for executeExport in
useAnalyticsExportIntegration and executeAlertSetup in useAlertIntegration so
each hook only depends on the specific function it invokes.
| contractId, | ||
| contentUrl, | ||
| userId, | ||
| platform: contentUrl.includes('instagram') ? 'instagram' : 'youtube', |
There was a problem hiding this comment.
Improve platform detection logic.
The current platform detection only checks for 'instagram' and defaults everything else to 'youtube'. This is brittle and will misclassify other platforms (TikTok, Facebook, Twitter, etc.).
Consider one of these approaches:
- platform: contentUrl.includes('instagram') ? 'instagram' : 'youtube',
+ platform: contentUrl.includes('instagram') ? 'instagram'
+ : contentUrl.includes('youtube') || contentUrl.includes('youtu.be') ? 'youtube'
+ : 'unknown',Or better yet, extract this logic into a helper function with proper URL parsing and validation.
📝 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.
| platform: contentUrl.includes('instagram') ? 'instagram' : 'youtube', | |
| platform: contentUrl.includes('instagram') ? 'instagram' | |
| : contentUrl.includes('youtube') || contentUrl.includes('youtu.be') ? 'youtube' | |
| : 'unknown', |
| // Brand ID for testing (in production, this would come from auth context) | ||
| const brandId = "6dbfcdd5-795f-49c1-8f7a-a5538b8c6f6f"; // Test brand ID | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Externalize brandId and API base; drop localhost in code.
Hardcoded test UUID and http://localhost:8000 will break in non-dev environments. Read brandId from auth/session and API base from env.
+ const API_BASE = import.meta.env.VITE_API_BASE_URL ?? "/api";
- const brandId = "6dbfcdd5-795f-49c1-8f7a-a5538b8c6f6f"; // Test brand ID
+ // TODO: wire to auth/session
+ const brandId = /* get from auth context */ "";
- const kpisResponse = await fetch(`http://localhost:8000/api/brand/dashboard/kpis?brand_id=${brandId}`);
+ const kpisResponse = await fetch(`${API_BASE}/brand/dashboard/kpis?brand_id=${brandId}`);
- const campaignsResponse = await fetch(`http://localhost:8000/api/brand/dashboard/campaigns/overview?brand_id=${brandId}`);
+ const campaignsResponse = await fetch(`${API_BASE}/brand/dashboard/campaigns/overview?brand_id=${brandId}`);
- const analyticsResponse = await fetch(`http://localhost:8000/api/brand/dashboard/analytics?brand_id=${brandId}`);
+ const analyticsResponse = await fetch(`${API_BASE}/brand/dashboard/analytics?brand_id=${brandId}`);
- const notificationsResponse = await fetch(`http://localhost:8000/api/brand/dashboard/notifications?brand_id=${brandId}`);
+ const notificationsResponse = await fetch(`${API_BASE}/brand/dashboard/notifications?brand_id=${brandId}`);Also applies to: 127-146
🤖 Prompt for AI Agents
In Frontend/src/pages/Brand/DashboardOverview.tsx around lines 42-44 (and
similarly lines 127-146), replace the hardcoded test brandId and any direct
"http://localhost:8000" usage by reading the brandId from the auth/session
context (or props) used in the app and obtaining the API base URL from an
environment variable (e.g. process.env.REACT_APP_API_BASE or
import.meta.env.VITE_API_BASE) instead of embedding localhost; add a safe
fallback or explicit error/warning if the env var or session brandId is missing
so the component fails fast in non-dev environments.
| <div style={{ fontSize: "32px", fontWeight: "700", color: "#fff" }}>${data.kpis.budgetSpent.toLocaleString()}</div> | ||
| <div style={{ display: "flex", alignItems: "center", marginTop: "8px", color: PRIMARY }}> | ||
| <span style={{ fontSize: "14px" }}>{data.kpis.budgetUtilization}% of allocated budget</span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Wrong source for budgetUtilization (user-visible bug).
You're reading budgetUtilization from data.kpis, but it's under data.financial. This renders "undefined% of allocated budget".
- <span style={{ fontSize: "14px" }}>{data.kpis.budgetUtilization}% of allocated budget</span>
+ <span style={{ fontSize: "14px" }}>{data.financial.budgetUtilization}% of allocated budget</span>📝 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.
| <div style={{ fontSize: "32px", fontWeight: "700", color: "#fff" }}>${data.kpis.budgetSpent.toLocaleString()}</div> | |
| <div style={{ display: "flex", alignItems: "center", marginTop: "8px", color: PRIMARY }}> | |
| <span style={{ fontSize: "14px" }}>{data.kpis.budgetUtilization}% of allocated budget</span> | |
| </div> | |
| </div> | |
| <div style={{ fontSize: "32px", fontWeight: "700", color: "#fff" }}>${data.kpis.budgetSpent.toLocaleString()}</div> | |
| <div style={{ display: "flex", alignItems: "center", marginTop: "8px", color: PRIMARY }}> | |
| <span style={{ fontSize: "14px" }}>{data.financial.budgetUtilization}% of allocated budget</span> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In Frontend/src/pages/Brand/DashboardOverview.tsx around lines 409 to 413, the
JSX is referencing data.kpis.budgetUtilization but the correct source is
data.financial.budgetUtilization; update the JSX to read budgetUtilization from
data.financial (e.g., replace data.kpis.budgetUtilization with
data.financial.budgetUtilization or guard with optional chaining like
data.financial?.budgetUtilization) and ensure it is formatted/displayed as a
number (handle undefined with a fallback such as 0 or “—”).
|
not relevant for the moment..closing |
1 participant
Closes #
📝 Description
This pull request integrates AI-driven price optimization features into the application. It includes backend enhancements for AI model integration, API endpoints for price recommendations, and frontend updates to display optimized pricing data. These changes aim to improve pricing strategies and maximize revenue.
🔧 Changes Made
Backend:
Integrated AI models for price optimization.
Added new API endpoints to fetch and update optimized pricing data.
Enhanced database schema to store AI-generated pricing recommendations.
Improved error handling for AI-related operations.
Frontend:
Updated the pricing dashboard to display AI-optimized prices.
Added UI components for viewing and applying price recommendations.
Improved the design and responsiveness of the pricing interface.
✅ Checklist
Summary by CodeRabbit
Release Notes
New Features
Enhancements