diff --git a/DSL/CronManager/script/service_enrichment.sh b/DSL/CronManager/script/service_enrichment.sh index c50a490a..c4bdb9d1 100644 --- a/DSL/CronManager/script/service_enrichment.sh +++ b/DSL/CronManager/script/service_enrichment.sh @@ -37,7 +37,7 @@ echo "[PACKAGES] Installing required packages..." "$UV_BIN" pip install --python "$VENV_PATH/bin/python3" "httpx>=0.27.0" || exit 1 "$UV_BIN" pip install --python "$VENV_PATH/bin/python3" "pydantic>=2.11.7" || exit 1 "$UV_BIN" pip install --python "$VENV_PATH/bin/python3" "qdrant-client>=1.15.1" || exit 1 -"$UV_BIN" pip install --python "$VENV_PATH/bin/python3" "loguru>=0.7.3" || exit 1 +"$UV_BIN" pip install --python "$VENV_PATH/bin/python3" "requests>=2.32.5" || exit 1 echo "[PACKAGES] All packages installed successfully" diff --git a/DSL/Ruuter.public/rag-search/POST/api-tools/index.yml b/DSL/Ruuter.public/rag-search/POST/api-tools/index.yml index 320e0353..7d476d46 100644 --- a/DSL/Ruuter.public/rag-search/POST/api-tools/index.yml +++ b/DSL/Ruuter.public/rag-search/POST/api-tools/index.yml @@ -80,13 +80,7 @@ execute_indexing: params: ${params} result: indexing_result on_error: handle_cron_error - next: check_indexing_status - -check_indexing_status: - switch: - - condition: ${200 <= indexing_result.response.statusCodeValue && indexing_result.response.statusCodeValue < 300} - next: assign_success - next: assign_cron_failure + next: assign_success handle_cron_error: log: "ERROR: Failed to queue api_tool indexing job - ${indexing_result.error || 'CronManager unreachable'}" @@ -98,7 +92,7 @@ assign_cron_failure: success: false error: "INDEXING_QUEUE_FAILED" message: "Failed to queue indexing job. CronManager may be unavailable." - details: ${indexing_result.error} + details: ${indexing_result.error || 'CronManager unreachable'} next: return_server_error assign_success: diff --git a/DSL/Ruuter.public/rag-search/POST/services/enrich.yml b/DSL/Ruuter.public/rag-search/POST/services/enrich.yml index 5748ad59..b177f3fe 100644 --- a/DSL/Ruuter.public/rag-search/POST/services/enrich.yml +++ b/DSL/Ruuter.public/rag-search/POST/services/enrich.yml @@ -75,13 +75,7 @@ execute_enrichment: is_common: ${service_is_common} result: enrichment_result on_error: handle_cron_error - next: check_enrichment_status - -check_enrichment_status: - switch: - - condition: ${200 <= enrichment_result.response.statusCodeValue && enrichment_result.response.statusCodeValue < 300} - next: assign_success - next: assign_cron_failure + next: assign_success handle_cron_error: log: "ERROR: Failed to queue enrichment job - ${enrichment_result.error || 'CronManager unreachable'}" @@ -93,7 +87,7 @@ assign_cron_failure: success: false error: "ENRICHMENT_QUEUE_FAILED" message: "Failed to queue enrichment job. CronManager may be unavailable." - details: ${enrichment_result.error} + details: ${enrichment_result.error || 'CronManager unreachable'} next: return_server_error assign_success: diff --git a/docs/CONTEXT_WORKFLOW_GREETING_DETECTION.md b/docs/CONTEXT_WORKFLOW_GREETING_DETECTION.md index 8a67e841..dd6990d3 100644 --- a/docs/CONTEXT_WORKFLOW_GREETING_DETECTION.md +++ b/docs/CONTEXT_WORKFLOW_GREETING_DETECTION.md @@ -2,12 +2,14 @@ ## Overview -The **Context Workflow (Layer 2)** intercepts user queries that can be answered without searching the knowledge base. It handles two categories: +The **Context Workflow (Layer 3)** intercepts user queries that can be answered without searching the knowledge base. It handles two categories: 1. **Greetings** — Detects and responds to social exchanges (hello, goodbye, thanks) in multiple languages 2. **Conversation history references** — Answers follow-up questions that refer to information already discussed in the session -When the context workflow can answer, a response is returned immediately, bypassing the RAG pipeline entirely. When it cannot answer, the query falls through to the RAG workflow (Layer 3). +Conversation history is now sourced from a **Redis-backed store** (canonical source) rather than the GUI-provided `request.conversationHistory`. The store retains the most recent 10 rounds per session and maintains an incremental summary of evicted older rounds, enabling context detection to cover the full conversation lifetime. + +When the context workflow can answer, a response is returned immediately, bypassing the RAG pipeline entirely. When it cannot answer, the query falls through to the RAG workflow (Layer 4). --- @@ -18,23 +20,31 @@ When the context workflow can answer, a response is returned immediately, bypass ``` User Query ↓ -Layer 1: SERVICE → External API calls +Layer 1: SERVICE → External API calls + ↓ (cannot handle) +Layer 2: API_TOOL_CALLING → Agentic API tool execution ↓ (cannot handle) -Layer 2: CONTEXT → Greetings + conversation history ←── This document +Layer 3: CONTEXT → Greetings + conversation history ←── This document ↓ (cannot handle) -Layer 3: RAG → Knowledge base retrieval +Layer 4: RAG → Knowledge base retrieval ↓ (cannot handle) -Layer 4: OOD → Out-of-domain fallback +Layer 5: OOD → Out-of-domain fallback ``` +> **Note**: The classifier also checks for an **active API tool session** (`chatId` present in `session_store`) before any layer evaluation. If found, the request short-circuits directly to the `API_TOOL_CALLING` workflow to continue parameter collection — the context workflow is never reached in this path. + ### Key Components | Component | File | Responsibility | |-----------|------|----------------| -| `ContextAnalyzer` | `src/tool_classifier/context_analyzer.py` | LLM-based greeting detection and context analysis | -| `ContextWorkflowExecutor` | `src/tool_classifier/workflows/context_workflow.py` | Orchestrates the workflow, handles streaming/non-streaming | +| `ContextAnalyzer` | `src/tool_classifier/context_analyzer.py` | LLM-based greeting detection, context analysis, summary generation | +| `ContextWorkflowExecutor` | `src/tool_classifier/workflows/context_workflow.py` | Orchestrates the workflow, history fetching, streaming/non-streaming | | `ToolClassifier` | `src/tool_classifier/classifier.py` | Invokes `ContextAnalyzer` during classification and routes to `ContextWorkflowExecutor` | -| `greeting_constants.py` | `src/tool_classifier/greeting_constants.py` | Fallback greeting responses for Estonian and English | +| `FeatureFlags.CONTEXT_WORKFLOW_ENABLED` | `src/llm_orchestrator_config/feature_flags.py` | Guards the context workflow; if `False`, the layer is skipped in the fallback chain and the request proceeds directly to RAG | +| `ConversationHistoryStore` | `src/utils/conversation_history_store.py` | Redis CRUD store for per-session rounds and incremental summary | +| `conversation_summary_generator` | `src/utils/conversation_summary_generator.py` | Factory for the incremental summarizer callable injected into the store | +| `redis_client` | `src/utils/redis_client.py` | Singleton async Redis client (db=1, TLS-capable) | +| `greeting_constants.py` | `src/tool_classifier/greeting_constants.py` | Static greeting response templates for Estonian and English | --- @@ -44,47 +54,105 @@ Layer 4: OOD → Out-of-domain fallback User Query + Conversation History ↓ ToolClassifier.classify() + ├─ Pre-check: active API tool session for chatId? + │ └─ Yes → short-circuit to API_TOOL_CALLING (context skipped) + │ ├─ Layer 1 (SERVICE): Embedding-based intent routing - │ └─ If no service tool matches → route to CONTEXT workflow + │ └─ If no service tool matches → try Layer 2 + │ + ├─ Layer 2 (API_TOOL_CALLING): Semantic search in api_tool_collection + │ └─ If no API tool matches (or feature disabled) → route to CONTEXT workflow │ └─ ClassificationResult(workflow=CONTEXT) ToolClassifier.route_to_workflow() ├─ Non-streaming → ContextWorkflowExecutor.execute_async() - │ ├─ Phase 1: _detect() → context_analyzer.detect_context() [classification only] - │ ├─ If greeting → return greeting OrchestrationResponse + │ ├─ _build_history() → ConversationHistoryStore.get_context() [Redis, with fallback] + │ ├─ Phase 1: _detect() → context_analyzer.detect_context_with_summary_fallback() + │ │ ├─ Step 1: detect_context() on last 10 turns + │ │ ├─ Step 2 (if needed): use pre_computed_summary from Redis OR generate summary + │ │ └─ Step 3 (if needed): _analyze_from_summary() on summary + │ ├─ If greeting → return greeting OrchestrationResponse (static template) │ ├─ If can_answer → _generate_response_async() → context_analyzer.generate_context_response() │ └─ Otherwise → return None (RAG fallback) │ └─ Streaming → ContextWorkflowExecutor.execute_streaming() - ├─ Phase 1: _detect() → context_analyzer.detect_context() [classification only] - ├─ If greeting → _stream_greeting() async generator + ├─ _build_history() → ConversationHistoryStore.get_context() [Redis, with fallback] + ├─ Phase 1: _detect() → context_analyzer.detect_context_with_summary_fallback() + ├─ If greeting → _stream_greeting() async generator (static template) ├─ If can_answer → _create_history_stream() → context_analyzer.stream_context_response() └─ Otherwise → return None (RAG fallback) ``` --- +## Redis-Backed Conversation History + +### Overview + +`ConversationHistoryStore` is a Redis-backed CRUD store (db=1) that holds per-session conversation data. It is the **canonical source of truth** for conversation history, replacing the GUI-provided `request.conversationHistory` when available. + +### Key Layout + +| Redis Key | Content | TTL | +|-----------|---------|-----| +| `conv:{chat_id}` | JSON list of up to 10 `ConversationRound` objects | 30 minutes (sliding) | +| `conv:summary:{chat_id}` | Plain-text incremental summary of evicted rounds | 30 minutes (sliding) | + +Both keys share a sliding TTL: every write resets the expiry on both keys to keep them in sync. + +### History Capping and Eviction + +The store caps history at **10 rounds** (`_MAX_ROUNDS`). When appending a new round causes the count to exceed 10, the oldest rounds are trimmed. Trimmed (evicted) rounds are passed to an optional `summarizer` callable as a fire-and-forget background `asyncio.Task`, which merges them into the running summary using `IncrementalSummarySignature`. + +### `_build_history()` — History Resolution in the Workflow + +`ContextWorkflowExecutor._build_history()` resolves the history and pre-computed summary to pass to Phase 1: + +1. If `ConversationHistoryStore` is wired in, call `get_context(chat_id)` to retrieve rounds and the optional Redis summary. +2. If rounds are present, flatten them into `{"authorRole", "message", "timestamp"}` dicts and return `(history, summary)`. +3. If the store is absent, raises, or returns no rounds → fall back to `request.conversationHistory` with `summary=None`. + +The returned `pre_computed_summary` is forwarded to `detect_context_with_summary_fallback()` to skip an expensive LLM summarisation step when Redis already has one. + +### Optimistic Locking + +`save_round()` uses Redis `WATCH`/`MULTI`/`EXEC` (optimistic locking) to detect concurrent writes and retries up to 3 times on conflict. + +--- + ## Phase 1: Detection (Classify Only) -### LLM Task +### Three-Step Detection Flow -Every query is checked against the **most recent 10 conversation turns** using a single LLM call (`detect_context()`). This phase **does not generate an answer** — it only classifies the query and extracts a relevant context snippet for Phase 2. +Every query is processed by `detect_context_with_summary_fallback()`, which implements a three-step detection pipeline: -The `ContextDetectionSignature` DSPy signature instructs the LLM to: +**Step 1 — Recent turns check (`detect_context`)** -1. Detect if the query is a greeting in any supported language -2. Check if the query references something discussed in the last 10 turns -3. If the query can be answered from history, extract the relevant snippet -4. Do **not** generate the final answer here — detection only +Runs `ContextDetectionSignature` via `dspy.ChainOfThought` against the **most recent 10 conversation turns**. This phase **does not generate an answer** — it only classifies the query and extracts a relevant context snippet for Phase 2. + +**Step 2 — Summary path (triggered when Step 1 cannot answer)** + +Triggered when `can_answer_from_context=False` AND one of the following is true: +- Total history exceeds 10 turns (older turns exist), OR +- Redis has a `pre_computed_summary` (covers evicted rounds beyond the current active window) + +Two sub-paths: +- **Redis path**: Pre-computed summary is available → used directly (no LLM call, zero cost). +- **On-demand path**: No pre-computed summary → older turns (beyond last 10) are summarised via `ConversationSummarySignature`. + +**Step 3 — Summary analysis (`_analyze_from_summary`)** + +Runs `SummaryAnalysisSignature` against the summary string to determine if the query can be answered from it. If so, the summary-derived answer is returned as `context_snippet` (with `answered_from_summary=True`) for Phase 2 generation. ### LLM Output Format -The LLM returns a JSON object parsed into `ContextDetectionResult`: +`ContextDetectionSignature` returns a JSON object parsed into `ContextDetectionResult`: ```json { "is_greeting": false, + "greeting_type": "hello", "can_answer_from_context": true, "reasoning": "User is asking about tax rate discussed earlier", "context_snippet": "Bot confirmed the flat rate is 20%, applying equally to all income brackets." @@ -94,18 +162,18 @@ The LLM returns a JSON object parsed into `ContextDetectionResult`: | Field | Type | Description | |-------|------|-------------| | `is_greeting` | `bool` | Whether the query is a greeting | +| `greeting_type` | `str` | One of `hello`, `goodbye`, `thanks`, `casual` (relevant when `is_greeting=True`) | | `can_answer_from_context` | `bool` | Whether the query can be answered from conversation history | | `reasoning` | `str` | Brief explanation of the detection decision | -| `context_snippet` | `str \| null` | Relevant excerpt from history for use in Phase 2, or `null` | - -> **Internal field**: `answered_from_summary` (bool, default `False`) is reserved for future summary-based detection paths. +| `context_snippet` | `str \| null` | Relevant excerpt from history or summary for Phase 2, or `null` | +| `answered_from_summary` | `bool` | `True` when the answer was derived from the summary path (internal, default `False`) | ### Decision After Phase 1 ``` -is_greeting=True → Phase 2: return greeting response (no LLM call) +is_greeting=True → Phase 2: return greeting response (static template, no LLM) can_answer_from_context=True AND snippet set → Phase 2: generate answer from snippet -Otherwise → Fall back to RAG +Otherwise (all steps exhausted) → Fall back to RAG ``` --- @@ -118,9 +186,12 @@ Calls `generate_context_response(query, context_snippet)` which uses `ContextRes ### Streaming (`_create_history_stream` → `stream_context_response`) -Calls `stream_context_response(query, context_snippet)` which uses DSPy native streaming (`dspy.streamify`) with `ContextResponseGenerationSignature`. Tokens are yielded in real time and passed through NeMo Guardrails before being SSE-formatted. +Calls `stream_context_response(query, context_snippet)` which uses DSPy native streaming (`dspy.streamify`) with `ContextResponseGenerationSignature`. A fresh `StreamListener` is created per call to avoid stale state. Tokens are yielded in real time and passed through NeMo Guardrails before being SSE-formatted. ---- +**Fallback chain inside `stream_context_response`:** +1. DSPy `streamify` → yield `StreamResponse` tokens as they arrive. +2. If no stream tokens received but the final `Prediction` has an answer, yield it in word-group chunks. +3. If that is also empty, call `generate_context_response()` directly and yield its result in word-group chunks. --- @@ -144,8 +215,10 @@ Calls `stream_context_response(query, context_snippet)` which uses DSPy native s ### Greeting Response Generation -Greeting detection is handled in **Phase 1 (`detect_context`)**, where the LLM classifies whether the query is a greeting and, if so, identifies the language and greeting type. This phase does **not** generate the final natural-language reply. -In **Phase 2**, `ContextWorkflowExecutor` calls `get_greeting_response(...)`, which returns a response based on predefined static templates in `greeting_constants.py`, ensuring the reply is in the detected language. If greeting detection fails or the greeting type is unsupported, the query falls through to the next workflow layer instead of attempting LLM-based greeting generation. +Greeting detection is handled in **Phase 1 (`detect_context`)**, where the LLM classifies whether the query is a greeting, identifies the `greeting_type`, and sets `is_greeting=True`. A message is only treated as a greeting if it contains **nothing beyond the greeting itself** — a greeting combined with a question is routed to RAG instead. + +In **Phase 2**, `ContextWorkflowExecutor` calls `get_greeting_response(greeting_type=..., language=...)`, which returns a static template from `greeting_constants.py`. The language is determined by `detect_language()` on the user query. No LLM call is made for greeting responses. + **Greeting response templates (`greeting_constants.py`):** ```python @@ -164,8 +237,6 @@ GREETINGS_EN = { } ``` -The fallback greeting type is determined by keyword matching in `_detect_greeting_type()` — checking for `thank/tänan/aitäh`, `bye/goodbye/nägemist/tšau`, before defaulting to `hello`. - --- ## Streaming Support @@ -174,7 +245,7 @@ The context workflow supports both response modes: ### Non-Streaming (`execute_async`) -Returns a complete `OrchestrationResponse` object with the answer as a single string. Output guardrails are applied before the response is returned. +Returns a complete `OrchestrationResponse` object with the answer as a single string. Output guardrails are applied before the response is returned. If a `pre_computed_analysis_result` is present in the classifier context, Phase 1 is skipped entirely (reuses the already-computed detection). ### Streaming (`execute_streaming`) @@ -184,6 +255,10 @@ Returns an `AsyncIterator[str]` that yields SSE (Server-Sent Events) chunks. **History responses** use DSPy native streaming (`dspy.streamify`) with `ContextResponseGenerationSignature`. Tokens are emitted in real time as they arrive from the LLM, then passed through NeMo Guardrails (`stream_with_guardrails`) before being SSE-formatted. If a guardrail violation is detected in a chunk, streaming stops and the violation message is sent instead. +### Conversation History Persistence (Streaming) + +After streaming completes, `llm_orchestration_service.py` saves a `ConversationRound` to `ConversationHistoryStore` — but **only for non-RAG workflows** (SERVICE, API_TOOL_CALLING, CONTEXT). RAG has its own internal save hook inside `_stream_rag_pipeline` and does not go through this path. The accumulated content is filtered to exclude SSE control tokens (`END`) and predefined excluded messages before saving. + **SSE Format:** ``` data: {"chatId": "abc123", "payload": {"content": "Tere! Kuidas ma"}, "timestamp": "...", "sentTo": []} @@ -199,12 +274,13 @@ data: {"chatId": "abc123", "payload": {"content": "END"}, "timestamp": "...", "s LLM token usage and cost is tracked via `get_lm_usage_since()` and stored in `costs_metric` within the workflow executor. Costs are logged via `orchestration_service.log_costs()` at the end of each execution path. -Two cost keys are tracked separately: +Two cost keys are tracked separately. When the summary fallback path is taken, its LLM calls are **merged into** `context_detection`: ```python costs_metric = { "context_detection": { - # Phase 1: detect_context() — single LLM call + # Phase 1: detect_context() + optional summary generation + summary analysis + # All summary-path costs are merged here via _merge_cost_dicts() "total_cost": 0.0012, "total_tokens": 180, "total_prompt_tokens": 150, @@ -222,9 +298,7 @@ costs_metric = { } ``` -Greeting responses skip Phase 2, so only `"context_detection"` cost is populated. - ---- +Greeting responses skip Phase 2, so only `"context_detection"` cost is populated. When the Redis pre-computed summary is used, the summary-generation cost is zero (no LLM call). --- @@ -232,10 +306,15 @@ Greeting responses skip Phase 2, so only `"context_detection"` cost is populated | Failure Point | Behaviour | |---------------|-----------| +| Redis unavailable (`ConversationHistoryStore`) | Logged as warning → falls back to `request.conversationHistory` | +| Redis fetch raises exception | Logged as warning → falls back to `request.conversationHistory` | | Phase 1 LLM call raises exception | `can_answer_from_context=False` → falls back to RAG | | Phase 1 returns invalid JSON | Logged as warning, all flags default to `False` → falls back to RAG | +| Summary generation (on-demand) fails | Logged as error → summary path skipped → falls back to RAG | +| Summary analysis returns no answer | Logged as info → falls back to RAG | | Phase 2 LLM call raises exception | Logged as error, `_generate_response_async` returns `None` → falls back to RAG | | Phase 2 returns empty answer | Logged as warning → falls back to RAG | +| All Phase 2 streaming fallbacks exhausted | Logged as error → empty response | | Output guardrails fail | Logged as warning, response returned without guardrail check | | Guardrail violation in streaming | `OUTPUT_GUARDRAIL_VIOLATION_MESSAGE` sent, stream terminated | | `orchestration_service` unavailable | History streaming skipped → `None` returned → RAG fallback | @@ -252,13 +331,21 @@ Key log entries emitted during a request: |-------|---------|------| | `INFO` | `CONTEXT WORKFLOW (NON-STREAMING) \| Query: '...'` | `execute_async()` entry | | `INFO` | `CONTEXT WORKFLOW (STREAMING) \| Query: '...'` | `execute_streaming()` entry | +| `DEBUG` | `[chatId] Using Redis history: N rounds, summary=present\|absent` | Redis history fetched successfully | +| `WARNING` | `[chatId] Redis history fetch failed, falling back to request history: ...` | Redis read error | | `INFO` | `CONTEXT DETECTOR: Phase 1 \| Query: '...' \| History: N turns` | `detect_context()` entry | | `INFO` | `DETECTION RESULT \| Greeting: ... \| Can Answer: ... \| Has snippet: ...` | Phase 1 LLM response parsed | | `INFO` | `Detection cost \| Total: $... \| Tokens: N` | After Phase 1 cost tracked | +| `INFO` | `Pre-computed summary available \| Skipping LLM summary generation, using Redis summary directly` | Redis summary reused | +| `INFO` | `History has N turns (> 10) \| Cannot answer from recent 10 \| Attempting summary-based detection` | On-demand summary path triggered | +| `INFO` | `DETECTION: Can answer from summary \| Reasoning: ...` | Summary path answered query | +| `INFO` | `Cannot answer from summary either \| Falling back to RAG` | Summary path failed | | `INFO` | `Detection: greeting=... can_answer=...` | After `_detect()` returns in executor | | `INFO` | `CONTEXT GENERATOR: Phase 2 non-streaming \| Query: '...'` | `generate_context_response()` entry | | `INFO` | `CONTEXT GENERATOR: Phase 2 streaming \| Query: '...'` | `stream_context_response()` entry | | `INFO` | `Context response streaming complete (final Prediction received)` | DSPy streaming finished | +| `WARNING` | `Stream tokens not received — yielding answer from final Prediction in chunks.` | Streaming fallback 1 | +| `WARNING` | `No answer from streamify — falling back to generate_context_response.` | Streaming fallback 2 | | `WARNING` | `[chatId] Phase 2 empty answer — fallback to RAG` | Phase 2 returned no content | | `WARNING` | `[chatId] Guardrails violation in context streaming` | Violation detected mid-stream | | `WARNING` | `[chatId] Cannot answer from context — falling back to RAG` | Neither phase could answer | @@ -267,32 +354,74 @@ Key log entries emitted during a request: ## Data Models +### `ConversationRound` (Redis storage unit) + +```python +class ConversationRound(BaseModel): + user_message: str # The user's message text + bot_message: str # The bot's response text + timestamp: float # Unix timestamp of the round +``` + +### `ConversationHistoryState` (Redis fetch result) + +```python +class ConversationHistoryState(BaseModel): + chat_id: str # Unique conversation identifier + rounds: list[ConversationRound] # Ordered rounds (newest last), capped at 10 + summary: Optional[str] # Incremental summary of evicted older rounds +``` + ### `ContextDetectionResult` (Phase 1 output) ```python class ContextDetectionResult(BaseModel): is_greeting: bool # True if query is a greeting - can_answer_from_context: bool # True if query can be answered from last 10 turns + greeting_type: str # "hello" | "goodbye" | "thanks" | "casual" + can_answer_from_context: bool # True if query can be answered from history or summary reasoning: str # LLM's brief explanation - answered_from_summary: bool # Reserved; always False in current workflow + answered_from_summary: bool # True when answer derived from summary path context_snippet: Optional[str] # Relevant excerpt for Phase 2 generation, or None ``` -### `ContextDetectionSignature` (DSPy — Phase 1) +### `ContextDetectionSignature` (DSPy — Phase 1, recent turns) | Field | Type | Description | |-------|------|-------------| | `conversation_history` | Input | Last 10 turns formatted as JSON | | `user_query` | Input | Current user query | -| `detection_result` | Output | JSON with `is_greeting`, `can_answer_from_context`, `reasoning`, `context_snippet` | +| `detection_result` | Output | JSON with `is_greeting`, `greeting_type`, `can_answer_from_context`, `reasoning`, `context_snippet` | > Detection only — **no answer generated here**. +### `ConversationSummarySignature` (DSPy — on-demand summary generation) + +| Field | Type | Description | +|-------|------|-------------| +| `conversation_history` | Input | JSON of older turns to summarize | +| `summary` | Output | Concise summary preserving key facts, names, numbers, dates | + +### `IncrementalSummarySignature` (DSPy — background eviction summary) + +| Field | Type | Description | +|-------|------|-------------| +| `existing_summary` | Input | Current summary (may be empty for first eviction) | +| `new_rounds` | Input | JSON array of just-evicted rounds | +| `updated_summary` | Output | Merged summary incorporating new rounds | + +### `SummaryAnalysisSignature` (DSPy — Phase 1, summary path) + +| Field | Type | Description | +|-------|------|-------------| +| `conversation_summary` | Input | Summary of earlier conversation | +| `user_query` | Input | Current user query | +| `analysis_result` | Output | JSON with `can_answer_from_context`, `answer`, `reasoning` | + ### `ContextResponseGenerationSignature` (DSPy — Phase 2) | Field | Type | Description | |-------|------|-------------| -| `context_snippet` | Input | Relevant excerpt from Phase 1 | +| `context_snippet` | Input | Relevant excerpt from Phase 1 (or summary-derived answer) | | `user_query` | Input | Current user query | | `answer` | Output | Natural language response in the same language as the query | @@ -302,11 +431,15 @@ class ContextDetectionResult(BaseModel): | Scenario | Phase 1 LLM Calls | Phase 2 LLM Calls | Outcome | |----------|--------------------|--------------------|---------| -| Greeting detected | 1 (`detect_context`) | 0 (static response) | Context responds (greeting) | +| Greeting detected | 1 (`detect_context`) | 0 (static template) | Context responds (greeting) | | Follow-up answerable from last 10 turns | 1 (`detect_context`) | 1 (`generate_context_response` or `stream_context_response`) | Context responds | -| Cannot answer from last 10 turns | 1 (`detect_context`) | 0 | Falls back to RAG | +| Cannot answer from 10 turns; Redis summary answers | 1 + 1 (`detect_context` + `_analyze_from_summary`) | 1 | Context responds (summary path) | +| Cannot answer from 10 turns; Redis summary reused (no new LLM call) | 1 + 1 (`detect_context` + `_analyze_from_summary`; 0 for summary gen) | 1 | Context responds (Redis summary path) | +| Cannot answer from 10 turns; on-demand summary answers | 1 + 1 + 1 (`detect_context` + `_generate_conversation_summary` + `_analyze_from_summary`) | 1 | Context responds (on-demand summary path) | +| Cannot answer from any path | 1–3 (all detection steps) | 0 | Falls back to RAG | | Phase 1 LLM error / JSON parse failure | — | 0 | Falls back to RAG | -| Phase 2 LLM error or empty answer | 1 | — | Falls back to RAG | +| Phase 2 LLM error or empty answer | 1–3 | — | Falls back to RAG | +| Redis unavailable | 0 (fallback to request history) | varies | Proceeds normally with request history | --- @@ -314,10 +447,14 @@ class ContextDetectionResult(BaseModel): | File | Purpose | |------|---------| -| `src/tool_classifier/context_analyzer.py` | Core LLM analysis logic (all three steps) | -| `src/tool_classifier/workflows/context_workflow.py` | Workflow executor (streaming + non-streaming) | +| `src/tool_classifier/context_analyzer.py` | Core LLM analysis logic (detection, summary generation, response generation) | +| `src/tool_classifier/workflows/context_workflow.py` | Workflow executor (history fetching, streaming + non-streaming) | | `src/tool_classifier/classifier.py` | Classification layer that invokes context analysis | -| `src/tool_classifier/greeting_constants.py` | Static fallback greeting responses (ET/EN) | +| `src/tool_classifier/greeting_constants.py` | Static greeting response templates (ET/EN) | +| `src/utils/conversation_history_store.py` | Redis CRUD store for rounds and incremental summary | +| `src/utils/conversation_summary_generator.py` | Factory for the incremental summarizer callable | +| `src/utils/redis_client.py` | Singleton async Redis client (db=1, TLS-capable) | +| `src/models/conversation_history_models.py` | Pydantic models: `ConversationRound`, `ConversationHistoryState` | | `tests/test_context_analyzer.py` | Unit tests for `ContextAnalyzer` | | `tests/test_context_workflow.py` | Unit tests for `ContextWorkflowExecutor` | | `tests/test_context_workflow_integration.py` | Integration tests for the full classify → route → execute chain | \ No newline at end of file diff --git a/src/api_tool_indexer/constants.py b/src/api_tool_indexer/constants.py index 43701a64..7d84095a 100644 --- a/src/api_tool_indexer/constants.py +++ b/src/api_tool_indexer/constants.py @@ -23,7 +23,9 @@ class ApiToolIndexerConstants: # LLM / Embedding API DEFAULT_API_BASE_URL = "http://llm-orchestration-service:8100" DEFAULT_ENVIRONMENT = "production" - DEFAULT_CONNECTION_ID = "" + # None → orchestration service resolves the embedding model via the + # DB-fetched vault UUID (path: embeddings/connections/{provider}/{vault_uuid}). + DEFAULT_CONNECTION_ID = None # Retry Configuration MAX_RETRIES = 3 diff --git a/src/intent_data_enrichment/api_client.py b/src/intent_data_enrichment/api_client.py index 081d0fb3..bfcd240d 100644 --- a/src/intent_data_enrichment/api_client.py +++ b/src/intent_data_enrichment/api_client.py @@ -20,7 +20,7 @@ def __init__( self, api_base_url: str = EnrichmentConstants.DEFAULT_API_BASE_URL, environment: str = EnrichmentConstants.DEFAULT_ENVIRONMENT, - connection_id: str = EnrichmentConstants.DEFAULT_CONNECTION_ID, + connection_id: Optional[str] = EnrichmentConstants.DEFAULT_CONNECTION_ID, max_retries: int = EnrichmentConstants.MAX_RETRIES, retry_delay_base: int = EnrichmentConstants.RETRY_DELAY_BASE, timeout: int = EnrichmentConstants.REQUEST_TIMEOUT, diff --git a/src/intent_data_enrichment/constants.py b/src/intent_data_enrichment/constants.py index f506880a..c85d736c 100644 --- a/src/intent_data_enrichment/constants.py +++ b/src/intent_data_enrichment/constants.py @@ -10,7 +10,9 @@ class EnrichmentConstants: # API Configuration DEFAULT_API_BASE_URL = "http://llm-orchestration-service:8100" DEFAULT_ENVIRONMENT = "production" - DEFAULT_CONNECTION_ID = "gpt-4o-mini" + # None → orchestration service resolves the embedding model via the + # DB-fetched vault UUID (path: embeddings/connections/{provider}/{vault_uuid}). + DEFAULT_CONNECTION_ID = None # Retry Configuration MAX_RETRIES = 3 diff --git a/src/llm_orchestration_service.py b/src/llm_orchestration_service.py index 543cc96f..4af9d7a3 100644 --- a/src/llm_orchestration_service.py +++ b/src/llm_orchestration_service.py @@ -60,6 +60,9 @@ from src.utils.language_detector import detect_language, get_language_name from src.utils.prompt_config_loader import PromptConfigurationLoader from src.utils.query_validator import validate_query_basic +from src.utils.sse_utils import extract_content_from_sse +from src.utils.conversation_history_store import should_save_history, save_history_round +from src.utils.conversation_history_helpers import get_conversation_history from src.guardrails import NeMoRailsAdapter, GuardrailCheckResult from src.contextual_retrieval import ContextualRetriever from src.contextual_retrieval.bm25_search import SmartBM25Search @@ -68,13 +71,27 @@ ContextualRetrievalFailureError, ) from src.llm_orchestrator_config.feature_flags import FeatureFlags -from src.tool_classifier import ToolClassifier +from src.tool_classifier import ToolClassifier, WorkflowType from src.tool_classifier.constants import SERVICE_STEP_PREFIXES from src.tool_classifier.workflows.service_workflow import ServiceWorkflowExecutor # Initialize Loki logger for orchestration service logger = LokiLogger(service_name="llm-orchestration-service") +# Set of content strings that must NOT be persisted in conversation history. +# Covers all multilingual error / OOS / guardrail-violation messages so that +# failed or blocked exchanges are never written to Redis. +_HISTORY_EXCLUDED_MESSAGES: frozenset[str] = frozenset( + { + *OUT_OF_SCOPE_MESSAGES.values(), + *TECHNICAL_ISSUE_MESSAGES.values(), + *INPUT_GUARDRAIL_VIOLATION_MESSAGES.values(), + *OUTPUT_GUARDRAIL_VIOLATION_MESSAGES.values(), + *QUERY_VALIDATION_FAILED_MESSAGES.values(), + STREAM_TOKEN_LIMIT_MESSAGE, + } +) + class LangfuseConfig: """Configuration for Langfuse integration.""" @@ -152,6 +169,11 @@ def __init__(self) -> None: # Workflow executors access it via self.orchestration_service.session_store. self.session_store: Any = None + # Redis-backed conversation history store. + # Set to None here; the FastAPI lifespan injects the live store after + # Redis initialises (app.state.orchestration_service.conversation_history_store = ...). + self.conversation_history_store: Any = None + # Shared BM25 search index pre-warmed at startup. # Populated by _prewarm_shared_bm25() which is called from the FastAPI # lifespan so it runs inside the async event loop. Until then it is None @@ -503,6 +525,18 @@ async def process_orchestration_request( }, ) langfuse.flush() + + # Persist successful exchange to conversation history (non-streaming) + if should_save_history( + self.conversation_history_store, response, _HISTORY_EXCLUDED_MESSAGES + ): + await save_history_round( + self.conversation_history_store, + request.chatId, + request.message, + response.content, + ) + return response except Exception as e: @@ -731,14 +765,51 @@ async def stream_orchestration_response( ) time_metric["classifier.route"] = time.time() - start_time + # Accumulate content for history only on non-RAG workflows; + # RAG routes through _stream_rag_pipeline which has its own hook. + _save_classifier_history = ( + self.conversation_history_store is not None + and classification.workflow != WorkflowType.RAG + ) + _classifier_accumulated: list[str] = [] + # Tracks whether an excluded marker (OOS / guardrail violation / + # error) was observed at any point during the stream. When True + # the entire accumulated buffer is discarded so no partial content + # from before the blocked marker is ever written to Redis. + _history_blocked = False + async for sse_chunk in stream_result: yield sse_chunk + if _save_classifier_history and not _history_blocked: + extracted = extract_content_from_sse(sse_chunk) + if extracted is not None and extracted != "END": + if extracted in _HISTORY_EXCLUDED_MESSAGES: + # Excluded marker observed — discard any partial + # content accumulated before this point and stop + # accumulating for the rest of the stream. + _classifier_accumulated.clear() + _history_blocked = True + else: + _classifier_accumulated.append(extracted) # Successfully completed streaming through classifier logger.info( f"[{request.chatId}] [{stream_ctx.stream_id}] Tool classifier streaming completed" ) + # Persist conversation history (classifier streaming, non-RAG workflows) + if ( + _save_classifier_history + and not _history_blocked + and _classifier_accumulated + ): + await save_history_round( + self.conversation_history_store, + request.chatId, + request.message, + "".join(_classifier_accumulated), + ) + # Log costs and timings self.log_costs(costs_metric) log_step_timings(time_metric, request.chatId) @@ -868,10 +939,16 @@ async def _stream_rag_pipeline( ) start_time = time.time() + conversation_history, conversation_summary = await get_conversation_history( + chat_id=request.chatId, + store=self.conversation_history_store, + fallback=request.conversationHistory, + ) refined_output, refiner_usage = self._refine_user_prompt( llm_manager=components["llm_manager"], original_message=request.message, - conversation_history=request.conversationHistory, + conversation_history=conversation_history, + conversation_summary=conversation_summary, ) time_metric["prompt_refiner"] = time.time() - start_time costs_metric["prompt_refiner"] = refiner_usage @@ -1173,6 +1250,17 @@ async def bot_response_generator() -> AsyncIterator[str]: f"Storage failed for chat_id: {request.chatId}, environment: {request.environment} - {str(storage_error)}" ) + # Persist conversation history (RAG streaming) + if self.conversation_history_store is not None: + _rag_bot_message = "".join(accumulated_response) + if _rag_bot_message not in _HISTORY_EXCLUDED_MESSAGES: + await save_history_round( + self.conversation_history_store, + request.chatId, + request.message, + _rag_bot_message, + ) + # Mark stream as completed successfully stream_ctx.mark_completed() @@ -1422,10 +1510,16 @@ async def _execute_orchestration_pipeline( # Step 1: Refine user prompt start_time = time.time() + conversation_history, conversation_summary = await get_conversation_history( + chat_id=request.chatId, + store=self.conversation_history_store, + fallback=request.conversationHistory, + ) refined_output, refiner_usage = self._refine_user_prompt( llm_manager=components["llm_manager"], original_message=request.message, - conversation_history=request.conversationHistory, + conversation_history=conversation_history, + conversation_summary=conversation_summary, ) timing_key = f"{prefix}.prompt_refiner" if prefix else "prompt_refiner" time_metric[timing_key] = time.time() - start_time @@ -2304,6 +2398,7 @@ def _refine_user_prompt( llm_manager: LLMManager, original_message: str, conversation_history: List[ConversationItem], + conversation_summary: Optional[str] = None, ) -> tuple[PromptRefinerOutput, Dict[str, Any]]: """ Refine user prompt using loaded LLM configuration and return usage info. @@ -2312,6 +2407,10 @@ def _refine_user_prompt( llm_manager: The LLM manager instance to use original_message: The original user message to refine conversation_history: Previous conversation context + conversation_summary: Optional summary of earlier conversation rounds + that were evicted from Redis. When provided it is prepended to the + DSPy history as a ``system`` turn so the refiner can use it for + context without re-summarising via an LLM call. Returns: Tuple of (PromptRefinerOutput, usage_dict): The refined prompt output and usage info @@ -2324,8 +2423,16 @@ def _refine_user_prompt( logger.info("Starting prompt refinement process") try: - # Convert conversation history to DSPy format + # Convert conversation history to DSPy format, optionally prepending + # a pre-computed summary of earlier (evicted) conversation rounds. history: List[Dict[str, str]] = [] + if conversation_summary: + history.append( + { + "role": "system", + "content": f"Summary of earlier conversation: {conversation_summary}", + } + ) for item in conversation_history: role = "assistant" if item.authorRole == "bot" else item.authorRole history.append({"role": role, "content": item.message}) diff --git a/src/llm_orchestration_service_api.py b/src/llm_orchestration_service_api.py index c136bdb7..df1f8427 100644 --- a/src/llm_orchestration_service_api.py +++ b/src/llm_orchestration_service_api.py @@ -1,5 +1,6 @@ """LLM Orchestration Service API - FastAPI application.""" +import asyncio import os import logging from contextlib import asynccontextmanager @@ -12,12 +13,15 @@ import uvicorn from llm_orchestration_service import LLMOrchestrationService +from llm_orchestrator_config.llm_manager import LLMManager from src.utils.redis_client import ( init_redis_client, close_redis_client, check_redis_health, ) from src.utils.api_tool_session_store import APIToolSessionStore +from src.utils.conversation_history_store import ConversationHistoryStore +from src.utils.conversation_summary_generator import create_incremental_summarizer from src.llm_orchestrator_config.llm_ochestrator_constants import ( STREAMING_ALLOWED_ENVS, STREAM_TIMEOUT_MESSAGE, @@ -98,23 +102,50 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: try: await init_redis_client() app.state.session_store = APIToolSessionStore() + + # Wire an incremental summarizer if the LLM manager singleton is available. + summarizer = None + try: + summarizer = create_incremental_summarizer(LLMManager()) + logger.info("Incremental conversation summarizer initialized") + except Exception as e: + logger.warning( + f"Could not create incremental summarizer, continuing without it: {e}" + ) + + app.state.conversation_history_store = ConversationHistoryStore( + summarizer=summarizer + ) logger.info("Redis session store initialized successfully") except Exception as e: logger.warning(f"Redis session store unavailable, continuing without it: {e}") app.state.session_store = None + app.state.conversation_history_store = None - # Expose session_store on the orchestration service so workflow executors - # (e.g. APIToolWorkflowExecutor) can reach it via self.orchestration_service. + # Expose session_store and conversation_history_store on the orchestration + # service so downstream components can reach them via self.orchestration_service. if ( hasattr(app.state, "orchestration_service") and app.state.orchestration_service is not None ): app.state.orchestration_service.session_store = app.state.session_store + app.state.orchestration_service.conversation_history_store = ( + app.state.conversation_history_store + ) yield # Shutdown logger.info("Shutting down LLM Orchestration Service API") + + # Await any in-flight incremental summary tasks to avoid lost work. + store = getattr(app.state, "conversation_history_store", None) + if store is not None and store._pending_tasks: + logger.info( + f"Waiting for {len(store._pending_tasks)} pending summary task(s) to complete..." + ) + await asyncio.gather(*store._pending_tasks, return_exceptions=True) + if ( hasattr(app.state, "orchestration_service") and app.state.orchestration_service is not None diff --git a/src/models/conversation_history_models.py b/src/models/conversation_history_models.py new file mode 100644 index 00000000..a91d8e97 --- /dev/null +++ b/src/models/conversation_history_models.py @@ -0,0 +1,45 @@ +"""Pydantic models for conversation history state.""" + +import time +from typing import Optional + +from pydantic import BaseModel, Field + + +class ConversationRound(BaseModel): + """A single user+bot exchange in a conversation. + + Stored as part of ``ConversationHistoryState`` in Redis, keyed by chat_id. + """ + + user_message: str = Field(..., description="The user's message text for this round") + bot_message: str = Field(..., description="The bot's response text for this round") + timestamp: float = Field( + default_factory=time.time, + description="Unix timestamp of when the round was recorded", + ) + + +class ConversationHistoryState(BaseModel): + """Persisted conversation history for a chat session. + + Keyed by chat_id in Redis with a sliding 30-minute TTL. + Retains up to the most recent 10 rounds; older rounds are trimmed. + An optional summary field holds a condensed representation of rounds + that have been evicted. When a summarizer is injected into the store, + it is automatically generated and persisted as rounds are evicted. + If no summarizer is provided, the summary must be managed by the caller. + """ + + chat_id: str = Field(..., description="Unique conversation identifier") + rounds: list[ConversationRound] = Field( + default_factory=list, + description="Ordered list of conversation rounds (newest last), capped at 10", + ) + summary: Optional[str] = Field( + default=None, + description=( + "Optional condensed summary of earlier conversation turns that have been " + "evicted from the rounds list. Generated and stored by the caller." + ), + ) diff --git a/src/tool_classifier/classifier.py b/src/tool_classifier/classifier.py index 3ce4b37e..9414d821 100644 --- a/src/tool_classifier/classifier.py +++ b/src/tool_classifier/classifier.py @@ -127,6 +127,9 @@ def __init__( self.context_workflow = ContextWorkflowExecutor( llm_manager=llm_manager, orchestration_service=orchestration_service, + conversation_history_store=getattr( + orchestration_service, "conversation_history_store", None + ), ) self.rag_workflow = RAGWorkflowExecutor( orchestration_service=orchestration_service, diff --git a/src/tool_classifier/context_analyzer.py b/src/tool_classifier/context_analyzer.py index d4016180..071ee36b 100644 --- a/src/tool_classifier/context_analyzer.py +++ b/src/tool_classifier/context_analyzer.py @@ -18,7 +18,7 @@ from src.utils.cost_utils import get_lm_usage_since from tool_classifier.greeting_constants import get_greeting_response -logger = LokiLogger(service_name="api-tool-calling") +logger = LokiLogger(service_name="context-workflow") def _get_current_model_name() -> str: @@ -107,6 +107,42 @@ class ConversationSummarySignature(dspy.Signature): ) +class IncrementalSummarySignature(dspy.Signature): + """Merge newly evicted conversation rounds into an existing summary. + + Given an existing summary (which may be empty for the first eviction) and a + JSON-formatted list of conversation rounds that were just evicted from the + active history window, produce an updated summary that incorporates all new + information. + + Guidelines: + - Preserve all factual details from the existing summary (names, numbers, dates). + - Integrate only new, non-redundant information from the evicted rounds. + - Keep the summary concise — omit filler, focus on actionable/memorable facts. + - Respond in the SAME language as the conversation. + """ + + existing_summary: str = dspy.InputField( + desc=( + "Current conversation summary. May be an empty string if no summary " + "exists yet (first eviction)." + ) + ) + new_rounds: str = dspy.InputField( + desc=( + "JSON array of conversation rounds that were just evicted from the " + "active history window, each with user_message, bot_message, and timestamp." + ) + ) + updated_summary: str = dspy.OutputField( + desc=( + "Updated summary that merges the existing summary with the new rounds. " + "Preserve all factual details; drop redundant information. " + "Same language as the conversation." + ) + ) + + class SummaryAnalysisSignature(dspy.Signature): """Analyze if a user query can be answered from a conversation summary. @@ -413,14 +449,18 @@ async def detect_context_with_summary_fallback( self, query: str, conversation_history: List[Dict[str, Any]], + pre_computed_summary: Optional[str] = None, ) -> tuple[ContextDetectionResult, Dict[str, Any]]: """ Phase 1 with summary fallback: detect if query can be answered from history. Implements a 3-step flow: 1. Check the last 10 turns via detect_context(). - 2. If cannot answer AND total history > 10 turns: - - Generate a concise summary of the older turns (everything before the last 10). + 2. If cannot answer AND (total history > 10 turns OR a pre-computed summary + is available from Redis): + - Use *pre_computed_summary* directly when provided (skips the expensive + LLM summarisation call). + - Otherwise generate a concise summary of the older turns. - Check whether the query can be answered from that summary. 3. If still cannot answer, return can_answer=False (workflow falls back to RAG). @@ -434,6 +474,11 @@ async def detect_context_with_summary_fallback( Args: query: User query to classify conversation_history: Full conversation history + pre_computed_summary: Running conversation summary retrieved from Redis. + When provided the LLM summary-generation step is skipped and this + value is used directly. The summary path is also attempted even + when ``total_turns <= 10`` because evicted rounds may exist in Redis + beyond what is currently in *conversation_history*. Returns: Tuple of (ContextDetectionResult, cost_dict) @@ -449,19 +494,35 @@ async def detect_context_with_summary_fallback( if result.is_greeting or result.can_answer_from_context: return result, cost_dict - # Step 2 & 3: if history exceeds 10 turns, try summary-based detection - if total_turns > 10: - logger.info( - f"History has {total_turns} turns (> 10) | " - f"Cannot answer from recent 10 | Attempting summary-based detection" - ) - older_history = conversation_history[:-10] - logger.info(f"Summarizing {len(older_history)} older turns") - + # Step 2 & 3: try summary-based detection when history is long *or* Redis + # has a pre-computed summary (which may cover evicted rounds beyond what is + # currently in memory). + if total_turns > 10 or pre_computed_summary is not None: try: - summary, summary_cost = await self._generate_conversation_summary( - older_history - ) + if pre_computed_summary is not None: + # Redis path: reuse the incremental summary, skip LLM generation. + logger.info( + "Pre-computed summary available | " + "Skipping LLM summary generation, using Redis summary directly" + ) + summary = pre_computed_summary + summary_cost: Dict[str, Any] = { + "total_cost": 0.0, + "total_tokens": 0, + "num_calls": 0, + } + else: + # On-demand path: summarise older turns via LLM. + logger.info( + f"History has {total_turns} turns (> 10) | " + f"Cannot answer from recent 10 | Attempting summary-based detection" + ) + older_history = conversation_history[:-10] + logger.info(f"Summarizing {len(older_history)} older turns") + summary, summary_cost = await self._generate_conversation_summary( + older_history + ) + cost_dict = self._merge_cost_dicts(cost_dict, summary_cost) if summary: diff --git a/src/tool_classifier/workflows/api_tool_workflow.py b/src/tool_classifier/workflows/api_tool_workflow.py index dfb03bab..d50cd691 100644 --- a/src/tool_classifier/workflows/api_tool_workflow.py +++ b/src/tool_classifier/workflows/api_tool_workflow.py @@ -32,6 +32,8 @@ from tool_classifier.enums import AgenticLoopStatus, ExecutionMode from tool_classifier.param_extractor import ParamExtractionModule from utils.api_tool_session_store import APIToolSessionStore +from utils.conversation_history_helpers import get_conversation_history +from utils.conversation_history_store import ConversationHistoryStore from utils.atc_cache_store import ATCCacheStore from tool_classifier.constants import ATC_CACHE_DEFAULT_TTL_SECONDS from tool_classifier.follow_up_detector import FollowUpDetectorModule @@ -154,6 +156,12 @@ def _get_session_store(self) -> Optional[APIToolSessionStore]: return None return getattr(self.orchestration_service, "session_store", None) + def _get_conversation_history_store(self) -> Optional[ConversationHistoryStore]: + """Return the conversation history store from the orchestration service, or None.""" + if self.orchestration_service is None: + return None + return getattr(self.orchestration_service, "conversation_history_store", None) + def _get_guardrails_adapter( self, environment: str, connection_id: Optional[str] = None ) -> Optional["NeMoRailsAdapter"]: @@ -974,14 +982,34 @@ async def _compute_loop_step( or session.detected_language ) - conversation_history_for_loop = ( - [] - if session.turn_count == 0 - else [ + _atc_conversation_summary: Optional[str] = None + conversation_history_for_loop: List[Dict[str, Any]] + if session.turn_count == 0: + # On the first ATC turn there is no prior ATC exchange to pass. + conversation_history_for_loop = [] + else: + # On subsequent turns prefer Redis as the authoritative history source. + _redis_history, _atc_conversation_summary = await get_conversation_history( + chat_id=chat_id, + store=self._get_conversation_history_store(), + fallback=list(request.conversationHistory or []), + ) + conversation_history_for_loop = [ {"authorRole": item.authorRole, "message": item.message} - for item in (request.conversationHistory or []) + for item in _redis_history ] - ) + + # Incorporate any Redis-supplied conversation summary into custom_instructions + # so the param extractor and response formatter have full conversational context. + if _atc_conversation_summary: + _summary_prefix = ( + f"Summary of earlier conversation: {_atc_conversation_summary}" + ) + custom_instructions = ( + f"{_summary_prefix}\n\n{custom_instructions}".strip() + if custom_instructions + else _summary_prefix + ) if session.execution_mode == ExecutionMode.PARALLEL.value: # Parallel path: MultiEndpointAgenticLoop operates on the full diff --git a/src/tool_classifier/workflows/context_workflow.py b/src/tool_classifier/workflows/context_workflow.py index 8ffddd5c..5352bd32 100644 --- a/src/tool_classifier/workflows/context_workflow.py +++ b/src/tool_classifier/workflows/context_workflow.py @@ -6,7 +6,9 @@ from langfuse import observe from src.loki_logger import LokiLogger +from src.models.conversation_history_models import ConversationHistoryState from src.models.request_models import OrchestrationRequest, OrchestrationResponse +from src.utils.conversation_history_store import ConversationHistoryStore from tool_classifier.base_workflow import BaseWorkflow from tool_classifier.context_analyzer import ContextAnalyzer, ContextDetectionResult from tool_classifier.workflows.service_workflow import LLMServiceProtocol @@ -14,7 +16,10 @@ from src.llm_orchestrator_config.llm_manager import LLMManager from src.utils.cost_utils import get_lm_usage_since from src.utils.language_detector import detect_language -from src.utils.observation_utils import update_observation_safe +from src.utils.observation_utils import ( + safe_observation_context, + update_observation_safe, +) from src.llm_orchestrator_config.llm_ochestrator_constants import ( GUARDRAILS_BLOCKED_PHRASES, OUTPUT_GUARDRAIL_VIOLATION_MESSAGE, @@ -51,6 +56,7 @@ def __init__( self, llm_manager: LLMManager, orchestration_service: Optional[LLMServiceProtocol] = None, + conversation_history_store: Optional[ConversationHistoryStore] = None, ) -> None: """ Initialize context workflow executor. @@ -58,15 +64,68 @@ def __init__( Args: llm_manager: LLM manager for context analysis orchestration_service: Reference to LLMOrchestrationService for cost logging + conversation_history_store: Redis-backed conversation history store; + when provided, history is read from Redis (canonical source) rather + than from ``request.conversationHistory``. """ self.llm_manager = llm_manager self.orchestration_service = orchestration_service + self.conversation_history_store = conversation_history_store self.context_analyzer = ContextAnalyzer(llm_manager) logger.info("Context workflow executor initialized") - @staticmethod - def _build_history(request: OrchestrationRequest) -> list[Dict[str, Any]]: - return [ + async def _build_history( + self, request: OrchestrationRequest + ) -> tuple[list[Dict[str, Any]], Optional[str]]: + """Fetch conversation history, preferring Redis over request payload. + + When a :class:`ConversationHistoryStore` is wired in and the session has + stored rounds, the Redis state is used as the canonical source of truth and + the GUI-provided ``request.conversationHistory`` is ignored. Any running + summary stored alongside the rounds is returned as ``pre_computed_summary`` + so that downstream callers can skip an expensive LLM summarisation step. + + If the store is absent, raises, or returns no rounds the method falls back + to the conversation history supplied in the request. + + Returns: + A ``(history_dicts, pre_computed_summary)`` tuple where + *history_dicts* is a list of ``{"authorRole", "message", "timestamp"}`` + dicts and *pre_computed_summary* is the Redis summary string or ``None``. + """ + if self.conversation_history_store is not None: + try: + state: ConversationHistoryState = ( + await self.conversation_history_store.get_context(request.chatId) + ) + if state.rounds: + history: list[Dict[str, Any]] = [] + for round_ in state.rounds: + history.append( + { + "authorRole": "user", + "message": round_.user_message, + "timestamp": str(round_.timestamp), + } + ) + history.append( + { + "authorRole": "bot", + "message": round_.bot_message, + "timestamp": str(round_.timestamp), + } + ) + logger.debug( + f"[{request.chatId}] Using Redis history: {len(state.rounds)} rounds, summary={'present' if state.summary else 'absent'}" + ) + return history, state.summary + except Exception as exc: + logger.warning( + f"[{request.chatId}] Redis history fetch failed, falling back to request history: {exc}" + ) + + # Fallback: use the conversation history supplied in the request + request_history: list[Dict[str, Any]] = [ { "authorRole": item.authorRole, "message": item.message, @@ -74,20 +133,28 @@ def _build_history(request: OrchestrationRequest) -> list[Dict[str, Any]]: } for item in request.conversationHistory ] + return request_history, None + @observe(name="context_workflow_detect", as_type="generation") async def _detect( self, message: str, history: list[Dict[str, Any]], time_metric: Dict[str, float], costs_metric: Dict[str, Dict[str, Any]], + pre_computed_summary: Optional[str] = None, ) -> Optional[ContextDetectionResult]: """Phase 1: run context detection with summary fallback. Checks the last 10 conversation turns first. If the query cannot be - answered from those and the history exceeds 10 turns, falls back to a - summary-based check over the older turns. Returns None on error so the - caller falls through to RAG. + answered from those and the history exceeds 10 turns (or a Redis summary + is available), falls back to a summary-based check. Returns None on error + so the caller falls through to RAG. + + Args: + pre_computed_summary: Running conversation summary retrieved from Redis. + When provided, the expensive LLM summarisation step is skipped and + this value is used directly. """ try: start = time.time() @@ -95,13 +162,29 @@ async def _detect( result, cost, ) = await self.context_analyzer.detect_context_with_summary_fallback( - query=message, conversation_history=history + query=message, + conversation_history=history, + pre_computed_summary=pre_computed_summary, ) time_metric["context.detection"] = time.time() - start costs_metric["context_detection"] = cost + update_observation_safe( + input_data={"query": message, "history_length": len(history)}, + output_data={ + "is_greeting": result.is_greeting if result else None, + "can_answer_from_context": result.can_answer_from_context + if result + else None, + }, + metadata={"usage": cost}, + ) return result except Exception as e: logger.error(f"Phase 1 detection failed: {e}", exc_info=True) + update_observation_safe( + output_data={"error": str(e)}, + metadata={"usage": {}}, + ) return None def _log_costs(self, costs_metric: Dict[str, Dict[str, Any]]) -> None: @@ -118,6 +201,7 @@ def _is_guardrail_violation(chunk: str) -> bool: for phrase in GUARDRAILS_BLOCKED_PHRASES ) + @observe(name="context_workflow_generate_response", as_type="generation") async def _generate_response_async( self, request: OrchestrationRequest, @@ -133,8 +217,21 @@ async def _generate_response_async( ) time_metric["context.generation"] = time.time() - start costs_metric["context_response"] = cost + update_observation_safe( + input_data={ + "chat_id": request.chatId, + "query": request.message, + "context_snippet_length": len(context_snippet), + }, + output_data={"has_answer": bool(answer)}, + metadata={"usage": cost}, + ) except Exception as e: logger.error(f"Phase 2 generation failed: {e}", exc_info=True) + update_observation_safe( + output_data={"error": str(e)}, + metadata={"usage": {}}, + ) self._log_costs(costs_metric) return None @@ -189,33 +286,70 @@ async def _stream_history_generator( if orchestration_service is None: return accumulated_response: list[str] = [] - async for validated_chunk in guardrails_adapter.stream_with_guardrails( - user_message=query, bot_message_generator=bot_generator - ): - if isinstance(validated_chunk, str) and self._is_guardrail_violation( - validated_chunk + with safe_observation_context( + as_type="generation", + name="context_workflow_streaming", + input={"query": query, "chat_id": chat_id}, + ) as _generation: + async for validated_chunk in guardrails_adapter.stream_with_guardrails( + user_message=query, bot_message_generator=bot_generator ): - logger.warning(f"[{chat_id}] Guardrails violation in context streaming") - yield orchestration_service.format_sse( - chat_id, OUTPUT_GUARDRAIL_VIOLATION_MESSAGE - ) - await orchestration_service.store_streaming_inference( - request, OUTPUT_GUARDRAIL_VIOLATION_MESSAGE - ) - yield orchestration_service.format_sse(chat_id, "END") - costs_metric["context_response"] = get_lm_usage_since( - history_length_before - ) - orchestration_service.log_costs(costs_metric) - return - accumulated_response.append(validated_chunk) - yield orchestration_service.format_sse(chat_id, validated_chunk) - final_answer = "".join(accumulated_response) - await orchestration_service.store_streaming_inference(request, final_answer) - yield orchestration_service.format_sse(chat_id, "END") - logger.info(f"[{chat_id}] Context streaming complete") - costs_metric["context_response"] = get_lm_usage_since(history_length_before) - orchestration_service.log_costs(costs_metric) + if isinstance(validated_chunk, str) and self._is_guardrail_violation( + validated_chunk + ): + logger.warning( + f"[{chat_id}] Guardrails violation in context streaming" + ) + yield orchestration_service.format_sse( + chat_id, OUTPUT_GUARDRAIL_VIOLATION_MESSAGE + ) + await orchestration_service.store_streaming_inference( + request, OUTPUT_GUARDRAIL_VIOLATION_MESSAGE + ) + yield orchestration_service.format_sse(chat_id, "END") + costs_metric["context_response"] = get_lm_usage_since( + history_length_before + ) + _usage = costs_metric["context_response"] + try: + if _generation is not None: + _generation.update( + usage_details={ + "input": _usage.get("total_prompt_tokens", 0), + "output": _usage.get("total_completion_tokens", 0), + "total": _usage.get("total_tokens", 0), + }, + cost_details={"total": _usage.get("total_cost", 0.0)}, + output={"guardrail_violation": True}, + ) + except Exception as _e: + logger.debug( + f"Langfuse streaming observation update skipped: {_e}" + ) + orchestration_service.log_costs(costs_metric) + return + accumulated_response.append(validated_chunk) + yield orchestration_service.format_sse(chat_id, validated_chunk) + final_answer = "".join(accumulated_response) + await orchestration_service.store_streaming_inference(request, final_answer) + yield orchestration_service.format_sse(chat_id, "END") + logger.info(f"[{chat_id}] Context streaming complete") + costs_metric["context_response"] = get_lm_usage_since(history_length_before) + _usage = costs_metric["context_response"] + try: + if _generation is not None: + _generation.update( + usage_details={ + "input": _usage.get("total_prompt_tokens", 0), + "output": _usage.get("total_completion_tokens", 0), + "total": _usage.get("total_tokens", 0), + }, + cost_details={"total": _usage.get("total_cost", 0.0)}, + output={"answer_preview": final_answer[:500]}, + ) + except Exception as _e: + logger.debug(f"Langfuse streaming observation update skipped: {_e}") + orchestration_service.log_costs(costs_metric) async def _create_history_stream( self, @@ -292,7 +426,7 @@ async def execute_async( time_metric = {} language = detect_language(request.message) - history = self._build_history(request) + history, pre_computed_summary = await self._build_history(request) # Check if analysis is pre-computed (e.g. from classifier classify step) pre_computed = context.get("analysis_result") @@ -310,7 +444,11 @@ async def execute_async( ) else: _detected = await self._detect( - request.message, history, time_metric, costs_metric + request.message, + history, + time_metric, + costs_metric, + pre_computed_summary, ) if _detected is None: update_observation_safe( @@ -404,10 +542,10 @@ async def execute_streaming( time_metric = {} language = detect_language(request.message) - history = self._build_history(request) + history, pre_computed_summary = await self._build_history(request) detection_result = await self._detect( - request.message, history, time_metric, costs_metric + request.message, history, time_metric, costs_metric, pre_computed_summary ) if detection_result is None: update_observation_safe( diff --git a/src/tool_classifier/workflows/service_workflow.py b/src/tool_classifier/workflows/service_workflow.py index 4b2c4572..a2490701 100644 --- a/src/tool_classifier/workflows/service_workflow.py +++ b/src/tool_classifier/workflows/service_workflow.py @@ -11,6 +11,8 @@ from llm_orchestrator_config.llm_manager import LLMManager from src.guardrails.nemo_rails_adapter import NeMoRailsAdapter +from src.utils.conversation_history_helpers import get_conversation_history +from src.utils.conversation_history_store import ConversationHistoryStore from src.utils.cost_utils import get_lm_usage_since from src.utils.observation_utils import update_observation_safe @@ -130,6 +132,12 @@ def __init__( self.llm_manager = llm_manager self.orchestration_service = orchestration_service + def _get_conversation_history_store(self) -> Optional[ConversationHistoryStore]: + """Return the conversation history store from the orchestration service, or None.""" + if self.orchestration_service is None: + return None + return getattr(self.orchestration_service, "conversation_history_store", None) + async def _semantic_search_services( self, query: str, @@ -263,9 +271,20 @@ async def _detect_service_intent( services: List[Dict[str, Any]], conversation_history: List[Any], chat_id: str, + conversation_summary: Optional[str] = None, ) -> tuple[Optional[Dict[str, Any]], Dict[str, Any]]: """Use DSPy + LLMManager to detect service intent and extract entities. + Args: + user_query: The user's query string. + services: List of available service dicts. + conversation_history: Recent conversation turns (``ConversationItem`` objects). + chat_id: Chat identifier for logging. + conversation_summary: Optional summary of earlier conversation rounds + evicted from Redis. When provided it is prepended to the history + passed to the intent detector so the LLM has additional context + without a separate summarisation call. + Returns: Tuple of (intent_result, usage_info): - intent_result: Intent detection result dict (or None on error) @@ -284,11 +303,21 @@ async def _detect_service_intent( ) intent_module = IntentDetectionModule() - history_dicts = [ - {"authorRole": msg.authorRole, "message": msg.message} - for msg in conversation_history - if hasattr(msg, "authorRole") and hasattr(msg, "message") - ] + history_dicts: List[Dict[str, str]] = [] + if conversation_summary: + history_dicts.append( + { + "authorRole": "system", + "message": f"Summary of earlier conversation: {conversation_summary}", + } + ) + history_dicts.extend( + [ + {"authorRole": msg.authorRole, "message": msg.message} + for msg in conversation_history + if hasattr(msg, "authorRole") and hasattr(msg, "message") + ] + ) with self.llm_manager.use_task_local(): intent_result = intent_module.forward( @@ -372,11 +401,17 @@ async def _process_intent_detection( context: Context dict to populate with results costs_metric: Dictionary to track LLM costs """ + conversation_history, conversation_summary = await get_conversation_history( + chat_id=request.chatId, + store=self._get_conversation_history_store(), + fallback=request.conversationHistory, + ) intent_result, intent_usage = await self._detect_service_intent( user_query=request.message, services=services, - conversation_history=request.conversationHistory, + conversation_history=conversation_history, chat_id=chat_id, + conversation_summary=conversation_summary, ) costs_metric["intent_detection"] = intent_usage diff --git a/src/utils/conversation_history_helpers.py b/src/utils/conversation_history_helpers.py new file mode 100644 index 00000000..7ceba47b --- /dev/null +++ b/src/utils/conversation_history_helpers.py @@ -0,0 +1,78 @@ +"""Shared helper for fetching conversation history from Redis. + +Mirrors the pattern established by ``ContextWorkflowExecutor._build_history()`` +so that all workflow entry points (RAG, service, ATC) resolve history in the +same way: Redis is preferred over the GUI-supplied request payload, and any +running conversation summary stored alongside the rounds is surfaced to callers +so they can skip an expensive LLM summarisation step. +""" + +from typing import List, Optional + +from src.loki_logger import LokiLogger + +from src.models.conversation_history_models import ConversationHistoryState +from models.request_models import ConversationItem +from src.utils.conversation_history_store import ConversationHistoryStore + +logger = LokiLogger(service_name="conversation-history") + + +async def get_conversation_history( + chat_id: str, + store: Optional[ConversationHistoryStore], + fallback: List[ConversationItem], +) -> tuple[List[ConversationItem], Optional[str]]: + """Fetch conversation history, preferring Redis over the request payload. + + When *store* is provided and the Redis session has rounds, those rounds are + returned as the authoritative history and *fallback* is ignored. Any running + summary attached to the stored state is returned as the second tuple element + so that callers can skip an expensive LLM summarisation step. + + If the store is absent, raises, or contains no rounds the function returns + *fallback* with ``None`` as the summary. + + Args: + chat_id: The conversation identifier. + store: Optional Redis-backed conversation history store. + fallback: ``request.conversationHistory`` — used when Redis is unavailable + or has no rounds for this session. + + Returns: + ``(history, summary)`` where *history* is a list of + :class:`~src.models.request_models.ConversationItem` objects (two per + stored round: one ``"user"`` and one ``"bot"`` item) and *summary* is + the Redis summary string or ``None``. + """ + if store is not None: + try: + state: ConversationHistoryState = await store.get_context(chat_id) + if state.rounds: + history: List[ConversationItem] = [] + for round_ in state.rounds: + history.append( + ConversationItem( + authorRole="user", + message=round_.user_message, + timestamp=str(round_.timestamp), + ) + ) + history.append( + ConversationItem( + authorRole="bot", + message=round_.bot_message, + timestamp=str(round_.timestamp), + ) + ) + logger.debug( + f"[{chat_id}] Using Redis history: {len(state.rounds)} rounds, " + f"summary={'present' if state.summary else 'absent'}" + ) + return history, state.summary + except Exception as exc: + logger.warning( + f"[{chat_id}] Redis history fetch failed, falling back to request history: {exc}" + ) + + return fallback, None diff --git a/src/utils/conversation_history_store.py b/src/utils/conversation_history_store.py new file mode 100644 index 00000000..85fb84a6 --- /dev/null +++ b/src/utils/conversation_history_store.py @@ -0,0 +1,364 @@ +"""Redis-backed conversation history store.""" + +import asyncio +import json +import weakref +from typing import TYPE_CHECKING, Optional, Union, cast + +from src.loki_logger import LokiLogger +from redis import WatchError + +from src.models.conversation_history_models import ( + ConversationHistoryState, + ConversationRound, +) +from src.utils.redis_client import get_redis_client + +if TYPE_CHECKING: + from src.models.request_models import ( + OrchestrationResponse, + TestOrchestrationResponse, + ) + from src.utils.conversation_summary_generator import SummarizerCallable + +logger = LokiLogger(service_name="conversation-history") + +_HISTORY_KEY_PREFIX = "conv:" +_SUMMARY_KEY_PREFIX = "conv:summary:" +_HISTORY_TTL_SECONDS = 1800 # 30 minutes, sliding +_MAX_ROUNDS = 10 +_APPEND_MAX_RETRIES = 3 + + +def _history_key(chat_id: str) -> str: + return f"{_HISTORY_KEY_PREFIX}{chat_id}" + + +def _summary_key(chat_id: str) -> str: + return f"{_SUMMARY_KEY_PREFIX}{chat_id}" + + +class ConversationHistoryStore: + """CRUD store for per-session conversation history backed by Redis. + + All operations are async and safe to call from FastAPI handlers. + The TTL is reset (sliding expiry) on every write that touches a key. + + Key layout (db=1, same as session store): + ``conv:{chat_id}`` — JSON list of up to 10 ``ConversationRound`` objects + ``conv:summary:{chat_id}`` — plain string summary (optional) + + An optional *summarizer* callable is injected at construction time. When + trimming evicts rounds (``len(rounds) > _MAX_ROUNDS``), a background + ``asyncio.Task`` is created to merge the evicted rounds into the existing + summary via the summarizer. If *summarizer* is ``None``, trimming still + occurs but no summary is generated. + """ + + def __init__( + self, + summarizer: Optional["SummarizerCallable"] = None, + ) -> None: + """Initialise the store. + + Args: + summarizer: Optional async callable that merges evicted rounds into + the running conversation summary. See + :func:`~src.utils.conversation_summary_generator.create_incremental_summarizer` + for a factory that creates one. + """ + self._summarizer = summarizer + # Hold strong references to background tasks to prevent GC collection + # before they complete (asyncio tasks are only weakly referenced by the + # event loop). + self._pending_tasks: set[asyncio.Task[None]] = set() + # Per-chat locks to serialize summary updates and prevent concurrent + # write races when multiple save_round() calls trigger evictions. + # WeakValueDictionary allows entries to be GC'd once no task holds a + # strong reference, preventing unbounded growth under high-cardinality + # chat_ids. + self._summary_locks: weakref.WeakValueDictionary[str, asyncio.Lock] = ( + weakref.WeakValueDictionary() + ) + + async def save_round(self, chat_id: str, round: ConversationRound) -> None: + """Append a round to the history, trim to ``_MAX_ROUNDS``, reset both TTLs. + + Uses optimistic locking (WATCH/MULTI/EXEC) to detect concurrent writes and + retries up to ``_APPEND_MAX_RETRIES`` times on conflict. + + Args: + chat_id: The conversation identifier. + round: The completed user+bot exchange to persist. + """ + client = get_redis_client() + if client is None: + logger.warning( + f"[ConversationHistoryStore] Redis unavailable - save_round({chat_id}) skipped" + ) + return + + hkey = _history_key(chat_id) + skey = _summary_key(chat_id) + + for attempt in range(_APPEND_MAX_RETRIES): + try: + async with client.pipeline(transaction=True) as pipe: + await pipe.watch(hkey) + + raw = await pipe.get(hkey) + if raw is not None: + rounds: list[dict] = json.loads(raw) + else: + rounds = [] + + rounds.append(round.model_dump()) + + # Capture rounds that will be evicted before trimming. + evicted: list[ConversationRound] = [] + if len(rounds) > _MAX_ROUNDS: + evicted = [ + ConversationRound.model_validate(r) + for r in rounds[: len(rounds) - _MAX_ROUNDS] + ] + rounds = rounds[-_MAX_ROUNDS:] + + pipe.multi() + pipe.set(hkey, json.dumps(rounds), ex=_HISTORY_TTL_SECONDS) + # Reset summary TTL without overwriting its value + pipe.expire(skey, _HISTORY_TTL_SECONDS) + await pipe.execute() + + logger.debug( + f"[ConversationHistoryStore] Round saved for chat_id={chat_id}" + ) + + # Fire-and-forget incremental summary generation for evicted rounds. + if evicted and self._summarizer is not None: + task = asyncio.create_task( + self._run_incremental_summary(chat_id, evicted) + ) + self._pending_tasks.add(task) + task.add_done_callback(self._pending_tasks.discard) + + return + + except WatchError: + logger.debug( + f"[ConversationHistoryStore] save_round({chat_id}) - concurrent modification " + f"detected, retrying (attempt {attempt + 1}/{_APPEND_MAX_RETRIES})" + ) + continue + except Exception as exc: + logger.error( + f"[ConversationHistoryStore] save_round({chat_id}) failed: {exc}" + ) + return + + logger.error( + f"[ConversationHistoryStore] save_round({chat_id}) - exhausted {_APPEND_MAX_RETRIES} retries due to " + f"concurrent writes" + ) + + def _get_summary_lock(self, chat_id: str) -> asyncio.Lock: + """Get or create a lock for serializing summary updates for this chat_id. + + Args: + chat_id: The conversation identifier. + + Returns: + An asyncio.Lock that serializes summary merges for this chat_id. + """ + if chat_id not in self._summary_locks: + self._summary_locks[chat_id] = asyncio.Lock() + return self._summary_locks[chat_id] + + async def _run_incremental_summary( + self, + chat_id: str, + evicted_rounds: list[ConversationRound], + ) -> None: + """Background task: merge *evicted_rounds* into the stored summary. + + Acquires a per-chat lock to serialize summary updates for the same + chat_id. This ensures that concurrent evictions do not lose information + due to simultaneous reads and writes. Only one summarizer runs at a time + for each chat_id, making summary merges deterministic. + + Fetches the current summary, calls the injected summarizer, and persists + the result. All exceptions are caught so the task never propagates. + + Args: + chat_id: The conversation identifier. + evicted_rounds: Rounds that were just trimmed from the active window. + """ + lock = self._get_summary_lock(chat_id) + try: + async with lock: + existing_summary = await self.get_summary(chat_id) + updated = await self._summarizer(existing_summary, evicted_rounds) # type: ignore[misc] + if updated: + await self.save_summary(chat_id, updated) + logger.debug( + f"[ConversationHistoryStore] Incremental summary updated for chat_id={chat_id}" + ) + except Exception as exc: + logger.error( + f"[ConversationHistoryStore] _run_incremental_summary({chat_id}) failed: {exc}" + ) + + async def get_history(self, chat_id: str) -> list[ConversationRound]: + """Retrieve the stored rounds for a conversation. + + Returns: + Ordered list of ``ConversationRound`` objects (newest last), + or an empty list if the key is missing or Redis is unavailable. + """ + client = get_redis_client() + if client is None: + logger.warning( + f"[ConversationHistoryStore] Redis unavailable - get_history({chat_id}) skipped" + ) + return [] + + try: + raw = await client.get(_history_key(chat_id)) + if raw is None: + return [] + return [ConversationRound.model_validate(r) for r in json.loads(raw)] + except Exception as exc: + logger.error( + f"[ConversationHistoryStore] get_history({chat_id}) failed: {exc}" + ) + return [] + + async def get_summary(self, chat_id: str) -> Optional[str]: + """Retrieve the optional summary for a conversation. + + Returns: + The summary string, or None if not set or Redis is unavailable. + """ + client = get_redis_client() + if client is None: + logger.warning( + f"[ConversationHistoryStore] Redis unavailable - get_summary({chat_id}) skipped" + ) + return None + + try: + raw = await client.get(_summary_key(chat_id)) + return raw if raw is not None else None + except Exception as exc: + logger.error( + f"[ConversationHistoryStore] get_summary({chat_id}) failed: {exc}" + ) + return None + + async def save_summary(self, chat_id: str, summary: str) -> None: + """Persist a summary string and reset the TTL on both keys. + + Args: + chat_id: The conversation identifier. + summary: The condensed text to store. + """ + client = get_redis_client() + if client is None: + logger.warning( + f"[ConversationHistoryStore] Redis unavailable - save_summary({chat_id}) skipped" + ) + return + + hkey = _history_key(chat_id) + skey = _summary_key(chat_id) + + try: + async with client.pipeline(transaction=False) as pipe: + pipe.set(skey, summary, ex=_HISTORY_TTL_SECONDS) + # Reset history key TTL to keep both keys in sync + pipe.expire(hkey, _HISTORY_TTL_SECONDS) + await pipe.execute() + logger.debug( + f"[ConversationHistoryStore] Summary saved for chat_id={chat_id}" + ) + except Exception as exc: + logger.error( + f"[ConversationHistoryStore] save_summary({chat_id}) failed: {exc}" + ) + + async def get_context(self, chat_id: str) -> ConversationHistoryState: + """Return the full conversation context (rounds + summary) for a chat. + + Fetches both keys concurrently via ``asyncio.gather``. + + Returns: + A ``ConversationHistoryState`` instance. Always succeeds — both + fields fall back to safe defaults if Redis is unavailable. + """ + rounds, summary = await asyncio.gather( + self.get_history(chat_id), + self.get_summary(chat_id), + ) + return ConversationHistoryState( + chat_id=chat_id, + rounds=rounds, + summary=summary, + ) + + +def should_save_history( + conversation_history_store: Optional["ConversationHistoryStore"], + response: Union["OrchestrationResponse", "TestOrchestrationResponse"], + excluded_messages: frozenset[str], +) -> bool: + """Return True when a successful exchange should be persisted to history. + + Args: + conversation_history_store: The active store instance, or None when Redis is unavailable. + response: The response produced by the orchestration pipeline. + excluded_messages: Set of content strings that must never be persisted (OOS, error, etc.). + """ + if conversation_history_store is None: + return False + # Use duck typing to distinguish response types, avoiding isinstance() issues + # caused by import path aliasing (models.request_models vs src.models.request_models). + # OrchestrationResponse has chatId; TestOrchestrationResponse does not. + if not hasattr(response, "chatId"): + # TestOrchestrationResponse (testing env) — skip history. + return False + + # After hasattr check, safely access chatId via cast for type safety + orch_response = cast("OrchestrationResponse", response) + if orch_response.chatId is None: + return False + if response.inputGuardFailed or response.questionOutOfLLMScope: + return False + if response.content in excluded_messages: + return False + return True + + +async def save_history_round( + store: ConversationHistoryStore, + chat_id: str, + user_message: str, + bot_message: str, +) -> None: + """Persist a completed user+bot exchange to Redis. Never raises. + + Args: + store: The active ConversationHistoryStore. + chat_id: Conversation identifier. + user_message: The user's original message. + bot_message: The bot's full response. + """ + try: + round_ = ConversationRound( + user_message=user_message, + bot_message=bot_message, + ) + await store.save_round(chat_id, round_) + logger.debug( + f"[{chat_id}] Conversation history round saved ({len(bot_message)} chars)" + ) + except Exception as exc: + logger.warning(f"[{chat_id}] Failed to save conversation history round: {exc}") diff --git a/src/utils/conversation_summary_generator.py b/src/utils/conversation_summary_generator.py new file mode 100644 index 00000000..71e1c078 --- /dev/null +++ b/src/utils/conversation_summary_generator.py @@ -0,0 +1,96 @@ +"""Factory for creating incremental conversation summarizer callables.""" + +from __future__ import annotations + +import json +from typing import Any, Protocol + +import dspy +from src.loki_logger import LokiLogger + +from src.models.conversation_history_models import ConversationRound +from tool_classifier.context_analyzer import IncrementalSummarySignature + +logger = LokiLogger(service_name="context-workflow") + + +class SummarizerCallable(Protocol): + """Protocol for incremental summary callables injected into the history store.""" + + async def __call__( + self, + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + """Merge *evicted_rounds* into *existing_summary* and return the result. + + Args: + existing_summary: The current summary string, or None / empty string + when no summary exists yet. + evicted_rounds: The rounds that were just trimmed from the active + history window. + + Returns: + The updated summary string, or an empty string on failure. + """ + ... + + +def _format_rounds_as_json(rounds: list[ConversationRound]) -> str: + """Serialise *rounds* to a compact JSON string suitable for LLM prompts.""" + return json.dumps( + [r.model_dump() for r in rounds], + ensure_ascii=False, + separators=(",", ":"), + ) + + +def create_incremental_summarizer(llm_manager: Any) -> SummarizerCallable: # noqa: ANN401 + """Return an async callable that merges evicted rounds into a running summary. + + The returned callable is safe to use as a fire-and-forget background task. + Any exception is caught and logged; the caller always receives either a + non-empty updated summary or an empty string (graceful degradation). + + Args: + llm_manager: The application-wide LLM manager instance. + + Returns: + An async callable matching the ``SummarizerCallable`` protocol. + """ + _module: dspy.Module | None = None + + async def _summarize( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + nonlocal _module + try: + rounds_json = _format_rounds_as_json(evicted_rounds) + current_summary = existing_summary or "" + + llm_manager.ensure_global_config() + with llm_manager.use_task_local(): + if _module is None: + _module = dspy.ChainOfThought(IncrementalSummarySignature) + response = _module( + existing_summary=current_summary, + new_rounds=rounds_json, + ) + + updated: str = response.updated_summary + if not updated or not updated.strip(): + logger.warning( + "[IncrementalSummarizer] LLM returned empty summary; " + "keeping existing summary unchanged." + ) + return "" + return updated.strip() + + except Exception as exc: + logger.error( + f"[IncrementalSummarizer] Failed to generate incremental summary: {exc}" + ) + return "" + + return _summarize # type: ignore[return-value] diff --git a/src/utils/sse_utils.py b/src/utils/sse_utils.py new file mode 100644 index 00000000..e0c0810e --- /dev/null +++ b/src/utils/sse_utils.py @@ -0,0 +1,16 @@ +"""Utilities for parsing Server-Sent Events (SSE) messages.""" + +import json +from typing import Optional + + +def extract_content_from_sse(sse_chunk: str) -> Optional[str]: + """Parse an SSE chunk and return payload.content, or None on failure.""" + if not sse_chunk.startswith("data: "): + return None + json_part = sse_chunk[len("data: ") :].strip() + try: + parsed = json.loads(json_part) + return parsed.get("payload", {}).get("content") + except (json.JSONDecodeError, AttributeError): + return None diff --git a/tests/test_api_semantic_searcher.py b/tests/test_api_semantic_searcher.py index f1b44040..e351d032 100644 --- a/tests/test_api_semantic_searcher.py +++ b/tests/test_api_semantic_searcher.py @@ -20,6 +20,7 @@ from tool_classifier.api_semantic_searcher import ( APISemanticSearcher, + DisambiguationResult, EndpointDisambiguatorModule, ) from tool_classifier.constants import ( @@ -160,7 +161,7 @@ def test_returns_winning_endpoint_id(self) -> None: }, ] result = module.forward("When is the next holiday?", candidates) - assert result == "ep-holidays" + assert result.winner_id == "ep-holidays" def test_returns_none_when_predictor_returns_none_string(self) -> None: module = self._make_module("none") @@ -173,7 +174,7 @@ def test_returns_none_when_predictor_returns_none_string(self) -> None: }, ] result = module.forward("Tell me a joke", candidates) - assert result is None + assert result.winner_id is None def test_returns_none_when_predictor_returns_none_uppercase(self) -> None: module = self._make_module("NONE") @@ -186,7 +187,7 @@ def test_returns_none_when_predictor_returns_none_uppercase(self) -> None: }, ] result = module.forward("Tell me a joke", candidates) - assert result is None + assert result.winner_id is None def test_returns_none_on_dspy_exception(self) -> None: module = EndpointDisambiguatorModule() @@ -200,7 +201,7 @@ def test_returns_none_on_dspy_exception(self) -> None: }, ] result = module.forward("Which holidays?", candidates) - assert result is None + assert result.winner_id is None def test_strips_whitespace_from_endpoint_id(self) -> None: module = self._make_module(" ep-holidays ") @@ -213,7 +214,7 @@ def test_strips_whitespace_from_endpoint_id(self) -> None: }, ] result = module.forward("Holidays?", candidates) - assert result == "ep-holidays" + assert result.winner_id == "ep-holidays" # --------------------------------------------------------------------------- @@ -451,7 +452,9 @@ async def test_multiple_medium_triggers_disambiguation(self) -> None: # Inject our disambiguator — searcher calls self._disambiguator(query, candidates) # which in turn calls forward() via __call__ async_disambiguator = MagicMock() - async_disambiguator.forward = MagicMock(return_value="ep-holidays") + async_disambiguator.forward = MagicMock( + return_value=DisambiguationResult(winner_id="ep-holidays") + ) searcher = _make_searcher(client, disambiguator=async_disambiguator) @@ -459,7 +462,7 @@ async def test_multiple_medium_triggers_disambiguation(self) -> None: with patch( "tool_classifier.api_semantic_searcher.asyncio.to_thread", new_callable=AsyncMock, - return_value="ep-holidays", + return_value=DisambiguationResult(winner_id="ep-holidays"), ): results = await searcher.search("something ambiguous") @@ -540,7 +543,9 @@ async def _to_thread_side_effect(fn: Any, *args: Any, **kwargs: Any) -> Any: _call_count += 1 if _call_count == 1: return precomputed # embedding call - return None # disambiguator call → rejects all candidates + return DisambiguationResult( + winner_id=None + ) # disambiguator call → rejects all candidates with patch( "tool_classifier.api_semantic_searcher.asyncio.to_thread", diff --git a/tests/test_api_tool_workflow.py b/tests/test_api_tool_workflow.py index 55483e00..e3275cdd 100644 --- a/tests/test_api_tool_workflow.py +++ b/tests/test_api_tool_workflow.py @@ -135,6 +135,7 @@ def _format_sse(chat_id: str, content: str) -> str: return f'data: {{"chatId":"{chat_id}","payload":{{"content":"{content}"}}}}\n\n' svc.format_sse = _format_sse + svc.store_streaming_inference = AsyncMock() svc.handle_output_guardrails = AsyncMock( side_effect=lambda _adapter, response, _req, _costs: response ) diff --git a/tests/test_api_tool_workflow_integration.py b/tests/test_api_tool_workflow_integration.py index 7db8a5a9..fda6d241 100644 --- a/tests/test_api_tool_workflow_integration.py +++ b/tests/test_api_tool_workflow_integration.py @@ -133,6 +133,7 @@ async def _mock_rag(**kwargs: Any) -> OrchestrationResponse: svc.handle_output_guardrails = AsyncMock( side_effect=lambda _adapter, response, _req, _costs: response ) + svc.store_streaming_inference = AsyncMock() async def _mock_rag_stream(**kwargs: Any) -> AsyncGenerator[str, None]: yield 'data: {"chatId":"test","payload":{"content":"RAG stream answer"}}\n\n' diff --git a/tests/test_conversation_history_helpers.py b/tests/test_conversation_history_helpers.py new file mode 100644 index 00000000..a5bbd61a --- /dev/null +++ b/tests/test_conversation_history_helpers.py @@ -0,0 +1,249 @@ +"""Unit tests for src.utils.conversation_history_helpers.get_conversation_history.""" + +from unittest.mock import AsyncMock + +import pytest + +from src.models.conversation_history_models import ( + ConversationHistoryState, + ConversationRound, +) +from models.request_models import ConversationItem +from src.utils.conversation_history_helpers import get_conversation_history + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_item(role: str = "user", msg: str = "hello") -> ConversationItem: + return ConversationItem( + authorRole=role, message=msg, timestamp="2024-01-01T00:00:00" + ) # type: ignore[arg-type] + + +def _make_round( + user: str = "What is the rate?", + bot: str = "The rate is 20%.", + ts: float = 1_700_000_000.0, +) -> ConversationRound: + return ConversationRound(user_message=user, bot_message=bot, timestamp=ts) + + +# --------------------------------------------------------------------------- +# Redis available with rounds +# --------------------------------------------------------------------------- + + +class TestGetConversationHistoryRedisRounds: + @pytest.mark.asyncio + async def test_returns_redis_rounds_as_conversation_items(self) -> None: + """Two ConversationItems (user + bot) per stored round.""" + round_ = _make_round() + state = ConversationHistoryState(chat_id="c1", rounds=[round_], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + fallback = [_make_item()] + history, summary = await get_conversation_history("c1", store, fallback) + + assert len(history) == 2 + assert history[0].authorRole == "user" + assert history[0].message == round_.user_message + assert history[1].authorRole == "bot" + assert history[1].message == round_.bot_message + assert summary is None + + @pytest.mark.asyncio + async def test_ignores_fallback_when_redis_has_rounds(self) -> None: + """Fallback list is not returned when Redis has valid rounds.""" + round_ = _make_round() + state = ConversationHistoryState(chat_id="c1", rounds=[round_], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + fallback = [_make_item(msg="fallback-only message")] + history, _ = await get_conversation_history("c1", store, fallback) + + messages = [item.message for item in history] + assert "fallback-only message" not in messages + + @pytest.mark.asyncio + async def test_returns_summary_alongside_rounds(self) -> None: + """Redis summary is returned as the second tuple element.""" + round_ = _make_round() + state = ConversationHistoryState( + chat_id="c1", + rounds=[round_], + summary="Earlier we discussed tax rates.", + ) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + _, summary = await get_conversation_history("c1", store, []) + + assert summary == "Earlier we discussed tax rates." + + @pytest.mark.asyncio + async def test_multiple_rounds_expand_to_all_items(self) -> None: + """N rounds → 2*N ConversationItems in order.""" + rounds = [ + _make_round(user="q1", bot="a1"), + _make_round(user="q2", bot="a2"), + _make_round(user="q3", bot="a3"), + ] + state = ConversationHistoryState(chat_id="c1", rounds=rounds, summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + history, _ = await get_conversation_history("c1", store, []) + + assert len(history) == 6 + assert history[0].message == "q1" + assert history[1].message == "a1" + assert history[4].message == "q3" + assert history[5].message == "a3" + + @pytest.mark.asyncio + async def test_timestamp_is_str_of_round_timestamp(self) -> None: + """Timestamp on returned items is the string form of the round's float timestamp.""" + round_ = _make_round(ts=1_700_000_123.456) + state = ConversationHistoryState(chat_id="c1", rounds=[round_], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + history, _ = await get_conversation_history("c1", store, []) + + assert history[0].timestamp == str(round_.timestamp) + assert history[1].timestamp == str(round_.timestamp) + + +# --------------------------------------------------------------------------- +# Redis empty → fallback +# --------------------------------------------------------------------------- + + +class TestGetConversationHistoryRedisEmpty: + @pytest.mark.asyncio + async def test_falls_back_when_redis_returns_no_rounds(self) -> None: + """Empty rounds list → fallback returned with summary=None.""" + state = ConversationHistoryState( + chat_id="c1", rounds=[], summary="stale summary" + ) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + fallback = [_make_item(msg="from request")] + history, summary = await get_conversation_history("c1", store, fallback) + + assert len(history) == 1 + assert history[0].message == "from request" + assert summary is None # summary not returned when rounds are empty + + +# --------------------------------------------------------------------------- +# Redis unavailable → graceful degradation +# --------------------------------------------------------------------------- + + +class TestGetConversationHistoryRedisUnavailable: + @pytest.mark.asyncio + async def test_falls_back_when_get_context_raises(self) -> None: + """Any exception from get_context → fallback with summary=None, no propagation.""" + store = AsyncMock() + store.get_context = AsyncMock(side_effect=RuntimeError("Redis down")) + + fallback = [_make_item(msg="fallback on error")] + history, summary = await get_conversation_history("c1", store, fallback) + + assert len(history) == 1 + assert history[0].message == "fallback on error" + assert summary is None + + @pytest.mark.asyncio + async def test_falls_back_when_store_is_none(self) -> None: + """When store=None, fallback is returned immediately without calling Redis.""" + fallback = [_make_item(role="bot", msg="bot message")] + history, summary = await get_conversation_history("c1", None, fallback) + + assert len(history) == 1 + assert history[0].authorRole == "bot" + assert summary is None + + @pytest.mark.asyncio + async def test_does_not_raise_on_connection_error(self) -> None: + """ConnectionError from Redis is caught and does not propagate.""" + store = AsyncMock() + store.get_context = AsyncMock(side_effect=ConnectionError("refused")) + + history, summary = await get_conversation_history("c1", store, []) + + assert history == [] + assert summary is None + + @pytest.mark.asyncio + async def test_get_context_called_with_correct_chat_id(self) -> None: + """The helper passes chat_id to store.get_context.""" + state = ConversationHistoryState(chat_id="my-chat", rounds=[], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + await get_conversation_history("my-chat", store, []) + + store.get_context.assert_awaited_once_with("my-chat") + + +# --------------------------------------------------------------------------- +# Conversion correctness +# --------------------------------------------------------------------------- + + +class TestGetConversationHistoryConversionCorrectness: + @pytest.mark.asyncio + async def test_returned_items_are_conversation_item_instances(self) -> None: + """All returned history items must be ConversationItem Pydantic objects.""" + round_ = _make_round() + state = ConversationHistoryState(chat_id="c1", rounds=[round_], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + history, _ = await get_conversation_history("c1", store, []) + + for item in history: + assert isinstance(item, ConversationItem) + + @pytest.mark.asyncio + async def test_user_role_is_literal_user(self) -> None: + """User item authorRole must be the literal string 'user'.""" + round_ = _make_round() + state = ConversationHistoryState(chat_id="c1", rounds=[round_], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + history, _ = await get_conversation_history("c1", store, []) + + assert history[0].authorRole == "user" + + @pytest.mark.asyncio + async def test_bot_role_is_literal_bot(self) -> None: + """Bot item authorRole must be the literal string 'bot'.""" + round_ = _make_round() + state = ConversationHistoryState(chat_id="c1", rounds=[round_], summary=None) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + history, _ = await get_conversation_history("c1", store, []) + + assert history[1].authorRole == "bot" + + @pytest.mark.asyncio + async def test_fallback_items_returned_unchanged(self) -> None: + """Fallback items are returned as-is (same objects, same content).""" + fallback = [ + _make_item(role="user", msg="user says"), + _make_item(role="bot", msg="bot replies"), + ] + history, _ = await get_conversation_history("c1", None, fallback) + + assert history is fallback diff --git a/tests/test_conversation_history_store.py b/tests/test_conversation_history_store.py new file mode 100644 index 00000000..59ab9775 --- /dev/null +++ b/tests/test_conversation_history_store.py @@ -0,0 +1,805 @@ +"""Unit tests for ConversationHistoryStore and conversation history models.""" + +import asyncio +import json +import time +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from pydantic import ValidationError +from redis import WatchError + +from src.models.conversation_history_models import ( + ConversationHistoryState, + ConversationRound, +) +from src.utils.conversation_history_store import ( + ConversationHistoryStore, + _HISTORY_TTL_SECONDS, + _MAX_ROUNDS, + _history_key, + _summary_key, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_round(**kwargs) -> ConversationRound: + defaults = { + "user_message": "Hello", + "bot_message": "Hi there!", + } + defaults.update(kwargs) + return ConversationRound(**defaults) + + +def _make_redis_mock() -> AsyncMock: + mock = AsyncMock() + mock.get = AsyncMock(return_value=None) + mock.set = AsyncMock() + mock.expire = AsyncMock() + mock.ping = AsyncMock(return_value=True) + return mock + + +def _make_pipe_mock() -> AsyncMock: + pipe = AsyncMock() + pipe.watch = AsyncMock() + pipe.unwatch = AsyncMock() + pipe.get = AsyncMock(return_value=None) + pipe.multi = MagicMock() + pipe.set = MagicMock() + pipe.expire = MagicMock() + pipe.execute = AsyncMock(return_value=[True, True]) + pipe.__aenter__ = AsyncMock(return_value=pipe) + pipe.__aexit__ = AsyncMock(return_value=False) + return pipe + + +# --------------------------------------------------------------------------- +# ConversationRound model tests +# --------------------------------------------------------------------------- + + +class TestConversationRound: + def test_required_fields(self): + r = _make_round() + assert r.user_message == "Hello" + assert r.bot_message == "Hi there!" + assert isinstance(r.timestamp, float) + + def test_timestamp_defaults_to_current_time(self): + before = time.time() + r = _make_round() + after = time.time() + assert before <= r.timestamp <= after + + def test_explicit_timestamp(self): + r = ConversationRound(user_message="u", bot_message="b", timestamp=12345.0) + assert r.timestamp == 12345.0 + + def test_user_message_required(self): + with pytest.raises(ValidationError): + ConversationRound(bot_message="b") # type: ignore[call-arg] + + def test_bot_message_required(self): + with pytest.raises(ValidationError): + ConversationRound(user_message="u") # type: ignore[call-arg] + + def test_serialization_roundtrip(self): + r = _make_round(user_message="What is the weather?", bot_message="It is sunny.") + restored = ConversationRound.model_validate_json(r.model_dump_json()) + assert restored == r + + +# --------------------------------------------------------------------------- +# ConversationHistoryState model tests +# --------------------------------------------------------------------------- + + +class TestConversationHistoryState: + def test_defaults(self): + state = ConversationHistoryState(chat_id="chat-1") + assert state.rounds == [] + assert state.summary is None + + def test_chat_id_required(self): + with pytest.raises(ValidationError): + ConversationHistoryState() # type: ignore[call-arg] + + def test_serialization_roundtrip(self): + state = ConversationHistoryState( + chat_id="chat-2", + rounds=[_make_round(), _make_round(user_message="q2", bot_message="a2")], + summary="User asked about weather and holidays.", + ) + restored = ConversationHistoryState.model_validate_json(state.model_dump_json()) + assert restored == state + assert len(restored.rounds) == 2 + assert restored.summary == "User asked about weather and holidays." + + +# --------------------------------------------------------------------------- +# Key helper tests +# --------------------------------------------------------------------------- + + +class TestKeyHelpers: + def test_history_key(self): + assert _history_key("abc") == "conv:abc" + + def test_summary_key(self): + assert _summary_key("abc") == "conv:summary:abc" + + +# --------------------------------------------------------------------------- +# ConversationHistoryStore.save_round +# --------------------------------------------------------------------------- + + +class TestSaveRound: + @pytest.mark.asyncio + async def test_save_round_appends_and_sets_ttl(self): + store = ConversationHistoryStore() + round_ = _make_round() + + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=None) # no existing rounds + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-1", round_) + + pipe.watch.assert_awaited_once_with(_history_key("chat-1")) + pipe.multi.assert_called_once() + pipe.set.assert_called_once() + set_call = pipe.set.call_args + assert set_call[0][0] == _history_key("chat-1") + assert set_call[1]["ex"] == _HISTORY_TTL_SECONDS + # Verify stored JSON contains the round + stored = json.loads(set_call[0][1]) + assert len(stored) == 1 + assert stored[0]["user_message"] == round_.user_message + + @pytest.mark.asyncio + async def test_save_round_appends_to_existing_rounds(self): + store = ConversationHistoryStore() + existing = [ + _make_round(user_message=f"q{i}", bot_message=f"a{i}").model_dump() + for i in range(3) + ] + new_round = _make_round(user_message="q_new", bot_message="a_new") + + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=json.dumps(existing)) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-2", new_round) + + stored_json = pipe.set.call_args[0][1] + stored = json.loads(stored_json) + assert len(stored) == 4 + assert stored[-1]["user_message"] == "q_new" + + @pytest.mark.asyncio + async def test_save_round_trims_to_max_rounds(self): + store = ConversationHistoryStore() + # Start with exactly MAX_ROUNDS rounds + existing = [ + _make_round(user_message=f"q{i}", bot_message=f"a{i}").model_dump() + for i in range(_MAX_ROUNDS) + ] + new_round = _make_round(user_message="q_overflow", bot_message="a_overflow") + + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=json.dumps(existing)) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-3", new_round) + + stored_json = pipe.set.call_args[0][1] + stored = json.loads(stored_json) + assert len(stored) == _MAX_ROUNDS + # Oldest round was evicted; newest is last + assert stored[-1]["user_message"] == "q_overflow" + assert stored[0]["user_message"] == "q1" + + @pytest.mark.asyncio + async def test_save_round_resets_summary_ttl(self): + store = ConversationHistoryStore() + round_ = _make_round() + + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=None) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-4", round_) + + pipe.expire.assert_called_once_with( + _summary_key("chat-4"), _HISTORY_TTL_SECONDS + ) + + @pytest.mark.asyncio + async def test_save_round_skips_when_redis_unavailable(self): + store = ConversationHistoryStore() + with patch( + "src.utils.conversation_history_store.get_redis_client", return_value=None + ): + # Should not raise + await store.save_round("chat-x", _make_round()) + + @pytest.mark.asyncio + async def test_save_round_retries_on_watch_error(self): + store = ConversationHistoryStore() + round_ = _make_round() + + call_count = 0 + + async def fake_execute(): + nonlocal call_count + call_count += 1 + if call_count < 3: + raise WatchError("conflict") + return [True, True] + + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=None) + pipe.execute = fake_execute + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-5", round_) + + assert call_count == 3 + + @pytest.mark.asyncio + async def test_save_round_exhausts_retries_gracefully(self): + store = ConversationHistoryStore() + round_ = _make_round() + + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=None) + pipe.execute = AsyncMock(side_effect=WatchError("always conflicts")) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + # Should not raise after exhausting retries + await store.save_round("chat-6", round_) + + @pytest.mark.asyncio + async def test_save_round_graceful_on_unexpected_error(self): + store = ConversationHistoryStore() + + pipe = _make_pipe_mock() + pipe.watch = AsyncMock(side_effect=RuntimeError("boom")) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-7", _make_round()) + + +# --------------------------------------------------------------------------- +# ConversationHistoryStore.get_history +# --------------------------------------------------------------------------- + + +class TestGetHistory: + @pytest.mark.asyncio + async def test_returns_empty_list_when_key_missing(self): + store = ConversationHistoryStore() + redis_mock = _make_redis_mock() + redis_mock.get = AsyncMock(return_value=None) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + result = await store.get_history("missing-chat") + + assert result == [] + + @pytest.mark.asyncio + async def test_returns_deserialized_rounds(self): + store = ConversationHistoryStore() + rounds = [ + _make_round(user_message="q1", bot_message="a1"), + _make_round(user_message="q2", bot_message="a2"), + ] + redis_mock = _make_redis_mock() + redis_mock.get = AsyncMock( + return_value=json.dumps([r.model_dump() for r in rounds]) + ) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + result = await store.get_history("chat-1") + + assert len(result) == 2 + assert result[0].user_message == "q1" + assert result[1].user_message == "q2" + + @pytest.mark.asyncio + async def test_returns_empty_list_when_redis_unavailable(self): + store = ConversationHistoryStore() + with patch( + "src.utils.conversation_history_store.get_redis_client", return_value=None + ): + result = await store.get_history("any-chat") + + assert result == [] + + @pytest.mark.asyncio + async def test_returns_empty_list_on_error(self): + store = ConversationHistoryStore() + redis_mock = _make_redis_mock() + redis_mock.get = AsyncMock(side_effect=RuntimeError("connection lost")) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + result = await store.get_history("chat-err") + + assert result == [] + + +# --------------------------------------------------------------------------- +# ConversationHistoryStore.get_summary +# --------------------------------------------------------------------------- + + +class TestGetSummary: + @pytest.mark.asyncio + async def test_returns_none_when_key_missing(self): + store = ConversationHistoryStore() + redis_mock = _make_redis_mock() + redis_mock.get = AsyncMock(return_value=None) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + result = await store.get_summary("chat-1") + + assert result is None + + @pytest.mark.asyncio + async def test_returns_summary_string(self): + store = ConversationHistoryStore() + redis_mock = _make_redis_mock() + redis_mock.get = AsyncMock(return_value="User asked about holidays.") + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + result = await store.get_summary("chat-1") + + assert result == "User asked about holidays." + + @pytest.mark.asyncio + async def test_returns_none_when_redis_unavailable(self): + store = ConversationHistoryStore() + with patch( + "src.utils.conversation_history_store.get_redis_client", return_value=None + ): + result = await store.get_summary("any-chat") + + assert result is None + + @pytest.mark.asyncio + async def test_returns_none_on_error(self): + store = ConversationHistoryStore() + redis_mock = _make_redis_mock() + redis_mock.get = AsyncMock(side_effect=RuntimeError("boom")) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + result = await store.get_summary("chat-err") + + assert result is None + + +# --------------------------------------------------------------------------- +# ConversationHistoryStore.save_summary +# --------------------------------------------------------------------------- + + +class TestSaveSummary: + @pytest.mark.asyncio + async def test_save_summary_sets_key_with_ttl(self): + store = ConversationHistoryStore() + + pipe = _make_pipe_mock() + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_summary("chat-1", "Some summary text.") + + pipe.set.assert_called_once() + set_call = pipe.set.call_args + assert set_call[0][0] == _summary_key("chat-1") + assert set_call[0][1] == "Some summary text." + assert set_call[1]["ex"] == _HISTORY_TTL_SECONDS + + @pytest.mark.asyncio + async def test_save_summary_resets_history_key_ttl(self): + store = ConversationHistoryStore() + + pipe = _make_pipe_mock() + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_summary("chat-2", "Summary.") + + pipe.expire.assert_called_once_with( + _history_key("chat-2"), _HISTORY_TTL_SECONDS + ) + + @pytest.mark.asyncio + async def test_save_summary_skips_when_redis_unavailable(self): + store = ConversationHistoryStore() + with patch( + "src.utils.conversation_history_store.get_redis_client", return_value=None + ): + await store.save_summary("chat-x", "text") + + @pytest.mark.asyncio + async def test_save_summary_graceful_on_error(self): + store = ConversationHistoryStore() + pipe = _make_pipe_mock() + pipe.execute = AsyncMock(side_effect=RuntimeError("io error")) + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_summary("chat-err", "text") + + +# --------------------------------------------------------------------------- +# ConversationHistoryStore.get_context +# --------------------------------------------------------------------------- + + +class TestGetContext: + @pytest.mark.asyncio + async def test_get_context_combines_history_and_summary(self): + store = ConversationHistoryStore() + rounds = [_make_round(user_message="hello", bot_message="hi")] + summary_text = "Earlier the user greeted the bot." + + async def _fake_get_history(chat_id: str): + return rounds + + async def _fake_get_summary(chat_id: str): + return summary_text + + with ( + patch.object(store, "get_history", side_effect=_fake_get_history), + patch.object(store, "get_summary", side_effect=_fake_get_summary), + ): + result = await store.get_context("chat-1") + + assert isinstance(result, ConversationHistoryState) + assert result.chat_id == "chat-1" + assert result.rounds == rounds + assert result.summary == summary_text + + @pytest.mark.asyncio + async def test_get_context_returns_empty_defaults_when_redis_unavailable(self): + store = ConversationHistoryStore() + with patch( + "src.utils.conversation_history_store.get_redis_client", return_value=None + ): + result = await store.get_context("chat-gone") + + assert isinstance(result, ConversationHistoryState) + assert result.chat_id == "chat-gone" + assert result.rounds == [] + assert result.summary is None + + @pytest.mark.asyncio + async def test_get_context_fetches_concurrently(self): + """Both sub-calls must run; verify gather behaviour by checking both are awaited.""" + store = ConversationHistoryStore() + history_called = False + summary_called = False + + async def _hist(chat_id: str): + nonlocal history_called + history_called = True + return [] + + async def _summ(chat_id: str): + nonlocal summary_called + summary_called = True + return None + + with ( + patch.object(store, "get_history", side_effect=_hist), + patch.object(store, "get_summary", side_effect=_summ), + ): + await store.get_context("chat-concurrent") + + assert history_called + assert summary_called + + +# --------------------------------------------------------------------------- +# Incremental summary: save_round eviction triggering +# --------------------------------------------------------------------------- + + +class TestSaveRoundIncrementalSummary: + """Tests for the fire-and-forget summarizer integration in save_round.""" + + @staticmethod + def _make_pipe_with_existing(rounds_data: list[dict]) -> AsyncMock: + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=json.dumps(rounds_data)) + return pipe + + @pytest.mark.asyncio + async def test_eviction_triggers_background_task(self): + """When len(rounds) exceeds _MAX_ROUNDS, summarizer is called with the + evicted round(s) and the existing summary.""" + received_summary: list[str | None] = [] + received_evicted: list[list] = [] + + async def mock_summarizer( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + received_summary.append(existing_summary) + received_evicted.append(list(evicted_rounds)) + return "merged summary" + + store = ConversationHistoryStore(summarizer=mock_summarizer) + + # Pre-populate with exactly _MAX_ROUNDS rounds so the new one triggers eviction. + existing = [ + _make_round(user_message=f"q{i}", bot_message=f"a{i}").model_dump() + for i in range(_MAX_ROUNDS) + ] + pipe = self._make_pipe_with_existing(existing) + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + new_round = _make_round(user_message="q_new", bot_message="a_new") + + with ( + patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ), + patch.object(store, "_get_summary_lock", return_value=asyncio.Lock()), + patch.object(store, "get_summary", AsyncMock(return_value="old summary")), + patch.object(store, "save_summary", AsyncMock()), + ): + await store.save_round("chat-evict", new_round) + # Drain the event loop so the background task completes. + await asyncio.gather(*store._pending_tasks, return_exceptions=True) + + assert len(received_evicted) == 1 + assert len(received_evicted[0]) == 1 + assert received_evicted[0][0].user_message == "q0" + assert received_summary[0] == "old summary" + + @pytest.mark.asyncio + async def test_no_eviction_does_not_trigger_summarizer(self): + """When rounds stay within _MAX_ROUNDS, the summarizer is never called.""" + called = False + + async def mock_summarizer( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + nonlocal called + called = True + return "should not be called" + + store = ConversationHistoryStore(summarizer=mock_summarizer) + + # Start with fewer than _MAX_ROUNDS rounds. + existing = [ + _make_round(user_message=f"q{i}", bot_message=f"a{i}").model_dump() + for i in range(_MAX_ROUNDS - 2) + ] + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=json.dumps(existing)) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-no-evict", _make_round()) + + assert not called + assert len(store._pending_tasks) == 0 + + @pytest.mark.asyncio + async def test_no_summarizer_no_task_on_eviction(self): + """Store without a summarizer still trims correctly; no tasks scheduled.""" + store = ConversationHistoryStore(summarizer=None) + + existing = [ + _make_round(user_message=f"q{i}", bot_message=f"a{i}").model_dump() + for i in range(_MAX_ROUNDS) + ] + pipe = _make_pipe_mock() + pipe.get = AsyncMock(return_value=json.dumps(existing)) + + redis_mock = _make_redis_mock() + redis_mock.pipeline = MagicMock(return_value=pipe) + + with patch( + "src.utils.conversation_history_store.get_redis_client", + return_value=redis_mock, + ): + await store.save_round("chat-no-summarizer", _make_round()) + + assert len(store._pending_tasks) == 0 + + # Verify that trimming still happened. + stored_json = pipe.set.call_args[0][1] + stored = json.loads(stored_json) + assert len(stored) == _MAX_ROUNDS + + +# --------------------------------------------------------------------------- +# Incremental summary: _run_incremental_summary +# --------------------------------------------------------------------------- + + +class TestRunIncrementalSummary: + """Tests for the _run_incremental_summary private method.""" + + @pytest.mark.asyncio + async def test_calls_save_summary_with_merged_result(self): + """Happy path: summarizer returns non-empty string → save_summary called.""" + + async def mock_summarizer( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + return "merged: " + (existing_summary or "") + " + new info" + + store = ConversationHistoryStore(summarizer=mock_summarizer) + evicted = [_make_round(user_message="old q", bot_message="old a")] + + with ( + patch.object(store, "_get_summary_lock", return_value=asyncio.Lock()), + patch.object(store, "get_summary", AsyncMock(return_value="prior summary")), + patch.object(store, "save_summary", AsyncMock()) as mock_save, + ): + await store._run_incremental_summary("chat-1", evicted) + + mock_save.assert_awaited_once_with("chat-1", "merged: prior summary + new info") + + @pytest.mark.asyncio + async def test_skips_save_when_summarizer_returns_empty(self): + """If summarizer returns empty string, save_summary must NOT be called.""" + + async def mock_summarizer( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + return "" + + store = ConversationHistoryStore(summarizer=mock_summarizer) + evicted = [_make_round()] + + with ( + patch.object(store, "_get_summary_lock", return_value=asyncio.Lock()), + patch.object(store, "get_summary", AsyncMock(return_value=None)), + patch.object(store, "save_summary", AsyncMock()) as mock_save, + ): + await store._run_incremental_summary("chat-2", evicted) + + mock_save.assert_not_awaited() + + @pytest.mark.asyncio + async def test_summarizer_exception_does_not_propagate(self): + """A failing summarizer must be caught; the method returns cleanly.""" + + async def exploding_summarizer( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + raise RuntimeError("LLM unavailable") + + store = ConversationHistoryStore(summarizer=exploding_summarizer) + evicted = [_make_round()] + + with ( + patch.object(store, "_get_summary_lock", return_value=asyncio.Lock()), + patch.object(store, "get_summary", AsyncMock(return_value=None)), + patch.object(store, "save_summary", AsyncMock()) as mock_save, + ): + # Must not raise. + await store._run_incremental_summary("chat-3", evicted) + + mock_save.assert_not_awaited() + + @pytest.mark.asyncio + async def test_passes_none_existing_summary_when_no_summary_stored(self): + """When get_summary returns None, summarizer receives None as first arg.""" + received: list[str | None] = [] + + async def capture_summarizer( + existing_summary: str | None, + evicted_rounds: list[ConversationRound], + ) -> str: + received.append(existing_summary) + return "new summary" + + store = ConversationHistoryStore(summarizer=capture_summarizer) + evicted = [_make_round()] + + with ( + patch.object(store, "_get_summary_lock", return_value=asyncio.Lock()), + patch.object(store, "get_summary", AsyncMock(return_value=None)), + patch.object(store, "save_summary", AsyncMock()), + ): + await store._run_incremental_summary("chat-4", evicted) + + assert received == [None] diff --git a/tests/test_direct_step_executor.py b/tests/test_direct_step_executor.py index be351ecf..f10cca0b 100644 --- a/tests/test_direct_step_executor.py +++ b/tests/test_direct_step_executor.py @@ -171,6 +171,7 @@ class TestExecuteDirectStepStreaming: async def test_yields_content_and_end(self) -> None: """Valid prefix → yields exactly 2 SSE chunks (content, END).""" mock_sse = MagicMock() + mock_sse.store_streaming_inference = AsyncMock() mock_sse.format_sse = MagicMock(side_effect=["sse_content", "sse_end"]) executor = _make_executor(orchestration_service=mock_sse) @@ -188,6 +189,7 @@ async def test_yields_content_and_end(self) -> None: async def test_format_sse_called_with_buttons(self) -> None: """format_sse receives content and buttons on first call, 'END' on second.""" mock_sse = MagicMock() + mock_sse.store_streaming_inference = AsyncMock() mock_sse.format_sse = MagicMock(return_value="data: ...\n\n") executor = _make_executor(orchestration_service=mock_sse) diff --git a/tests/test_history_integration.py b/tests/test_history_integration.py new file mode 100644 index 00000000..09d8c321 --- /dev/null +++ b/tests/test_history_integration.py @@ -0,0 +1,1466 @@ +"""Unit tests for conversation history integration in LLMOrchestrationService. + +Tests cover: +- should_save_history(): filtering logic for when to persist rounds (standalone function) +- save_history_round(): Redis persistence with error isolation (standalone function) +- _extract_content_from_sse(): SSE chunk parsing +- Non-streaming hook in process_orchestration_request() +- RAG streaming hook in _stream_rag_pipeline() (accumulated_response saved after END) +- Classifier streaming hook in stream_orchestration_response() (non-RAG workflows) +- ContextWorkflowExecutor._build_history(): Redis-first history retrieval +- ContextAnalyzer.detect_context_with_summary_fallback(): pre_computed_summary fast path +""" + +import json +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from src.llm_orchestration_service import ( + LLMOrchestrationService, + _HISTORY_EXCLUDED_MESSAGES, +) +from src.llm_orchestrator_config.llm_ochestrator_constants import ( + INPUT_GUARDRAIL_VIOLATION_MESSAGE, + OUT_OF_SCOPE_MESSAGE, + OUTPUT_GUARDRAIL_VIOLATION_MESSAGE, + TECHNICAL_ISSUE_MESSAGE, +) +from src.models.conversation_history_models import ( + ConversationHistoryState, + ConversationRound, +) +from src.utils.conversation_history_store import should_save_history, save_history_round +from src.utils.sse_utils import extract_content_from_sse + +# Use the same import path as llm_orchestration_service.py uses internally +# (``from models.request_models import ...``) to avoid the Python dual-import +# problem where isinstance() fails across two module-path aliases. +from models.request_models import OrchestrationResponse, TestOrchestrationResponse + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_service() -> LLMOrchestrationService: + """Return a bare LLMOrchestrationService instance with __init__ bypassed.""" + svc: LLMOrchestrationService = object.__new__(LLMOrchestrationService) + svc.conversation_history_store = None # default off + return svc + + +def _make_ok_response(content: str = "Here is the answer.") -> OrchestrationResponse: + return OrchestrationResponse( + chatId="chat-1", + llmServiceActive=True, + questionOutOfLLMScope=False, + inputGuardFailed=False, + content=content, + ) + + +def _make_sse(chat_id: str, content: str) -> str: + payload = { + "chatId": chat_id, + "payload": {"content": content}, + "timestamp": "1234567890", + "sentTo": [], + } + return f"data: {json.dumps(payload)}\n\n" + + +# --------------------------------------------------------------------------- +# _HISTORY_EXCLUDED_MESSAGES content +# --------------------------------------------------------------------------- + + +class TestHistoryExcludedMessages: + def test_english_out_of_scope_excluded(self): + assert OUT_OF_SCOPE_MESSAGE in _HISTORY_EXCLUDED_MESSAGES + + def test_english_technical_issue_excluded(self): + assert TECHNICAL_ISSUE_MESSAGE in _HISTORY_EXCLUDED_MESSAGES + + def test_english_input_guardrail_excluded(self): + assert INPUT_GUARDRAIL_VIOLATION_MESSAGE in _HISTORY_EXCLUDED_MESSAGES + + def test_english_output_guardrail_excluded(self): + assert OUTPUT_GUARDRAIL_VIOLATION_MESSAGE in _HISTORY_EXCLUDED_MESSAGES + + def test_normal_answer_not_excluded(self): + assert "Here is the answer." not in _HISTORY_EXCLUDED_MESSAGES + + def test_set_is_frozenset(self): + assert isinstance(_HISTORY_EXCLUDED_MESSAGES, frozenset) + + +# --------------------------------------------------------------------------- +# _should_save_history +# --------------------------------------------------------------------------- + + +class TestShouldSaveHistory: + def test_returns_false_when_store_is_none(self): + assert ( + should_save_history(None, _make_ok_response(), _HISTORY_EXCLUDED_MESSAGES) + is False + ) + + def test_returns_false_for_test_orchestration_response(self): + test_resp = TestOrchestrationResponse( + llmServiceActive=True, + questionOutOfLLMScope=False, + inputGuardFailed=False, + content="answer", + ) + assert ( + should_save_history(MagicMock(), test_resp, _HISTORY_EXCLUDED_MESSAGES) + is False + ) + + def test_returns_false_when_input_guard_failed(self): + resp = OrchestrationResponse( + chatId="c1", + llmServiceActive=True, + questionOutOfLLMScope=False, + inputGuardFailed=True, + content=INPUT_GUARDRAIL_VIOLATION_MESSAGE, + ) + assert ( + should_save_history(MagicMock(), resp, _HISTORY_EXCLUDED_MESSAGES) is False + ) + + def test_returns_false_when_out_of_scope(self): + resp = OrchestrationResponse( + chatId="c1", + llmServiceActive=True, + questionOutOfLLMScope=True, + inputGuardFailed=False, + content=OUT_OF_SCOPE_MESSAGE, + ) + assert ( + should_save_history(MagicMock(), resp, _HISTORY_EXCLUDED_MESSAGES) is False + ) + + def test_returns_false_when_content_is_excluded_message(self): + resp = _make_ok_response(content=TECHNICAL_ISSUE_MESSAGE) + assert ( + should_save_history(MagicMock(), resp, _HISTORY_EXCLUDED_MESSAGES) is False + ) + + def test_returns_true_for_valid_successful_response(self): + assert ( + should_save_history( + MagicMock(), _make_ok_response(), _HISTORY_EXCLUDED_MESSAGES + ) + is True + ) + + def test_returns_true_for_output_guardrail_violation_content_with_flags_false(self): + """Content match on OUTPUT_GUARDRAIL_VIOLATION_MESSAGE must also exclude.""" + resp = _make_ok_response(content=OUTPUT_GUARDRAIL_VIOLATION_MESSAGE) + assert ( + should_save_history(MagicMock(), resp, _HISTORY_EXCLUDED_MESSAGES) is False + ) + + +# --------------------------------------------------------------------------- +# save_history_round +# --------------------------------------------------------------------------- + + +class TestSaveHistoryRound: + @pytest.mark.asyncio + async def test_calls_store_save_round_with_correct_round(self): + store = AsyncMock() + + await save_history_round(store, "chat-99", "user question", "bot answer") + + store.save_round.assert_awaited_once() + call_args = store.save_round.call_args + chat_id_arg, round_arg = call_args.args + assert chat_id_arg == "chat-99" + assert isinstance(round_arg, ConversationRound) + assert round_arg.user_message == "user question" + assert round_arg.bot_message == "bot answer" + + @pytest.mark.asyncio + async def test_does_not_raise_when_store_raises(self): + store = AsyncMock() + store.save_round.side_effect = RuntimeError("Redis down") + + # Must not propagate + await save_history_round(store, "chat-1", "q", "a") + + +# --------------------------------------------------------------------------- +# extract_content_from_sse +# --------------------------------------------------------------------------- + + +class TestExtractContentFromSse: + def test_extracts_content_from_valid_chunk(self): + chunk = _make_sse("c1", "Hello world") + result = extract_content_from_sse(chunk) + assert result == "Hello world" + + def test_extracts_end_marker(self): + chunk = _make_sse("c1", "END") + result = extract_content_from_sse(chunk) + assert result == "END" + + def test_returns_none_for_non_sse_string(self): + result = extract_content_from_sse("not sse data") + assert result is None + + def test_returns_none_for_malformed_json(self): + result = extract_content_from_sse("data: {bad json}\n\n") + assert result is None + + def test_returns_none_when_payload_missing(self): + chunk = "data: " + json.dumps({"chatId": "c1", "sentTo": []}) + "\n\n" + result = extract_content_from_sse(chunk) + assert result is None + + def test_returns_none_when_content_key_missing(self): + chunk = "data: " + json.dumps({"chatId": "c1", "payload": {}}) + "\n\n" + result = extract_content_from_sse(chunk) + assert result is None + + def test_extracts_excluded_message_content(self): + chunk = _make_sse("c1", TECHNICAL_ISSUE_MESSAGE) + result = extract_content_from_sse(chunk) + assert result == TECHNICAL_ISSUE_MESSAGE + + +# --------------------------------------------------------------------------- +# Non-streaming hook: process_orchestration_request +# --------------------------------------------------------------------------- + + +class TestNonStreamingHistoryHook: + @pytest.mark.asyncio + async def test_save_called_on_successful_response(self): + store = AsyncMock() + response = _make_ok_response("The answer.") + + # Exercise the logic directly (mirrors what process_orchestration_request does) + if should_save_history(store, response, _HISTORY_EXCLUDED_MESSAGES): + await save_history_round(store, "chat-1", "my question", response.content) + + store.save_round.assert_awaited_once() + + @pytest.mark.asyncio + async def test_save_not_called_on_guardrail_blocked_response(self): + store = AsyncMock() + blocked = OrchestrationResponse( + chatId="c1", + llmServiceActive=True, + questionOutOfLLMScope=False, + inputGuardFailed=True, + content=INPUT_GUARDRAIL_VIOLATION_MESSAGE, + ) + + if should_save_history(store, blocked, _HISTORY_EXCLUDED_MESSAGES): + await save_history_round(store, "c1", "bad query", blocked.content) + + store.save_round.assert_not_awaited() + + @pytest.mark.asyncio + async def test_save_not_called_on_out_of_scope_response(self): + store = AsyncMock() + oos = OrchestrationResponse( + chatId="c1", + llmServiceActive=True, + questionOutOfLLMScope=True, + inputGuardFailed=False, + content=OUT_OF_SCOPE_MESSAGE, + ) + + if should_save_history(store, oos, _HISTORY_EXCLUDED_MESSAGES): + await save_history_round(store, "c1", "obscure query", oos.content) + + store.save_round.assert_not_awaited() + + @pytest.mark.asyncio + async def test_save_not_called_when_store_is_none(self): + response = _make_ok_response() + + if should_save_history(None, response, _HISTORY_EXCLUDED_MESSAGES): + await save_history_round(None, "c1", "q", response.content) + + # When store is None, save_history_round returns early without calling save_round + + +# --------------------------------------------------------------------------- +# RAG streaming hook: _stream_rag_pipeline logic +# --------------------------------------------------------------------------- + + +class TestRagStreamingHistoryHook: + @pytest.mark.asyncio + async def test_save_called_with_accumulated_response_after_successful_stream(self): + """Simulate the relevant section of _stream_rag_pipeline after streaming.""" + store = AsyncMock() + + accumulated_response = ["Hello", " world", " from", " RAG."] + _rag_bot_message = "".join(accumulated_response) + + # Mirrors the hook added to _stream_rag_pipeline + if store is not None: + if _rag_bot_message not in _HISTORY_EXCLUDED_MESSAGES: + await save_history_round( + store, "chat-5", "what is RAG?", _rag_bot_message + ) + + store.save_round.assert_awaited_once() + _, round_arg = store.save_round.call_args.args + assert round_arg.bot_message == "Hello world from RAG." + assert round_arg.user_message == "what is RAG?" + + @pytest.mark.asyncio + async def test_save_skipped_when_accumulated_is_excluded_message(self): + """OOS/violations yielded as single chunks must not be saved.""" + store = AsyncMock() + + # Simulate what happens when OOS message ends up in accumulated_response + accumulated_response = [OUT_OF_SCOPE_MESSAGE] + _rag_bot_message = "".join(accumulated_response) + + if store is not None: + if _rag_bot_message not in _HISTORY_EXCLUDED_MESSAGES: + await save_history_round(store, "chat-5", "q", _rag_bot_message) + + store.save_round.assert_not_awaited() + + @pytest.mark.asyncio + async def test_save_skipped_when_store_is_none(self): + store = None + + accumulated_response = ["some", " answer"] + _rag_bot_message = "".join(accumulated_response) + + if store is not None: + if _rag_bot_message not in _HISTORY_EXCLUDED_MESSAGES: + await save_history_round(store, "c1", "q", _rag_bot_message) + + # When store is None, save_history_round returns early without doing anything + + +# --------------------------------------------------------------------------- +# Classifier streaming hook: stream_orchestration_response accumulation logic +# --------------------------------------------------------------------------- + + +class TestClassifierStreamingHistoryHook: + @pytest.mark.asyncio + async def test_accumulates_non_end_non_excluded_content(self): + store = AsyncMock() + + tokens = ["The ", "answer ", "is 42."] + sse_chunks = [_make_sse("c1", t) for t in tokens] + sse_chunks.append(_make_sse("c1", "END")) + + # Mirrors the classifier streaming accumulation logic + _save_classifier_history = store is not None + _classifier_accumulated: list[str] = [] + + for sse_chunk in sse_chunks: + if _save_classifier_history: + extracted = extract_content_from_sse(sse_chunk) + if ( + extracted is not None + and extracted != "END" + and extracted not in _HISTORY_EXCLUDED_MESSAGES + ): + _classifier_accumulated.append(extracted) + + if _save_classifier_history and _classifier_accumulated: + await save_history_round( + store, "c1", "what is 42?", "".join(_classifier_accumulated) + ) + + store.save_round.assert_awaited_once() + + @pytest.mark.asyncio + async def test_does_not_save_when_only_violation_message_streamed(self): + store = AsyncMock() + + sse_chunks = [ + _make_sse("c1", INPUT_GUARDRAIL_VIOLATION_MESSAGE), + _make_sse("c1", "END"), + ] + + _save_classifier_history = store is not None + _classifier_accumulated: list[str] = [] + + for sse_chunk in sse_chunks: + if _save_classifier_history: + extracted = extract_content_from_sse(sse_chunk) + if ( + extracted is not None + and extracted != "END" + and extracted not in _HISTORY_EXCLUDED_MESSAGES + ): + _classifier_accumulated.append(extracted) + + if _save_classifier_history and _classifier_accumulated: + await save_history_round( + store, "c1", "bad q", "".join(_classifier_accumulated) + ) + + store.save_round.assert_not_awaited() + + @pytest.mark.asyncio + async def test_does_not_save_for_rag_workflow(self): + """RAG workflow has its own hook in _stream_rag_pipeline; skip classifier hook.""" + from src.tool_classifier import WorkflowType + + store = AsyncMock() + sse_chunks = [_make_sse("c1", "token"), _make_sse("c1", "END")] + + # Mirrors: _save_classifier_history = store is not None AND workflow != RAG + workflow_type = WorkflowType.RAG + _save_classifier_history = ( + store is not None and workflow_type != WorkflowType.RAG # False for RAG + ) + _classifier_accumulated: list[str] = [] + + for sse_chunk in sse_chunks: + if _save_classifier_history: + extracted = extract_content_from_sse(sse_chunk) + if extracted is not None and extracted != "END": + _classifier_accumulated.append(extracted) + + if _save_classifier_history and _classifier_accumulated: + await save_history_round(store, "c1", "q", "".join(_classifier_accumulated)) + + store.save_round.assert_not_awaited() + + @pytest.mark.asyncio + async def test_saves_for_non_rag_workflow(self): + """Non-RAG workflows (SERVICE, API_TOOL, etc.) should save via classifier hook.""" + from src.tool_classifier import WorkflowType + + store = AsyncMock() + tokens = ["answer"] + sse_chunks = [_make_sse("c1", t) for t in tokens] + sse_chunks.append(_make_sse("c1", "END")) + + # Use SERVICE workflow (not RAG) so the gate passes + workflow_type = WorkflowType.SERVICE + _save_classifier_history = ( + store is not None and workflow_type != WorkflowType.RAG # True for SERVICE + ) + _classifier_accumulated: list[str] = [] + + for sse_chunk in sse_chunks: + if _save_classifier_history: + extracted = extract_content_from_sse(sse_chunk) + if ( + extracted is not None + and extracted != "END" + and extracted not in _HISTORY_EXCLUDED_MESSAGES + ): + _classifier_accumulated.append(extracted) + + if _save_classifier_history and _classifier_accumulated: + await save_history_round(store, "c1", "q", "".join(_classifier_accumulated)) + + store.save_round.assert_awaited_once() + + @pytest.mark.asyncio + async def test_does_not_save_when_store_is_none(self): + store = None + sse_chunks = [_make_sse("c1", "answer"), _make_sse("c1", "END")] + _save_classifier_history = store is not None + _classifier_accumulated: list[str] = [] + + for sse_chunk in sse_chunks: + if _save_classifier_history: + extracted = extract_content_from_sse(sse_chunk) + if extracted is not None and extracted != "END": + _classifier_accumulated.append(extracted) + + if _save_classifier_history and _classifier_accumulated: + await save_history_round(store, "c1", "q", "".join(_classifier_accumulated)) + + # When store is None, save_history_round returns early without doing anything + + +# --------------------------------------------------------------------------- +# Helpers shared by the new test suites below +# --------------------------------------------------------------------------- + + +def _make_llm_manager() -> MagicMock: + mgr = MagicMock() + mgr.ensure_global_config = MagicMock() + mgr.use_task_local = MagicMock() + return mgr + + +def _make_orchestration_request( + chat_id: str = "chat-1", + message: str = "What did you say earlier?", + history: list | None = None, +) -> MagicMock: + """Return a lightweight mock that mimics the fields accessed by _build_history.""" + req = MagicMock() + req.chatId = chat_id + req.message = message + req.conversationHistory = history or [] + return req + + +def _make_round( + user: str = "What is the tax rate?", + bot: str = "The tax rate is 20%.", + ts: float = 1_700_000_000.0, +) -> ConversationRound: + return ConversationRound(user_message=user, bot_message=bot, timestamp=ts) + + +# --------------------------------------------------------------------------- +# ContextWorkflowExecutor._build_history — Redis-first retrieval +# --------------------------------------------------------------------------- + + +class TestBuildHistoryRedisFirst: + """Tests for the async _build_history method in ContextWorkflowExecutor.""" + + @pytest.mark.asyncio + async def test_uses_redis_rounds_when_available(self) -> None: + """When Redis has rounds, _build_history returns them and ignores request history.""" + from src.tool_classifier.workflows.context_workflow import ( + ContextWorkflowExecutor, + ) + + round_ = _make_round() + state = ConversationHistoryState( + chat_id="chat-1", rounds=[round_], summary=None + ) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + workflow = ContextWorkflowExecutor( + llm_manager=_make_llm_manager(), + conversation_history_store=store, + ) + req = _make_orchestration_request(chat_id="chat-1") + + history, summary = await workflow._build_history(req) + + assert len(history) == 2 # one round → two messages + assert history[0]["authorRole"] == "user" + assert history[0]["message"] == round_.user_message + assert history[1]["authorRole"] == "bot" + assert history[1]["message"] == round_.bot_message + assert summary is None + + @pytest.mark.asyncio + async def test_returns_redis_summary_with_rounds(self) -> None: + """Summary stored in Redis is returned as pre_computed_summary.""" + from src.tool_classifier.workflows.context_workflow import ( + ContextWorkflowExecutor, + ) + + round_ = _make_round() + state = ConversationHistoryState( + chat_id="chat-1", + rounds=[round_], + summary="Earlier we discussed tax rates.", + ) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + workflow = ContextWorkflowExecutor( + llm_manager=_make_llm_manager(), + conversation_history_store=store, + ) + req = _make_orchestration_request(chat_id="chat-1") + + history, summary = await workflow._build_history(req) + + assert len(history) == 2 + assert summary == "Earlier we discussed tax rates." + + @pytest.mark.asyncio + async def test_falls_back_to_request_when_redis_raises(self) -> None: + """When get_context() raises, _build_history falls back to request.conversationHistory.""" + from src.tool_classifier.workflows.context_workflow import ( + ContextWorkflowExecutor, + ) + + store = AsyncMock() + store.get_context = AsyncMock(side_effect=RuntimeError("Redis down")) + + workflow = ContextWorkflowExecutor( + llm_manager=_make_llm_manager(), + conversation_history_store=store, + ) + + item = MagicMock() + item.authorRole = "user" + item.message = "fallback message" + item.timestamp = "2024-01-01T00:00:00" + + req = _make_orchestration_request(chat_id="chat-1", history=[item]) + + history, summary = await workflow._build_history(req) + + assert len(history) == 1 + assert history[0]["message"] == "fallback message" + assert summary is None + + @pytest.mark.asyncio + async def test_falls_back_to_request_when_store_is_none(self) -> None: + """When conversation_history_store is None, request history is used.""" + from src.tool_classifier.workflows.context_workflow import ( + ContextWorkflowExecutor, + ) + + workflow = ContextWorkflowExecutor( + llm_manager=_make_llm_manager(), + conversation_history_store=None, + ) + + item = MagicMock() + item.authorRole = "bot" + item.message = "bot reply" + item.timestamp = "2024-01-01T00:00:01" + + req = _make_orchestration_request(chat_id="chat-1", history=[item]) + + history, summary = await workflow._build_history(req) + + assert len(history) == 1 + assert history[0]["authorRole"] == "bot" + assert summary is None + + @pytest.mark.asyncio + async def test_falls_back_to_request_when_redis_returns_empty_rounds(self) -> None: + """Redis state with no rounds → fall back to request.conversationHistory.""" + from src.tool_classifier.workflows.context_workflow import ( + ContextWorkflowExecutor, + ) + + state = ConversationHistoryState( + chat_id="chat-1", rounds=[], summary="old summary" + ) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + workflow = ContextWorkflowExecutor( + llm_manager=_make_llm_manager(), + conversation_history_store=store, + ) + + item = MagicMock() + item.authorRole = "user" + item.message = "from request" + item.timestamp = "2024-01-01T00:00:00" + + req = _make_orchestration_request(chat_id="chat-1", history=[item]) + + history, summary = await workflow._build_history(req) + + # Empty Redis rounds → fall back to request; summary not returned + assert len(history) == 1 + assert history[0]["message"] == "from request" + assert summary is None + + +# --------------------------------------------------------------------------- +# ContextAnalyzer.detect_context_with_summary_fallback — pre_computed_summary +# --------------------------------------------------------------------------- + + +class TestDetectContextWithPrecomputedSummary: + """Tests for the pre_computed_summary fast-path in detect_context_with_summary_fallback.""" + + def _make_analyzer(self) -> object: + from src.tool_classifier.context_analyzer import ContextAnalyzer + + return ContextAnalyzer(_make_llm_manager()) + + def _no_answer_detection(self) -> tuple: + from src.tool_classifier.context_analyzer import ContextDetectionResult + + result = ContextDetectionResult( + is_greeting=False, + can_answer_from_context=False, + reasoning="cannot answer", + ) + cost: dict = {"total_cost": 0.001, "total_tokens": 10, "num_calls": 1} + return result, cost + + def _answer_from_summary(self) -> tuple: + from src.tool_classifier.context_analyzer import ContextAnalysisResult + + result = ContextAnalysisResult( + is_greeting=False, + can_answer_from_context=True, + answer="The tax rate is 20%.", + reasoning="found in summary", + ) + cost: dict = {"total_cost": 0.002, "total_tokens": 20, "num_calls": 1} + return result, cost + + @pytest.mark.asyncio + async def test_skips_generate_summary_when_pre_computed_provided(self) -> None: + """_generate_conversation_summary must NOT be called when pre_computed_summary is set.""" + analyzer = self._make_analyzer() + + with ( + patch.object( + analyzer, + "detect_context", + new_callable=AsyncMock, + return_value=self._no_answer_detection(), + ), + patch.object( + analyzer, + "_generate_conversation_summary", + new_callable=AsyncMock, + ) as mock_generate, + patch.object( + analyzer, + "_analyze_from_summary", + new_callable=AsyncMock, + return_value=self._answer_from_summary(), + ), + ): + await analyzer.detect_context_with_summary_fallback( + query="What was the tax rate?", + conversation_history=[], + pre_computed_summary="Tax rate is 20%.", + ) + + mock_generate.assert_not_awaited() + + @pytest.mark.asyncio + async def test_still_runs_analyze_from_summary_when_pre_computed_provided( + self, + ) -> None: + """_analyze_from_summary IS called even when the summary comes from Redis.""" + analyzer = self._make_analyzer() + + with ( + patch.object( + analyzer, + "detect_context", + new_callable=AsyncMock, + return_value=self._no_answer_detection(), + ), + patch.object( + analyzer, + "_generate_conversation_summary", + new_callable=AsyncMock, + ), + patch.object( + analyzer, + "_analyze_from_summary", + new_callable=AsyncMock, + return_value=self._answer_from_summary(), + ) as mock_analyze, + ): + await analyzer.detect_context_with_summary_fallback( + query="What was the tax rate?", + conversation_history=[], + pre_computed_summary="Tax rate is 20%.", + ) + + mock_analyze.assert_awaited_once() + call_kwargs = mock_analyze.call_args.kwargs + assert call_kwargs["summary"] == "Tax rate is 20%." + + @pytest.mark.asyncio + async def test_returns_answer_from_pre_computed_summary(self) -> None: + """When the summary analysis succeeds, result has answered_from_summary=True.""" + from src.tool_classifier.context_analyzer import ContextDetectionResult + + analyzer = self._make_analyzer() + + with ( + patch.object( + analyzer, + "detect_context", + new_callable=AsyncMock, + return_value=self._no_answer_detection(), + ), + patch.object( + analyzer, "_generate_conversation_summary", new_callable=AsyncMock + ), + patch.object( + analyzer, + "_analyze_from_summary", + new_callable=AsyncMock, + return_value=self._answer_from_summary(), + ), + ): + result, _ = await analyzer.detect_context_with_summary_fallback( + query="What was the tax rate?", + conversation_history=[], + pre_computed_summary="Tax rate is 20%.", + ) + + assert isinstance(result, ContextDetectionResult) + assert result.can_answer_from_context is True + assert result.answered_from_summary is True + assert result.context_snippet == "The tax rate is 20%." + + @pytest.mark.asyncio + async def test_summary_path_attempted_for_short_history_with_pre_computed( + self, + ) -> None: + """Summary analysis runs even with <=10 turns when pre_computed_summary is set.""" + analyzer = self._make_analyzer() + + # Only 2 items in history (well below 10) + short_history = [ + {"authorRole": "user", "message": "hi", "timestamp": "0"}, + {"authorRole": "bot", "message": "hello", "timestamp": "1"}, + ] + + with ( + patch.object( + analyzer, + "detect_context", + new_callable=AsyncMock, + return_value=self._no_answer_detection(), + ), + patch.object( + analyzer, "_generate_conversation_summary", new_callable=AsyncMock + ) as mock_gen, + patch.object( + analyzer, + "_analyze_from_summary", + new_callable=AsyncMock, + return_value=self._answer_from_summary(), + ) as mock_analyze, + ): + await analyzer.detect_context_with_summary_fallback( + query="What was the tax rate?", + conversation_history=short_history, + pre_computed_summary="Tax rate was discussed previously.", + ) + + mock_gen.assert_not_awaited() # skipped because pre_computed_summary is set + mock_analyze.assert_awaited_once() # still validated against query + + +# --------------------------------------------------------------------------- +# End-to-end: no LLM summarisation when Redis supplies a summary +# --------------------------------------------------------------------------- + + +class TestEndToEndNoLLMSummarisationWhenRedisHasSummary: + """Verify that _generate_conversation_summary is never called when the + workflow retrieves a summary from Redis via _build_history.""" + + @pytest.mark.asyncio + async def test_no_summarisation_call_when_redis_has_summary(self) -> None: + """Full _detect() path: Redis summary present → no LLM summarisation.""" + from src.tool_classifier.context_analyzer import ( + ContextAnalysisResult, + ContextDetectionResult, + ) + from src.tool_classifier.workflows.context_workflow import ( + ContextWorkflowExecutor, + ) + + # Redis returns one round + a running summary + round_ = _make_round() + state = ConversationHistoryState( + chat_id="chat-e2e", + rounds=[round_], + summary="We discussed tax rates earlier.", + ) + store = AsyncMock() + store.get_context = AsyncMock(return_value=state) + + # detect_context returns "cannot answer" so the summary path is tried + cannot_answer = ContextDetectionResult( + is_greeting=False, + can_answer_from_context=False, + reasoning="not in recent history", + ) + + llm_manager = _make_llm_manager() + workflow = ContextWorkflowExecutor( + llm_manager=llm_manager, + conversation_history_store=store, + ) + + with ( + patch.object( + workflow.context_analyzer, + "detect_context", + new_callable=AsyncMock, + return_value=( + cannot_answer, + {"total_cost": 0.001, "total_tokens": 10, "num_calls": 1}, + ), + ), + patch.object( + workflow.context_analyzer, + "_generate_conversation_summary", + new_callable=AsyncMock, + ) as mock_gen, + patch.object( + workflow.context_analyzer, + "_analyze_from_summary", + new_callable=AsyncMock, + return_value=( + ContextAnalysisResult( + is_greeting=False, + can_answer_from_context=True, + answer="The tax rate is 20%.", + reasoning="from summary", + ), + {"total_cost": 0.002, "total_tokens": 20, "num_calls": 1}, + ), + ), + ): + time_metric: dict = {} + costs_metric: dict = {} + history, pre_computed_summary = await workflow._build_history( + _make_orchestration_request(chat_id="chat-e2e") + ) + result = await workflow._detect( + message="What was the tax rate?", + history=history, + time_metric=time_metric, + costs_metric=costs_metric, + pre_computed_summary=pre_computed_summary, + ) + + mock_gen.assert_not_awaited() + assert result is not None + assert result.can_answer_from_context is True + assert result.answered_from_summary is True + + +# --------------------------------------------------------------------------- +# RAG Workflow: Redis history used in _refine_user_prompt +# --------------------------------------------------------------------------- + + +class TestRagWorkflowUsesRedisHistory: + """Verify that the RAG pipeline fetches history from Redis before refinement.""" + + @pytest.mark.asyncio + async def test_redis_history_passed_to_refine_when_available(self) -> None: + """get_conversation_history is called and its result replaces request history.""" + + svc = _make_service() + svc.conversation_history_store = AsyncMock() + + with patch( + "src.llm_orchestration_service.get_conversation_history", + new_callable=AsyncMock, + return_value=([], None), + ) as mock_get_history: + # Stub out the rest of the pipeline so we only test the history fetch + svc._refine_user_prompt = MagicMock( + return_value=( + MagicMock( + original_question="q", + refined_questions=["q1"], + ), + {}, + ) + ) + svc._safe_retrieve_contextual_chunks = AsyncMock(return_value=[]) + svc.format_sse = MagicMock(return_value="data: {}\n\n") + + components = { + "llm_manager": _make_llm_manager(), + "contextual_retriever": AsyncMock(), + "response_generator": MagicMock(), + "guardrails_adapter": None, + } + stream_ctx = MagicMock() + stream_ctx.stream_id = "sid" + stream_ctx.mark_completed = MagicMock() + + request = _make_orchestration_request(chat_id="chat-rag") + + # Drain the generator to trigger the history fetch + async for _ in svc._stream_rag_pipeline( + request=request, + components=components, + stream_ctx=stream_ctx, + costs_metric={}, + time_metric={}, + ): + pass + + mock_get_history.assert_awaited_once_with( + chat_id="chat-rag", + store=svc.conversation_history_store, + fallback=request.conversationHistory, + ) + + @pytest.mark.asyncio + async def test_redis_fallback_used_when_store_is_none(self) -> None: + """When conversation_history_store is None, request history is used.""" + svc = _make_service() + svc.conversation_history_store = None + + with patch( + "src.llm_orchestration_service.get_conversation_history", + new_callable=AsyncMock, + return_value=([], None), + ) as mock_get_history: + svc._refine_user_prompt = MagicMock( + return_value=( + MagicMock(original_question="q", refined_questions=["q1"]), + {}, + ) + ) + svc._safe_retrieve_contextual_chunks = AsyncMock(return_value=[]) + svc.format_sse = MagicMock(return_value="data: {}\n\n") + + request = _make_orchestration_request(chat_id="chat-rag-fallback") + components = { + "llm_manager": _make_llm_manager(), + "contextual_retriever": AsyncMock(), + "response_generator": MagicMock(), + "guardrails_adapter": None, + } + stream_ctx = MagicMock() + stream_ctx.stream_id = "sid" + stream_ctx.mark_completed = MagicMock() + + # Drain the generator to trigger the history fetch + async for _ in svc._stream_rag_pipeline( + request=request, + components=components, + stream_ctx=stream_ctx, + costs_metric={}, + time_metric={}, + ): + pass + + # Fallback is request.conversationHistory + mock_get_history.assert_awaited_once_with( + chat_id="chat-rag-fallback", + store=None, + fallback=request.conversationHistory, + ) + + +class TestRefineUserPromptSummary: + """Verify that _refine_user_prompt prepends summary as a system turn.""" + + def test_summary_prepended_to_dspy_history(self) -> None: + """When conversation_summary is provided, a system message is first in history.""" + svc = _make_service() + svc.langfuse_config = MagicMock() + svc.langfuse_config.langfuse_client = None + + captured_history: list = [] + + class _FakeRefiner: + def forward_structured( + self, history: list, question: str, **_: object + ) -> dict: + captured_history.extend(history) + return { + "original_question": question, + "refined_questions": [question], + "usage": {}, + "module_info": {}, + } + + llm_manager = _make_llm_manager() + llm_manager.use_task_local = MagicMock() + llm_manager.use_task_local.return_value.__enter__ = MagicMock(return_value=None) + llm_manager.use_task_local.return_value.__exit__ = MagicMock(return_value=False) + + with patch( + "src.llm_orchestration_service.PromptRefinerAgent", + return_value=_FakeRefiner(), + ): + svc._refine_user_prompt( + llm_manager=llm_manager, + original_message="What is the rate?", + conversation_history=[], + conversation_summary="We discussed tax earlier.", + ) + + assert len(captured_history) >= 1 + first = captured_history[0] + assert first["role"] == "system" + assert "We discussed tax earlier." in first["content"] + + def test_no_summary_entry_when_summary_is_none(self) -> None: + """When conversation_summary is None, no system turn is prepended.""" + svc = _make_service() + svc.langfuse_config = MagicMock() + svc.langfuse_config.langfuse_client = None + + captured_history: list = [] + + class _FakeRefiner: + def forward_structured( + self, history: list, question: str, **_: object + ) -> dict: + captured_history.extend(history) + return { + "original_question": question, + "refined_questions": [question], + "usage": {}, + "module_info": {}, + } + + llm_manager = _make_llm_manager() + + with patch( + "src.llm_orchestration_service.PromptRefinerAgent", + return_value=_FakeRefiner(), + ): + svc._refine_user_prompt( + llm_manager=llm_manager, + original_message="What is the rate?", + conversation_history=[], + conversation_summary=None, + ) + + assert all(item.get("role") != "system" for item in captured_history) + + +# --------------------------------------------------------------------------- +# Service Workflow: Redis history used in _process_intent_detection +# --------------------------------------------------------------------------- + + +class TestServiceWorkflowUsesRedisHistory: + """Verify ServiceWorkflowExecutor fetches history from Redis before intent detection.""" + + @pytest.mark.asyncio + async def test_get_conversation_history_called_in_process_intent_detection( + self, + ) -> None: + """get_conversation_history is called with the history store from the service.""" + from src.tool_classifier.workflows.service_workflow import ( + ServiceWorkflowExecutor, + ) + + orchestration_service = MagicMock() + history_store = AsyncMock() + orchestration_service.conversation_history_store = history_store + + executor = ServiceWorkflowExecutor( + llm_manager=_make_llm_manager(), + orchestration_service=orchestration_service, + ) + + with ( + patch( + "src.tool_classifier.workflows.service_workflow.get_conversation_history", + new_callable=AsyncMock, + return_value=([], None), + ) as mock_get_history, + patch.object( + executor, + "_detect_service_intent", + new_callable=AsyncMock, + return_value=(None, {}), + ), + ): + request = _make_orchestration_request(chat_id="chat-svc") + await executor._process_intent_detection( + services=[], + request=request, + chat_id="chat-svc", + context={}, + costs_metric={}, + ) + + mock_get_history.assert_awaited_once_with( + chat_id="chat-svc", + store=history_store, + fallback=request.conversationHistory, + ) + + @pytest.mark.asyncio + async def test_summary_passed_to_detect_service_intent(self) -> None: + """Summary from Redis is forwarded to _detect_service_intent.""" + from src.tool_classifier.workflows.service_workflow import ( + ServiceWorkflowExecutor, + ) + + executor = ServiceWorkflowExecutor(llm_manager=_make_llm_manager()) + + with ( + patch( + "src.tool_classifier.workflows.service_workflow.get_conversation_history", + new_callable=AsyncMock, + return_value=([], "Earlier we discussed registration."), + ), + patch.object( + executor, + "_detect_service_intent", + new_callable=AsyncMock, + return_value=(None, {}), + ) as mock_detect, + ): + request = _make_orchestration_request(chat_id="chat-svc2") + await executor._process_intent_detection( + services=[], + request=request, + chat_id="chat-svc2", + context={}, + costs_metric={}, + ) + + _, kwargs = mock_detect.call_args + assert ( + kwargs.get("conversation_summary") == "Earlier we discussed registration." + ) + + @pytest.mark.asyncio + async def test_get_conversation_history_store_returns_none_when_no_service( + self, + ) -> None: + """_get_conversation_history_store returns None when orchestration_service is None.""" + from src.tool_classifier.workflows.service_workflow import ( + ServiceWorkflowExecutor, + ) + + executor = ServiceWorkflowExecutor() + assert executor._get_conversation_history_store() is None + + @pytest.mark.asyncio + async def test_summary_prepended_to_history_dicts_in_detect_service_intent( + self, + ) -> None: + """When conversation_summary is set, a 'system' message is first in history_dicts.""" + from src.tool_classifier.workflows.service_workflow import ( + ServiceWorkflowExecutor, + ) + import dspy + + executor = ServiceWorkflowExecutor(llm_manager=_make_llm_manager()) + captured: list = [] + + class _FakeModule: + def forward( + self, + user_query: str, + services: list, + conversation_history: list | None = None, + ) -> dict: + if conversation_history: + captured.extend(conversation_history) + return { + "matched_service_id": None, + "confidence": 0.0, + "entities": {}, + "reasoning": "", + } + + with ( + patch( + "src.tool_classifier.workflows.service_workflow.IntentDetectionModule", + return_value=_FakeModule(), + ), + patch.object(executor.llm_manager, "ensure_global_config"), + patch.object(executor.llm_manager, "use_task_local"), + ): + # Patch dspy.settings.lm to avoid NoneType + mock_lm = MagicMock() + mock_lm.history = [] + with patch.object(dspy, "settings", MagicMock(lm=mock_lm)): + await executor._detect_service_intent( + user_query="register my car", + services=[], + conversation_history=[], + chat_id="c1", + conversation_summary="User was asking about vehicle registration.", + ) + + assert len(captured) >= 1 + assert captured[0]["authorRole"] == "system" + assert "vehicle registration" in captured[0]["message"] + + +# --------------------------------------------------------------------------- +# ATC Workflow: Redis history used in _compute_loop_step +# --------------------------------------------------------------------------- + + +class TestATCWorkflowUsesRedisHistory: + """Verify APIToolWorkflowExecutor fetches history from Redis on turns > 0.""" + + def _make_executor( + self, + history_store: object = None, + ) -> object: + from src.tool_classifier.workflows.api_tool_workflow import ( + APIToolWorkflowExecutor, + ) + + orchestration_service = MagicMock() + orchestration_service.conversation_history_store = history_store + orchestration_service.session_store = None + orchestration_service.prompt_config_loader = None + executor = APIToolWorkflowExecutor(orchestration_service=orchestration_service) + return executor + + def test_get_conversation_history_store_returns_store(self) -> None: + """_get_conversation_history_store returns the store from orchestration_service.""" + from src.tool_classifier.workflows.api_tool_workflow import ( + APIToolWorkflowExecutor, + ) + + store = AsyncMock() + orchestration_service = MagicMock() + orchestration_service.conversation_history_store = store + executor = APIToolWorkflowExecutor(orchestration_service=orchestration_service) + assert executor._get_conversation_history_store() is store + + def test_get_conversation_history_store_returns_none_when_no_service(self) -> None: + """_get_conversation_history_store returns None when orchestration_service is None.""" + from src.tool_classifier.workflows.api_tool_workflow import ( + APIToolWorkflowExecutor, + ) + + executor = APIToolWorkflowExecutor(orchestration_service=None) + assert executor._get_conversation_history_store() is None + + @pytest.mark.asyncio + async def test_no_redis_call_on_turn_zero(self) -> None: + """On turn 0 get_conversation_history must NOT be called.""" + from src.models.session_models import APIToolSession + from src.tool_classifier.enums import ExecutionMode + + executor = self._make_executor() + + session = MagicMock(spec=APIToolSession) + session.turn_count = 0 + session.selected_endpoint = {"name": "test_endpoint", "params": []} + session.collected_params = {} + session.max_turns = 5 + session.awaiting_continuation = False + session.detected_language = "en" + session.original_query = "book a slot" + session.execution_mode = ExecutionMode.SINGLE.value + session.parallel_endpoints = [] + + with ( + patch( + "src.tool_classifier.workflows.api_tool_workflow.get_conversation_history", + new_callable=AsyncMock, + ) as mock_get_history, + patch.object(executor, "_get_session_store", return_value=None), + patch.object( + executor, + "_get_custom_instructions", + new_callable=AsyncMock, + return_value="", + ), + patch.object( + executor, + "_build_agentic_loop", + return_value=MagicMock( + stream_run_turn=AsyncMock( + return_value=( + MagicMock( + status=MagicMock(value="NEEDS_INPUT"), + clarifying_question="What time?", + turn_count=1, + ), + ["What", " time", "?"], + ) + ) + ), + ), + ): + # Force session_store.get to return our mocked session + with patch.object( + executor, + "_get_session_store", + return_value=MagicMock( + get=AsyncMock(return_value=session), + delete=AsyncMock(), + ), + ): + request = _make_orchestration_request(chat_id="chat-atc-t0") + await executor._compute_loop_step( + request=request, + context={"matched_endpoint": {"name": "ep", "params": []}}, + ) + + mock_get_history.assert_not_awaited() + + @pytest.mark.asyncio + async def test_redis_history_fetched_on_turn_greater_than_zero(self) -> None: + """On turn > 0 get_conversation_history IS called.""" + from src.models.session_models import APIToolSession + from src.tool_classifier.enums import ExecutionMode + + executor = self._make_executor() + + session = MagicMock(spec=APIToolSession) + session.turn_count = 1 + session.selected_endpoint = {"name": "test_endpoint", "params": []} + session.collected_params = {} + session.max_turns = 5 + session.awaiting_continuation = False + session.detected_language = "en" + session.original_query = "book a slot" + session.execution_mode = ExecutionMode.SINGLE.value + session.parallel_endpoints = [] + + with ( + patch( + "src.tool_classifier.workflows.api_tool_workflow.get_conversation_history", + new_callable=AsyncMock, + return_value=([], None), + ) as mock_get_history, + patch.object( + executor, + "_get_custom_instructions", + new_callable=AsyncMock, + return_value="", + ), + patch.object( + executor, + "_build_agentic_loop", + return_value=MagicMock( + stream_run_turn=AsyncMock( + return_value=( + MagicMock( + status=MagicMock(value="NEEDS_INPUT"), + clarifying_question="What time?", + turn_count=2, + ), + ["What", " time", "?"], + ) + ) + ), + ), + patch.object( + executor, + "_get_session_store", + return_value=MagicMock( + get=AsyncMock(return_value=session), + delete=AsyncMock(), + ), + ), + ): + request = _make_orchestration_request(chat_id="chat-atc-t1") + await executor._compute_loop_step( + request=request, + context={"matched_endpoint": {"name": "ep", "params": []}}, + ) + + mock_get_history.assert_awaited_once() + _, kwargs = mock_get_history.call_args + assert kwargs["chat_id"] == "chat-atc-t1"