diff --git a/bin/gstack-question-log b/bin/gstack-question-log index 2e9c054c9..da86add41 100755 --- a/bin/gstack-question-log +++ b/bin/gstack-question-log @@ -168,9 +168,17 @@ if (j.recommended !== undefined) { if (j.recommended.length > 64) j.recommended = j.recommended.slice(0, 64); } -// followed_recommendation — compute if both sides present. -if (j.recommended !== undefined && j.user_choice !== undefined) { - j.followed_recommendation = j.user_choice === j.recommended; +// followed_recommendation — compute if both sides present. An __unknown__ +// choice means extraction failed, not that the user rejected the +// recommendation — leave the field absent so metrics can't be poisoned. +// Strip a trailing (Recommended) marker from BOTH sides before comparing: +// recommended usually arrives pre-stripped while user_choice is the raw +// option label, so a user who picked the recommended option was scored as +// NOT following it (#2400). NB: this JS lives inside a double-quoted +// bun -e string — never use double quotes in it. +if (j.recommended !== undefined && j.user_choice !== undefined && j.user_choice !== '__unknown__') { + const stripRec = (s) => String(s).replace(/\s*\(recommended\)\s*$/i, '').trim(); + j.followed_recommendation = stripRec(j.user_choice) === stripRec(j.recommended); } // session_id — kebab-friendly; <=64 chars diff --git a/hosts/claude/hooks/question-log-hook.ts b/hosts/claude/hooks/question-log-hook.ts index 304a505f5..505bee415 100644 --- a/hosts/claude/hooks/question-log-hook.ts +++ b/hosts/claude/hooks/question-log-hook.ts @@ -156,21 +156,63 @@ function extractRecommended(questionText: string, opts: string[]): string | unde * AUQ tool_response shape varies by Claude Code variant (native vs MCP), * and the hook stdin docs don't pin a single canonical shape. We handle * the common cases gracefully. + * + * Shape D is the current native AskUserQuestion result: + * { answers: { "": "" }, + * annotations?: { "": { notes?, preview? } } } + * The map is keyed by the question text exactly as passed in tool_input, + * so extraction needs the questions themselves, not just a count. */ function extractUserChoices( response: unknown, - questionCount: number, + questions: Array<{ question?: string; options?: Array }>, + diag?: (msg: string) => void, ): Array<{ choice: string; free_text?: string }> { + const questionCount = questions.length; const out: Array<{ choice: string; free_text?: string }> = []; if (!response) { + diag?.(`answer-extract: empty tool_response (typeof=${typeof response})`); for (let i = 0; i < questionCount; i++) out.push({ choice: '__unknown__' }); return out; } - // Shape A: { answers: [{option_label, free_text?}] } - // Shape B: { questions: [{user_answer}] } - // Shape C: { content: [...] } or array. - // We probe lazily. const rec = response as Record; + // Shape D: { answers: {questionText: answer}, annotations?: {questionText: {notes}} } + if (rec.answers && typeof rec.answers === 'object' && !Array.isArray(rec.answers)) { + const answers = rec.answers as Record; + const annotations = + rec.annotations && typeof rec.annotations === 'object' && !Array.isArray(rec.annotations) + ? (rec.annotations as Record>) + : {}; + const keys = Object.keys(answers); + const norm = (s: string) => s.replace(/\s+/g, ' ').trim().toLowerCase(); + for (const q of questions) { + const qText = q.question || ''; + let key: string | undefined = Object.prototype.hasOwnProperty.call(answers, qText) + ? qText + : keys.find((k) => norm(k) === norm(qText)); + // Single question, single answer: pair them even if the key drifted. + if (key === undefined && keys.length === 1 && questionCount === 1) key = keys[0]; + if (key === undefined) { + diag?.(`answer-extract: no answers key matched question "${qText.slice(0, 60)}"`); + out.push({ choice: '__unknown__' }); + continue; + } + const v = answers[key]; + const rawChoice = Array.isArray(v) ? v.map(String).join(', ') : String(v ?? '__unknown__'); + // The bin compares user_choice === recommended, and recommended is + // stored with the "(recommended)" suffix stripped — strip it here too. + const choice = rawChoice.replace(RECOMMENDED_LABEL_RE, '').trim() || '__unknown__'; + const labels = optionLabels(q.options || []).map((l) => + l.replace(RECOMMENDED_LABEL_RE, '').trim().toLowerCase(), + ); + const notes = annotations[key]?.notes; + const isFreeText = !Array.isArray(v) && labels.length > 0 && !labels.includes(choice.toLowerCase()); + const freeText = notes !== undefined ? String(notes) : isFreeText ? rawChoice : undefined; + out.push(freeText !== undefined ? { choice, free_text: freeText } : { choice }); + } + return out; + } + // Shape A: { answers: [{option_label, free_text?}] } if (Array.isArray(rec.answers)) { for (const a of rec.answers as Array>) { const choice = (a.option_label || a.label || a.choice || a.answer || '__unknown__') as string; @@ -180,6 +222,7 @@ function extractUserChoices( while (out.length < questionCount) out.push({ choice: '__unknown__' }); return out; } + // Shape B: { questions: [{user_answer}] } if (Array.isArray(rec.questions)) { for (const q of rec.questions as Array>) { const choice = (q.user_answer || q.answer || q.choice || '__unknown__') as string; @@ -188,9 +231,11 @@ function extractUserChoices( while (out.length < questionCount) out.push({ choice: '__unknown__' }); return out; } - // Fall back: stringify and log first 100 chars to help future debugging. + // Unrecognized shape: log it for postmortem (never embed it in the record — + // that poisons user_choice for every downstream metric). + diag?.(`answer-extract: unrecognized tool_response shape: ${JSON.stringify(response).slice(0, 300)}`); for (let i = 0; i < questionCount; i++) { - out.push({ choice: `__response-shape-unknown:${JSON.stringify(response).slice(0, 80)}__` }); + out.push({ choice: '__unknown__' }); } return out; } @@ -251,7 +296,9 @@ async function main(): Promise { } const skill = detectSkill(stdin.cwd); - const choices = extractUserChoices(stdin.tool_response, questions.length); + const choices = extractUserChoices(stdin.tool_response, questions, (msg) => + logHookError(`${msg} (tool_use_id=${stdin.tool_use_id || 'n/a'})`), + ); for (let i = 0; i < questions.length; i++) { const q = questions[i]; diff --git a/test/question-log-hook.test.ts b/test/question-log-hook.test.ts index 43b75d0ff..faa0cfc8b 100644 --- a/test/question-log-hook.test.ts +++ b/test/question-log-hook.test.ts @@ -138,6 +138,91 @@ describe('PostToolUse hook (native AskUserQuestion)', () => { }); }); +// ---------------------------------------------------------------------- +// Shape D: current native AUQ result — answers object keyed by question text +// ---------------------------------------------------------------------- + +describe('PostToolUse hook (Shape D: answers object keyed by question text)', () => { + test('captures user_choice from the answers map, stripping the (Recommended) suffix', () => { + const qText = 'D-shape test: apply the edits?'; + runHook({ + session_id: 'sessD1', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d1', + tool_input: { + questions: [ + { question: qText, options: [{ label: 'Full revision (Recommended)' }, { label: 'Hold' }] }, + ], + }, + tool_response: { answers: { [qText]: 'Full revision (Recommended)' } }, + cwd: ROOT, + }); + const events = readLog(); + expect(events.length).toBe(1); + expect(events[0].user_choice).toBe('Full revision'); + expect(events[0].recommended).toBe('Full revision'); + expect(events[0].followed_recommendation).toBe(true); + }); + + test('free-text answer (not an option label) → source=auq-other with free_text', () => { + const qText = 'D-shape free text test'; + runHook({ + session_id: 'sessD2', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d2', + tool_input: { + questions: [{ question: qText, options: [{ label: 'Alpha' }, { label: 'Beta' }] }], + }, + tool_response: { answers: { [qText]: 'I cannot see what the proposal is' } }, + cwd: ROOT, + }); + const events = readLog(); + expect(events.length).toBe(1); + expect(events[0].source).toBe('auq-other'); + expect(events[0].free_text).toContain('cannot see'); + }); + + test('annotations notes are captured as free_text alongside a selected option', () => { + const qText = 'D-shape annotations test'; + runHook({ + session_id: 'sessD3', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d3', + tool_input: { + questions: [{ question: qText, options: [{ label: 'Alpha' }, { label: 'Beta' }] }], + }, + tool_response: { + answers: { [qText]: 'Alpha' }, + annotations: { [qText]: { notes: 'but only after the demo' } }, + }, + cwd: ROOT, + }); + const events = readLog(); + expect(events.length).toBe(1); + expect(events[0].user_choice).toBe('Alpha'); + expect(events[0].free_text).toContain('after the demo'); + }); + + test('empty tool_response → user_choice __unknown__ and NO followed_recommendation', () => { + runHook({ + session_id: 'sessD4', + tool_name: 'AskUserQuestion', + tool_use_id: 'tu-d4', + tool_input: { + questions: [ + { question: 'D-shape unknown test', options: [{ label: 'Alpha (Recommended)' }, { label: 'Beta' }] }, + ], + }, + cwd: ROOT, + }); + const events = readLog(); + expect(events.length).toBe(1); + expect(events[0].user_choice).toBe('__unknown__'); + expect(events[0].recommended).toBe('Alpha'); + expect(events[0].followed_recommendation).toBeUndefined(); + }); +}); + // ---------------------------------------------------------------------- // MCP AskUserQuestion variant (Conductor) // ----------------------------------------------------------------------