Merged
Conversation
…ic Server/Database nodes when throwing Regional Failover events - The UI will instantly trap the user with a pulsing red 5-Minute Panic Timer, forcing them to either mitigate the problem before the clock runs out or hit submit - The Canvas will vividly display the broken node (grayed out, marked with a pulsing red warning icon) rendering it dead
Feature/chaos
- Implemented graph traversal for realtime load simulation. - Added UI widget to canvas with variable RPS slider. - Integrated deterministic graph bottlenecks with SVG marching ants and warning badges. - Created Backend API and frontend trigger for 10-minute 'Final Validation Phase'. - Fixed React hooks ordering rules violation with . - Automatically open AI Interviewer sidebar during chaos events and validation.
feat(simulation): add dynamic load engine and final validation phase
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for system-craft-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds server APIs for constraint timeouts and final-validation, a client-side chaos timer, a simulation engine with UI controls integrated into the design canvas, and extends InterviewSession constraint schema with per-constraint timing and impacted-node metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Interview Page
participant API as Chaos Timeout API
participant DB as MongoDB
participant UI as Interview UI
Client->>API: POST /api/interview/[id]/chaos-timeout {type, constraintId}
API->>DB: Load InterviewSession by id & user
alt constraint found & actionable
API->>DB: Set `overtimeAt` or `failedAt`, set status, append interviewer message
API->>DB: Save session
DB-->>API: Updated session
API-->>Client: {success, messages, constraintChanges}
Client->>UI: setMessages(...) & update constraintChanges
UI->>UI: Re-render header/canvas (open interviewer panel)
else not found / already processed
API-->>Client: {error or success w/warning}
end
sequenceDiagram
participant Timer as Page Timer
participant Client as Interview Page
participant API as Final Validation API
participant DB as MongoDB
Timer->>Client: detects minutes ≤ 10 & status='in_progress'
Client->>API: POST /api/interview/[id]/final-validation
API->>DB: Load session, check existing "Final Validation Phase" message
alt already sent or not in_progress
API-->>Client: {success, messages}
else first time
API->>DB: Append interviewer message, save
DB-->>API: Updated session
API-->>Client: {success, messages}
Client->>Client: ai.setMessages(...), open interviewer panel
end
sequenceDiagram
participant UI as SimulationControls
participant Canvas as DesignCanvas
participant Hook as useSimulationEngine
participant Engine as runSimulation
UI->>Canvas: onToggle(true), onChangeRps(500)
Canvas->>Hook: isRunning=true, targetRps=500, nodes, edges
Hook->>Engine: runSimulation(nodes, edges, 500)
Engine-->>Hook: SimulationResult (nodeMetrics, edgeMetrics, globalStatus)
Hook-->>Canvas: SimulationResult
Canvas->>Canvas: Render animated edges, node badges, state classes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
src/hooks/useInterviewAI.ts (1)
152-154: Consider documenting the intended usage ofsetMessages.Exposing the raw
setMessagessetter allows external callers to bypass the hook's internal message management logic (e.g.,requestHint). This is intentional for syncing with server-returned message arrays from/final-validationand/chaos-timeoutendpoints, but consider adding a brief comment or JSDoc to clarify this is for server-sync scenarios only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useInterviewAI.ts` around lines 152 - 154, Add a short JSDoc/comment on the exported setMessages in the useInterviewAI hook to document its intended usage: explain that setMessages is exposed only for server-sync scenarios (e.g., syncing arrays returned from /final-validation and /chaos-timeout) and should not be used to bypass the hook's internal message-management functions like requestHint or sendReply; place the comment adjacent to the return object that includes sendReply and setMessages so callers see it when importing the hook.app/api/interview/[id]/hint/route.ts (1)
71-80: Node failure victim selection excludes valid infrastructure components.The filter only includes
['SQL', 'Blob', 'Cache', 'Server'], but the system supports 10 node types (perComponentPalette.tsxandconstants.ts). Notable omissions:
- LB (Load Balancer) - common chaos scenario
- Queue/Kafka - tests async resilience
- Function - serverless failure modes
- CDN - edge availability
This limits the realism of reliability chaos scenarios.
♻️ Consider expanding the victim pool
- const candidateNodes = nodes.filter((n) => ['SQL', 'Blob', 'Cache', 'Server'].includes(n.type)); + // Exclude Client (load generator) from failure candidates + const candidateNodes = nodes.filter((n) => n.type !== 'Client');Or explicitly include all infrastructure types:
- const candidateNodes = nodes.filter((n) => ['SQL', 'Blob', 'Cache', 'Server'].includes(n.type)); + const candidateNodes = nodes.filter((n) => + ['SQL', 'Blob', 'Cache', 'Server', 'LB', 'CDN', 'Queue', 'Kafka', 'Function'].includes(n.type) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/interview/`[id]/hint/route.ts around lines 71 - 80, The victim-selection for chaos scenarios restricts candidateNodes to ['SQL','Blob','Cache','Server'], excluding valid infrastructure types like LB, Queue/Kafka, Function, CDN; update the filter in the block that builds candidateNodes (and the subsequent victim and selected objects) to either use the canonical infrastructure type list from the shared source (e.g., ComponentPalette / constants export) or expand the hardcoded array to include LB, Queue/Kafka, Function, CDN (and any other types present in constants.ts), and add a guard to handle empty candidateNodes before computing Math.random() so victim may be undefined safely.app/interview/[id]/page.tsx (1)
305-321: Dependency array includes unstableaiobject reference.Including
aiin the dependency array causeshandleChaosTimeoutto be recreated on every render sinceuseInterviewAIreturns a new object each time. This could cause unnecessary re-renders of child components receiving this callback.♻️ Proposed fix - use stable reference
const handleChaosTimeout = useCallback(async (type: 'warning' | 'penalty', constraintId: string) => { try { const res = await authFetch(`/api/interview/${id}/chaos-timeout`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ type, constraintId }) }); + if (!res.ok) throw new Error('Chaos timeout request failed'); const data = await res.json(); if (data.success && data.messages) { if (ai.setMessages) ai.setMessages(data.messages); setSession(prev => prev ? { ...prev, constraintChanges: data.constraintChanges } : null); setIsInterviewPanelOpen(true); } } catch (err) { console.error('Chaos timeout failed:', err); } - }, [id, ai]); + // eslint-disable-next-line react-hooks/exhaustive-deps -- ai.setMessages is stable from useState + }, [id]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/interview/`[id]/page.tsx around lines 305 - 321, The handleChaosTimeout callback depends on the unstable ai object returned by useInterviewAI, causing the callback to be recreated each render; fix by extracting the stable function you need (e.g., const { setMessages } = ai) before defining handleChaosTimeout and use that stable reference inside the callback, then update the dependency array to [id, setMessages] (or if setMessages can be undefined handle that case) so handleChaosTimeout is stable; reference symbols: handleChaosTimeout, useInterviewAI, ai, setMessages, id.src/lib/simulation/engine.ts (1)
75-82: Minor optimization opportunity in source lookup.The
sources.find()call on line 78 runs for every node on every pass, resulting in O(passes × nodes × sources) complexity. For typical small UI graphs this is fine, but if performance becomes a concern, consider using aSetfor O(1) lookups.💡 Optional optimization
+ const sourceIds = new Set(sources.map(s => s.id)); + for (let pass = 0; pass < 10; pass++) { edges.forEach(e => { edgeMetrics[e.id].trafficFlow = 0; }); const currentTrafficIn = Object.keys(nodeMetrics).reduce((acc, id) => { acc[id] = nodeMetrics[id].trafficIn; - if (!sources.find(s => s.id === id)) { + if (!sourceIds.has(id)) { nodeMetrics[id].trafficIn = 0; } return acc; }, {} as Record<string, number>);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/simulation/engine.ts` around lines 75 - 82, The reduce over nodeMetrics repeatedly calls sources.find(...) causing O(nodes×sources) per pass; create a Set of source ids once (e.g., const sourceIds = new Set(sources.map(s => s.id))) before computing currentTrafficIn and replace the per-node lookup sources.find(s => s.id === id) with sourceIds.has(id) so you get O(1) membership checks while preserving the existing behavior in the currentTrafficIn reduce and the conditional that clears nodeMetrics[id].trafficIn for non-sources.components/canvas/SimulationControls.tsx (1)
38-47: Consider allowing RPS adjustment before starting simulation.The range slider is disabled when
isRunningisfalse(line 45), which prevents users from configuring the target RPS before starting the simulation. This seems counterintuitive — users typically want to set their target load before running the test.If this is intentional (to prevent confusion about when changes take effect), consider adding a visual hint or tooltip explaining that RPS can only be adjusted while running.
💡 Proposed fix to allow pre-configuration
value={targetRps} onChange={(e) => onChangeRps(parseInt(e.target.value, 10))} - disabled={!isRunning} + disabled={false} className="w-full h-2 rounded-lg appearance-none cursor-pointer bg-slate-200 dark:bg-slate-700 accent-primary focus:outline-none focus:ring-2 focus:ring-primary/50 disabled:opacity-50 disabled:cursor-not-allowed"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/canvas/SimulationControls.tsx` around lines 38 - 47, The range slider is currently disabled when isRunning is false which blocks pre-configuration; update the SimulationControls input (the range element bound to targetRps and onChangeRps) so the slider is enabled before starting by removing or changing the disabled condition (remove disabled={!isRunning} or change the predicate to only disable when truly needed), and ensure onChangeRps can accept changes while not running (or guard inside onChangeRps if needed) so users can set targetRps before starting the simulation; optionally, if you want to preserve the original behavior, add a tooltip/visual hint near the slider explaining that changes only take effect when the simulation is running.app/api/interview/[id]/chaos-timeout/route.ts (1)
46-56: Penalty handling leaves constraint in 'active' status permanently.The comment clarifies this is intentional to keep the node visually broken. However, since
statusremains'active'and is never set to'addressed', theChaosTimerwill display "Unresolved Incident" indefinitely per the check inInterviewHeader.tsx(line 120-122:latestConstraint.status !== 'addressed').If this is by design for evaluation purposes, consider adding a way to dismiss or conclude the constraint after the interview, or document this behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/interview/`[id]/chaos-timeout/route.ts around lines 46 - 56, The penalty branch currently sets constraint.failedAt but leaves constraint.status as 'active', causing ChaosTimer/InterviewHeader (check latestConstraint.status !== 'addressed') to show "Unresolved Incident" forever; update the penalty handling in route.ts to mark the constraint as concluded—set constraint.status = 'addressed' (or add and set a new flag like constraint.dismissed = true plus a resolution reason field) and persist those changes via session.markModified('constraintChanges') so the UI stops treating it as unresolved, or alternatively implement a dismiss/conclude endpoint that flips that same status/flag after the interview ends; reference constraint.failedAt, constraint.status, session.markModified('constraintChanges'), and the UI check in InterviewHeader.tsx to guide placement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/interview/`[id]/chaos-timeout/route.ts:
- Line 22: The findIndex callback currently types parameter as any; change the
callback to use the IConstraintChange type (e.g.,
constraintChanges.findIndex((c: IConstraintChange) => c.id === constraintId))
and ensure IConstraintChange is imported from the model where it’s exported (add
the import if missing) so the `@typescript-eslint/no-explicit-any` error is
resolved.
In `@app/api/interview/`[id]/final-validation/route.ts:
- Around line 21-23: The code uses an unsafe any for messages when checking
session.aiMessages in the alreadySent computation; replace the any with the
appropriate message interface (e.g., whatever type defines AI messages in your
schema such as AiMessage, ChatMessage, or MessageRecord) and import or reference
that type, then change the callback signature to (m: AiMessage) => m.role ===
'interviewer' && m.content.includes("Final Validation Phase") so static analysis
understands the properties used; update the session type if necessary so
session.aiMessages is typed as an array of that message interface.
- Around line 5-8: The POST handler (function POST) lacks the authentication and
session-ownership checks present in hint/route.ts; add the same authentication
flow used there: extract and validate the session/user (the helper(s) used in
hint/route.ts that verify authenticated user and session ownership), confirm the
requesting user is the session owner for the interview ID, and return the
appropriate 401/403 response if validation fails; only proceed to run the final
validation logic when the authenticated user check passes.
- Around line 13-16: The query uses a non-queryable virtual `id` field so
InterviewSession.findOne({ id }) always fails; update the lookup to query by the
real Mongo identifier and user ownership—replace the call to
InterviewSession.findOne in route.ts with a query that uses {_id: id, userId:
user._id} (or use findById with an explicit userId check) so the document can be
found the same way other routes (hint/route.ts and route.ts) do.
In `@app/interview/`[id]/page.tsx:
- Around line 105-119: Extract the stable setter from the ai object (e.g., const
{ setMessages } = ai) before the useEffect and use setMessages in the dependency
array instead of ai or ai.setMessages to satisfy the linter without causing
re-renders; inside the effect, after calling authFetch check the fetch response
(if (!res.ok) throw new Error(await res.text() || res.statusText)) before
calling res.json(), and guard setMessages and setIsInterviewPanelOpen calls
(e.g., if (data.success && data.messages) { if (setMessages)
setMessages(data.messages); setIsInterviewPanelOpen(true); }) so errors are
caught and surfaced in the existing catch block.
In `@components/interview/ChaosTimer.tsx`:
- Around line 47-68: The reset of hasTriggeredWarning.current and
hasTriggeredPenalty.current inside the useEffect is inverted and unnecessary;
remove the two lines that set hasTriggeredWarning.current = false and
hasTriggeredPenalty.current = false so the component relies on the existing
trigger guards (!constraint.overtimeAt / !constraint.failedAt) and the refs to
prevent duplicate callbacks, and add a brief comment near the hasTriggered* refs
(or rename them to hasPendingWarningRequest/hasPendingPenaltyRequest if you
prefer clearer intent) to document their purpose; ensure calculateRemaining,
onTimeoutRef, and constraint usage remains unchanged.
- Around line 22-39: calculateRemaining references PENALTY_MS and WARNING_MS but
they are not listed in the useCallback dependency array; add PENALTY_MS and
WARNING_MS to the dependency array for calculateRemaining (or alternatively move
those constants outside the component so they are truly static) to satisfy
React's exhaustive-deps rule and silence the ESLint warning while keeping the
logic around constraint, penalty, and warning intact (symbols:
calculateRemaining, PENALTY_MS, WARNING_MS, constraint).
---
Nitpick comments:
In `@app/api/interview/`[id]/chaos-timeout/route.ts:
- Around line 46-56: The penalty branch currently sets constraint.failedAt but
leaves constraint.status as 'active', causing ChaosTimer/InterviewHeader (check
latestConstraint.status !== 'addressed') to show "Unresolved Incident" forever;
update the penalty handling in route.ts to mark the constraint as concluded—set
constraint.status = 'addressed' (or add and set a new flag like
constraint.dismissed = true plus a resolution reason field) and persist those
changes via session.markModified('constraintChanges') so the UI stops treating
it as unresolved, or alternatively implement a dismiss/conclude endpoint that
flips that same status/flag after the interview ends; reference
constraint.failedAt, constraint.status,
session.markModified('constraintChanges'), and the UI check in
InterviewHeader.tsx to guide placement.
In `@app/api/interview/`[id]/hint/route.ts:
- Around line 71-80: The victim-selection for chaos scenarios restricts
candidateNodes to ['SQL','Blob','Cache','Server'], excluding valid
infrastructure types like LB, Queue/Kafka, Function, CDN; update the filter in
the block that builds candidateNodes (and the subsequent victim and selected
objects) to either use the canonical infrastructure type list from the shared
source (e.g., ComponentPalette / constants export) or expand the hardcoded array
to include LB, Queue/Kafka, Function, CDN (and any other types present in
constants.ts), and add a guard to handle empty candidateNodes before computing
Math.random() so victim may be undefined safely.
In `@app/interview/`[id]/page.tsx:
- Around line 305-321: The handleChaosTimeout callback depends on the unstable
ai object returned by useInterviewAI, causing the callback to be recreated each
render; fix by extracting the stable function you need (e.g., const {
setMessages } = ai) before defining handleChaosTimeout and use that stable
reference inside the callback, then update the dependency array to [id,
setMessages] (or if setMessages can be undefined handle that case) so
handleChaosTimeout is stable; reference symbols: handleChaosTimeout,
useInterviewAI, ai, setMessages, id.
In `@components/canvas/SimulationControls.tsx`:
- Around line 38-47: The range slider is currently disabled when isRunning is
false which blocks pre-configuration; update the SimulationControls input (the
range element bound to targetRps and onChangeRps) so the slider is enabled
before starting by removing or changing the disabled condition (remove
disabled={!isRunning} or change the predicate to only disable when truly
needed), and ensure onChangeRps can accept changes while not running (or guard
inside onChangeRps if needed) so users can set targetRps before starting the
simulation; optionally, if you want to preserve the original behavior, add a
tooltip/visual hint near the slider explaining that changes only take effect
when the simulation is running.
In `@src/hooks/useInterviewAI.ts`:
- Around line 152-154: Add a short JSDoc/comment on the exported setMessages in
the useInterviewAI hook to document its intended usage: explain that setMessages
is exposed only for server-sync scenarios (e.g., syncing arrays returned from
/final-validation and /chaos-timeout) and should not be used to bypass the
hook's internal message-management functions like requestHint or sendReply;
place the comment adjacent to the return object that includes sendReply and
setMessages so callers see it when importing the hook.
In `@src/lib/simulation/engine.ts`:
- Around line 75-82: The reduce over nodeMetrics repeatedly calls
sources.find(...) causing O(nodes×sources) per pass; create a Set of source ids
once (e.g., const sourceIds = new Set(sources.map(s => s.id))) before computing
currentTrafficIn and replace the per-node lookup sources.find(s => s.id === id)
with sourceIds.has(id) so you get O(1) membership checks while preserving the
existing behavior in the currentTrafficIn reduce and the conditional that clears
nodeMetrics[id].trafficIn for non-sources.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bac0d99-aa20-4640-a12b-a67a79534142
📒 Files selected for processing (13)
app/api/interview/[id]/chaos-timeout/route.tsapp/api/interview/[id]/final-validation/route.tsapp/api/interview/[id]/hint/route.tsapp/interview/[id]/page.tsxcomponents/canvas/DesignCanvas.tsxcomponents/canvas/SimulationControls.tsxcomponents/interview/ChaosTimer.tsxcomponents/interview/InterviewHeader.tsxsrc/hooks/useInterviewAI.tssrc/hooks/useSimulationEngine.tssrc/lib/db/models/InterviewSession.tssrc/lib/simulation/constants.tssrc/lib/simulation/engine.ts
fix: Lint issues
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 7 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/canvas/DesignCanvas.tsx (1)
842-882:⚠️ Potential issue | 🟠 MajorImpacted nodes won't receive the mouse events this branch relies on.
Line 842 disables pointer events for the whole node layer. The normal node branch opts back in with
pointer-events-auto, but the impacted branch on Lines 858-866 does not, so itsonMouseDown/onMouseUphandlers never fire and the node cannot actually be selected.Suggested fix
- className={`absolute w-[60px] h-[60px] rounded-xl flex flex-col items-center justify-center select-none shadow-lg transition-all duration-300 ${isImpacted + className={`absolute w-[60px] h-[60px] rounded-xl flex flex-col items-center justify-center select-none shadow-lg transition-all duration-300 pointer-events-auto ${isImpacted ? 'bg-red-500/10 border-2 border-red-500/50 opacity-80 grayscale-[50%] cursor-not-allowed' : isBottlenecked ? 'bg-red-600 border-2 border-red-500 shadow-[0_0_20px_rgba(220,38,38,0.7)] text-white ring-2 ring-red-500 animate-pulse' : isWarning ? 'bg-amber-500/20 border-2 border-amber-500 text-amber-500 shadow-[0_0_15px_rgba(245,158,11,0.5)]' : 'bg-white dark:bg-[`#1e1e24`] cursor-move transition-shadow pointer-events-auto ' + (isSelected ? 'ring-2 ring-primary ring-offset-2 ring-offset-white dark:ring-offset-[`#0f1115`] shadow-[0_0_20px_rgba(71,37,244,0.3)] z-20' : 'border-2 border-transparent hover:border-primary') }`} onMouseDown={(e) => { if (isImpacted) { e.stopPropagation(); setSelectedNodeId(node.id); + setSelectedConnectionId(null); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/canvas/DesignCanvas.tsx` around lines 842 - 882, The node layer currently has pointer-events disabled globally which prevents the impacted branch from receiving mouse events; update the impacted branch in the className logic for displayNodes (where isImpacted is computed) to include "pointer-events-auto" (or remove the parent pointer-events-none and ensure non-impacted nodes keep pointer-events-auto) so that onMouseDown/onMouseUp handlers (handleNodeMouseDown, handleNodeMouseUp) and setSelectedNodeId can run for impacted nodes while preserving the visual disabled styling (e.g., keep cursor-not-allowed/grayscale).
♻️ Duplicate comments (1)
app/interview/[id]/page.tsx (1)
105-119:⚠️ Potential issue | 🟠 MajorDon't consume the one-shot final-validation trigger before the POST succeeds.
Line 107 is acting as both “request in flight” and “request completed”. On a transient 401/500/network failure, this phase is skipped for the rest of the session.
One way to fix it
+ const finalValidationInFlightRef = useRef(false); ... - if (!finalValidationTriggered && timer.minutes !== undefined && timer.minutes <= 10 && session?.status === 'in_progress') { - setFinalValidationTriggered(true); + if (!finalValidationTriggered && !finalValidationInFlightRef.current && timer.minutes !== undefined && timer.minutes <= 10 && session?.status === 'in_progress') { + finalValidationInFlightRef.current = true; authFetch(`/api/interview/${id}/final-validation`, { method: 'POST' }) - .then(res => res.json()) + .then(async res => { + if (!res.ok) throw new Error((await res.text()) || res.statusText); + return res.json(); + }) .then(data => { if (data.success && data.messages) { + setFinalValidationTriggered(true); if (ai.setMessages) ai.setMessages(data.messages); setIsInterviewPanelOpen(true); } }) - .catch(err => console.error('Final validation trigger failed:', err)); + .catch(err => console.error('Final validation trigger failed:', err)) + .finally(() => { finalValidationInFlightRef.current = false; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/interview/`[id]/page.tsx around lines 105 - 119, The effect sets finalValidationTriggered before the POST completes, so transient failures prevent retries; change the logic to only mark the trigger as completed after a successful POST (or introduce a separate in-flight flag). Specifically, in the useEffect that checks timer.minutes and session?.status, remove the early setFinalValidationTriggered(true) and instead set it only when the response is successful (e.g., inside the then where data.success is true), or add a new boolean state like finalValidationInFlight to guard duplicate requests while leaving finalValidationTriggered false until success; keep the authFetch call, ai.setMessages, and setIsInterviewPanelOpen usage the same and ensure catch leaves finalValidationTriggered unset so retries can occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/interview/`[id]/chaos-timeout/route.ts:
- Around line 29-31: Update the early-return condition in the route handler so
that any request short-circuits if constraint.failedAt is already set (not only
when type === 'penalty'); specifically, in the block that currently checks
constraint.status, type and overtimeAt/failedAt, add a top-level check for
constraint.failedAt (alongside constraint.status === 'addressed') and return the
same NextResponse.json({ success: true, warning: 'Already processed' }) for
warnings that arrive after a penalty has been recorded; reference the existing
symbols constraint, type and NextResponse.json in your change.
- Around line 15-18: The handler currently proceeds even if the interview is
closed; after fetching the session with InterviewSession.findOne({ id }) check
the session state and early-return when it is not "in_progress" (same guard used
in final-validation/route.ts) so the timeout update becomes a no-op for
closed/submitted sessions; update the code around the session null-check to
verify session.state === 'in_progress' and return a non-mutating response (e.g.,
JSON no-op or 409/200) to avoid appending interviewer messages or modifying
constraint timing after submit/evaluate.
---
Outside diff comments:
In `@components/canvas/DesignCanvas.tsx`:
- Around line 842-882: The node layer currently has pointer-events disabled
globally which prevents the impacted branch from receiving mouse events; update
the impacted branch in the className logic for displayNodes (where isImpacted is
computed) to include "pointer-events-auto" (or remove the parent
pointer-events-none and ensure non-impacted nodes keep pointer-events-auto) so
that onMouseDown/onMouseUp handlers (handleNodeMouseDown, handleNodeMouseUp) and
setSelectedNodeId can run for impacted nodes while preserving the visual
disabled styling (e.g., keep cursor-not-allowed/grayscale).
---
Duplicate comments:
In `@app/interview/`[id]/page.tsx:
- Around line 105-119: The effect sets finalValidationTriggered before the POST
completes, so transient failures prevent retries; change the logic to only mark
the trigger as completed after a successful POST (or introduce a separate
in-flight flag). Specifically, in the useEffect that checks timer.minutes and
session?.status, remove the early setFinalValidationTriggered(true) and instead
set it only when the response is successful (e.g., inside the then where
data.success is true), or add a new boolean state like finalValidationInFlight
to guard duplicate requests while leaving finalValidationTriggered false until
success; keep the authFetch call, ai.setMessages, and setIsInterviewPanelOpen
usage the same and ensure catch leaves finalValidationTriggered unset so retries
can occur.
🪄 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: 9f881bd0-adcc-4190-bee4-5f49b4eca944
📒 Files selected for processing (4)
app/api/interview/[id]/chaos-timeout/route.tsapp/api/interview/[id]/final-validation/route.tsapp/interview/[id]/page.tsxcomponents/canvas/DesignCanvas.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/interview/[id]/final-validation/route.ts
fix: Lint and build issues
fix: Lint and build issues
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
app/interview/[id]/page.tsx (1)
324-330:⚠️ Potential issue | 🟠 MajorHandle non-2xx chaos-timeout responses before parsing JSON.
This helper currently treats server-side failures as a silent no-op because it only checks
data.success. If the route returns 4xx/5xx, the UI never updates and the timeout looks like it already fired successfully.Suggested fix
const res = await authFetch(`/api/interview/${id}/chaos-timeout`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ type, constraintId }) }); + if (!res.ok) { + const errText = await res.text(); + throw new Error(errText || res.statusText); + } const data = await res.json(); if (data.success && data.messages) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/interview/`[id]/page.tsx around lines 324 - 330, The code calls authFetch and immediately does res.json() without checking HTTP status, so 4xx/5xx responses are treated like successful responses; update the handler around the authFetch call (the res variable returned from authFetch when posting to `/api/interview/${id}/chaos-timeout`) to first check res.ok (or res.status) and handle non-2xx responses (e.g., throw or read error text and surface an error state) before calling res.json(); ensure the UI update branch that currently looks at data.success only runs for successful HTTP responses and surface/log errors when res.ok is false so the timeout UI doesn’t silently appear to have fired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/interview/`[id]/chaos-timeout/route.ts:
- Around line 14-18: The route handler currently fetches the InterviewSession
with InterviewSession.findOne({ id }) and proceeds to update constraintChanges
and aiMessages without authenticating or verifying ownership; require the caller
to be authenticated, obtain the current user's id (e.g., from your auth helper
or request context), then scope the session lookup to include userId (e.g.,
InterviewSession.findOne({ id, userId })) and return a 403 if no session is
found for that user before performing any mutations to constraintChanges or
aiMessages; also ensure any downstream saves/updates (the code paths that modify
constraintChanges/aiMessages) only run after this ownership check.
- Around line 11-13: The handler reads body via req.json() and uses
type/constraintId without validation so invalid JSON throws 500 and unexpected
type values silently succeed; validate the parsed body immediately in the route
(after const { type, constraintId } = await req.json()) to ensure type is
present and equals 'warning' or 'penalty' and constraintId is the expected shape
(e.g., non-empty string/number), and if validation fails return a new Response
with status 400 and a clear error message; update the route's early-return logic
to reject malformed payloads before any branching that references type or
constraintId.
In `@components/canvas/DesignCanvas.tsx`:
- Around line 185-189: Add an effect that stops the simulation when the canvas
becomes read-only: watch the readOnly prop and, if readOnly is true and the
isSimulationRunning state is true, call setIsSimulationRunning(false) so
overlays/badges stop rendering; place this alongside the existing state hooks
(isSimulationRunning, setIsSimulationRunning, targetRps, useSimulationEngine)
and ensure it cleans up only by flipping the running flag (no additional side
effects).
In `@components/interview/ChaosTimer.tsx`:
- Around line 43-66: The refs hasPendingWarningRequest and
hasPendingPenaltyRequest persist across component lifetime so a prior constraint
can block callbacks for a new constraint; inside the useEffect that depends on
calculateRemaining and constraint (the one creating the interval), reset
hasPendingWarningRequest.current = false and hasPendingPenaltyRequest.current =
false (or reset based on constraint.id) before creating the interval so each new
active constraint starts with fresh dispatch guards; reference the useEffect
that calls calculateRemaining and onTimeoutRef.current to locate where to reset
them.
In `@src/lib/simulation/engine.ts`:
- Around line 69-114: The fixed 10-pass loop only snapshots currentTrafficIn
once per pass, so traffic moves one hop per pass and cycles amplify; replace
this with a true iterative propagation that runs until convergence (change <
epsilon) or a maxIterations cap: maintain two maps (e.g., prevTrafficIn and
nextTrafficIn) or update nodeMetrics.trafficIn incrementally each sub-iteration,
compute trafficOut from nodeMetrics.capacity, distribute to children via adj and
edgeMetrics, then swap and measure total delta across nodeMetrics to stop when
stable; ensure cycles are stabilized by using the delta-based convergence check
and a sensible epsilon/maxIterations to avoid runaway loops.
---
Duplicate comments:
In `@app/interview/`[id]/page.tsx:
- Around line 324-330: The code calls authFetch and immediately does res.json()
without checking HTTP status, so 4xx/5xx responses are treated like successful
responses; update the handler around the authFetch call (the res variable
returned from authFetch when posting to `/api/interview/${id}/chaos-timeout`) to
first check res.ok (or res.status) and handle non-2xx responses (e.g., throw or
read error text and surface an error state) before calling res.json(); ensure
the UI update branch that currently looks at data.success only runs for
successful HTTP responses and surface/log errors when res.ok is false so the
timeout UI doesn’t silently appear to have fired.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93c31900-9e9b-4b63-b443-0c7028d99a88
📒 Files selected for processing (10)
app/api/interview/[id]/chaos-timeout/route.tsapp/api/interview/[id]/final-validation/route.tsapp/api/interview/[id]/hint/route.tsapp/interview/[id]/page.tsxcomponents/canvas/DesignCanvas.tsxcomponents/canvas/SimulationControls.tsxcomponents/interview/ChaosTimer.tsxsrc/hooks/useInterviewAI.tssrc/hooks/useSimulationEngine.tssrc/lib/simulation/engine.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- app/api/interview/[id]/hint/route.ts
- src/hooks/useInterviewAI.ts
- src/hooks/useSimulationEngine.ts
- components/canvas/SimulationControls.tsx
- app/api/interview/[id]/final-validation/route.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 5 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
Fixed 4 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <[email protected]>
…0bfc16 fix: CodeRabbit auto-fixes for PR #98
fix: lint and build errors
Merged
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.
Summary by CodeRabbit
New Features
UI/UX Improvements