Skip to content

Fix the five bugs that would have broken the shadow run - #4

Merged
sparsh-j01 merged 15 commits into
mainfrom
fix/launch-blockers
Jul 14, 2026
Merged

Fix the five bugs that would have broken the shadow run#4
sparsh-j01 merged 15 commits into
mainfrom
fix/launch-blockers

Conversation

@sparsh-j01

@sparsh-j01 sparsh-j01 commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Five launch blockers, found by a deploy-focused engineering review. Every one of them is invisible on next dev and only shows up in production, on real candidates — which is exactly the class of bug a shadow run should not have to discover for you.

What was broken

1. Every report would have come back failed (7115ea9)
Nothing in apps/web set maxDuration, so every route ran on Vercel's 10s default. Inngest invokes the endpoint once per step, so the grade step is a single request that must outlive a 30s Gemini call. It gets killed at 10s, retried twice, and the interview is marked failed. Same class of break on /api/resumes/parse, where the real work runs in after() and Vercel bills it to the same invocation — the résumé's embedding was being silently dropped.

Also unified the scorer timeout: it was 30s in production and 60s in the eval, so the eval was measuring a model with twice the budget prod gave it. A grade taking 40s passed the gate and failed the customer. Both now read SCORER_TIMEOUT_MS.

2. Free users were charged for interviews that never happened (3b1b5a3)
The quota gate counted every row created this month, with no status filter. Interview start is admin-gated, so a free user who requested three interviews an admin never approved had spent their entire month on nothing — and a scorer outage spent it for them. The rule is now: you are billed when the interview actually starts, not when it's asked for. /end also no longer strands a silent room live forever; it hands the interview back, unbilled and re-joinable.

3. The coding round was guillotined mid-problem (583deab)
The 10-minute cap fired cold — a candidate mid-DSA-problem was cut off with no warning, then graded on the fragment they were cut off in. The interviewer now gets a two-minute warning to land the question and close. Judge0 also had no retry, so one 503 from RapidAPI graded a correct solution as a failure. And the agent now reads its own .env, so running it against production can't put prod credentials where drizzle-kit reads them.

4. A missing agent worker was completely silent (717ba76)
live only meant the candidate reached the room. If no worker was running, nobody joined, nothing errored, and the candidate sat watching a silent orb until they gave up. Now: a 15s watchdog, a real failure state, and the interview handed back unbilled. The FAQ also stopped promising "10–15 minutes" when the cap hard-stops at 10.

5. Dead config (ff4d7f7)
CLERK_WEBHOOK_SECRET is read by nothing — there is no Clerk webhook route.

Test plan

  • pnpm typecheck — clean (4 packages)
  • pnpm test — 53 passing (4 new tests pin the billing rule, including fail-closed on an unknown status)
  • pnpm lint — clean
  • python -m pytest — 12 passing; python coding.py — 36 graders / 72 cases verified
  • Tests confirmed non-vacuous by mutation: sabotaging transcriptIsThin turns 3 tests red

Not in this PR

README.md and .github/assets/ are deliberately untouched — those land after the shadow run, with screenshots.

Summary by CodeRabbit

  • New Features
    • Added a time-warning phase shortly before the session’s hard limit, with safer automatic teardown on expiry.
    • Added an interviewer room join watchdog that surfaces clearer errors and gracefully ends when needed.
  • Bug Fixes
    • Ending interviews now handles race conditions and empty-candidate scenarios more reliably.
    • Improved grading and résumé/coding reliability with longer route time windows, retries, and shared grading timeouts.
    • Monthly usage/cap logic now properly excludes non-billable statuses (and bills unknown statuses).
  • Documentation
    • Updated the FAQ to clarify the fixed hard time-limit behavior.

Nothing in apps/web set maxDuration, so every route ran on Vercel's 10s
default. Three paths are designed to take longer:

  - /api/inngest — Inngest invokes the endpoint once PER STEP, so the
    `grade` step is one request that must outlive a 30s Gemini call. It
    was killed at 10s, retried twice, and the interview marked `failed`.
    Every report in production would have come back broken.
  - /api/resumes/parse — responds fast, then does the real work in
    after() (structure + embed + R2), which Vercel bills to the same
    invocation. The résumé's embedding was being silently dropped: the
    user sees "uploaded", RAG never sees the résumé.

Invisible locally — `next dev` has no timeout — so this bug could only
have surfaced in production, on real candidates.

Also unify the scorer timeout. It was 30s in production and 60s in the
eval, so the eval was measuring a model with twice the budget prod gave
it: a grade taking 40s passed the gate and failed the customer. Both now
read SCORER_TIMEOUT_MS from shared/models.ts, next to SCORER_TEMPERATURE.

45s, not 60s: the abort must fire BEFORE the platform kills the
invocation, or there's no clean error for Inngest to retry — just a dead
request. It's still a guess, not a measurement; `pnpm eval:live` prints
the real duration.
…sked for

The quota gate counted every row created this month, with no status
filter. Since interview start is admin-gated, a free user who requested
three interviews an admin never approved had spent their whole month on
nothing — and a scorer outage spent it for them, because a `failed`
interview counted too. There was no way to give a slot back.

The rule is now one shared constant: you are billed when the interview
actually STARTS (live → processing → ready). Unbilled:

  requested — an admin never approved it
  approved  — granted but never taken
  failed    — it broke on our side

Unrecognised statuses are billed: fail closed, so a new status can never
quietly hand out free interviews.

Two places had to agree and didn't. The API gate ran a real count(*); the
dashboard derived "used this month" by filtering a row list capped at 20.
Two sources of truth for one number is how a user gets told "1 of 3 used"
and then refused at 2. Both now read UNBILLED_STATUSES.

And /end no longer strands a silent room. It used to return early when
nobody spoke, leaving the row `live` forever — so an interview the agent
never joined burned a slot permanently, and the candidate was charged for
silence caused by our outage. It now hands the interview back to
`approved`, which is unbilled and which the token route already accepts
for a re-join, so "retry" works with no new endpoint and no new state.

Letting `approved` go unbilled is safe: approvals are admin-gated, and the
hourly cap still bounds request spam.

Adds maxDuration=60 to /api/interviews — personalizePlan runs retrieval
(embed, 8s) then the plan LLM (7s) sequentially, ~15s worst case, past
Vercel's 10s default.
…sient Judge0

Three agent-side fixes.

1. Warn before the hard cap. MAX_SESSION_MIN=10 fired cold: a candidate
   mid-DSA-problem was cut off with no warning, then graded on the
   fragment they were cut off in. The interviewer is now told at 8:00 that
   two minutes remain — finish the current thought, close, call
   end_interview, start nothing new. Guarded on _ended, because
   end_interview deliberately leaves the cap task armed; without the guard
   a finished interview would be told to "wrap up" into a closed session.

2. Retry Judge0. There was no retry, so ONE 503 from RapidAPI mid-round
   graded a correct solution as a failure — the candidate blamed for our
   infrastructure. Three attempts, 1s/3s backoff, on 429/5xx and transport
   errors only. A 4xx that isn't 429 (bad key, exhausted quota, bad
   language id) raises immediately: retrying it would just burn the ~50/day
   free tier faster.

3. Isolate production credentials. `pnpm bootstrap` symlinks the root .env
   into packages/db, where drizzle-kit reads it. The agent dials OUT to
   LiveKit and needs no inbound port, so it can run against production from
   a laptop — but that means putting the prod DATABASE_URL in its env, and
   in the root .env one absent-minded `pnpm db:push` would rewrite the
   production schema. main.py now prefers apps/agent/.env when it exists,
   which nothing else reads. Local dev is unchanged: no file, no setup.
…nutes

"live" only meant the CANDIDATE reached the room. LiveKit is happy to hold
a room with nobody in it, so if no agent worker was running — crashed, not
deployed, laptop asleep — nobody joined, nothing errored, and the candidate
sat watching a silent orb until they gave up. The single worst failure in
the product, and it was completely silent.

A 15s watchdog now arms only when the room has no remote participant, and
disarms on ParticipantConnected (the agent is usually in within a second or
two, often before the candidate). If it fires, the candidate gets a real
failure state, and we POST /end first so the interview is handed back
unbilled — otherwise the error message would be lying when it says this
doesn't count against their monthly interviews.

Also: the FAQ promised "10–15 minutes" while MAX_SESSION_MIN hard-stops at
10. Don't advertise a range the product can't keep. It now says ten minutes,
says plainly that it's a hard limit, and — true as of the previous commit —
that the interviewer warns you before closing rather than cutting you off.
No code reads it. Users are mirrored into the DB lazily on first write
(api/interviews), not via a Clerk webhook — there is no webhook route. The
line only sent people hunting for a thing that doesn't exist.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: af1fad8b-6d45-4902-8638-dc6b4648544d

📥 Commits

Reviewing files that changed from the base of the PR and between 92e6cf8 and d73dc73.

📒 Files selected for processing (2)
  • apps/web/app/interview/[id]/page.tsx
  • packages/shared/src/models.ts
 __________________________________________________________
< If your code was a carrot, I'd bury it and forget where. >
 ----------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
📝 Walkthrough

Walkthrough

The changes add agent environment configuration, Judge0 retries, session-cap enforcement, LiveKit join failure handling, quota-status rules, shared scoring timeouts, repeated evaluation reporting, route duration limits, and corrected interview completion handling.

Changes

Interview reliability and billing

Layer / File(s) Summary
Shared quota and scoring contracts
packages/shared/src/entitlements.ts, packages/shared/src/entitlements.test.ts, packages/shared/src/models.ts
Defines unbilled statuses, fail-closed quota consumption, and validated shared scorer timeout configuration with tests.
Quota counting and interview completion
apps/web/app/api/interviews/route.ts, apps/web/app/dashboard/page.tsx, apps/web/app/api/interviews/[id]/end/route.ts
Excludes unbilled statuses from quota counts and updates no-speech and concurrent-ending status responses.
Agent configuration and runtime resilience
apps/agent/...
Adds agent-specific environment loading, model normalization, Judge0 retries, session-cap warnings, cleanup, and tests.
Interview connection and execution limits
apps/web/app/interview/[id]/page.tsx, apps/web/app/api/inngest/route.ts, apps/web/app/api/resumes/parse/route.ts, apps/web/app/page.tsx
Adds an agent join watchdog, extends route durations, and documents the hard interview duration cap.
Timed scoring and evaluation reporting
apps/web/lib/score-interview.ts, packages/evals/src/grade.ts, packages/evals/src/suite.ts, packages/evals/src/live.ts
Uses shared scoring timeouts, retries transient grading requests, records latency, and reports repeated-run stability.
Environment template updates
.env.example, apps/agent/.env.example
Updates root placeholders and documents agent database, realtime, provider, sandbox, and model configuration.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Candidate
  participant InterviewPage
  participant LiveKitRoom
  participant EndInterviewAPI
  participant InterviewsDB
  Candidate->>InterviewPage: enter interview room
  InterviewPage->>LiveKitRoom: connect and wait for agent
  LiveKitRoom-->>InterviewPage: participant connects or timeout expires
  InterviewPage->>EndInterviewAPI: end interview after timeout
  EndInterviewAPI->>InterviewsDB: update eligible interview status
  EndInterviewAPI-->>InterviewPage: return interview status
Loading

Possibly related PRs

  • sparsh-j01/maven-ai#2: Both changes modify the Judge0 execution wrapper with authentication or retry-oriented execution behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the PR’s main theme of fixing multiple blockers that affected the shadow run.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/launch-blockers

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/agent/.env.example`:
- Line 17: Update the blank optional environment defaults in .env.example,
including LIVEKIT_URL and the other affected fields, to use explicit empty
quotes before their inline comments. Preserve the existing comments and ensure
copied values are parsed as empty strings so fallback behavior works correctly.

In `@apps/agent/main.py`:
- Around line 670-685: Protect the warning call in the session-cap flow so a
failing or hanging session.generate_reply does not bypass finalization. Bound it
with a timeout and handle non-cancellation exceptions, while preserving
cancellation propagation; ensure execution continues to the existing
agent._finalize("time cap"), session.aclose(), and ctx.delete_room() cleanup
regardless of warning failure.

In `@apps/web/app/api/interviews/`[id]/end/route.ts:
- Around line 45-62: Update the reset-to-approved branch in the interview end
route to inspect the UPDATE result using returning-row semantics, like the
scored branch. Only return the approved response when a matching live or
provisioning row was updated; otherwise handle the concurrent status change
through the existing finalized/scored flow rather than unconditionally reporting
scored: false.

In `@packages/shared/src/models.ts`:
- Around line 44-46: Update the SCORER_TIMEOUT_MS validation around rawTimeout
so environment overrides above the safe 45-second budget are rejected or clamped
to 45_000. Preserve finite positive overrides within that limit and ensure the
exported timeout never reaches the 60-second route execution cap.
🪄 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 Plus

Run ID: 4b12ce3e-1300-4962-8ed0-c18a6fe35854

📥 Commits

Reviewing files that changed from the base of the PR and between 9d62c52 and ff4d7f7.

📒 Files selected for processing (16)
  • .env.example
  • apps/agent/.env.example
  • apps/agent/coding.py
  • apps/agent/main.py
  • apps/web/app/api/inngest/route.ts
  • apps/web/app/api/interviews/[id]/end/route.ts
  • apps/web/app/api/interviews/route.ts
  • apps/web/app/api/resumes/parse/route.ts
  • apps/web/app/dashboard/page.tsx
  • apps/web/app/interview/[id]/page.tsx
  • apps/web/app/page.tsx
  • apps/web/lib/score-interview.ts
  • packages/evals/src/grade.ts
  • packages/shared/src/entitlements.test.ts
  • packages/shared/src/entitlements.ts
  • packages/shared/src/models.ts
💤 Files with no reviewable changes (1)
  • .env.example

Comment thread apps/agent/.env.example Outdated
Comment thread apps/agent/main.py Outdated
Comment thread apps/web/app/api/interviews/[id]/end/route.ts Outdated
Comment thread packages/shared/src/models.ts Outdated
Comment on lines +44 to +46
const rawTimeout = Number(env("SCORER_TIMEOUT_MS", "45000"));
export const SCORER_TIMEOUT_MS =
Number.isFinite(rawTimeout) && rawTimeout > 0 ? rawTimeout : 45_000;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound the environment override below the route execution limit.

Any finite positive value is accepted, including values at or above the 60-second route cap. In that case, Vercel can terminate the invocation before the abort timer fires, leaving no clean retryable error. Reject or clamp values above the safe 45-second budget.

Suggested validation
 const rawTimeout = Number(env("SCORER_TIMEOUT_MS", "45000"));
 export const SCORER_TIMEOUT_MS =
-  Number.isFinite(rawTimeout) && rawTimeout > 0 ? rawTimeout : 45_000;
+  Number.isFinite(rawTimeout) &&
+  rawTimeout > 0 &&
+  rawTimeout <= 45_000
+    ? rawTimeout
+    : 45_000;
📝 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.

Suggested change
const rawTimeout = Number(env("SCORER_TIMEOUT_MS", "45000"));
export const SCORER_TIMEOUT_MS =
Number.isFinite(rawTimeout) && rawTimeout > 0 ? rawTimeout : 45_000;
const rawTimeout = Number(env("SCORER_TIMEOUT_MS", "45000"));
export const SCORER_TIMEOUT_MS =
Number.isFinite(rawTimeout) &&
rawTimeout > 0 &&
rawTimeout <= 45_000
? rawTimeout
: 45_000;
🤖 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 `@packages/shared/src/models.ts` around lines 44 - 46, Update the
SCORER_TIMEOUT_MS validation around rawTimeout so environment overrides above
the safe 45-second budget are rejected or clamped to 45_000. Preserve finite
positive overrides within that limit and ensure the exported timeout never
reaches the 60-second route execution cap.

python-dotenv reads `STT_MODEL=    # default nova-3` as the VALUE "# default
nova-3" — the comment, not an empty string. Both .env.example files handed out
blank-with-comment lines for every optional key, and main.py loads the ROOT .env
when apps/agent/.env is absent, so a copied example booted the agent with a model
id made of prose. Quote the blanks.

The other half was in models.py: os.getenv(k, default) only falls back when the
key is ABSENT, so even a correctly-quoted `STT_MODEL=""` yielded an empty model
id. _env() treats blank as unset, matching the TS side's env().
The 8-minute "let's wrap up" line was an unguarded await. If generate_reply raised
(provider blip, session tearing down) or hung, the cap task died with it — and the
finalize + aclose + delete_room BELOW it never ran. The one job the cap exists to
do (stop the meter) was the thing that got skipped. Admin approval bounds how many
interviews start; nothing but this bounds how long one bills.

Warning is now bounded by wait_for and its failures swallowed, and the teardown is
anchored to a monotonic deadline so a slow warning can't push the cap out past
MAX_SESSION_SECONDS.

Extracted to session_cap.py because CI installs pytest only — a test importing
main.py would pull in LiveKit and never run. Against the old code the new tests
fail: the raise case errors out, and the hang case never terminates at all.
Three exits from the end route each computed "scored" their own way. The early
return said `scored: status !== "live"` — so an interview handed back as
`approved` by a no-speech leave was reported as SCORED, and a double-clicked Leave
redirected the candidate to a feedback report that is never going to be written.

One scoredFor() rule for all of them. The reset-to-approved write is now checked
like the processing write beside it: a caller that loses the race to the agent's
own finalize reads the row instead of asserting a status it didn't set.
Any finite positive override was accepted, including 90000 — which lets Vercel kill
the 60s invocation (maxDuration in api/inngest) before the abort timer fires: no
clean error to retry, just a dead request. Clamp to 50s, warn when clamping.
SCORER_TIMEOUT_MS (45s) and the stability of the grader's own scores were both
guesses — nobody had ever timed a real grade or run the suite twice. eval:live
--runs=N now reports median/max grade latency against the configured timeout, and
the run-to-run spread of every axis (anchored correctness should be flat; vibe
completeness is the one that drifts).

grade() also retries 429/5xx now. Production's grade step is an Inngest function
with retries: 2, so a Gemini 503 costs a retry, not the interview — the eval had
none, so a blip failed a model production would have graded fine. Same budget,
same attempt count, or the gate isn't measuring what ships.

The measurement itself is still unrun: SCORER_TIMEOUT_MS remains the 45s guess
until someone spends the API calls.
pnpm eval:live --runs=3, gemini-2.5-flash, n=15: median 22.7s, max 32.8s, per-run
worst case 30.9 / 31.6 / 32.8s.

The 30s timeout production shipped until last week was BELOW the typical worst-case
grade — it was aborting grades that were about to succeed and telling the candidate
their interview failed. 45s was 1.4x the slowest observed, thinner than it looked.

50s is not a comfortable margin, it's the whole margin: the scorer runs inside a 60s
Vercel function and the abort has to fire first. Two known gaps, documented at the
constant: the eval grades the free schema (Pro adds a studyPlan — more output, slower)
and the fixtures are shorter than real transcripts. So 32.8s is a floor for the Pro
path, not a ceiling. If Pro grades time out, the answer is a bigger budget, not a
bigger number.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
apps/agent/models.py (1)

9-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Correct fix for the blank-env-var bug; consider a unit test.

The _env helper correctly normalizes None/""/whitespace-only values to default, matching os.getenv's "absent-only" default gap it's working around.

Since this exact bug (blank env values not triggering defaults) is one of this PR's stated fixes, a couple of parametrized asserts (None, "", " ", real value) would guard against regressions cheaply.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/agent/models.py` around lines 9 - 22, The _env helper fixes blank
environment values but lacks regression coverage. Add a focused parametrized
unit test for _env asserting that None, an empty string, and whitespace-only
values return the default while a real value is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/agent/session_cap.py`:
- Around line 69-79: The cap teardown path must isolate and bound the
`agent._finalize("time cap")` call so a publish failure or hang cannot prevent
later cleanup. Wrap it in a local exception handler with an appropriate timeout,
log failures, and preserve execution of the existing grace sleep,
`session.aclose()`, and `ctx.delete_room()` steps.

In `@packages/evals/src/grade.ts`:
- Around line 54-89: Update the retry loop around the fetch and response parsing
in the grading function to catch non-HTTP exceptions, including fetch
rejections, timeout AbortErrors, JSON.parse failures, and feedbackReport.parse
errors, and retry them through the existing backoff until ATTEMPTS is exhausted.
Preserve immediate failure for non-transient HTTP statuses, while storing the
latest retryable error in lastErr and throwing it after the final attempt.

In `@packages/evals/src/live.ts`:
- Around line 24-27: Validate the parsed --runs value before applying Math.max
in the runs initialization, rejecting NaN or otherwise malformed values with a
clear failure instead of allowing zero iterations and an empty successful
result. Preserve the minimum of one run for valid numeric values.

---

Nitpick comments:
In `@apps/agent/models.py`:
- Around line 9-22: The _env helper fixes blank environment values but lacks
regression coverage. Add a focused parametrized unit test for _env asserting
that None, an empty string, and whitespace-only values return the default while
a real value is preserved.
🪄 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 Plus

Run ID: bf4c7766-0f2a-42d5-81a8-cb357a8a64af

📥 Commits

Reviewing files that changed from the base of the PR and between ff4d7f7 and c5498dc.

📒 Files selected for processing (11)
  • .env.example
  • apps/agent/.env.example
  • apps/agent/main.py
  • apps/agent/models.py
  • apps/agent/session_cap.py
  • apps/agent/test_session_cap.py
  • apps/web/app/api/interviews/[id]/end/route.ts
  • packages/evals/src/grade.ts
  • packages/evals/src/live.ts
  • packages/evals/src/suite.ts
  • packages/shared/src/models.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/shared/src/models.ts
  • apps/agent/main.py

Comment thread apps/agent/session_cap.py Outdated
Comment thread packages/evals/src/grade.ts
Comment thread packages/evals/src/live.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@packages/shared/src/models.ts`:
- Around line 53-59: Revise the timing comment near the 32.8s value to describe
it only as the observed maximum from the 15 free-schema fixture runs. Remove the
claim that it is a Pro-path floor, and state that Pro evaluations and longer
transcripts require separate tail-latency measurements.
🪄 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 Plus

Run ID: 54730167-411b-4dff-92e3-d6e55595ffe7

📥 Commits

Reviewing files that changed from the base of the PR and between c5498dc and 9897662.

📒 Files selected for processing (1)
  • packages/shared/src/models.ts

Comment thread packages/shared/src/models.ts
CodeRabbit caught _finalize() being the one unguarded await in the cap teardown. Its
stated mechanism was half right — _publish already swallows exceptions — but the hole
is real and bigger than reported: _finalize does a DB UPDATE and a room publish, and
either can HANG, which try/except doesn't help with. aclose() had the same hole. Three
steps, same shape: an unbounded await on any of them skips the ones below, and the
ones below are what stop the billing.

One _guarded() helper: bound each step, log, keep going. Tests cover finalize raising,
finalize hanging, and aclose hanging — in every case the room still gets deleted.
…ut grading

Retry only covered non-2xx statuses, so the things that really go wrong — a dropped
socket, our own TIMEOUT_MS abort, a truncated body that won't JSON.parse — escaped on
the first attempt and never spent the retry budget. They all route through the backoff
now; only a permanent 4xx skips it (retrying a 400 just fails slower).

--runs=thre made Number() return NaN, Math.max(1, NaN) return NaN, and the loop run
zero times — so results.every() was vacuously true and the gate printed 'passes all
checks' having graded nothing. Validate the flag, and refuse to exit 0 on an empty run.

The stability table now includes overallScore and the rubric dims — the numbers the
candidate actually reads, which is what this whole exercise was supposed to check.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/agent/test_session_cap.py (1)

108-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make failure-path tests prove the injected failure occurred.

These tests only assert downstream cleanup. A regression that skips _finalize or aclose but still deletes the room would pass. Add assert agent.finalized == ["time cap"] to the finalize tests and assert not session.closed to the close-hang test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/agent/test_session_cap.py` around lines 108 - 127, Strengthen the
failure-path assertions in test_finalize_raises_but_the_meter_still_stops and
test_finalize_hangs_but_the_meter_still_stops by asserting agent.finalized
equals ["time cap"]. In test_session_close_hangs_but_the_room_is_still_deleted,
assert session.closed is false while preserving the existing room-deletion
assertion.
🤖 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.

Nitpick comments:
In `@apps/agent/test_session_cap.py`:
- Around line 108-127: Strengthen the failure-path assertions in
test_finalize_raises_but_the_meter_still_stops and
test_finalize_hangs_but_the_meter_still_stops by asserting agent.finalized
equals ["time cap"]. In test_session_close_hangs_but_the_room_is_still_deleted,
assert session.closed is false while preserving the existing room-deletion
assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f01ed136-8675-46d3-a990-4ac86c46be30

📥 Commits

Reviewing files that changed from the base of the PR and between 9897662 and 92e6cf8.

📒 Files selected for processing (5)
  • apps/agent/session_cap.py
  • apps/agent/test_session_cap.py
  • packages/evals/src/grade.ts
  • packages/evals/src/live.ts
  • packages/evals/src/suite.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/evals/src/grade.ts
  • apps/agent/session_cap.py
  • packages/evals/src/live.ts

The agent watchdog fired /api/interviews/:id/end without awaiting it and then
told the candidate the interview "doesn't count against your monthly
interviews". Billing charges at interview START, so on a failed request that
sentence is a lie: the row stays `live`, the candidate stays charged, and we've
just told them they weren't. Await the call, check res.ok, and only make the
claim when the reset actually landed.

Leave the room before showing the failure, too. Nothing was disconnecting it, so
an agent that turned up late would start talking over the error screen — and an
abandoned room keeps burning LiveKit minutes. The watchdog now owns the terminal
state so its own disconnect isn't misread as the candidate dropping out.
The clamp warning quoted ROUTE_LIMIT_MS while clamping to CEILING_MS, so
SCORER_TIMEOUT_MS=55000 got told it "exceeds the 60000ms route limit" — which it
does not. Quote the ceiling it was actually measured against.

Also stop calling 32.8s a floor for the Pro path. It's the observed max of 15
FREE-schema runs and bounds nothing on a path that emits a studyPlan on top. Pro
tail latency has never been measured; say so instead of implying otherwise.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details
putComment timed out

@sparsh-j01
sparsh-j01 merged commit bbdb6d3 into main Jul 14, 2026
4 checks passed
@sparsh-j01
sparsh-j01 deleted the fix/launch-blockers branch July 14, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant