From 8d7a18ac031be5e70d1153918edea91e9ef2b044 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 26 May 2026 22:50:35 -0700 Subject: [PATCH] fix(conversation-parser): threshold-gated fallback + acceptance floor (closes #1533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gbrain conversation-parser scan` reported `phase: no_match` on meeting pages where 175 of 226 lines (77.8%) were valid `imessage-slack` format. The 36 reformatted Circleback meetings could not flow through the conversation facts pipeline. Root cause: `scorePattern` only scans the first 10 non-blank lines. A meeting page's `## Summary` + blockquote + `## Transcript` preamble takes all 10 head slots, so every pattern scored 0 and the orchestrator short-circuited to `no_match` without ever seeing the transcript. Fix: two-tier scoring with threshold gates. 1. Fast path unchanged: chat-only pages match on line 1, scoring 1.0, skipping the fallback entirely. 2. Full-body fallback fires when `top.score < SCORING_HEAD_TRIGGER_THRESHOLD` (0.3). NOT `=== 0` — Codex P1 #1 caught the bug class where a stray head match (blockquote that accidentally matches an unrelated pattern at 0.1) would suppress the fallback. 0.3 leaves the fast path untouched while triggering on any preamble-dominated page. 3. Minimum acceptance floor `SCORING_MIN_ACCEPTANCE` (0.05) prevents essay false positives: a 300-line essay with one stray `**Name** (date time):` line scores ~0.003 — without the floor it would flip to `regex_match` with `messages.length = 1`. Closes Codex P1 #2. DRY refactor: extract `getNonBlankLines` + `scoreFromLines` so the quick_reject + regex loop lives in one place. New exported `scorePatternFull` for direct unit testing. Fallback pre-splits the body ONCE per pass to avoid 12 redundant splits. Plan + decisions + Codex consult absorption at: ~/.claude/plans/system-instruction-you-are-working-starry-frost.md Tests: 10 new cases in test/conversation-parser/parse.test.ts (87 pass). Highlights: - #1533 IRON-RULE regression pin (meeting page → regex_match, imessage-slack, 20 messages) - Stray-head-match guard (Codex P1 #1: irc-classic 0.1 in head does not suppress fallback; imessage-slack wins on full body) - Essay false-positive guard (Codex P1 #2: 1/301 score below acceptance floor stays no_match) - 300-line preamble + 50 chat lines hits fallback - Cap test reshaped (Codex P2 #6): pins behavior not constant value Once landed and a brain has `cycle.conversation_facts_backfill.enabled = true` (opt-in), the 36 Circleback meetings flow through the fact extractor automatically. Operators on the manual path run `gbrain extract-conversation-facts ` directly. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/core/conversation-parser/parse.ts | 137 +++++++++++++++--- test/conversation-parser/parse.test.ts | 188 ++++++++++++++++++++++++- 2 files changed, 297 insertions(+), 28 deletions(-) diff --git a/src/core/conversation-parser/parse.ts b/src/core/conversation-parser/parse.ts index 7368f6b28..215cbeab8 100644 --- a/src/core/conversation-parser/parse.ts +++ b/src/core/conversation-parser/parse.ts @@ -48,6 +48,33 @@ export type { ParseConversationOpts, ParseResult, MatchedMessage } from './types */ const SCORING_HEAD_LINES = 10; +/** + * Head-pass score below which `parseConversation` falls back to a + * full-body re-score (v0.41.18+ fix for #1533 + Codex P1 #1). + * + * Why a threshold instead of `=== 0`: meeting pages start with + * `## Summary` + blockquotes + `## Transcript` headings. A + * blockquote like `> [12:00] Foo` can accidentally match an + * unrelated pattern's regex (irc-classic at score 0.1) which would + * suppress the fallback even when 175 of 226 lines further down are + * valid imessage-slack. 0.3 = "fewer than 3 of 10 head lines + * matched"; chat-only pages still score 1.0 and skip the fallback + * entirely, so the fast path is preserved. + */ +const SCORING_HEAD_TRIGGER_THRESHOLD = 0.3; + +/** + * Minimum final winner score required to accept `regex_match` + * (v0.41.18+ Codex P1 #2). A 500-line essay with one stray + * `**Name** (date time):` line scores ~1/500 = 0.002 for + * imessage-slack, which without this floor would flip to + * `regex_match` with `messages.length = 1` — a false positive that + * silently corrupts downstream fact extraction. 0.05 = "at least 5% + * of non-blank lines anchored a message"; real transcript pages + * typically score 0.5+ and sail through, accidental anchors do not. + */ +const SCORING_MIN_ACCEPTANCE = 0.05; + /** * Tie-breaker priority: lower index wins on score tie. Mirrors * BUILTIN_PATTERNS declaration order. User-declared patterns get @@ -334,6 +361,41 @@ export function applyPattern( return out; } +/** + * Split a body into non-blank trimmed lines. `headCap` (when set) + * limits the result to the first N lines — used by the head-pass + * scorer; omit for full-body scoring. + */ +function getNonBlankLines(body: string, headCap?: number): string[] { + if (!body) return []; + const all = body + .split(/\r?\n/) + .map((l) => l.trim()) + .filter((l) => l.length > 0); + return headCap !== undefined ? all.slice(0, headCap) : all; +} + +/** + * Core scorer over a pre-split line array. Both `scorePattern` (head + * window) and `scorePatternFull` (whole body) delegate here so the + * quick_reject + regex loop lives in one place. Reused by + * `parseConversation`'s fallback path which pre-splits ONCE and + * passes the array to all 12 candidates (saves 11 redundant body + * splits per fallback pass). + */ +function scoreFromLines( + lines: readonly string[], + entry: PatternEntry, +): number { + if (lines.length === 0) return 0; + let anchored = 0; + for (const line of lines) { + if (entry.quick_reject && !entry.quick_reject.test(line)) continue; + if (entry.regex.test(line)) anchored++; + } + return anchored / lines.length; +} + /** * Score how well a pattern matches the first N lines of a body (D18). * Returns 0..1 ratio of matched lines. Higher = more confident. @@ -344,19 +406,22 @@ export function applyPattern( * Exported for tests. */ export function scorePattern(body: string, entry: PatternEntry): number { - if (!body) return 0; - const lines = body - .split(/\r?\n/) - .map((l) => l.trim()) - .filter((l) => l.length > 0) - .slice(0, SCORING_HEAD_LINES); - if (lines.length === 0) return 0; - let anchored = 0; - for (const line of lines) { - if (entry.quick_reject && !entry.quick_reject.test(line)) continue; - if (entry.regex.test(line)) anchored++; - } - return anchored / lines.length; + return scoreFromLines(getNonBlankLines(body, SCORING_HEAD_LINES), entry); +} + +/** + * Score how well a pattern matches the FULL body, no head cap + * (v0.41.18+ Codex P1 #1 — full-body fallback when head pass falls + * below `SCORING_HEAD_TRIGGER_THRESHOLD`). + * + * Cost-aware: in `parseConversation`'s fallback path we pre-split + * ONCE and route through `scoreFromLines` directly, NOT through this + * wrapper, to avoid 12 redundant body splits per pass. This wrapper + * exists for direct unit testing and for any future caller that + * needs full-body scoring of a single pattern. + */ +export function scorePatternFull(body: string, entry: PatternEntry): number { + return scoreFromLines(getNonBlankLines(body), entry); } /** @@ -392,23 +457,51 @@ export function parseConversation( // declared priority order (built-in declaration order; user patterns // lose every tie). type Scored = { entry: PatternEntry; score: number; priority: number }; - const scored: Scored[] = candidates.map((entry) => ({ + const sortScored = (arr: Scored[]) => + arr.sort((a, b) => { + if (b.score !== a.score) return b.score - a.score; + return a.priority - b.priority; + }); + let scored: Scored[] = candidates.map((entry) => ({ entry, score: scorePattern(body, entry), priority: priorityOf(entry.id), })); - // Sort: score desc, then priority asc. - scored.sort((a, b) => { - if (b.score !== a.score) return b.score - a.score; - return a.priority - b.priority; - }); + sortScored(scored); + + // REGRESSION (closes #1533 + Codex P1 #1): meeting pages have + // ## Summary + blockquote + ## Transcript ahead of the chat. The + // pre-fix "trigger fallback only when score === 0" shape left a + // real bug class open — a stray head match (e.g. a blockquote + // that accidentally matches an unrelated pattern at 0.1) + // suppressed the fallback even when 175 of 226 lines further down + // were valid imessage-slack. Threshold 0.3 means "fewer than 3 of + // 10 head lines matched"; chat-only pages still score 1.0 and skip + // the fallback. Re-score every candidate against the full body, + // pre-splitting ONCE to avoid 12 redundant body splits. + if (scored[0].score < SCORING_HEAD_TRIGGER_THRESHOLD) { + const allLines = getNonBlankLines(body); + scored = candidates.map((entry) => ({ + entry, + score: scoreFromLines(allLines, entry), + priority: priorityOf(entry.id), + })); + sortScored(scored); + // NOTE: patterns_scored stays as scored.length (= candidate + // count, typically 12) even when the fallback runs — the + // diagnostic reports "candidates considered" not "scoring + // attempts" (Codex P2 #7). + } const top = scored[0]; const patternsScored = scored.length; - // If even the top scorer matches zero lines, no regex won. Caller - // may invoke LLM fallback (T4); for now return no_match. - if (top.score === 0) { + // Minimum acceptance floor (closes Codex P1 #2): an essay with + // one stray `**Name** (date time):` line scores ~1/300 ≈ 0.003 — + // below the 5% floor we stay no_match instead of returning a + // 1-message false positive. Real transcript pages typically score + // 0.5+ and sail through. + if (top.score < SCORING_MIN_ACCEPTANCE) { return { messages: [], phase: 'no_match', diff --git a/test/conversation-parser/parse.test.ts b/test/conversation-parser/parse.test.ts index 577312a2e..5711ba323 100644 --- a/test/conversation-parser/parse.test.ts +++ b/test/conversation-parser/parse.test.ts @@ -22,6 +22,7 @@ import { deriveDateContext, applyPattern, scorePattern, + scorePatternFull, } from '../../src/core/conversation-parser/parse.ts'; import { BUILTIN_PATTERNS } from '../../src/core/conversation-parser/builtins.ts'; import type { Page } from '../../src/core/types.ts'; @@ -336,13 +337,188 @@ describe('scorePattern — boundary', () => { const tg = BUILTIN_PATTERNS.find((p) => p.id === 'telegram-bracket')!; expect(scorePattern('\n\n \n', tg)).toBe(0); }); - test('caps at SCORING_HEAD_LINES (10) lines', () => { - // 100 telegram lines → still scores 1.0 because only first 10 sampled. - const body = Array.from( - { length: 100 }, - (_, i) => `**[18:${String(i).padStart(2, '0')}] \u{1f464} Alice:** msg ${i}`, - ).join('\n'); + // T5 reshape (Codex P2 #6): pins BEHAVIOR not the constant value. + // The prior test ("100 matching lines score 1.0") would pass with + // head=10 or head=1000 — it didn't prove anything about the cap. + test('head cap ignores lines past line 10 (10 match + 1 non-match scores 1.0)', () => { const tg = BUILTIN_PATTERNS.find((p) => p.id === 'telegram-bracket')!; + const matching = Array.from( + { length: 10 }, + (_, i) => `**[18:${String(i).padStart(2, '0')}] \u{1f464} Alice:** msg ${i}`, + ); + const body = [...matching, 'plain text outside the head window'].join('\n'); + // First 10 lines all match → 10/10. Line 11 was ignored. expect(scorePattern(body, tg)).toBe(1); }); + test('head cap stops at line 10 (9 non-match + 1 match at line 10 + 100 match after scores 0.1)', () => { + const tg = BUILTIN_PATTERNS.find((p) => p.id === 'telegram-bracket')!; + const nonMatches = Array.from({ length: 9 }, (_, i) => `non-matching prose line ${i}`); + const matchingLate = Array.from( + { length: 100 }, + (_, i) => `**[18:${String(i).padStart(2, '0')}] \u{1f464} Alice:** msg ${i}`, + ); + // Line 10 (index 9 in the matching array) IS a match; lines 11-109 are too + // but are past the head cap and don't count. + const body = [...nonMatches, matchingLate[0], ...matchingLate.slice(1)].join('\n'); + // Head sees 9 non-matches + 1 match = 1/10 = 0.1. Pre-fix: same result. + // Post-fix: same result (this test pins head-cap behavior, not the new + // fallback path — that's tested separately below). + expect(scorePattern(body, tg)).toBe(0.1); + }); +}); + +// --------------------------------------------------------------------------- +// scorePatternFull — direct unit tests (v0.41.18+ T3 #5) +// --------------------------------------------------------------------------- + +describe('scorePatternFull — full-body scoring (v0.41.18+ Codex P1 #1)', () => { + test('empty body scores 0', () => { + const im = BUILTIN_PATTERNS.find((p) => p.id === 'imessage-slack')!; + expect(scorePatternFull('', im)).toBe(0); + }); + test('preamble + 20 matching lines scores 20/(preamble + 20)', () => { + const im = BUILTIN_PATTERNS.find((p) => p.id === 'imessage-slack')!; + const preamble = ['## Summary', 'Three sentences.', '> Source: ref', '## Transcript']; + const matches = Array.from( + { length: 20 }, + (_, i) => `**Garry Tan** (2026-01-29 12:00 PM): message ${i}`, + ); + const body = [...preamble, ...matches].join('\n'); + // 24 total non-blank, 20 match → 20/24 ≈ 0.833 + expect(scorePatternFull(body, im)).toBeCloseTo(20 / 24, 5); + }); + test('preamble-only-no-match scores 0', () => { + const im = BUILTIN_PATTERNS.find((p) => p.id === 'imessage-slack')!; + const body = '## Summary\nProse paragraph.\n> Blockquote\n## Heading'; + expect(scorePatternFull(body, im)).toBe(0); + }); +}); + +// --------------------------------------------------------------------------- +// parseConversation — full-body fallback (v0.41.18+ #1533 + Codex P1 #1, #2, #8) +// --------------------------------------------------------------------------- + +describe('parseConversation — full-body fallback', () => { + // T3 #1: IRON-RULE regression pin for #1533. Pre-fix this returns + // no_match because head 10 sees only preamble. + test('#1533: meeting page with ## Summary + blockquote + ## Transcript before chat hits fallback', () => { + const preamble = [ + '## Summary', + 'This meeting covered Q1 roadmap discussion.', + 'Three engineers participated in the call.', + 'Action items were captured during the conversation.', + '> Source: [meeting recording](https://example.com/rec/123)', + '## Topics Discussed', + '- Product roadmap for Q1', + '- Engineering team allocation', + '- Customer feedback synthesis', + '## Transcript', + ]; + const transcript = Array.from( + { length: 20 }, + (_, i) => `**Garry Tan** (2026-01-29 12:00 PM): line ${i}`, + ); + const body = [...preamble, ...transcript].join('\n'); + const r = parseConversation(body); + expect(r.phase).toBe('regex_match'); + expect(r.matched_pattern_id).toBe('imessage-slack'); + expect(r.messages).toHaveLength(20); + }); + + // T3 #2: diagnostic now reports total_non_blank - matched, not total. + test('#1533: unmatched_line_count subtracts matched messages after fallback', () => { + const preamble = [ + '## Summary', + 'Prose A.', + 'Prose B.', + '> Blockquote', + '## Transcript', + ]; + const transcript = Array.from( + { length: 20 }, + (_, i) => `**Garry Tan** (2026-01-29 12:00 PM): line ${i}`, + ); + const body = [...preamble, ...transcript].join('\n'); + const r = parseConversation(body, { diagnostic: true }); + expect(r.phase).toBe('regex_match'); + expect(r.unmatched_line_count).toBe(5); // 25 total non-blank - 20 messages = 5 + }); + + // T3 #3: a 50-line essay with no chat shape stays no_match. + test('pure-prose 50-line essay stays no_match (fallback found nothing to anchor)', () => { + const body = Array.from( + { length: 50 }, + (_, i) => `This is the ${i + 1}th paragraph of a pure-prose article.`, + ).join('\n'); + const r = parseConversation(body); + expect(r.phase).toBe('no_match'); + expect(r.messages).toHaveLength(0); + }); + + // T3 #4: proves "full-body" not just "wider window" — 300-line preamble + // far exceeds any reasonable head-bump alternative. + test('300-line preamble + 50 chat lines hits fallback (any preamble length)', () => { + const preamble = Array.from( + { length: 300 }, + (_, i) => `Preamble paragraph ${i + 1} with prose content here.`, + ); + const transcript = Array.from( + { length: 50 }, + (_, i) => `**Garry Tan** (2026-01-29 12:00 PM): chat line ${i}`, + ); + const body = [...preamble, ...transcript].join('\n'); + const r = parseConversation(body); + expect(r.phase).toBe('regex_match'); + expect(r.matched_pattern_id).toBe('imessage-slack'); + expect(r.messages).toHaveLength(50); + }); + + // T3 #6 (Codex P1 #1 + #8): stray-head-match doesn't suppress fallback. + // Pre-fix: irc-classic 0.1 in head → no fallback → irc-classic wins with 1 + // message. Post-fix: 0.1 < 0.3 trigger → fallback re-scores → imessage-slack + // wins (50/60 ≈ 0.83 vs irc-classic 1/60 ≈ 0.017). + test('Codex P1 #1: stray irc-classic match in head does not suppress fallback', () => { + const preamble = [ + '## Meeting Notes', + ' Garry Tan opening remarks', // stray irc-classic match + '- agenda item 1', + '- agenda item 2', + '- agenda item 3', + '- agenda item 4', + '- agenda item 5', + '- agenda item 6', + '- agenda item 7', + '## Transcript', + ]; + const transcript = Array.from( + { length: 50 }, + (_, i) => `**Garry Tan** (2024-01-29 12:00 PM): real transcript line ${i}`, + ); + const body = [...preamble, ...transcript].join('\n'); + const r = parseConversation(body); + expect(r.phase).toBe('regex_match'); + // The critical assertion: imessage-slack wins, NOT irc-classic. + expect(r.matched_pattern_id).toBe('imessage-slack'); + expect(r.messages).toHaveLength(50); + }); + + // T3 #7 (Codex P1 #2): essay with one stray chat-shape line stays + // no_match. 1/301 ≈ 0.003, below SCORING_MIN_ACCEPTANCE (0.05). + test('Codex P1 #2: 300-line essay with one stray chat line stays no_match (acceptance floor)', () => { + const prose = Array.from( + { length: 150 }, + (_, i) => `Essay paragraph ${i + 1} of pure prose with no chat shape.`, + ); + const strayChatLine = '**Author Name** (2024-01-01 9:00 AM): stray quoted snippet'; + const morePros = Array.from( + { length: 150 }, + (_, i) => `Essay continuation paragraph ${i + 151}.`, + ); + const body = [...prose, strayChatLine, ...morePros].join('\n'); + const r = parseConversation(body); + // Pre-fix: regex_match with messages.length === 1. + // Post-fix: no_match because 1/301 < 0.05 acceptance floor. + expect(r.phase).toBe('no_match'); + expect(r.messages).toHaveLength(0); + }); });