-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Initial Report Reading Trigger #273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -87,7 +87,7 @@ function checkForClearAffirmation(text: string): boolean { | |||||||||||||
|
|
||||||||||||||
| // Check if user is requesting to start/continue the reading (not OSR) | ||||||||||||||
| function checkForReadingStartRequest(text: string): boolean { | ||||||||||||||
| const lower = text.toLowerCase(); | ||||||||||||||
| const lower = text.toLowerCase().trim(); | ||||||||||||||
| const startReadingPhrases = [ | ||||||||||||||
| 'give me the reading', | ||||||||||||||
| 'start the reading', | ||||||||||||||
|
|
@@ -107,9 +107,13 @@ function checkForReadingStartRequest(text: string): boolean { | |||||||||||||
| 'let\'s start', | ||||||||||||||
| 'please continue', | ||||||||||||||
| 'go ahead', | ||||||||||||||
| 'proceed' | ||||||||||||||
| 'proceed', | ||||||||||||||
| 'continue', | ||||||||||||||
| 'okay', | ||||||||||||||
| 'yes', | ||||||||||||||
| 'read it' | ||||||||||||||
| ]; | ||||||||||||||
|
|
||||||||||||||
| return startReadingPhrases.some(phrase => lower.includes(phrase)); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -345,41 +349,49 @@ Give a short, plain-language summary of the current planetary weather in two par | |||||||||||||
| return new Response(responseBody, { headers: { 'Content-Type': 'text/plain; charset=utf-8' }}); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Check for natural follow-up flow based on user response type | ||||||||||||||
| // | ||||||||||||||
| // DESIGN NOTE: Skip OSR checks on first turn UNLESS explicit OSR phrases used | ||||||||||||||
| // Rationale: First turn after "Session Started" is typically a command, not a resonance response. | ||||||||||||||
| // However, we still detect explicit OSR phrases like "doesn't resonate" for edge cases. | ||||||||||||||
| // | ||||||||||||||
| // The OSR check is duplicated here (before classifyUserResponse) intentionally: | ||||||||||||||
| // - Early exit for first-turn commands (performance optimization) | ||||||||||||||
| // - Preserves classification logic independence in classifyUserResponse() | ||||||||||||||
| // - Makes the first-turn special case explicit and easy to understand | ||||||||||||||
| const skipOSRCheck = isFirstTurn && !checkForOSRIndicators(text); | ||||||||||||||
| const responseType = skipOSRCheck | ||||||||||||||
| ? 'CLEAR_WB' | ||||||||||||||
| : classifyUserResponse(text); | ||||||||||||||
|
|
||||||||||||||
| // Mock session context (in production, this would be persisted) | ||||||||||||||
| const mockSessionContext: SessionContext = { | ||||||||||||||
| wbHits: [], | ||||||||||||||
| abeHits: [], | ||||||||||||||
| osrMisses: [], | ||||||||||||||
| actorWeighting: 0, | ||||||||||||||
| roleWeighting: 0, | ||||||||||||||
| driftIndex: 0, | ||||||||||||||
| sessionActive: true | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| // Generate natural follow-up based on response type | ||||||||||||||
| if (responseType === 'CLEAR_WB') { | ||||||||||||||
| const followUp = naturalFollowUpFlow.generateFollowUp({ | ||||||||||||||
| type: 'AFFIRM', | ||||||||||||||
| content: text, | ||||||||||||||
| originalMirror: text | ||||||||||||||
| }, mockSessionContext); | ||||||||||||||
|
|
||||||||||||||
| 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'); | ||||||||||||||
|
||||||||||||||
| 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)); |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable followUp.
| const followUp = naturalFollowUpFlow.generateFollowUp({ | |
| type: 'AFFIRM', | |
| content: text, | |
| originalMirror: text | |
| }, mockSessionContext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding generic trigger phrases like 'continue', 'okay', 'yes', and 'read it' to
checkForReadingStartRequestcreates 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
checkForReadingStartRequestfunction. This would preserve the existing conversation flow while fixing the report-ready trigger issue.