Spaces:
Sleeping
Sleeping
Commit ·
d547d5a
1
Parent(s): f068094
docs: KI-091 + KI-094 — extractor gates + None-guard added to audit + CLAUDE + ADR-032
Browse files
70-docs/60-decisions/ADR-032-llm-chain-architecture.md
CHANGED
|
@@ -4,7 +4,7 @@
|
|
| 4 |
**Type:** Architecture reference (not a decision ADR)
|
| 5 |
**Owner:** Rohit Saraf
|
| 6 |
**Consolidates:** [ADR-019](ADR-019-nim-single-provider-consolidation.md), [ADR-026](ADR-026-provider-load-balancing.md) (superseded), [ADR-030](ADR-030-llm-driven-fact-find.md), [ADR-031](ADR-031-sticky-primary-election.md)
|
| 7 |
-
**Related KIs:** KI-079 (`87ee522`), KI-080 (`6159c54`), KI-081 (HF Space env), KI-084 (`119e0fd`), KI-085 (`8fc7979`)
|
| 8 |
|
| 9 |
> This is **not a decision ADR.** No alternatives or trade-offs are weighed here.
|
| 10 |
> ADR-032 is the single readable spec for how the LLM chain works in production
|
|
|
|
| 4 |
**Type:** Architecture reference (not a decision ADR)
|
| 5 |
**Owner:** Rohit Saraf
|
| 6 |
**Consolidates:** [ADR-019](ADR-019-nim-single-provider-consolidation.md), [ADR-026](ADR-026-provider-load-balancing.md) (superseded), [ADR-030](ADR-030-llm-driven-fact-find.md), [ADR-031](ADR-031-sticky-primary-election.md)
|
| 7 |
+
**Related KIs:** KI-079 (`87ee522`), KI-080 (`6159c54`), KI-081 (HF Space env), KI-084 (`119e0fd`), KI-085 (`8fc7979`), KI-091 (`9813994` — skip `profile_extractor` + faithfulness judge on fact-find turns; saturated-chain hang + mid-session field-clear), KI-094 (`f068094` — defensive `None`-guard in extractor merge; extractor cannot clear a filled profile field even when it runs)
|
| 8 |
|
| 9 |
> This is **not a decision ADR.** No alternatives or trade-offs are weighed here.
|
| 10 |
> ADR-032 is the single readable spec for how the LLM chain works in production
|
80-audit/ENTERPRISE_AUDIT.md
CHANGED
|
@@ -276,6 +276,8 @@ The full eval was sending **every** QA question to `needs_finder`. Sample bot an
|
|
| 276 |
| **KI-088** | **`14ee008`** | **NIM concurrency semaphore + serial probe + dropped inner retry.** Pre-KI-088 the process could fire 6+ concurrent NIM HTTP calls (probe burst `asyncio.gather` across 6 candidates + admin pollers + per-user turns), self-saturating the NIM endpoint and producing `timeout_after_escalation` failures at 41s wall-clock — the NIM endpoint serialises internally so every overlap added pure queueing latency. Three changes in one commit. (1) Module-level `asyncio.Semaphore(2)` at `backend/nvidia_nim_llm.py:104` wraps every `httpx.post` to `integrate.api.nvidia.com` so the entire process never has more than 2 NIM requests in flight simultaneously, regardless of source — probe loop, admin polls, and per-user turns all serialise through the same semaphore. (2) `backend/llm_health.py::probe_all` changed from `asyncio.gather(...)` to a serial `for m in models:` loop so the 6-NIM probe burst becomes a 1-slot trickle over ~12s instead of contending with live user turns. (3) The 4-attempt exponential-backoff inner retry inside `NvidiaNimLLM.chat()` was deleted — KI-080 sticky-primary election + KI-079 heavy-chain escalation already handle failover at the right layer, so the inner retry only amplified the self-saturation. Live verify post-deploy: failure mode flipped from `fallback:timeout_after_escalation` at 41s → `fallback:no_trailer` at 4-10s. NIM concurrency bottleneck closed; new parser-side bottleneck surfaced and is addressed in KI-090. |
|
| 277 |
| **KI-089** | **`8a87526`** | **Credits-election test fix + paired NIM-empty test.** `test_groq_above_water_picked_in_election` had been failing on `main` since KI-087 landed: it asserted Groq wins election on raw latency (161ms LPU TTFT), but KI-087 inverted election to prefer any eligible NIM candidate over all non-NIM candidates regardless of latency. Replaced with two paired tests that pin KI-087's invariant explicitly. (1) `test_nim_preferred_over_faster_groq_when_eligible` — when an eligible NIM candidate exists, election picks it as PRIMARY even though Groq is measurably faster. (2) `test_groq_picked_when_nim_pool_empty` — when every NIM candidate is dead / throttled / `no_credits`, election correctly falls through to Groq as PRIMARY. Together the pair pin both halves of the KI-087 contract (NIM-first AND fallthrough-when-NIM-empty) so a future regression that breaks either half fails loudly. Full inline test count: credits-election 12/12 pass, routing-regression 15/15 pass. |
|
| 278 |
| **KI-090** | **`11cf4b3`** | **Lenient FF-block parser.** Post-KI-088 live probe showed ~70% of brain calls returned successfully in 4-10s (NIM concurrency fix surfaced the real bottleneck) but `_parse_ff_block` rejected the reply with `fallback:no_trailer` because the brain had dropped the literal `<FF>...</FF>` tags around its JSON tail. Real LLMs under load (Qwen 3-Next 80B, Nemotron Nano 30B, Groq Llama-3.3-70B) regularly drop the wrapper even when the structured payload is otherwise contract-compliant. New `_parse_ff_block` tries three strategies in order: (a) strict `<FF>{...}</FF>` (the original contract); (b) fenced ```` ```json {...} ``` ```` (common LLM habit); (c) bare `{...}` JSON object at the end of the reply. Each candidate must `json.loads` cleanly AND contain at least one contract key (`captured` / `slot_driving` / `complete`) before it counts — prevents false positives from prose that happens to contain `{...}`. `_strip_ff_block` mirrors the three strategies in reverse so prose-only output to the user never leaks the structured metadata block. Inline tests: 7/7 parse tests + 7/7 strip tests pass. Brain success rate climbs from ~30% (post-KI-088 baseline blocked by parser) toward ~95% (NIM concurrency healthy + parser accepts contract-compliant tails regardless of wrapper). |
|
|
|
|
|
|
|
| 279 |
| D-001 | (multi) | ChromaDB HNSW bloat 3-layer prevention (ADR-029) |
|
| 280 |
| D-002 | (LaunchAgent edit) | Three silently-failing LaunchAgent scripts fixed |
|
| 281 |
| D-009 | `bcb7079` | Removed `tmp_*.py` debug files from repo root |
|
|
|
|
| 276 |
| **KI-088** | **`14ee008`** | **NIM concurrency semaphore + serial probe + dropped inner retry.** Pre-KI-088 the process could fire 6+ concurrent NIM HTTP calls (probe burst `asyncio.gather` across 6 candidates + admin pollers + per-user turns), self-saturating the NIM endpoint and producing `timeout_after_escalation` failures at 41s wall-clock — the NIM endpoint serialises internally so every overlap added pure queueing latency. Three changes in one commit. (1) Module-level `asyncio.Semaphore(2)` at `backend/nvidia_nim_llm.py:104` wraps every `httpx.post` to `integrate.api.nvidia.com` so the entire process never has more than 2 NIM requests in flight simultaneously, regardless of source — probe loop, admin polls, and per-user turns all serialise through the same semaphore. (2) `backend/llm_health.py::probe_all` changed from `asyncio.gather(...)` to a serial `for m in models:` loop so the 6-NIM probe burst becomes a 1-slot trickle over ~12s instead of contending with live user turns. (3) The 4-attempt exponential-backoff inner retry inside `NvidiaNimLLM.chat()` was deleted — KI-080 sticky-primary election + KI-079 heavy-chain escalation already handle failover at the right layer, so the inner retry only amplified the self-saturation. Live verify post-deploy: failure mode flipped from `fallback:timeout_after_escalation` at 41s → `fallback:no_trailer` at 4-10s. NIM concurrency bottleneck closed; new parser-side bottleneck surfaced and is addressed in KI-090. |
|
| 277 |
| **KI-089** | **`8a87526`** | **Credits-election test fix + paired NIM-empty test.** `test_groq_above_water_picked_in_election` had been failing on `main` since KI-087 landed: it asserted Groq wins election on raw latency (161ms LPU TTFT), but KI-087 inverted election to prefer any eligible NIM candidate over all non-NIM candidates regardless of latency. Replaced with two paired tests that pin KI-087's invariant explicitly. (1) `test_nim_preferred_over_faster_groq_when_eligible` — when an eligible NIM candidate exists, election picks it as PRIMARY even though Groq is measurably faster. (2) `test_groq_picked_when_nim_pool_empty` — when every NIM candidate is dead / throttled / `no_credits`, election correctly falls through to Groq as PRIMARY. Together the pair pin both halves of the KI-087 contract (NIM-first AND fallthrough-when-NIM-empty) so a future regression that breaks either half fails loudly. Full inline test count: credits-election 12/12 pass, routing-regression 15/15 pass. |
|
| 278 |
| **KI-090** | **`11cf4b3`** | **Lenient FF-block parser.** Post-KI-088 live probe showed ~70% of brain calls returned successfully in 4-10s (NIM concurrency fix surfaced the real bottleneck) but `_parse_ff_block` rejected the reply with `fallback:no_trailer` because the brain had dropped the literal `<FF>...</FF>` tags around its JSON tail. Real LLMs under load (Qwen 3-Next 80B, Nemotron Nano 30B, Groq Llama-3.3-70B) regularly drop the wrapper even when the structured payload is otherwise contract-compliant. New `_parse_ff_block` tries three strategies in order: (a) strict `<FF>{...}</FF>` (the original contract); (b) fenced ```` ```json {...} ``` ```` (common LLM habit); (c) bare `{...}` JSON object at the end of the reply. Each candidate must `json.loads` cleanly AND contain at least one contract key (`captured` / `slot_driving` / `complete`) before it counts — prevents false positives from prose that happens to contain `{...}`. `_strip_ff_block` mirrors the three strategies in reverse so prose-only output to the user never leaks the structured metadata block. Inline tests: 7/7 parse tests + 7/7 strip tests pass. Brain success rate climbs from ~30% (post-KI-088 baseline blocked by parser) toward ~95% (NIM concurrency healthy + parser accepts contract-compliant tails regardless of wrapper). |
|
| 279 |
+
| **KI-091** | **`9813994`** | **Skip `profile_extractor` + `faithfulness` judge on fact-find turns (saturated-chain hang + mid-session field-clear).** Pre-KI-091 every fact-find turn ran two dependent post-brain LLM chains: (a) `extract_profile_updates` (LLM re-reads the user message to pull profile fields), and (b) `check_faithfulness` (judge LLM grades the bot's reply against retrieved context). Both chains were credit-exhausted on the steady-state primary, hung the turn for 20+ seconds on `asyncio.wait_for`, and — critically — the extractor periodically returned `{"name": null}` for utterances that had nothing to do with name, causing `session.update_profile_field("name", None)` to wipe the captured value mid-session. `next_question(profile)` then re-asked the name slot the user had already answered. KI-091 gates both chains behind an `intent == "fact_find"` short-circuit in `backend/orchestrator.py` so fact-find turns skip them entirely — the fact-find brain (KI-070) already extracts fields natively from its `<FF>` JSON tail, and faithfulness scoring is meaningless on a turn whose reply is "what's your annual income?". QA-mode turns (recommendation / comparison / clarification) still run both chains as before. Live verify: name re-ask loop eliminated; fact-find turn latency drops from p95 28s → p95 6-8s. |
|
| 280 |
+
| **KI-094** | **`f068094`** | **Defensive `None`-guard in extractor merge — extractor cannot clear a filled profile field even when it runs.** Belt-and-braces companion to KI-091. On QA-mode turns the `extract_profile_updates` chain still runs (it's the right behaviour: a user can mention "I'm now 35" mid-recommendation and the profile should update). But the extractor under load periodically returns `{"name": null, "age": null, "dependents": null}` — semantically "nothing to extract" but the merge loop was writing every key including nulls back into the session, wiping filled fields. Added `if new_value in (None, "", []): continue` guard at the top of the extracted-fields loop in `backend/orchestrator.py` so the merge step ONLY overwrites a profile field when the extractor returns a real value. Null / empty-string / empty-list returns are now no-ops, regardless of which fields they target. Closes the same root-cause hole as KI-091 (LLM returning nulls clears state) at a second layer — extractor running and returning nulls is now safe even when KI-091's intent gate doesn't fire. Together: KI-091 prevents the extractor from running on fact-find turns at all; KI-094 ensures that if it DOES run (QA-mode turn), null returns can't wipe filled fields like `name`. |
|
| 281 |
| D-001 | (multi) | ChromaDB HNSW bloat 3-layer prevention (ADR-029) |
|
| 282 |
| D-002 | (LaunchAgent edit) | Three silently-failing LaunchAgent scripts fixed |
|
| 283 |
| D-009 | `bcb7079` | Removed `tmp_*.py` debug files from repo root |
|
CLAUDE.md
CHANGED
|
@@ -34,6 +34,8 @@ Every LLM role is a `NimChainLLM` candidate pool, NOT a hardcoded single model.
|
|
| 34 |
- **KI-079 escalation as last bite (`87ee522`).** If both PRIMARY and BACKUP fail in a single fact-find turn, orchestrator retries once on `BRAIN_CHAIN` (heavy brain, `_TIMEOUT_S_ESCALATION = 15s`, 35s chain budget) before falling to `_canonical_fallback` (KI-072 / KI-074 greedy slot capture). Worst-case wall-clock before canonical: 25s FAST + 15s heavy = 40s.
|
| 35 |
- **NIM concurrency semaphore + serial probe (KI-088, `14ee008`).** Module-level `asyncio.Semaphore(2)` wraps every NIM HTTP call so our process never has >2 NIM requests in flight simultaneously, regardless of source (probe loop + admin polls + per-user turns all serialise through the same semaphore). Probe loop changed parallel→serial so the 6-NIM probe burst becomes a 1-slot trickle over ~12s. Inner 4-attempt exponential-backoff retry deleted from `NvidiaNimLLM.chat()` — KI-080 election + KI-079 escalation now handle failover. Result: latency-based failures (41s timeouts under self-saturation) dropped to zero; replaced by a parser-side bottleneck (KI-090).
|
| 36 |
- **Lenient FF-block parser (KI-090, `11cf4b3`).** Real LLMs (Qwen, Nemotron under load, Groq Llama-3.3) sometimes drop the literal `<FF>...</FF>` tags around their JSON tail. Pre-KI-090 those replies fell to `fallback:no_trailer` even though the brain had produced a perfectly valid structured response. Now `_parse_ff_block` tries strict → fenced ```` ```json``` ```` → bare-JSON-tail, each candidate validated by presence of a contract key (`captured` / `slot_driving` / `complete`). `_strip_ff_block` mirrors the strategies so prose-only output never leaks structured metadata.
|
|
|
|
|
|
|
| 37 |
- **Chain budgets:** brain 20s × 35s total, fast-brain 12s × 22s total, judge 30s × 75s total. With KI-080 only PRIMARY + BACKUP consume budget in the common case — leaves headroom for KI-079 escalation. KI-084 per-phase httpx timeouts are nested inside these budgets.
|
| 38 |
- **STT/TTS/Translator** = Sarvam (Saarika v2.5 / Bulbul v2 / Sarvam-M). **Embeddings** = local BGE-small-en-v1.5.
|
| 39 |
- **Provider keys.** `NVIDIA_NIM_API_KEY` + `GROQ_API_KEY` + `OPENROUTER_API_KEY` required in `.env` (local) and HF Space environment (production — KI-081).
|
|
|
|
| 34 |
- **KI-079 escalation as last bite (`87ee522`).** If both PRIMARY and BACKUP fail in a single fact-find turn, orchestrator retries once on `BRAIN_CHAIN` (heavy brain, `_TIMEOUT_S_ESCALATION = 15s`, 35s chain budget) before falling to `_canonical_fallback` (KI-072 / KI-074 greedy slot capture). Worst-case wall-clock before canonical: 25s FAST + 15s heavy = 40s.
|
| 35 |
- **NIM concurrency semaphore + serial probe (KI-088, `14ee008`).** Module-level `asyncio.Semaphore(2)` wraps every NIM HTTP call so our process never has >2 NIM requests in flight simultaneously, regardless of source (probe loop + admin polls + per-user turns all serialise through the same semaphore). Probe loop changed parallel→serial so the 6-NIM probe burst becomes a 1-slot trickle over ~12s. Inner 4-attempt exponential-backoff retry deleted from `NvidiaNimLLM.chat()` — KI-080 election + KI-079 escalation now handle failover. Result: latency-based failures (41s timeouts under self-saturation) dropped to zero; replaced by a parser-side bottleneck (KI-090).
|
| 36 |
- **Lenient FF-block parser (KI-090, `11cf4b3`).** Real LLMs (Qwen, Nemotron under load, Groq Llama-3.3) sometimes drop the literal `<FF>...</FF>` tags around their JSON tail. Pre-KI-090 those replies fell to `fallback:no_trailer` even though the brain had produced a perfectly valid structured response. Now `_parse_ff_block` tries strict → fenced ```` ```json``` ```` → bare-JSON-tail, each candidate validated by presence of a contract key (`captured` / `slot_driving` / `complete`). `_strip_ff_block` mirrors the strategies so prose-only output never leaks structured metadata.
|
| 37 |
+
- **Skip `profile_extractor` + faithfulness judge on fact-find turns (KI-091, `9813994`).** Both chains were credit-exhausted on the steady-state primary, hung fact-find turns for 20+s, and the extractor periodically returned `{"name": null}` which wrote into `session.update_profile_field` and wiped the captured name mid-session — causing `next_question` to re-ask the name slot the user had already answered. Orchestrator now short-circuits both chains behind `intent == "fact_find"`; the fact-find brain (KI-070) extracts fields natively from its `<FF>` JSON tail, and faithfulness scoring is meaningless on "what's your annual income?". QA-mode turns still run both chains. Live: name re-ask loop gone; fact-find p95 28s → 6-8s.
|
| 38 |
+
- **Defensive `None`-guard in extractor merge (KI-094, `f068094`).** Belt-and-braces companion to KI-091. On QA-mode turns the extractor still runs (correct: "I'm now 35" mid-recommendation should update the profile), but under load it periodically returns `{"name": null, "age": null, ...}` and the merge loop was writing every key including nulls back into the session. Added `if new_value in (None, "", []): continue` at the top of the extracted-fields loop in `backend/orchestrator.py` — null / empty-string / empty-list returns are now no-ops. Closes the same root cause (LLM-returned nulls wiping state) at a second layer; KI-091 prevents the extractor from running on fact-find turns at all, KI-094 makes it safe even when it does run.
|
| 39 |
- **Chain budgets:** brain 20s × 35s total, fast-brain 12s × 22s total, judge 30s × 75s total. With KI-080 only PRIMARY + BACKUP consume budget in the common case — leaves headroom for KI-079 escalation. KI-084 per-phase httpx timeouts are nested inside these budgets.
|
| 40 |
- **STT/TTS/Translator** = Sarvam (Saarika v2.5 / Bulbul v2 / Sarvam-M). **Embeddings** = local BGE-small-en-v1.5.
|
| 41 |
- **Provider keys.** `NVIDIA_NIM_API_KEY` + `GROQ_API_KEY` + `OPENROUTER_API_KEY` required in `.env` (local) and HF Space environment (production — KI-081).
|