rohitsar567 Claude Opus 4.7 (1M context) commited on
Commit
8ef5c43
·
1 Parent(s): 4bb8da0

fix(fact-find): KI-103 — break no_trailer fallback loop after 2 failed attempts

Browse files

Live 15-persona smoke test on 2026-05-15 caught the brain repeating the
same slot question 4-8 times in a row even after the user explicitly
answered:
A3 (Priya): age 42 stated T2, bot asked "First, your age?" 7 times
A5 (Sanjay): employer-cover answered T4, bot asked same Q 4 times
B3 (Mehul): age 45 stated T2, bot asked age 8 times despite the answer

Root cause: when the LLM brain returns a reply WITHOUT the <FF>{...}</FF>
trailer (KI-090 lenient parser still fails), _canonical_fallback runs.
Its greedy multi-slot capture only appends to profile.asked when a value
is successfully captured. When greedy fails to match anything in the
user_text, profile.asked stays unchanged and next_question(profile)
returns the SAME slot every turn — infinite re-ask loop.

Fix — per-session canonical-fallback failure counters with loop breaker:

• _ff_failed_attempts (dict[slot_id, int]) on the session counts
consecutive surfaces of an unfilled slot via canonical fallback
without a capture for that slot.
• _ff_skipped_slots (list[str]) records slots the loop-breaker has
marked SKIPPED so the orchestrator / scorecard knows they were
intentionally unanswered (not silently dropped).
• _MAX_FAILED_ATTEMPTS = 2. On the 3rd surface, the slot is marked
asked + appended to _ff_skipped_slots, then we loop next_question()
forward to the next unfilled slot. Bounded at 20 iterations so the
loop-breaker itself can never loop on a pathological state.
• A successful greedy capture for a slot pops its counter so a
temporary LLM outage that recovers doesn't permanently ghost the
slot.

This stays inside fact_find_brain.py per the KI-101 coordination note —
orchestrator.py and needs_finder.py are untouched.

Regression test tests/test_fact_find_loop_break.py — three cases:
1. Three calls with non-extracting user_text → slot on call 3 differs
from call 1 + stuck slot is recorded in _ff_skipped_slots.
2. Successful name capture pops the name failure counter.
3. Pins _MAX_FAILED_ATTEMPTS=2 so a future bump to 5+ re-enables the
live-smoke loop bug and fails this gate.

Sample 3-turn trace (with user_text="hello there how are you"):
T1: slot=name attempts={name:1} skipped=[]
T2: slot=name attempts={name:2} skipped=[]
T3: slot=age attempts={age:1} skipped=['name'] ← loop broken

Tests: 34/34 pass (31 baseline + 3 new) across test_routing_regression,
test_credits_election, test_name_persistence, test_fact_find_loop_break.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

backend/fact_find_brain.py CHANGED
@@ -337,6 +337,51 @@ def _bump_brain_history(session, slot_driving: Optional[str]) -> int:
337
  return n
338
 
339
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
340
  # ----------------------------------------------------------------------------
341
  # Public entry
342
  # ----------------------------------------------------------------------------
@@ -755,12 +800,60 @@ def _canonical_fallback(session, user_text: str, *, reason: str) -> FactFindOutc
755
  session.update_profile_field(q_obj.field, val)
756
  if slot_id not in profile.asked:
757
  profile.asked.append(slot_id)
 
 
 
 
758
  except Exception as e:
759
  logging.info("canonical_fallback greedy capture failed: %s", e)
760
 
 
 
 
 
 
 
 
 
761
  try:
762
  from backend.needs_finder import next_question
763
- q = next_question(profile)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
764
  except Exception:
765
  q = None
766
  if q is not None:
 
337
  return n
338
 
339
 
340
+ # ----------------------------------------------------------------------------
341
+ # KI-103 (2026-05-15) — Canonical-fallback loop breaker
342
+ # ----------------------------------------------------------------------------
343
+ # When the LLM brain returns `no_trailer` (or any reason that drives
344
+ # `_canonical_fallback`) on consecutive turns, the legacy code re-surfaced
345
+ # the SAME unfilled slot indefinitely. Live 15-persona smoke test caught
346
+ # the brain repeating "First, your age?" 7-8 times in a row even after the
347
+ # user explicitly stated their age. Root cause: `next_question(profile)`
348
+ # iterates GRAPH skipping ids in `profile.asked` and filled fields — but
349
+ # the greedy capture in `_canonical_fallback` only appends to `asked` when
350
+ # a value was successfully extracted, so a failed capture loop keeps the
351
+ # slot un-asked forever.
352
+ #
353
+ # Fix: track per-slot failed-fallback counts on the session. After
354
+ # `_MAX_FAILED_ATTEMPTS` (=2) consecutive surfaces of the same unfilled
355
+ # slot via canonical fallback WITHOUT capturing anything, mark the slot as
356
+ # asked (so `next_question` advances) AND record it on `session._ff_skipped_slots`
357
+ # so the orchestrator / scorecard can render it as intentionally unanswered.
358
+ #
359
+ # Counters live on the SessionState (not the Profile) — they are transient
360
+ # fact-find-turn state, not part of the persisted profile schema. Resetting
361
+ # them happens automatically on greedy-capture success below.
362
+ _MAX_FAILED_ATTEMPTS = 2
363
+
364
+
365
+ def _failed_attempts(session) -> dict[str, int]:
366
+ """Per-slot canonical-fallback failure counter, lazily attached to the
367
+ session. Maps `question.id` (NOT field name) → consecutive failure count.
368
+ """
369
+ if not hasattr(session, "_ff_failed_attempts"):
370
+ session._ff_failed_attempts = {}
371
+ return session._ff_failed_attempts
372
+
373
+
374
+ def _skipped_slots(session) -> list[str]:
375
+ """Per-session list of slot ids the loop-breaker has marked SKIPPED
376
+ after `_MAX_FAILED_ATTEMPTS` failed canonical-fallback surfaces. The
377
+ orchestrator / scorecard reads this to render the skipped slots as
378
+ intentionally unanswered (not silently dropped).
379
+ """
380
+ if not hasattr(session, "_ff_skipped_slots"):
381
+ session._ff_skipped_slots = []
382
+ return session._ff_skipped_slots
383
+
384
+
385
  # ----------------------------------------------------------------------------
386
  # Public entry
387
  # ----------------------------------------------------------------------------
 
800
  session.update_profile_field(q_obj.field, val)
801
  if slot_id not in profile.asked:
802
  profile.asked.append(slot_id)
803
+ # KI-103 — successful capture resets the failed-attempt
804
+ # counter for this slot so a temporary loss of the LLM
805
+ # brain doesn't permanently ghost the slot.
806
+ _failed_attempts(session).pop(slot_id, None)
807
  except Exception as e:
808
  logging.info("canonical_fallback greedy capture failed: %s", e)
809
 
810
+ # KI-103 (2026-05-15) — loop breaker. The legacy code surfaced the SAME
811
+ # unfilled slot every turn the brain returned `no_trailer`, even when
812
+ # the user explicitly answered it on turn 1. Now we track per-slot
813
+ # consecutive failed-fallback surfaces on the session; after
814
+ # _MAX_FAILED_ATTEMPTS consecutive surfaces of slot S with NO capture
815
+ # for S, we mark S as asked (so `next_question` advances) and append
816
+ # it to `_ff_skipped_slots` so the orchestrator/scorecard can render
817
+ # it as intentionally unanswered.
818
  try:
819
  from backend.needs_finder import next_question
820
+ attempts = _failed_attempts(session)
821
+ skipped = _skipped_slots(session)
822
+ # Bounded loop: at most len(GRAPH)+1 iterations so we can never
823
+ # infinite-loop on a pathological state. In practice we exit on
824
+ # the first slot that's either fresh OR has been skipped now.
825
+ q = None
826
+ for _ in range(20):
827
+ q = next_question(profile)
828
+ if q is None:
829
+ break
830
+ slot_id = q.id
831
+ # If this turn's greedy capture filled this slot, surface
832
+ # the NEXT slot (we already captured the answer; don't re-ask).
833
+ if q.field in captured:
834
+ profile.asked.append(slot_id) if slot_id not in profile.asked else None
835
+ continue
836
+ # Count this as a failed surface for the slot — we're about
837
+ # to re-ask it without having captured a value for it.
838
+ attempts[slot_id] = attempts.get(slot_id, 0) + 1
839
+ if attempts[slot_id] > _MAX_FAILED_ATTEMPTS:
840
+ # 3rd-or-later attempt: degrade gracefully — skip the slot
841
+ # entirely so we don't re-ask. Mark asked + record in
842
+ # _ff_skipped_slots and loop to pick the next unfilled slot.
843
+ logging.info(
844
+ "KI-103: canonical fallback skipping slot=%s after %d failed attempts",
845
+ slot_id, attempts[slot_id] - 1,
846
+ )
847
+ if slot_id not in profile.asked:
848
+ profile.asked.append(slot_id)
849
+ if slot_id not in skipped:
850
+ skipped.append(slot_id)
851
+ # Reset the counter so if we ever do capture later via a
852
+ # different code path, the slot can be re-introduced cleanly.
853
+ attempts.pop(slot_id, None)
854
+ continue
855
+ # Within tolerance — surface this slot to the user.
856
+ break
857
  except Exception:
858
  q = None
859
  if q is not None:
tests/test_fact_find_loop_break.py ADDED
@@ -0,0 +1,191 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """Regression tests for the fact-find canonical-fallback loop bug (KI-103).
2
+
3
+ Pre-fix bug
4
+ -----------
5
+ Live 15-persona smoke test on 2026-05-15 caught the brain repeating the
6
+ SAME slot question for 4-8 consecutive turns even though the user explicitly
7
+ answered on the first turn. Concrete evidence:
8
+ A3 (Priya): age 42 stated T2, bot asked "First, your age?" 7 times
9
+ A5 (Sanjay): employer-cover answered T4, bot asked the same Q 4 times
10
+ B3 (Mehul): age 45 stated T2, bot asked age 8 times despite the answer
11
+
12
+ Root cause
13
+ ----------
14
+ When the LLM brain returned a reply WITHOUT the `<FF>{...}</FF>` JSON tail
15
+ (reason=`no_trailer`), `_canonical_fallback` ran. Its greedy multi-slot
16
+ capture only appends to `profile.asked` when a value is successfully
17
+ captured. When greedy fails (user_text doesn't match any slot's strict
18
+ regex), `profile.asked` stays unchanged, so `next_question(profile)`
19
+ returns the SAME slot every turn. Loop.
20
+
21
+ Fix
22
+ ---
23
+ `_canonical_fallback` now tracks per-slot consecutive failed-fallback
24
+ surfaces on the SessionState (`_ff_failed_attempts`). After
25
+ `_MAX_FAILED_ATTEMPTS` (=2) consecutive surfaces of slot S with no
26
+ capture for S, the slot is marked asked + appended to
27
+ `_ff_skipped_slots` and the fallback advances to the next unfilled slot.
28
+
29
+ Run:
30
+ cd /Users/rohitsar/Developer/Insurance\\ Sales\\ Bot
31
+ PYTHONPATH=$PWD .venv/bin/python -m pytest tests/test_fact_find_loop_break.py -v
32
+ """
33
+
34
+ from __future__ import annotations
35
+
36
+ import unittest
37
+ import uuid
38
+ from dataclasses import dataclass, field
39
+ from typing import Optional
40
+
41
+ # Bootstrap import path so this file runs from either pytest or unittest.
42
+ import sys
43
+ from pathlib import Path
44
+ _REPO_ROOT = Path(__file__).resolve().parent.parent
45
+ if str(_REPO_ROOT) not in sys.path:
46
+ sys.path.insert(0, str(_REPO_ROOT))
47
+
48
+
49
+ def _fresh_session_id() -> str:
50
+ return f"test_ff_loop_break_{uuid.uuid4().hex[:10]}"
51
+
52
+
53
+ def _cleanup_session_file(session_id: str) -> None:
54
+ target = _REPO_ROOT / "40-data" / "sessions" / f"{session_id}.json"
55
+ if target.exists():
56
+ try:
57
+ target.unlink()
58
+ except OSError:
59
+ pass
60
+
61
+
62
+ class TestCanonicalFallbackBreaksLoop(unittest.TestCase):
63
+ """KI-103 — three consecutive `_canonical_fallback` calls with user
64
+ text that captures nothing must NOT return the same slot on call 3
65
+ as on call 1. The session-level failure counter must advance past
66
+ the stuck slot after two failed surfaces.
67
+ """
68
+
69
+ def setUp(self) -> None:
70
+ from backend.session_state import _sessions, get_session
71
+ _sessions.clear()
72
+ self.session_id = _fresh_session_id()
73
+ # Build a fresh session with name + age unset (everything unset
74
+ # actually). No awaiting_question_id, no asked slots.
75
+ self.session = get_session(self.session_id)
76
+
77
+ def tearDown(self) -> None:
78
+ _cleanup_session_file(self.session_id)
79
+
80
+ def test_three_failed_attempts_advance_slot(self) -> None:
81
+ """Drive `_canonical_fallback` 3 times with non-extracting input.
82
+ Assert the slot driven on call 3 differs from call 1 — the
83
+ loop-breaker has skipped the stuck slot and advanced.
84
+ """
85
+ from backend.fact_find_brain import _canonical_fallback
86
+
87
+ # user_text that doesn't match ANY slot's strict capture regex:
88
+ # - "name" strict mode needs intro phrase ("I'm X", "my name is X")
89
+ # - "age" needs age trigger or bare number
90
+ # - dependents / income_band / etc. need family / amount / etc. keywords
91
+ non_extracting_text = "hello there how are you"
92
+
93
+ # Call 1
94
+ out1 = _canonical_fallback(
95
+ self.session, non_extracting_text, reason="no_trailer",
96
+ )
97
+ # Call 2
98
+ out2 = _canonical_fallback(
99
+ self.session, non_extracting_text, reason="no_trailer",
100
+ )
101
+ # Call 3
102
+ out3 = _canonical_fallback(
103
+ self.session, non_extracting_text, reason="no_trailer",
104
+ )
105
+
106
+ # Sanity: all three are ambiguous canonical fallbacks.
107
+ self.assertTrue(out1.ambiguous, "Call 1 must be a canonical fallback")
108
+ self.assertTrue(out2.ambiguous, "Call 2 must be a canonical fallback")
109
+ self.assertTrue(out3.ambiguous, "Call 3 must be a canonical fallback")
110
+
111
+ # Headline assertion: by call 3 the loop-breaker has advanced past
112
+ # the stuck slot — the slot returned on call 3 MUST differ from call 1.
113
+ self.assertIsNotNone(out1.slot_driving, "Call 1 must surface a slot")
114
+ self.assertIsNotNone(out3.slot_driving, "Call 3 must surface a slot")
115
+ self.assertNotEqual(
116
+ out3.slot_driving, out1.slot_driving,
117
+ "REGRESSION (KI-103): canonical fallback re-asked the same slot "
118
+ f"on call 3 ({out3.slot_driving!r}) as on call 1 "
119
+ f"({out1.slot_driving!r}) — loop-breaker is broken. After "
120
+ "2 failed attempts the stuck slot must be marked asked and the "
121
+ "fallback must advance to the next unfilled slot.",
122
+ )
123
+
124
+ # The stuck slot from call 1 must now be in _ff_skipped_slots
125
+ # so the orchestrator/scorecard knows it's intentionally unanswered.
126
+ # `slot_driving` on the outcome is the FIELD name, not the question
127
+ # id; map back to the question id for the assertion.
128
+ from backend.fact_find_brain import FIELD_TO_QUESTION_ID
129
+ stuck_field = out1.slot_driving
130
+ stuck_qid = FIELD_TO_QUESTION_ID.get(stuck_field, stuck_field)
131
+ skipped = getattr(self.session, "_ff_skipped_slots", [])
132
+ self.assertIn(
133
+ stuck_qid, skipped,
134
+ f"REGRESSION (KI-103): stuck slot {stuck_qid!r} (field "
135
+ f"{stuck_field!r}) must be recorded on session._ff_skipped_slots "
136
+ f"so the scorecard knows it was intentionally skipped after "
137
+ f"hitting the failed-attempt threshold. Got skipped={skipped!r}.",
138
+ )
139
+
140
+ def test_successful_capture_resets_counter(self) -> None:
141
+ """A successful greedy capture for a slot must reset its failure
142
+ counter — so a temporary LLM outage that recovers doesn't
143
+ permanently ghost that slot for the session.
144
+ """
145
+ from backend.fact_find_brain import _canonical_fallback
146
+
147
+ # Call 1: non-extracting input → name surfaces, counter[name]=1.
148
+ _canonical_fallback(self.session, "hello there", reason="no_trailer")
149
+ attempts = getattr(self.session, "_ff_failed_attempts", {})
150
+ # Note: the surfaced slot id is determined by GREEDY_ORDER + GRAPH
151
+ # iteration; we don't hard-code it — just confirm a counter advanced.
152
+ self.assertTrue(
153
+ any(v >= 1 for v in attempts.values()),
154
+ "Failure counter must record the first surface as a failed attempt.",
155
+ )
156
+
157
+ # Call 2: user supplies a clear name in lenient form. The strict
158
+ # name parser requires an intro phrase, so use that explicitly.
159
+ out2 = _canonical_fallback(
160
+ self.session, "my name is Rohit", reason="no_trailer",
161
+ )
162
+ # Name should now be captured.
163
+ self.assertEqual(
164
+ self.session.profile.name, "Rohit",
165
+ "Greedy capture in canonical fallback should have extracted "
166
+ "the name from 'my name is Rohit'.",
167
+ )
168
+ # The name slot's counter must be reset (popped) after capture.
169
+ attempts_after = getattr(self.session, "_ff_failed_attempts", {})
170
+ self.assertNotIn(
171
+ "name", attempts_after,
172
+ "REGRESSION (KI-103): successful name capture must reset/pop "
173
+ "the failed-attempt counter for 'name' so the slot can be "
174
+ "re-introduced cleanly if the brain recovers.",
175
+ )
176
+
177
+ def test_failed_attempt_threshold_constant(self) -> None:
178
+ """Pin the threshold constant. If anyone tunes it back to 5+ they
179
+ have to update this test (and re-prove the loop is still broken).
180
+ """
181
+ from backend.fact_find_brain import _MAX_FAILED_ATTEMPTS
182
+ self.assertEqual(
183
+ _MAX_FAILED_ATTEMPTS, 2,
184
+ "KI-103 threshold is 2 (skip on the 3rd surface). Bumping this "
185
+ "above 2 re-enables the live-smoke-test loop bug where users "
186
+ "saw the same slot question 4-8 times in a row.",
187
+ )
188
+
189
+
190
+ if __name__ == "__main__":
191
+ unittest.main(verbosity=2)