Developer metrics - #42
Conversation
…ss with session validation and abort handling
…rror statistics into developer metrics
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reached
More reviews will be available in 23 minutes and 32 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThe PR adds developer analytics and request logging, updates prompt and optimize schemas and flows, and adjusts dashboard, domain, account, and bridge UI behavior. ChangesAdmin analytics and observability
Prompt library and optimize flows
Account, domain, and bridge UI updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
qa-chatbot/src/promptly/api/v1/prompts.py (1)
75-83: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep advisory and health-score charges on the fixed 5-credit contract.
These two calls now persist
credits_spent=token_count, which makes usage records variable even though both endpoints are priced at a fixed 5 credits. That will skew billing analytics and any reconciliation againsttoken_balancewhenevertoken_count !== 5. Align bothdeduct_tokens(...)andusage_repo.log(...)to the same fixed operation cost instead of the model token count.Proposed fix
+PROMPT_REVIEW_COST = 5 + async def prompt_health_score( @@ - token_count = result.pop("_token_usage", 0) - if token_count: - await user_repo.deduct_tokens(current_user.user_id, token_count) + result.pop("_token_usage", 0) + await user_repo.deduct_tokens(current_user.user_id, PROMPT_REVIEW_COST) @@ usage_repo = UsageEventRepository(db) await usage_repo.log( - user_id=current_user.user_id, action="health_score", credits_spent=token_count + user_id=current_user.user_id, action="health_score", credits_spent=PROMPT_REVIEW_COST ) @@ - token_count = result.pop("_token_usage", 0) - if token_count: - await user_repo.deduct_tokens(current_user.user_id, token_count) + result.pop("_token_usage", 0) + await user_repo.deduct_tokens(current_user.user_id, PROMPT_REVIEW_COST) @@ usage_repo = UsageEventRepository(db) - await usage_repo.log(user_id=current_user.user_id, action="advisory", credits_spent=token_count) + await usage_repo.log( + user_id=current_user.user_id, + action="advisory", + credits_spent=PROMPT_REVIEW_COST, + )As per coding guidelines, "Each optimization costs 10 credits; health-score and advisory operations cost 5 credits each; return HTTP 402 when a user has insufficient credits".
Also applies to: 117-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa-chatbot/src/promptly/api/v1/prompts.py` around lines 75 - 83, The health-score/advisory billing path is recording and deducting the model’s token count instead of the fixed 5-credit contract. Update the logic in the prompt handling flow around the user token deduction and UsageEventRepository.log call so both use the same constant operation cost for health-score and advisory actions. Make sure the relevant branches/functions in prompts.py (including the code that calls deduct_tokens, log, and the advisory path it also applies to) consistently charge and persist 5 credits, not token_count.Source: Coding guidelines
frontend/src/types/bridge.ts (1)
58-71: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep transfer job API shapes in
src/types/api.ts.These added fields make
TransferJobSummaryan even closer mirror of the backend payload, but this file is still acting as the source of truth for that API shape. Please keepTransferJobSummary/TransferJobListResponsedeclared insrc/types/api.tsand have bridge-specific code import them from there.As per coding guidelines,
frontend/src/{types,lib}/!(api).{ts,tsx}: "Define all TypeScript types mirroring backend response shapes insrc/types/api.tsand Zod schemas for form validation insrc/lib/schemas.ts."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/types/bridge.ts` around lines 58 - 71, The transfer job response types are defined in the wrong place, so move TransferJobSummary and TransferJobListResponse back to src/types/api.ts and keep that file as the single source of truth for backend-shaped API types. Update bridge-specific code in TransferJobSummary consumers to import these types from api.ts instead of declaring or extending them in bridge.ts, and leave bridge.ts only for bridge-specific types/helpers.Source: Coding guidelines
frontend/src/components/layout/sidebar.tsx (1)
206-215: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDon't reuse the loading placeholder for
/users/mefailures.
fetchedUser?.token_balanceisundefinedduring the initial fetch and after a query error, so this leaves the token card in the shimmer state forever when/api/v1/users/mefails. Please threadisLoading/isErrorthrough the user query here and render a distinct error or hidden state instead of treating both cases as loading.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/layout/sidebar.tsx` around lines 206 - 215, The user query in the sidebar token card currently treats both initial loading and `/api/v1/users/me` failures as the same `fetchedUser?.token_balance === undefined` state, which leaves the shimmer stuck on errors. Update the `useQuery` usage in `sidebar.tsx` to read and pass through `isLoading` and `isError` alongside `fetchedUser`, and make the token card render a real error or hidden state when `useQuery<User>` fails instead of reusing the loading placeholder.
🧹 Nitpick comments (1)
qa-chatbot/src/promptly/config/app.py (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep Sentry settings in a dedicated config class.
These fields extend
AppSettingsinstead of keeping observability config isolated by concern. Please move them into a Sentry/observability settings class and compose that where the analytics code reads them. As per coding guidelines, "Organize configuration by concern using separate settings classes insrc/promptly/config/."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa-chatbot/src/promptly/config/app.py` around lines 26 - 28, The Sentry fields are currently mixed into AppSettings instead of being isolated by concern. Move SENTRY_AUTH_TOKEN, SENTRY_ORG_SLUG, and SENTRY_PROJECT_SLUG into a dedicated Sentry/observability settings class under promptly/config, then compose or inject that class wherever the analytics/Sentry setup reads these values. Keep AppSettings focused on app-wide config and update any references to use the new settings class name.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/admin/analytics/developer-metrics.tsx`:
- Around line 132-140: The threshold coloring logic in developer-metrics.tsx
treats placeholder values like “—” as failures because parseFloat() produces NaN
and the color falls through to red. Update the value-to-number handling in the
metrics color calculation so thresholds are only applied when the parsed value
is finite, and otherwise keep the default text color. Use the existing threshold
block around the color assignment in the DeveloperMetrics component to make this
check before applying the inverse/non-inverse comparisons.
In `@frontend/src/components/domain-prompts/domain-workspace.tsx`:
- Around line 1556-1568: The delete confirmation state in domain-workspace is
global, which lets a confirmation carry over to a different domain after the
selection changes. Update the delete flow around handleDelete and confirmDelete
so the confirmation is tied to the selected domain id (or reset whenever
selectedId changes), and ensure the delete affordance is only enabled/shown when
a domain is currently selected. Apply the same fix to the related delete UI
block referenced by the other occurrence in this component.
In `@frontend/src/hooks/use-job-stream.ts`:
- Around line 120-127: The missing-token branch in use-job-stream is vulnerable
to a stale update because getSession() is not checked against ctrl.signal before
setting failed state; after awaiting the session, verify the abort signal and
return early before calling setError or setStatus. Also replace the raw fetch
usage in use-job-stream with the shared Axios instance from src/lib/api.ts,
updating the HTTP calls in the stream setup and request path to use
axios.get/axios.post so token handling, 401 behavior, and abort support stay
centralized.
In `@qa-chatbot/src/promptly/admin/api/router.py`:
- Around line 191-205: The Sentry stats cache in _fetch_sentry_stats should be
keyed by days instead of using one global payload, since different ranges like
7/30/365 days can currently reuse the wrong data. Update the cache state and
lookup so each days value has its own entry, and apply the same fix to the
stale-on-error fallback used when building the daily chart so the zero-filled
result is also stored and reused per days. Use the existing _fetch_sentry_stats
helper and the downstream stats/chart path around the stale-error handling to
locate the changes.
- Around line 1763-1764: The daily-series window in the router logic is starting
from a timestamp with the current time-of-day, which can cause partial-day data
to be counted in totals but omitted from the per-day buckets. Update the
`cutoff` calculation in `router.py` to normalize the window start to the
beginning of the day before it is used by the later `DATE(...)` queries and
`_fill_days()`, so the aggregates and daily series stay aligned.
- Around line 226-256: The unresolved issue count is being derived from
`top_issues`, so it only reflects the first 5 displayed items instead of the
full unresolved set. Update the Sentry handling in the admin router method that
builds `result` to use a true total from the API response or a separate count
source, and do not base `unresolved_issue_count` on the sliced `raw_issues[:5]`
list. Keep `top_issues` limited for display, but compute the count independently
from that limit so it represents all unresolved issues.
In `@qa-chatbot/src/promptly/core/middleware.py`:
- Around line 27-43: The request log writer in _write_request_log currently
creates an unbounded async DB write per request, which can pile up under slow
database conditions. Update the middleware logging path to route writes through
a bounded queue or semaphore, and ensure saturated paths either drop logs or
batch them instead of spawning more tasks. Use the existing _write_request_log
and the middleware request logging flow as the main integration points.
---
Outside diff comments:
In `@frontend/src/components/layout/sidebar.tsx`:
- Around line 206-215: The user query in the sidebar token card currently treats
both initial loading and `/api/v1/users/me` failures as the same
`fetchedUser?.token_balance === undefined` state, which leaves the shimmer stuck
on errors. Update the `useQuery` usage in `sidebar.tsx` to read and pass through
`isLoading` and `isError` alongside `fetchedUser`, and make the token card
render a real error or hidden state when `useQuery<User>` fails instead of
reusing the loading placeholder.
In `@frontend/src/types/bridge.ts`:
- Around line 58-71: The transfer job response types are defined in the wrong
place, so move TransferJobSummary and TransferJobListResponse back to
src/types/api.ts and keep that file as the single source of truth for
backend-shaped API types. Update bridge-specific code in TransferJobSummary
consumers to import these types from api.ts instead of declaring or extending
them in bridge.ts, and leave bridge.ts only for bridge-specific types/helpers.
In `@qa-chatbot/src/promptly/api/v1/prompts.py`:
- Around line 75-83: The health-score/advisory billing path is recording and
deducting the model’s token count instead of the fixed 5-credit contract. Update
the logic in the prompt handling flow around the user token deduction and
UsageEventRepository.log call so both use the same constant operation cost for
health-score and advisory actions. Make sure the relevant branches/functions in
prompts.py (including the code that calls deduct_tokens, log, and the advisory
path it also applies to) consistently charge and persist 5 credits, not
token_count.
---
Nitpick comments:
In `@qa-chatbot/src/promptly/config/app.py`:
- Around line 26-28: The Sentry fields are currently mixed into AppSettings
instead of being isolated by concern. Move SENTRY_AUTH_TOKEN, SENTRY_ORG_SLUG,
and SENTRY_PROJECT_SLUG into a dedicated Sentry/observability settings class
under promptly/config, then compose or inject that class wherever the
analytics/Sentry setup reads these values. Keep AppSettings focused on app-wide
config and update any references to use the new settings class name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a81bb491-098b-4d01-878b-4e6990142ec2
📒 Files selected for processing (26)
frontend/src/app/(dashboard)/prompt-library/[id]/page.tsxfrontend/src/components/admin/analytics/developer-metrics.tsxfrontend/src/components/admin/view-tab.tsxfrontend/src/components/bridge/transfer-detail.tsxfrontend/src/components/domain-prompts/domain-card.tsxfrontend/src/components/domain-prompts/domain-workspace.tsxfrontend/src/components/layout/header.tsxfrontend/src/components/layout/sidebar.tsxfrontend/src/components/optimize/optimize-chat.tsxfrontend/src/components/optimize/result-card.tsxfrontend/src/hooks/use-favorites.tsfrontend/src/hooks/use-job-stream.tsfrontend/src/lib/schemas.tsfrontend/src/types/api.tsfrontend/src/types/bridge.tsqa-chatbot/src/promptly/admin/api/router.pyqa-chatbot/src/promptly/api/v1/prompts.pyqa-chatbot/src/promptly/config/app.pyqa-chatbot/src/promptly/core/middleware.pyqa-chatbot/src/promptly/main.pyqa-chatbot/src/promptly/migrations/versions/l6m7n8o9p0q1_add_api_request_logs.pyqa-chatbot/src/promptly/models/__init__.pyqa-chatbot/src/promptly/models/api_request_log.pyqa-chatbot/src/promptly/optimize/api/schemas.pyqa-chatbot/src/promptly/schemas/prompt.pyqa-chatbot/src/promptly/schemas/user.py
💤 Files with no reviewable changes (2)
- qa-chatbot/src/promptly/schemas/prompt.py
- frontend/src/components/layout/header.tsx
| const { data: { session } } = await supabase.auth.getSession(); | ||
| const token = session?.access_token ?? ''; | ||
|
|
||
| if (!token) { | ||
| setError('Session expired — please refresh the page'); | ||
| setStatus('failed'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/src/hooks/use-job-stream.tsRepository: ananthanarayanan431/promptly
Length of output: 8307
Correct the race condition in the missing-token branch and replace raw fetch with the Axios instance.
The getSession() call at line 120 is not bound to the abort signal. If the effect cancels while waiting for the session, the stale closure continues and overwrites the UI state with a "failed" error at lines 124‑125. Check ctrl.signal.aborted immediately after awaiting the session and before mutating state.
Additionally, the raw fetch calls at lines 39 and 130 violate the coding guideline requiring all frontend HTTP requests to use the centralized axios instance in src/lib/api.ts. This instance handles token injection, 401 redirects, and abort signaling consistently.
Proposed fix for race condition
const { data: { session } } = await supabase.auth.getSession();
+ if (ctrl.signal.aborted) return;
const token = session?.access_token ?? '';
if (!token) {
+ if (ctrl.signal.aborted) return;
setError('Session expired — please refresh the page');
setStatus('failed');
return;
}Refactor HTTP calls: Replace fetch with axios.get/post from src/lib/api.ts to comply with frontend network standards.
📝 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 { data: { session } } = await supabase.auth.getSession(); | |
| const token = session?.access_token ?? ''; | |
| if (!token) { | |
| setError('Session expired — please refresh the page'); | |
| setStatus('failed'); | |
| return; | |
| } | |
| const { data: { session } } = await supabase.auth.getSession(); | |
| if (ctrl.signal.aborted) return; | |
| const token = session?.access_token ?? ''; | |
| if (!token) { | |
| if (ctrl.signal.aborted) return; | |
| setError('Session expired — please refresh the page'); | |
| setStatus('failed'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/hooks/use-job-stream.ts` around lines 120 - 127, The
missing-token branch in use-job-stream is vulnerable to a stale update because
getSession() is not checked against ctrl.signal before setting failed state;
after awaiting the session, verify the abort signal and return early before
calling setError or setStatus. Also replace the raw fetch usage in
use-job-stream with the shared Axios instance from src/lib/api.ts, updating the
HTTP calls in the stream setup and request path to use axios.get/axios.post so
token handling, 401 behavior, and abort support stay centralized.
…racking, endpoint latency reporting, and robust error handling.
…GPT-4.1-mini in analytics panel
…s using TanStack Query cache
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
qa-chatbot/src/promptly/api/v1/prompts.py (1)
75-80: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the same fixed credit cost for deduction and usage logging.
Lines 80 and 120 now persist
credits_spent=5, but Lines 77 and 117 still deduct_token_usage. If_token_usageis not exactly5, balances and usage metrics diverge. Use one 5-credit constant for the preflight check, deduction, and usage log.As per coding guidelines, “Each optimization costs 10 credits; health-score and advisory operations cost 5 credits each; return HTTP 402 when a user has insufficient credits.”
<coding_guidelines>Suggested direction
+HEALTH_SCORE_CREDIT_COST = 5 +ADVISORY_CREDIT_COST = 5 + @@ - token_count = result.pop("_token_usage", 0) - if token_count: - await user_repo.deduct_tokens(current_user.user_id, token_count) + result.pop("_token_usage", None) + await user_repo.deduct_tokens(current_user.user_id, HEALTH_SCORE_CREDIT_COST) usage_repo = UsageEventRepository(db) - await usage_repo.log(user_id=current_user.user_id, action="health_score", credits_spent=5) + await usage_repo.log( + user_id=current_user.user_id, + action="health_score", + credits_spent=HEALTH_SCORE_CREDIT_COST, + ) @@ - token_count = result.pop("_token_usage", 0) - if token_count: - await user_repo.deduct_tokens(current_user.user_id, token_count) + result.pop("_token_usage", None) + await user_repo.deduct_tokens(current_user.user_id, ADVISORY_CREDIT_COST) usage_repo = UsageEventRepository(db) - await usage_repo.log(user_id=current_user.user_id, action="advisory", credits_spent=5) + await usage_repo.log( + user_id=current_user.user_id, + action="advisory", + credits_spent=ADVISORY_CREDIT_COST, + )Also applies to: 115-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa-chatbot/src/promptly/api/v1/prompts.py` around lines 75 - 80, The health_score and advisory flows are deducting _token_usage while logging a fixed credits_spent=5, which can desync balances and usage metrics. Update the relevant prompt handlers in prompts.py, including the health_score path and the advisory path, to use one shared 5-credit constant for the preflight credit check, the user_repo.deduct_tokens call, and UsageEventRepository.log. Make sure the same constant is used consistently in both branches so the deduction and logged usage always match.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/admin/analytics/issues-table.tsx`:
- Around line 98-114: The selectable row in issues-table.tsx uses role="button"
but only handles Enter and has no visible focus treatment, so keyboard
interaction is incomplete. Update the row element used in the issues table to
either a real button or keep the current clickable div and add Space-key
activation alongside Enter in onKeyDown; also add a clear focus style for the
row so tab focus is visible when navigating issues in the admin analytics table.
In `@frontend/src/types/analytics.ts`:
- Around line 16-56: Move the backend-response contract types out of
analytics.ts and into api.ts: EndpointLatency, SentryIssue, SentryRelease, and
the raw shape on AnalyticsResponse should be defined in src/types/api.ts and
then imported back into analytics.ts. Update any references to these symbols so
analytics.ts only contains analytics-specific types, keeping all backend mirror
types in the shared API types module.
In `@qa-chatbot/src/promptly/admin/api/router.py`:
- Around line 3327-3437: The latest_event payload is unstable when the event
fetch fails because event_data stays as an empty object, which breaks consumers
that expect user, tags, breadcrumbs, and other fields. Update the latest event
handling in the issue detail response builder to either fail the endpoint when
the /events/latest/ call does not succeed, or always return a fully shaped
default latest_event object from the same code path that builds event_data, so
callers like issue-detail-panel.tsx can safely dereference it.
- Around line 3363-3372: The request info builder in router.py is returning raw
Sentry headers, which can expose secret-bearing values to the admin dashboard.
Update the request handling logic around the request_info construction to filter
headers through a small safe allowlist instead of slicing raw_headers, and keep
only non-sensitive headers such as content-type, accept, and user-agent before
returning them.
- Around line 3528-3568: The OpenRouter request in router.py forwards
issue_context from _build_ai_fix_prompt(payload) without sanitizing sensitive
incident data. Add a server-side redaction step before the httpx.AsyncClient
post call to strip stack-frame locals, request query strings, IPs, emails,
tokens, and other high-entropy values from the content sent in the messages
payload. Keep the prompt-building flow intact, but ensure only sanitized context
is included in the user message passed to openrouter.ai.
- Around line 3323-3324: The issue handling in the Sentry lookup currently turns
every non-200 response into a 404, which hides auth, rate-limit, and upstream
server failures. Update the response handling around issue_resp so it checks the
actual upstream status code: keep HTTPException with 404 only when Sentry
returns 404, and map other non-200 responses to an appropriate 502/503 instead.
Use the existing issue_resp.status_code branch and the HTTPException raise path
to preserve the right error semantics for callers.
---
Outside diff comments:
In `@qa-chatbot/src/promptly/api/v1/prompts.py`:
- Around line 75-80: The health_score and advisory flows are deducting
_token_usage while logging a fixed credits_spent=5, which can desync balances
and usage metrics. Update the relevant prompt handlers in prompts.py, including
the health_score path and the advisory path, to use one shared 5-credit constant
for the preflight credit check, the user_repo.deduct_tokens call, and
UsageEventRepository.log. Make sure the same constant is used consistently in
both branches so the deduction and logged usage always match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f5b5fff-e49f-49ae-b977-3707826c7c57
📒 Files selected for processing (13)
frontend/src/components/admin/analytics/developer-metrics.tsxfrontend/src/components/admin/analytics/issue-detail-panel.tsxfrontend/src/components/admin/analytics/issues-table.tsxfrontend/src/components/admin/analytics/releases-card.tsxfrontend/src/components/domain-prompts/domain-workspace.tsxfrontend/src/components/layout/sidebar.tsxfrontend/src/hooks/use-job-stream.tsfrontend/src/types/analytics.tsqa-chatbot/src/promptly/admin/api/router.pyqa-chatbot/src/promptly/admin/api/schemas.pyqa-chatbot/src/promptly/api/v1/prompts.pyqa-chatbot/src/promptly/core/middleware.pyqa-chatbot/src/promptly/main.py
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/hooks/use-job-stream.ts
- frontend/src/components/domain-prompts/domain-workspace.tsx
- qa-chatbot/src/promptly/core/middleware.py
- frontend/src/components/layout/sidebar.tsx
- frontend/src/components/admin/analytics/developer-metrics.tsx
| if issue_resp.status_code != 200: | ||
| raise HTTPException(status_code=404, detail="Issue not found in Sentry") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't collapse upstream Sentry failures into 404s.
Line 3323 maps every non-200 issue response to “not found”. Invalid credentials, rate limits, and Sentry 5xxs will all be misreported as 404s, which breaks operator triage and retry behavior. Return 404 only for an upstream 404, and translate other failures to 502/503.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@qa-chatbot/src/promptly/admin/api/router.py` around lines 3323 - 3324, The
issue handling in the Sentry lookup currently turns every non-200 response into
a 404, which hides auth, rate-limit, and upstream server failures. Update the
response handling around issue_resp so it checks the actual upstream status
code: keep HTTPException with 404 only when Sentry returns 404, and map other
non-200 responses to an appropriate 502/503 instead. Use the existing
issue_resp.status_code branch and the HTTPException raise path to preserve the
right error semantics for callers.
| event_data: dict[str, Any] = {} | ||
|
|
||
| if event_resp.status_code == 200: | ||
| event = event_resp.json() | ||
| exception_info: dict[str, Any] | None = None | ||
| request_info: dict[str, Any] | None = None | ||
| breadcrumbs: list[dict[str, Any]] = [] | ||
|
|
||
| for entry in event.get("entries", []): | ||
| etype = entry.get("type", "") | ||
|
|
||
| if etype == "exception": | ||
| values = entry["data"].get("values", []) | ||
| if values: | ||
| exc = values[-1] | ||
| raw_frames = (exc.get("stacktrace") or {}).get("frames", []) | ||
| frames = [ | ||
| { | ||
| "filename": f.get("filename", ""), | ||
| "lineno": f.get("lineno"), | ||
| "function": f.get("function", ""), | ||
| "context": f.get("context", []), | ||
| "in_app": bool(f.get("inApp", False)), | ||
| "vars": {k: str(v)[:120] for k, v in (f.get("vars") or {}).items()}, | ||
| } | ||
| for f in raw_frames | ||
| ] | ||
| exception_info = { | ||
| "exc_type": exc.get("type", ""), | ||
| "exc_value": str(exc.get("value", ""))[:500], | ||
| "mechanism": (exc.get("mechanism") or {}).get("type", ""), | ||
| "frames": frames[-20:], | ||
| } | ||
|
|
||
| elif etype == "request": | ||
| req = entry["data"] | ||
| raw_headers = req.get("headers") or [] | ||
| exception_info_req_headers = ( | ||
| raw_headers if isinstance(raw_headers, list) else list(raw_headers.items()) | ||
| ) | ||
| request_info = { | ||
| "method": req.get("method", ""), | ||
| "url": req.get("url", ""), | ||
| "query_string": req.get("query", "") or "", | ||
| "headers": exception_info_req_headers[:15], | ||
| } | ||
|
|
||
| elif etype == "breadcrumbs": | ||
| crumbs = (entry["data"].get("values") or [])[-12:] | ||
| breadcrumbs = [ | ||
| { | ||
| "type": c.get("type", ""), | ||
| "category": c.get("category", ""), | ||
| "message": str(c.get("message") or "")[:120], | ||
| "level": c.get("level", ""), | ||
| "timestamp": c.get("timestamp", ""), | ||
| } | ||
| for c in crumbs | ||
| ] | ||
|
|
||
| user = event.get("user") or {} | ||
| geo = user.get("geo") or {} | ||
| tags = event.get("tags") or [] | ||
|
|
||
| event_data = { | ||
| "event_id": event.get("eventID", ""), | ||
| "timestamp": event.get("dateCreated", ""), | ||
| "user": { | ||
| "id": user.get("id"), | ||
| "email": user.get("email"), | ||
| "ip": user.get("ip_address"), | ||
| "geo_city": geo.get("city"), | ||
| "geo_country": geo.get("country_code"), | ||
| "geo_region": geo.get("region"), | ||
| }, | ||
| "tags": [ | ||
| {"key": str(t[0]), "value": str(t[1])} | ||
| if isinstance(t, list | tuple) | ||
| else {"key": str(t.get("key", "")), "value": str(t.get("value", ""))} | ||
| for t in tags | ||
| ], | ||
| "exception": exception_info, | ||
| "request": request_info, | ||
| "breadcrumbs": breadcrumbs, | ||
| "release": ( | ||
| event["release"].get("version") | ||
| if isinstance(event.get("release"), dict) | ||
| else str(event["release"]) | ||
| if event.get("release") is not None | ||
| else None | ||
| ), | ||
| } | ||
|
|
||
| return JSONResponse( | ||
| content={ | ||
| "success": True, | ||
| "data": { | ||
| "issue": { | ||
| "id": str(issue.get("id", "")), | ||
| "short_id": issue.get("shortId", ""), | ||
| "title": issue.get("title", ""), | ||
| "level": issue.get("level", "error"), | ||
| "count": int(issue.get("count", 0) or 0), | ||
| "user_count": int(issue.get("userCount", 0) or 0), | ||
| "first_seen": issue.get("firstSeen", ""), | ||
| "last_seen": issue.get("lastSeen", ""), | ||
| "permalink": issue.get("permalink", ""), | ||
| "culprit": issue.get("culprit", ""), | ||
| "status": issue.get("status", ""), | ||
| }, | ||
| "latest_event": event_data, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep latest_event shape stable when the event fetch fails.
Line 3327 initializes event_data to {}, and that empty object is returned unchanged whenever /events/latest/ is unavailable. frontend/src/components/admin/analytics/issue-detail-panel.tsx then dereferences data.latest_event.user, tags, and breadcrumbs unconditionally, so a transient Sentry event failure turns into a client-side crash. Either fail this endpoint when the latest event cannot be loaded, or return a fully shaped empty payload.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@qa-chatbot/src/promptly/admin/api/router.py` around lines 3327 - 3437, The
latest_event payload is unstable when the event fetch fails because event_data
stays as an empty object, which breaks consumers that expect user, tags,
breadcrumbs, and other fields. Update the latest event handling in the issue
detail response builder to either fail the endpoint when the /events/latest/
call does not succeed, or always return a fully shaped default latest_event
object from the same code path that builds event_data, so callers like
issue-detail-panel.tsx can safely dereference it.
| raw_headers = req.get("headers") or [] | ||
| exception_info_req_headers = ( | ||
| raw_headers if isinstance(raw_headers, list) else list(raw_headers.items()) | ||
| ) | ||
| request_info = { | ||
| "method": req.get("method", ""), | ||
| "url": req.get("url", ""), | ||
| "query_string": req.get("query", "") or "", | ||
| "headers": exception_info_req_headers[:15], | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Allowlist request headers before returning them to the dashboard.
These lines forward raw request headers from Sentry into the admin response. The frontend only strips cookie and authorization, so headers like x-api-key, cf-access-token, vendor session headers, or other custom secret-bearing headers will still be exposed to any admin who opens the issue. Prefer returning a small safe allowlist such as content-type, accept, and user-agent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@qa-chatbot/src/promptly/admin/api/router.py` around lines 3363 - 3372, The
request info builder in router.py is returning raw Sentry headers, which can
expose secret-bearing values to the admin dashboard. Update the request handling
logic around the request_info construction to filter headers through a small
safe allowlist instead of slicing raw_headers, and keep only non-sensitive
headers such as content-type, accept, and user-agent before returning them.
| issue_context = _build_ai_fix_prompt(payload) | ||
|
|
||
| system_prompt = ( | ||
| "You are a senior backend engineer performing root-cause analysis on production errors " | ||
| "from a FastAPI / Python application. Be concise, specific, and actionable. " | ||
| "Always reference exact file paths and line numbers from the stack trace.\n\n" | ||
| "Respond in this exact markdown format:\n\n" | ||
| "## Root Cause\n" | ||
| "[2-3 sentences explaining *why* the error occurs]\n\n" | ||
| "## Location\n" | ||
| "`filename.py:line_number` in `function_name()`\n" | ||
| "[One sentence on what this code does and why it fails]\n\n" | ||
| "## Fix\n" | ||
| "```python\n" | ||
| "[Corrected code snippet, 5-15 lines]\n" | ||
| "```\n" | ||
| "[1-2 sentences explaining the change]\n\n" | ||
| "## Prevention\n" | ||
| "[One concrete tip to prevent this class of error recurring]" | ||
| ) | ||
|
|
||
| async with httpx.AsyncClient(timeout=30.0) as client: | ||
| resp = await client.post( | ||
| "https://openrouter.ai/api/v1/chat/completions", | ||
| headers={ | ||
| "Authorization": f"Bearer {llm.OPENROUTER_API_KEY.get_secret_value()}", | ||
| "Content-Type": "application/json", | ||
| }, | ||
| json={ | ||
| "model": "openai/gpt-4.1-mini", | ||
| "max_tokens": 800, | ||
| "messages": [ | ||
| {"role": "system", "content": system_prompt}, | ||
| { | ||
| "role": "user", | ||
| "content": ( | ||
| "Analyze this production error and provide a fix:\n\n" + issue_context | ||
| ), | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact incident context before sending it to OpenRouter.
issue_context can include stack-frame locals, full request URLs, and breadcrumb messages from production traffic, and Lines 3549-3568 forward that payload verbatim to a third-party model. That is enough to leak secrets, tokens, emails, or customer data during incident analysis. Strip locals, query strings, IPs/emails, and other high-entropy values server-side before the outbound call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@qa-chatbot/src/promptly/admin/api/router.py` around lines 3528 - 3568, The
OpenRouter request in router.py forwards issue_context from
_build_ai_fix_prompt(payload) without sanitizing sensitive incident data. Add a
server-side redaction step before the httpx.AsyncClient post call to strip
stack-frame locals, request query strings, IPs, emails, tokens, and other
high-entropy values from the content sent in the messages payload. Keep the
prompt-building flow intact, but ensure only sanitized context is included in
the user message passed to openrouter.ai.
…ead of user prompt
…date token usage logic, and improve issues table accessibility
2 participants
What does this PR do?
Type of change
Checklist
make checkpasses for backend,npm run lintfor frontend)make testpasses)make migration name=...).env.exampleupdated if new env vars addedHow to test
Related issues
Summary by CodeRabbit
New Features
Bug Fixes