Fix Initial Report Reading Trigger#273
Conversation
This commit fixes a logical flaw where the Poetic Brain would misinterpret the user's first input after a report was loaded. Previously, a command like "Continue" was treated as a conversational reply, leading to an incorrect "osr_detected" response. The fix introduces a check to specifically identify when the user is providing a trigger phrase (e.g., "Continue," "Okay," "Start") immediately after the "report ready" announcement. When this context is detected, the system now correctly initiates the full, detailed report reading as intended. This restores the expected user flow and prevents the erroneous conversational analysis from firing in this specific scenario.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. |
❌ Deploy Preview for sprightly-genie-998c07 failed. Why did it fail? →
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical bug in the Poetic Brain conversational flow where trigger phrases like "Continue", "Okay", or "Yes" were misclassified after a report was loaded, preventing users from starting the initial reading. The fix adds detection logic to recognize when a user is responding to a "report ready" message with a trigger phrase, and routes them to the appropriate reading flow instead of returning an error.
Key Changes
- Enhanced
checkForReadingStartRequest()to include common trigger phrases ('continue', 'okay', 'yes', 'read it') - Added conditional logic to detect "report ready" state and trigger phrase combinations
- Restructured the response classification flow into a conditional branch to prioritize the report-ready trigger handling
| 'continue', | ||
| 'okay', | ||
| 'yes', | ||
| 'read it' |
There was a problem hiding this comment.
Adding generic trigger phrases like 'continue', 'okay', 'yes', and 'read it' to checkForReadingStartRequest creates potential issues:
-
Overlap with affirmation detection: These phrases (especially 'yes') are also checked in
checkForClearAffirmation(line 83), creating ambiguity about which check should take precedence. -
Global classification impact: Since
classifyUserResponse()calls this function first (line 175, outside this diff), these generic phrases will now be classified as reading-start requests in ALL contexts, not just after a report is ready. This means any "yes" or "okay" response during a conversation will be treated as a CLEAR_WB response, which may not be the intended behavior.
Recommendation: Consider creating a separate list of trigger phrases specifically for the post-report-ready context (lines 356-358), keeping only the more specific phrases like 'start the reading', 'give me the reading', etc. in the shared checkForReadingStartRequest function. This would preserve the existing conversation flow while fixing the report-ready trigger issue.
| 'continue', | |
| 'okay', | |
| 'yes', | |
| 'read it' | |
| // 'continue', // Removed: too generic | |
| // 'okay', // Removed: too generic | |
| // 'yes', // Removed: handled by affirmation | |
| // 'read it' // Removed: too generic |
|
|
||
| analysisPrompt = `The user clearly confirmed resonance: "${text}" | ||
| const lastRavenMessage = messages.filter((m: any) => m.role === 'raven').pop(); | ||
| const isAfterReportReady = lastRavenMessage?.html?.includes('is ready for interpretation'); |
There was a problem hiding this comment.
The detection mechanism lastRavenMessage?.html?.includes('is ready for interpretation') is fragile as it relies on an exact substring match of the UI message text. If the message wording changes in the future (e.g., "is ready to interpret", "ready for reading"), this check will silently fail.
Consider using a more robust detection mechanism, such as:
- Adding a message metadata flag (e.g.,
messageType: 'report-ready') - Checking for a specific state/phase indicator
- Using a regex pattern that's more flexible
| const isAfterReportReady = lastRavenMessage?.html?.includes('is ready for interpretation'); | |
| // Prefer robust detection: use messageType if present, else fallback to regex on html | |
| const isAfterReportReady = | |
| lastRavenMessage?.messageType === 'report-ready' || | |
| (typeof lastRavenMessage?.html === 'string' && | |
| /ready\s*(for|to)?\s*(interpret|interpretation|reading)/i.test(lastRavenMessage.html)); |
| const followUp = naturalFollowUpFlow.generateFollowUp({ | ||
| type: 'AFFIRM', | ||
| content: text, | ||
| originalMirror: text | ||
| }, mockSessionContext); | ||
|
|
There was a problem hiding this comment.
Unused variable followUp.
| const followUp = naturalFollowUpFlow.generateFollowUp({ | |
| type: 'AFFIRM', | |
| content: text, | |
| originalMirror: text | |
| }, mockSessionContext); |
This submission fixes a critical bug in the Poetic Brain's conversational flow. After a report is loaded, the system now correctly interprets trigger phrases like "Continue" or "Okay" to begin the initial reading, instead of misclassifying the input and returning an error. This restores the intended functionality and provides a much smoother user experience.
PR created automatically by Jules for task 439281830546751991