Merged
Conversation
- Real time interview constraint changes - Interviewer changes the constraints after when 30-50% of the interview is done
feat: Real time constraints
✅ Deploy Preview for system-craft-staging ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces a "Live Constraint Changes" feature that dynamically triggers design constraints during interviews. It adds domain models for constraint metadata, backend logic to generate and persist constraint changes during hint requests, extends the reasoning evaluator to assess candidate adaptation to constraints, and updates UI components to display both constraint information and adaptation analysis. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client / Frontend
participant HintAPI as Hint API Route
participant SessionDB as Session DB
participant Evaluator as Reasoning Evaluator
participant Hook as useInterviewAI Hook
Client->>HintAPI: POST /hint (with current progress)
HintAPI->>HintAPI: shouldTriggerConstraintChange() → true
HintAPI->>HintAPI: generateConstraintChange()
HintAPI->>SessionDB: session.constraintChanges.push(newConstraint)
HintAPI->>SessionDB: await session.save()
HintAPI->>Client: { constraintChange: {...}, messages: [...] }
Client->>Hook: Response received
Hook->>Hook: onConstraintChange(constraintChange)
Hook->>Client: Update local session.constraintChanges
Client->>Client: Render updated constraint display
Note over Client,Evaluator: Later: Interview evaluation
Client->>Evaluator: evaluateReasoning(question, snapshot, results, constraintChanges)
Evaluator->>Evaluator: Include constraints in prompt
Evaluator->>Evaluator: Analyze adaptation response
Evaluator->>Client: { adaptationSummary, addressedConstraintChanges, missedConstraintChanges }
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/interview/[id]/evaluate/route.ts (1)
82-90:⚠️ Potential issue | 🟠 MajorMissing constraint adaptation fields in final evaluation document.
The
evaluateReasoningfunction returnsadaptationSummary,addressedConstraintChanges, andmissedConstraintChanges, butcombineEvaluationsinsrc/lib/evaluation/scoringEngine.tsdoes not include these fields in thereasoningobject. This causes the constraint adaptation data to be discarded before persistence, and the result page will always display fallback text ("no adaptation summary was generated") regardless of whether the AI generated adaptation feedback.Update
combineEvaluationsto include the missing fields:Fix
reasoning: { score: reasoning.score, strengths: reasoning.strengths, weaknesses: reasoning.weaknesses, suggestions: reasoning.suggestions, adaptationSummary: reasoning.adaptationSummary, addressedConstraintChanges: reasoning.addressedConstraintChanges, missedConstraintChanges: reasoning.missedConstraintChanges },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/interview/`[id]/evaluate/route.ts around lines 82 - 90, The final evaluation omits constraint-adaptation fields returned by evaluateReasoning, so update combineEvaluations (in scoringEngine.ts) to include adaptationSummary, addressedConstraintChanges, and missedConstraintChanges inside the returned reasoning object (alongside score, strengths, weaknesses, suggestions) so those values are persisted and shown; locate the combineEvaluations function and add the three fields (adaptationSummary, addressedConstraintChanges, missedConstraintChanges) to the reasoning property of the combined evaluation object.
🧹 Nitpick comments (3)
src/hooks/useInterviewAI.ts (1)
42-46: Potential message state reset on re-fetch.This
useEffectresetsmessageswheneverinitialMessageschanges and has content. If the parent component re-fetches session data (e.g., after a network reconnection), this could overwrite locally-added messages that haven't been persisted yet.Consider comparing array contents rather than just length, or removing this effect if initial sync is only needed on mount:
♻️ Option 1: Remove effect if initial sync happens via useState initializer
The initial state is already set via
useState<AIMessage[]>(() => initialMessages). If the intent is only to initialize once, this effect may be unnecessary.- useEffect(() => { - if (initialMessages.length > 0) { - setMessages(initialMessages); - } - }, [initialMessages]);♻️ Option 2: Track if initial load completed to avoid resetting user messages
+ const hasInitializedRef = useRef(false); + useEffect(() => { - if (initialMessages.length > 0) { + if (!hasInitializedRef.current && initialMessages.length > 0) { setMessages(initialMessages); + hasInitializedRef.current = true; } }, [initialMessages]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useInterviewAI.ts` around lines 42 - 46, The useEffect that resets messages when initialMessages changes can overwrite local, unsaved messages; remove this effect or guard it to run only once on mount: stop using the effect that references initialMessages and setMessages, and instead rely on the useState initializer useState<AIMessage[]>(() => initialMessages) for the initial sync, or add a boolean flag (e.g., hasInitialized) checked/updated by the effect so useEffect(() => { if (!hasInitialized) { setMessages(initialMessages); setHasInitialized(true); } }, [initialMessages]) runs only the first time; update references to messages, setMessages, and initialMessages accordingly.app/interview/[id]/page.tsx (1)
86-98: Consider memoizing theonConstraintChangecallback.The
onConstraintChangecallback is defined inline, creating a new function reference on every render. This could trigger unnecessary re-invocations of theuseEffectinuseInterviewAIthat depends ononConstraintChange(viarequestHint).♻️ Proposed fix using useCallback
+ const handleConstraintChange = useCallback((change: IConstraintChange) => { + setSession(prev => { + if (!prev) return prev; + const existing = prev.constraintChanges || []; + if (existing.some(item => item.id === change.id)) { + return prev; + } + return { + ...prev, + constraintChanges: [...existing, change] + }; + }); + }, []); // AI Interviewer hook const ai = useInterviewAI({ sessionId: id, stateRef: canvasStateRef, timeRemaining: timer.minutes, initialMessages: session?.aiMessages || [], - onConstraintChange: (change) => { - setSession(prev => { - if (!prev) return prev; - const existing = prev.constraintChanges || []; - if (existing.some(item => item.id === change.id)) { - return prev; - } - return { - ...prev, - constraintChanges: [...existing, change] - }; - }); - } + onConstraintChange: handleConstraintChange });🤖 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 86 - 98, The inline onConstraintChange callback recreates a new function each render and can cause downstream effects (like useInterviewAI/useEffect that depends on requestHint) to re-run; wrap this handler in React.useCallback to memoize it (e.g., memoize the function that calls setSession and checks/updates constraintChanges) and include only necessary stable dependencies (setSession is stable so the deps can be [] or [setSession] depending on your hooks setup). Ensure you replace the inline definition passed to the component with the memoized callback name so its identity remains stable across renders and update any dependent effects if required.src/lib/db/models/InterviewSession.ts (1)
124-146: Consider adding validation bounds forintroducedAtMinute.The schema is well-structured with proper enum validation. One minor improvement:
introducedAtMinutecould benefit from a minimum bound to prevent invalid negative values.🛡️ Optional: Add min validation
- introducedAtMinute: { type: Number, required: true }, + introducedAtMinute: { type: Number, required: true, min: 0 },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/db/models/InterviewSession.ts` around lines 124 - 146, The introducedAtMinute field in ConstraintChangeSchema allows invalid negative (and out-of-range) values; update the introducedAtMinute definition in ConstraintChangeSchema to include numeric validation bounds (e.g., add min: 0 and max: 59) so minutes are constrained to valid minute values, and ensure required: true remains set; adjust any related type/interface if present to reflect these bounds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/api/interview/`[id]/evaluate/route.ts:
- Around line 82-90: The final evaluation omits constraint-adaptation fields
returned by evaluateReasoning, so update combineEvaluations (in
scoringEngine.ts) to include adaptationSummary, addressedConstraintChanges, and
missedConstraintChanges inside the returned reasoning object (alongside score,
strengths, weaknesses, suggestions) so those values are persisted and shown;
locate the combineEvaluations function and add the three fields
(adaptationSummary, addressedConstraintChanges, missedConstraintChanges) to the
reasoning property of the combined evaluation object.
---
Nitpick comments:
In `@app/interview/`[id]/page.tsx:
- Around line 86-98: The inline onConstraintChange callback recreates a new
function each render and can cause downstream effects (like
useInterviewAI/useEffect that depends on requestHint) to re-run; wrap this
handler in React.useCallback to memoize it (e.g., memoize the function that
calls setSession and checks/updates constraintChanges) and include only
necessary stable dependencies (setSession is stable so the deps can be [] or
[setSession] depending on your hooks setup). Ensure you replace the inline
definition passed to the component with the memoized callback name so its
identity remains stable across renders and update any dependent effects if
required.
In `@src/hooks/useInterviewAI.ts`:
- Around line 42-46: The useEffect that resets messages when initialMessages
changes can overwrite local, unsaved messages; remove this effect or guard it to
run only once on mount: stop using the effect that references initialMessages
and setMessages, and instead rely on the useState initializer
useState<AIMessage[]>(() => initialMessages) for the initial sync, or add a
boolean flag (e.g., hasInitialized) checked/updated by the effect so
useEffect(() => { if (!hasInitialized) { setMessages(initialMessages);
setHasInitialized(true); } }, [initialMessages]) runs only the first time;
update references to messages, setMessages, and initialMessages accordingly.
In `@src/lib/db/models/InterviewSession.ts`:
- Around line 124-146: The introducedAtMinute field in ConstraintChangeSchema
allows invalid negative (and out-of-range) values; update the introducedAtMinute
definition in ConstraintChangeSchema to include numeric validation bounds (e.g.,
add min: 0 and max: 59) so minutes are constrained to valid minute values, and
ensure required: true remains set; adjust any related type/interface if present
to reflect these bounds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: beccf8c2-df86-4337-bce3-6b2d0349b2b1
📒 Files selected for processing (9)
app/api/interview/[id]/evaluate/route.tsapp/api/interview/[id]/hint/route.tsapp/api/interview/[id]/route.tsapp/interview/[id]/page.tsxapp/interview/[id]/result/page.tsxcomponents/interview/QuestionPanel.tsxsrc/hooks/useInterviewAI.tssrc/lib/db/models/InterviewSession.tssrc/lib/evaluation/reasoningEvaluator.ts
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