CodeRabbit full-repo review (do not merge)#2
Closed
LEANDERANTONY wants to merge 326 commits into
Closed
Conversation
Removed local setup instructions and testing section from README.
… event routing
backend/webhooks/lemonsqueezy.py verifies HMAC-SHA256 of raw body against
AIJOBAGENT_LEMONSQUEEZY_WEBHOOK_SECRET, parses the LS event envelope, and routes
each of 10 subscription events to a state transition. Returns 401 on signature
failure, 200 with {status: duplicate} on idempotent replays, 200 on success.
backend/routers/billing.py mounts the webhook + a customer-portal helper endpoint
(POST /api/billing/portal). backend/app.py mounts the router under /api.
Idempotency: subscription_webhook_log table keyed by meta.webhook_id. The
upsert against subscriptions is also keyed on user_id, so even if the log
lookup fails-open, repeated events produce the same row.
Event-to-status mapping:
created/resumed/payment_recovered/payment_success -> active
updated -> mirrors payload status
cancelled -> cancelled (kept paid until ends_at)
payment_failed -> past_due
expired -> expired (downgrades to Free)
paused/unpaused -> paused/active
Divergence from brief: status=cancelled (not active) on subscription_cancelled
so the resolver can render distinct UI ('renewing' vs 'ends X'). The tier
resolver still treats cancelled-with-active-period as paid, matching brief
intent.
+28 tests (signature verification, every event type, idempotency, end-to-end
route). Suite: 476 -> 504.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
frontend/src/lib/api.ts adds getCheckoutUrl(tier), isLemonSqueezyEnabled(), and getCustomerPortalUrl(). When NEXT_PUBLIC_LEMONSQUEEZY_* env vars are empty, isLemonSqueezyEnabled() returns false so the pricing CTAs render as 'Coming soon' fallback — lets this branch ship to main before LS KYC clears or live variants exist. frontend/src/components/landing-page.tsx PRICING_TIERS CTAs route Pro/Business buttons to the LS hosted checkout URL with: ?checkout[custom][user_id]=<supabase-uid> &checkout[success_url]=<workspace>?ls_checkout=success The custom_data binding lands in the webhook payload so the backend can resolve which user just subscribed without trusting the LS-side email. frontend/src/components/workspace/WorkspaceShell.tsx adds a 'Manage subscription' button in the account popover (only visible when quotaSnapshot.tier !== 'free') that POSTs to /api/billing/portal and window.location's to the one-time LS customer portal URL. Also wires a post-checkout effect that detects ?ls_checkout=success in the URL on workspace mount and refreshes quota so the tier badge updates immediately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
.env.example documents 7 new env vars (5 backend AIJOBAGENT_LEMONSQUEEZY_* secrets + 2 NEXT_PUBLIC_LEMONSQUEEZY_* public values for the frontend bundle). All default to empty; backend webhook returns 503 + frontend disables the CTA with 'Coming soon' when unset, so a deploy without LS configured is safe. docs/lemon-squeezy.md covers: - Architecture diagram (table + helper + webhook + frontend wiring) - Event-to-status mapping table (all 10 LS subscription events) - Setup walkthrough for sandbox + live mode - Webhook URL to register, signature secret rotation - Customer portal URL contract Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Day 42 covers the 8-step tier enforcement series end-to-end: tier shim + TIER_CAPS, atomic quota with refund-on-failure, gates at /workspace/analyze + /workspace/assistant (sync+stream) + /workspace/parse-resume + resume builder + /jobs/search + saved_jobs + saved_workspaces, tier-aware model selection (premium routes review/resume_gen/cover to gpt-5.5), /workspace/quota endpoint + frontend Premium toggle, and the tier-aware saved_workspaces retention sweeper. 99 new tier- enforcement tests (451 -> 504 then 504 -> 511 with the lint fix). Deployed to api.job-application-copilot.xyz. Day 43 covers the Lemon Squeezy scaffold on feat/lemonsqueezy-integration: subscriptions + webhook_log tables, 60s LRU cache on get_active_subscription, HMAC-verified webhook with full event-to-status mapping, frontend Pro CTA wiring with ?checkout[custom][user_id] binding, env-gated 'Coming soon' fallback so the branch can merge to main before LS KYC clears. 51 new tests (501 -> 504+ before the docs commits). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four ADRs covering the load-bearing architectural decisions from the recent tier enforcement + payments work: ADR-020 — resolve_user_tier(app_user) as the single shim every gate routes through. Mirror of HelpmateAI's ADR-015 with AI Job Agent's naming + TIER_CAPS shape. Payment swap is a one-function change. ADR-021 — atomic SQL function (increment_aijobagent_counter) with embedded cap check raising P0001 on overrun; Python wrapper translates to QuotaExceededError -> global 429 handler. Diverges from HelpmateAI's pre-check + post-increment pattern (atomic is safer under concurrent workspace runs from the same user). Refund-on-failure for the LLM-cost counters (tailored_applications, premium_applications, assistant_turns, etc.); no refund for cheap counters like job_searches. ADR-022 — tier-aware model selection via constructor injection (model_overrides dict at ApplicationOrchestrator construction time) rather than per-call model_override. Keeps the per-agent retry + deterministic fallback paths tier-correct without extra plumbing. Premium routes review/resume_gen/cover_letter to gpt-5.5; tailoring stays mini (COGS pinning). ADR-023 — Lemon Squeezy as Merchant of Record for v1. Same rationale as HelpmateAI's ADR-017: solo dev unregistered in India, global reach, tax compliance off-loading. processor-neutral subscriptions schema reserves the Stripe + Razorpay slots for a post-incorporation migration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a 'Tier enforcement' row to the feature matrix covering the gates, the premium GPT-5.5 routing, atomic quota + refund-on-failure, and the retention sweeper. Bumps the ADR count from 19 to 23 with the four new ADRs (020 tier shim, 021 atomic quota, 022 tier-aware model selection, 023 Lemon Squeezy MoR) called out by number. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 275 files, which is 125 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (25)
📒 Files selected for processing (275)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dacf9c0cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+28
to
+29
| _ACCESS_TOKEN_MAX_AGE_SECONDS = 60 * 60 * 24 * 7 # 7 days | ||
| _REFRESH_TOKEN_MAX_AGE_SECONDS = 60 * 60 * 24 * 30 # 30 days |
There was a problem hiding this comment.
The access token cookie expires after 7 days while the refresh token cookie lasts 30 days, but session restore requires both tokens (resolve_authenticated_context rejects when either is missing). In practice, a user who returns after 7+ days of inactivity is logged out even though their refresh token is still valid, which breaks the intended "stay signed in" behavior and causes avoidable re-authentication churn.
Useful? React with 👍 / 👎.
Comment on lines
+148
to
+149
| if existing_count >= cap: | ||
| raise QuotaExceededError( |
There was a problem hiding this comment.
This cap check rejects saves when existing_count >= cap, which blocks Free users (cap=1) from updating their existing workspace even though storage is an upsert-by-user and comments describe overwrite as supported. The frontend autosave path calls /workspace/save directly (without pre-delete), so once a Free user has one saved workspace, every subsequent save fails with a quota error and the snapshot can no longer refresh.
Useful? React with 👍 / 👎.
Comment on lines
+256
to
+260
| if not code_verifier and not auth_flow: | ||
| latest_flow = _consume_latest_pkce_flow() | ||
| if latest_flow: | ||
| auth_flow = latest_flow.get("auth_flow") | ||
| code_verifier = latest_flow.get("code_verifier") |
There was a problem hiding this comment.
When auth_flow is missing, the exchange path consumes the most recently created PKCE flow from a global store. Because that store is shared across users, one malformed or malicious /auth/google/exchange request can steal another user's pending verifier, causing their legitimate login callback to fail. This cross-request coupling makes sign-in reliability depend on unrelated traffic and enables easy denial of sign-in attempts.
Useful? React with 👍 / 👎.
The saved_workspaces quota gate was rejecting every re-save from a Free user (cap=1) because it checked `existing_count >= cap` without distinguishing UPDATES to an existing slot from creation of NEW slots. Today's store upserts on user_id (one row per user), so any "existing row" means the save is an update — not a new slot — and the cap shouldn't apply. Real-world impact: the frontend autosave path calls /workspace/save directly without pre-delete. Once a Free user has one saved workspace, every subsequent autosave got rejected with a 429, breaking the snapshot-refresh UX for every Free user past their first save. Fix splits the gate into two questions: * is_existing_slot_update — does the user already own the slot? * is_creating_new_slot — would this save add to the slot count? The cap fires only on the second case. Today, with one-row-per-user storage, the gate is effectively dormant (existing_count == 1 always means update, existing_count == 0 with cap >= 1 always passes). It will start firing correctly the moment we migrate to per-slot rows (future PR with slugs/labels), without revisiting this code path. test_2nd_saved_workspace_on_free_returns_429 was inverted to test_free_re_save_upserts_existing_workspace and now pins the correct UX: re-saving is allowed because it's an update, not a new slot. test_saved_workspace_delete_then_save_works was updated to drop the intermediate 429 assertion (no longer fires under the fix) and instead pin that DELETE + save round-trips cleanly. Found by ChatGPT Codex review on PR #2 (May 2026). Full backend suite: 501 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The auth_cookies module set ACCESS_TOKEN_MAX_AGE to 7 days while
REFRESH_TOKEN_MAX_AGE was 30 days. The docstring on those constants
claims "the access token mirrors [refresh] so a quiet tab doesn't
lose only half its credentials" — but the actual values didn't
mirror.
Real-world failure mode:
* Session restore (resolve_authenticated_context) requires BOTH
cookies present.
* A user returning after 7+ days (but within 30) has a valid
refresh cookie but a purged access cookie.
* Restore rejects the half-credentialed pair and forces a fresh
OAuth re-login, defeating the "stay signed in" UX the 30-day
refresh window was meant to deliver.
Fix matches the docstring's stated intent: both cookies live for
30 days. The COOKIE's max_age governs browser purge time, NOT the
JWT's validity — Supabase rotates the access JWT on every restore
using the still-valid refresh JWT, so the inside-the-cookie token
being temporally stale is fine and expected.
Codex P1 finding on PR #2 (May 2026). Auth-test suite (14 tests)
still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The /auth/google/exchange path had a defensive fallback that consumed the most-recently-created PKCE flow from the shared _pkce_flows SQLite store when both `auth_flow` and `code_verifier` were missing on the request. The intent was to recover legitimate users who lost their cookie mid-flow. The unintended consequence (Codex P2 finding on PR #2, May 2026): the store is shared across users. A malformed or malicious exchange request could "steal" another user's pending verifier: * User A starts sign-in -> a PKCE flow lands in the store * User B (or an attacker) hits /auth/google/exchange with no auth_flow + no cookie * Fallback consumes User A's flow * User A's legitimate Google callback then fails ("session expired") because their flow was deleted from the store * In the worst case (attacker controls a matching auth_code), the exchange completes with User A's verifier on the attacker's behalf Cross-request coupling like this makes sign-in reliability depend on unrelated traffic. Removed in favor of failing fast with the clean "Google sign-in session expired" error. Legitimate users who hit this case re-run sign-in (rare, easy recovery); the attack surface goes from "any user can hijack any other user's sign-in flow" to zero. Test inverted: the previous test_exchange_code_for_session_uses_latest_local_flow_store_when_auth_flow_is_missing pinned the buggy behavior. New test test_exchange_code_for_session_does_not_consume_other_users_pkce_flow_when_auth_flow_is_missing pins the secure behavior: when the caller arrives without auth_flow + code_verifier, exchange raises "session expired" and the victim's pending PKCE flow stays in the store untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror of HelpmateAI commit 8016457 (Codex P1 finding on HelpmateAI's PR #4). The LS scaffold for both apps was built from the same template, so the same URL bug exists here too — even though Codex didn't flag it on AI Job Agent's PR #2 (Codex's review surfaces sample issues rather than every instance). Old: https://<store>.lemonsqueezy.com/buy/<variant> New: https://<store>.lemonsqueezy.com/checkout/buy/<variant> Per the LS docs at docs.lemonsqueezy.com/help/checkout/hosted-checkouts, the `/buy/` path is a non-checkout endpoint. Every Pro upgrade CTA would have resolved there and failed silently the moment LS goes live with real variant IDs. This fix lands BEFORE KYC clears so the broken state never reaches paid customers. No test changes — the helper is exercised in real LS sandbox testing which happens post-merge once variant IDs exist. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Programmatic wrapper around 5 quality runners (resume_parser, jd_parser, tailoring, review, orchestrator_e2e). Captures pass/fail per runner + key metrics in a JSON summary. Exits non-zero on regression beyond 5pt threshold (configurable via --threshold). docs/operations.md adds the VPS crontab line and alerting paths. Catches Prisha-style silent model-version drift before customers see malformed outputs in production. 13 new tests in tests/backend/test_nightly_eval.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/schemas_llm_outputs.py defines strict Pydantic models for every agent + service that consumes a JSON-shape LLM response: * TailoringOutput, ReviewOutput, ResumeGenerationOutput, CoverLetterOutput * JDParserOutput, ResumeParserOutput, JDSummaryOutput * ResumeBuilderTurnOutput, ResumeBuilderStructuringOutput All models use extra='forbid' so unknown fields raise instead of silently passing through — defends against Prisha-style silent model-version updates that add/remove/rename fields. Located at src/schemas_llm_outputs.py rather than src/schemas/ as a package because converting schemas.py to a package would break 30+ existing imports. No test changes in this commit — the schemas are exercised by the agent migration in the next commit + its tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/openai_service.py adds run_structured_prompt(...) — wraps OpenAI's
response_format={'type':'json_schema',...} feature so the model is
CONSTRAINED to match the Pydantic schema at generation time, not just
validated post-hoc. Supporting helpers _inline_refs() and
_enforce_strict_object_constraints() rewrite Pydantic schemas into
OpenAI's strict-mode JSON Schema format (additionalProperties: false,
all required fields explicit, no $ref indirection).
Migrated 6 high-value call sites onto run_structured_prompt:
* src/agents/tailoring_agent.py
* src/agents/review_agent.py
* src/agents/resume_generation_agent.py
* src/agents/cover_letter_agent.py
* src/services/jd_summary_service.py
* backend/services/resume_builder_service.py (structuring pass)
Each agent uses a hasattr-shim that falls back to the old
run_json_prompt + model_validate(...) path so existing test fakes
that only mock run_json_prompt keep working. Test files for the
three migrated agents (cover_letter, resume_generation, orchestrator)
get small updates to add run_structured_prompt onto the existing
fakes alongside run_json_prompt.
Deferred call sites (still use run_json_prompt + key check):
* resume_builder intake turn
* ResumeLLMParserService
* JobDescriptionLLMParserService
* assistant_service
All have schemas defined; only the call-site swap is deferred.
18 new tests in tests/backend/test_structured_outputs.py + small
fixture updates in test_cover_letter_agent / test_resume_generation_agent
/ test_orchestrator.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs/sql/supabase-run-traces.sql creates the aijobagent_run_traces table for per-LLM-call cost + token tracking. Schema: * trace_id (uuid PK, default gen_random_uuid()) * user_id (FK auth.users, on delete cascade) * task_name (text — which agent / pipeline stage) * model_name (text — gpt-5.4-mini, gpt-5.5, etc.) * prompt_tokens, completion_tokens (int) * cost_usd (numeric 10,6) * success (boolean) * created_at (timestamptz) RLS: service-role-only writes (mirrors quota_counters), user-can-read-own via the policy on user_id = auth.uid(). backend/run_traces.py exposes record_trace(...) with an in-memory fallback for tests. Supabase backend used when SUPABASE_URL + SERVICE_ROLE_KEY are configured. Migration NOT yet applied to Supabase — leave as SQL file for the user to apply via MCP or SQL editor post-merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/openai_service.py adds:
* _MODEL_PRICING_USD_PER_MILLION map for all 4 models
gpt-5.4-nano: $0.10 in / $0.40 out
gpt-5.4-mini: $0.75 / $4.50
gpt-5.4: $2.00 / $10.00
gpt-5.5: $5.00 / $30.00
* compute_call_cost_usd(prompt_tokens, completion_tokens, model_name)
* _record_cost_trace() hook called from run_json_prompt /
run_structured_prompt / run_text_stream after each successful call
* user_id constructor arg so the trace row gets attributed properly
backend/services/auth_session_service.py threads user_id into the
OpenAIService instance so per-call traces are user-attributable.
17 new tests in tests/backend/test_cost_tracking.py covering:
* Pricing-table correctness per model
* Cost computation across token shapes
* Trace row contract vs the SQL column shape (drift catch)
* Tier-margin guard — Pro 500 questions on mini stays under $4.50 floor
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two deterministic runners (_run_resume_parser, _run_jd_parser) were defined with positional underscore-prefixed while the rest used (no underscore) and the test suite called all of them with the openai_service= keyword. Aligned to the keyword form across all 5 runners; the deterministic ones explicitly to mark intentional-ignore. Renames a positional argument; no behavior change. 20/20 tests pass in tests/backend/test_nightly_eval.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Whitelists `coderabbit-root` as an allowed auto-review base so the full-repo audit PRs against that branch get reviewed instead of "Review skipped." Without this config, CodeRabbit refuses both auto and manual `@coderabbitai review` triggers on any PR whose base is not the repo's default branch. profile: chill — surface issues without nit-blocking style noise. request_changes_workflow: false — manual triggers always work. Mirrors the HelpmateAI config (same audit-PR workflow, same trick). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review --path "backend/**" Scoping to backend to stay under the 150-file ceiling. Will follow with |
|
✅ Actions performedFull review triggered. |
LEANDERANTONY
added a commit
that referenced
this pull request
May 30, 2026
…(LLM-1, OBS-1) Two spend-accounting holes left the per-user COGS the pricing rests on understated. (LLM-1) The resume-builder web_search tool called openai_service._client.responses.create directly, bypassing the accounting layer entirely — its tokens (the priciest part of an intake turn) never hit the weekly meter and never cost-traced. (OBS-1) _record_cost_trace checked only self._user_id while _record_token_meter had a meter_user_scope ContextVar fallback, so every JD/resume parse (bare OpenAIService inside a scope) was metered but wrote zero run-trace rows; create_embeddings never cost-traced at all and the embedding model had no pricing entry. Per Rec #2, OpenAIService is now the single door. New OpenAIService.run_builtin_web_search fires the search AND records usage + cost-trace + enforces the session budget (and applies WEB_SEARCH_TIMEOUT_SECONDS, incidentally fixing the dead-30s-timeout LLM-2); _web_search routes through it. _record_cost_trace gained the same effective_user_id = self._user_id or _METER_USER_ID.get() fallback _record_token_meter uses, so metered and cost-traced are now the same set. create_embeddings cost-traces, and text-embedding-3-small was added to the pricing map. tests/backend/test_cost_tracking.py adds: a bare service inside meter_user_scope now writes a cost-trace row (was zero); embeddings cost-trace with the embedding price; run_builtin_web_search meters + traces. The web_search dispatcher tests now run against a REAL OpenAIService (the raw-client bypass is gone) and a new test asserts the dispatcher cost-traces. The pricing-map test was updated for the 5th model. Fixes: LLM-1, OBS-1 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@coderabbitai full review