diff --git a/src/config/env.ts b/src/config/env.ts index ddffb0c..a09c88e 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -668,4 +668,15 @@ export const config = { sessions: { revokedRetainDays: parseInt(process.env.REVOKED_SESSION_RETAIN_DAYS || '7'), }, + nlp: { + /** + * Minimum confidence (0-1) a parsed Intent must carry to be acted on + * directly (#401). Below this, the rule-based parser returns a + * 'clarification' intent instead of guessing at the nearest pattern or + * falling straight through to 'unknown'. + */ + confidenceThreshold: parseFloat( + process.env.NLP_CONFIDENCE_THRESHOLD || '0.6' + ), + }, } diff --git a/src/nlp/parser.ts b/src/nlp/parser.ts index 6ed8150..292bf57 100644 --- a/src/nlp/parser.ts +++ b/src/nlp/parser.ts @@ -1,47 +1,64 @@ import Anthropic from '@anthropic-ai/sdk' import { HttpClientAdapter } from '../utils/http-client' import { config } from '../config' +import { responses } from './responses' /** * Represents a parsed user intent for the financial bot. * * This is a discriminated union so TypeScript can safely narrow the * available properties based on `action`. + * + * Every variant carries a `confidence` in [0, 1] (#401): 1 for a clean + * deterministic regex match, a fixed heuristic for a Claude classification + * (Claude doesn't expose a calibrated probability), and 0 for `unknown`. + * `parseWithRegex` uses this score internally to decide between returning a + * specific intent and returning `clarification` — callers that only switch + * on `action` can ignore the field entirely. */ export type Intent = | { action: 'deposit' + confidence: number amount?: number currency?: string } | { action: 'withdraw' + confidence: number amount?: number currency?: string all?: boolean } | { action: 'balance' + confidence: number } | { action: 'earnings' + confidence: number } | { action: 'help' + confidence: number } | { action: 'goal' + confidence: number } | { action: 'create_recurring_deposit' + confidence: number amount?: number cadence?: 'WEEKLY' | 'BIWEEKLY' | 'MONTHLY' } | { action: 'pause_recurring_deposit' + confidence: number } | { action: 'alert_create' + confidence: number metric: string protocolName: string comparator: string @@ -49,13 +66,28 @@ export type Intent = } | { action: 'alert_list' + confidence: number } | { action: 'alert_delete' + confidence: number alertId?: string } | { action: 'unknown' + confidence: number + } + | { + /** + * The parser recognized signals for two or more competing actions + * (e.g. both "deposit" and "withdraw") but wasn't confident enough in + * either to act on it directly. `prompt` is ready to show verbatim; + * `candidates` lists the actions it couldn't choose between. + */ + action: 'clarification' + confidence: number + prompt: string + candidates: string[] } // --------------------------------------------------------------------------- @@ -232,6 +264,110 @@ function isValidIntent(value: unknown): value is Intent { } } +// --------------------------------------------------------------------------- +// Ambiguity detection (#401) +// --------------------------------------------------------------------------- + +interface IntentSignal { + action: string + /** Short human phrase for the clarification prompt, e.g. "deposit money". */ + label: string + pattern: RegExp +} + +/** + * Loose keyword signals for each action family — deliberately broader than + * the strict patterns above, so a message that mentions an action without + * fully matching its pattern still registers as "about" that action. Used + * only to detect competing intents in a message the strict patterns above + * couldn't already resolve; never used to pick an action outright. + */ +const INTENT_SIGNALS: IntentSignal[] = [ + { + action: 'deposit', + label: 'deposit money', + pattern: /\bdeposit(?:s|ing)?\b|\badd\s+(?:money|funds)\b|\bput\s+in\b/i, + }, + { + action: 'withdraw', + label: 'withdraw money', + pattern: /\bwithdraw(?:s|al|ing)?\b|\btake\s+out\b|\bcash\s+out\b/i, + }, + { + action: 'balance', + label: 'check your balance', + pattern: /\bbalance\b/i, + }, + { + action: 'earnings', + label: 'check your earnings', + pattern: /\b(?:earnings|yield|apy|performance)\b/i, + }, + { + action: 'goal', + label: 'check your savings goal', + pattern: /\bgoal\b/i, + }, + { + action: 'create_recurring_deposit', + label: 'set up a recurring deposit', + pattern: /\brecurring\b|\bautomatic\b|\bscheduled\b/i, + }, + { + action: 'pause_recurring_deposit', + label: 'pause your recurring deposit', + pattern: /\bpause\b/i, + }, + { + action: 'alert_create', + label: 'set up an alert', + pattern: /\balert\b/i, + }, +] + +function detectIntentSignals(lowerMsg: string): IntentSignal[] { + return INTENT_SIGNALS.filter((signal) => signal.pattern.test(lowerMsg)) +} + +/** + * Confidence for a message that carries signals for `signalCount` competing + * actions. Two competing actions halve confidence from a hypothetical + * certain match; each additional competing action divides it further, with + * a floor so the score stays a meaningful (if low) number rather than + * collapsing to ~0. + */ +function ambiguityConfidence(signalCount: number): number { + return Math.max(0.15, 1 / signalCount) +} + +/** + * When a message couldn't be resolved to a single confident intent but + * mentions two or more known actions (e.g. "should I deposit or withdraw?"), + * returns a 'clarification' intent asking the user to pick one — rather than + * silently guessing the nearest pattern or falling straight through to + * 'unknown'. Returns null when there's zero or one signal (nothing to + * disambiguate) or when the resulting confidence isn't actually below the + * configured threshold. + */ +function buildClarificationIntent(lowerMsg: string): Intent | null { + const signals = detectIntentSignals(lowerMsg) + if (signals.length < 2) { + return null + } + + const confidence = ambiguityConfidence(signals.length) + if (confidence >= config.nlp.confidenceThreshold) { + return null + } + + return { + action: 'clarification', + confidence, + candidates: signals.map((s) => s.action), + prompt: responses.clarification(signals.map((s) => s.label)), + } +} + // --------------------------------------------------------------------------- // Regex parser // --------------------------------------------------------------------------- @@ -265,6 +401,7 @@ export function parseWithRegex(message: string): Intent | null { ) { return { action: 'withdraw', + confidence: 1, all: true, } } @@ -286,10 +423,12 @@ export function parseWithRegex(message: string): Intent | null { action === 'deposit' ? { action: 'deposit', + confidence: 1, amount, } : { action: 'withdraw', + confidence: 1, amount, } @@ -312,6 +451,7 @@ export function parseWithRegex(message: string): Intent | null { ) { return { action: 'balance', + confidence: 1, } } @@ -329,6 +469,7 @@ export function parseWithRegex(message: string): Intent | null { ) { return { action: 'earnings', + confidence: 1, } } @@ -345,6 +486,7 @@ export function parseWithRegex(message: string): Intent | null { ) { return { action: 'goal', + confidence: 1, } } @@ -355,6 +497,7 @@ export function parseWithRegex(message: string): Intent | null { if (/^(?:help|commands|what\s+can\s+you\s+do)[.!?]*$/i.test(lowerMsg)) { return { action: 'help', + confidence: 1, } } @@ -379,6 +522,7 @@ export function parseWithRegex(message: string): Intent | null { if (amount !== null && cadence !== null) { return { action: 'create_recurring_deposit', + confidence: 1, amount, cadence, } @@ -404,6 +548,7 @@ export function parseWithRegex(message: string): Intent | null { if (amount !== null && cadence !== null) { return { action: 'create_recurring_deposit', + confidence: 1, amount, cadence, } @@ -421,6 +566,7 @@ export function parseWithRegex(message: string): Intent | null { ) { return { action: 'pause_recurring_deposit', + confidence: 1, } } @@ -434,6 +580,7 @@ export function parseWithRegex(message: string): Intent | null { ) { return { action: 'alert_list', + confidence: 1, } } @@ -452,6 +599,7 @@ export function parseWithRegex(message: string): Intent | null { if (alertDeleteMatch) { return { action: 'alert_delete', + confidence: 1, alertId: alertDeleteMatch[1], } } @@ -496,6 +644,7 @@ export function parseWithRegex(message: string): Intent | null { return { action: 'alert_create', + confidence: 1, metric, protocolName, comparator, @@ -505,10 +654,11 @@ export function parseWithRegex(message: string): Intent | null { } // ------------------------------------------------------------------------- - // No match + // No confident match — check for competing-intent ambiguity (#401) before + // giving up entirely, e.g. "should I deposit or withdraw?" // ------------------------------------------------------------------------- - return null + return buildClarificationIntent(lowerMsg) } // --------------------------------------------------------------------------- @@ -648,7 +798,7 @@ Do not invent an alert ID. ) if (!contentBlock || contentBlock.type !== 'text') { - return { action: 'unknown' } + return { action: 'unknown', confidence: 0 } } const textContent = contentBlock.text.trim() @@ -657,7 +807,7 @@ Do not invent an alert ID. const end = textContent.lastIndexOf('}') if (start === -1 || end === -1 || end < start) { - return { action: 'unknown' } + return { action: 'unknown', confidence: 0 } } const jsonStr = textContent.substring(start, end + 1) @@ -665,12 +815,15 @@ Do not invent an alert ID. const parsed: unknown = JSON.parse(jsonStr) if (!isValidIntent(parsed)) { - return { action: 'unknown' } + return { action: 'unknown', confidence: 0 } } - return parsed + // Claude doesn't expose a calibrated probability, so a valid + // classification gets a fixed heuristic confidence — comfortably above + // the default threshold, but (unlike a regex match) not certain. + return { ...parsed, confidence: parsed.action === 'unknown' ? 0 : 0.85 } } catch { - return { action: 'unknown' } + return { action: 'unknown', confidence: 0 } } } @@ -684,15 +837,20 @@ Do not invent an alert ID. * Parsing order: * * 1. Empty input -> unknown - * 2. Regex -> deterministic/local result + * 2. Regex -> deterministic/local result, including a 'clarification' intent + * (#401) when the message names two or more competing actions without a + * confident single match — see buildClarificationIntent above * 3. Claude -> only when AI_MODE !== "local" * 4. Unknown fallback * - * This guarantees the parser never throws. + * This guarantees the parser never throws. Every returned Intent carries a + * `confidence`; callers that want to gate on it can compare against + * config.nlp.confidenceThreshold, the same threshold this function's own + * clarification step already uses. */ export async function parseIntent(message: string): Promise { if (!message || message.trim() === '') { - return { action: 'unknown' } + return { action: 'unknown', confidence: 0 } } try { @@ -709,5 +867,5 @@ export async function parseIntent(message: string): Promise { // Intent parsing must never break the WhatsApp handler. } - return { action: 'unknown' } + return { action: 'unknown', confidence: 0 } } diff --git a/src/nlp/responses.ts b/src/nlp/responses.ts index b844cb6..f7e34b9 100644 --- a/src/nlp/responses.ts +++ b/src/nlp/responses.ts @@ -26,4 +26,26 @@ export const responses = { // message into a known intent (i.e. Intent.action === 'unknown'). unrecognized: () => "I'm sorry, I couldn't understand that command. Please try 'deposit 100', 'withdraw everything', or 'balance'.", + + // Prompt shown when the parser recognized signals for one or more actions + // but wasn't confident enough to act on any of them directly (#401) — e.g. + // a message that could plausibly be a deposit or a withdrawal. `labels` are + // short human phrases like "deposit money" or "check your balance"; at + // least one is always provided. + clarification: (labels: string[]) => { + if (labels.length === 0) { + return "I'm not sure what you'd like to do. Could you rephrase that?" + } + + if (labels.length === 1) { + return `Did you want to ${labels[0]}? Could you give me a bit more detail?` + } + + const list = + labels.length === 2 + ? labels.join(' or ') + : `${labels.slice(0, -1).join(', ')}, or ${labels[labels.length - 1]}` + + return `Did you mean to ${list}?` + }, } diff --git a/src/telegram/handler.ts b/src/telegram/handler.ts index fdeea84..f47f914 100644 --- a/src/telegram/handler.ts +++ b/src/telegram/handler.ts @@ -103,6 +103,8 @@ async function executeIntent( } case 'help': return { body: formatHelpMessage() } + case 'clarification': + return { body: intent.prompt } default: return { body: formatUnknownMessage() } } diff --git a/src/whatsapp/handler.ts b/src/whatsapp/handler.ts index d3d3e8e..ba1e16f 100644 --- a/src/whatsapp/handler.ts +++ b/src/whatsapp/handler.ts @@ -345,6 +345,9 @@ async function executeIntent( return { body: formatAlertDeletedReply(deleted) } } + case 'clarification': + return { body: intent.prompt } + case 'unknown': default: return { body: formatUnknownMessage() } diff --git a/tests/unit/nlp/parser.test.ts b/tests/unit/nlp/parser.test.ts new file mode 100644 index 0000000..dd9922b --- /dev/null +++ b/tests/unit/nlp/parser.test.ts @@ -0,0 +1,144 @@ +/** + * #401 — confidence scoring and the clarification fallback on the rule-based + * intent parser. parseWithRegex is synchronous and network-free, so it's + * exercised directly; parseIntent is covered end-to-end with AI_MODE=local + * so it never reaches out to Claude. + */ +process.env.AI_MODE = 'local' + +import { parseWithRegex, parseIntent } from '../../../src/nlp/parser' +import { responses } from '../../../src/nlp/responses' +import { config } from '../../../src/config' + +describe('parseWithRegex confidence', () => { + it('gives a clean deposit match full confidence', () => { + const intent = parseWithRegex('deposit 100') + expect(intent).toEqual( + expect.objectContaining({ action: 'deposit', confidence: 1, amount: 100 }) + ) + }) + + it('gives a clean withdraw-all match full confidence', () => { + const intent = parseWithRegex('withdraw everything') + expect(intent).toEqual( + expect.objectContaining({ + action: 'withdraw', + confidence: 1, + all: true, + }) + ) + }) + + it('gives a clean balance match full confidence', () => { + expect(parseWithRegex('balance')).toEqual( + expect.objectContaining({ action: 'balance', confidence: 1 }) + ) + }) + + it('returns null for input with no recognizable signal at all', () => { + expect(parseWithRegex('xyzzy plugh')).toBeNull() + }) +}) + +describe('parseWithRegex clarification fallback', () => { + it.each([ + 'should I deposit or withdraw?', + 'do you want me to deposit or withdraw money', + 'deposit or withdraw', + ])('asks to clarify deposit vs withdraw for %p', (message) => { + const intent = parseWithRegex(message) + expect(intent).not.toBeNull() + expect(intent!.action).toBe('clarification') + if (intent!.action === 'clarification') { + expect(intent.confidence).toBeLessThan(config.nlp.confidenceThreshold) + expect(intent.candidates).toEqual( + expect.arrayContaining(['deposit', 'withdraw']) + ) + expect(intent.prompt).toBe( + responses.clarification(['deposit money', 'withdraw money']) + ) + } + }) + + it('asks to clarify across three competing actions', () => { + const intent = parseWithRegex( + 'not sure if I want to check my balance or my earnings or my goal' + ) + expect(intent).not.toBeNull() + expect(intent!.action).toBe('clarification') + if (intent!.action === 'clarification') { + expect(intent.candidates).toEqual( + expect.arrayContaining(['balance', 'earnings', 'goal']) + ) + // More competing actions -> lower confidence than the two-way case. + expect(intent.confidence).toBeLessThan(1 / 2) + } + }) + + it('does not clarify when only one action is mentioned but incomplete', () => { + // "deposit" alone mentions only one action family — nothing to + // disambiguate between, so this stays null (escalates to Claude/unknown + // exactly as before #401). + expect(parseWithRegex('deposit')).toBeNull() + }) + + it('never fires for an unambiguous full match even if a second keyword appears incidentally', () => { + // Contains the word "balance" only inside a fully-matched deposit + // sentence structure it doesn't actually satisfy — falls through to the + // ambiguity check, which correctly finds both signals present. + const intent = parseWithRegex('deposit 50 and check my balance') + expect(intent).not.toBeNull() + expect(intent!.action).toBe('clarification') + }) +}) + +describe('parseIntent confidence end-to-end (AI_MODE=local)', () => { + it('resolves a clean command with full confidence', async () => { + const intent = await parseIntent('withdraw 25') + expect(intent).toEqual( + expect.objectContaining({ action: 'withdraw', confidence: 1, amount: 25 }) + ) + }) + + it('returns a clarification intent for ambiguous phrasing instead of unknown', async () => { + const intent = await parseIntent('should I deposit or withdraw?') + expect(intent.action).toBe('clarification') + expect(intent.confidence).toBeLessThan(config.nlp.confidenceThreshold) + }) + + it('falls back to unknown (confidence 0) for empty input', async () => { + const intent = await parseIntent('') + expect(intent).toEqual({ action: 'unknown', confidence: 0 }) + }) + + it('falls back to unknown (confidence 0) with AI_MODE=local for unrecognized text', async () => { + const intent = await parseIntent('xyzzy plugh') + expect(intent).toEqual({ action: 'unknown', confidence: 0 }) + }) +}) + +describe('responses.clarification', () => { + it('phrases a two-way choice with "or"', () => { + expect(responses.clarification(['deposit money', 'withdraw money'])).toBe( + 'Did you mean to deposit money or withdraw money?' + ) + }) + + it('phrases a three-way choice as a comma list with a trailing "or"', () => { + expect( + responses.clarification([ + 'check your balance', + 'check your earnings', + 'check your savings goal', + ]) + ).toBe( + 'Did you mean to check your balance, check your earnings, or check your savings goal?' + ) + }) + + it('phrases a single label as a yes/no-style question asking for more detail', () => { + expect(responses.clarification(['deposit money'])).toBe( + 'Did you want to deposit money? Could you give me a bit more detail?' + ) + }) +})