diff --git a/backend/app/api/docs/evaluation/get_evaluation.md b/backend/app/api/docs/evaluation/get_evaluation.md index 2c8e96351..e03002feb 100644 --- a/backend/app/api/docs/evaluation/get_evaluation.md +++ b/backend/app/api/docs/evaluation/get_evaluation.md @@ -32,6 +32,9 @@ Returns comprehensive evaluation information including processing status, config "question": "What is 2+2?", "llm_answer": "4", "ground_truth_answer": "4", + "guardrail": null, + "input_to_llm": null, + "output_from_llm": null, "scores": [ { "name": "cosine_similarity", @@ -82,3 +85,5 @@ Returns comprehensive evaluation information including processing status, config * CATEGORICAL scores include distribution counts in summary * Only complete scores are included (all traces have been rated) * Numeric values are rounded to 2 decimal places +* `guardrail` (row format only) reports what the config's guardrails did to that row: `"blocked: "` (no answer was generated, the row is unscoreable with reason `guardrail_blocked` and is not counted as a failure), `"rephrased"` (guardrails answered directly and the answer is scored normally), `"applied"` (guardrails ran and passed the content through), or `null`/absent (guardrails did not run, or were bypassed because the service was unreachable). Fast runs only; batch runs never carry it. +* `input_to_llm` / `output_from_llm` (row format only, fast runs) are set on `"applied"` rows: `input_to_llm` is the prompt the LLM actually received after input guardrails (e.g. with PII redacted, and after `prompt_template` interpolation), `output_from_llm` is the LLM's answer before output guardrails changed it (`llm_answer` stays the post-guardrail text that is scored). Each is `null` when that side's guardrails did not apply, and on blocked or rephrased rows. `output_from_llm` is the pre-redaction text, so it can contain content an output PII guardrail removed. diff --git a/backend/app/crud/evaluations/fast.py b/backend/app/crud/evaluations/fast.py index c4d9e9b52..5eeffc5c0 100644 --- a/backend/app/crud/evaluations/fast.py +++ b/backend/app/crud/evaluations/fast.py @@ -32,6 +32,7 @@ from concurrent.futures import ThreadPoolExecutor, as_completed from dataclasses import dataclass, field from typing import Any +from uuid import uuid4 import openai from langfuse import Langfuse @@ -76,6 +77,10 @@ ) from app.crud.evaluations.fast_results import ( EMBEDDING_USAGE_KEYS, + GUARDRAIL_APPLIED, + GUARDRAIL_METADATA_KEYS, + INPUT_GUARDRAIL_METADATA_KEY, + OUTPUT_GUARDRAIL_METADATA_KEY, RESPONSE_USAGE_KEYS, EmbeddingResult, ResponseResult, @@ -83,6 +88,7 @@ build_response_result, extract_usage, is_failure_threshold_breached, + is_guardrail_blocked, parse_embedding_pair, sum_usage, ) @@ -98,8 +104,11 @@ create_langfuse_dataset_run, update_traces_with_cosine_scores, ) -from app.crud.evaluations.response_parsing import extract_response_text -from app.crud.evaluations.retry import retry_openai_call +from app.crud.evaluations.response_parsing import ( + extract_file_search_chunks, + field_value, +) +from app.crud.evaluations.retry import retry_llm_call, retry_openai_call from app.crud.evaluations.score import ( JUDGE_FAILED_REASON, EvaluationScore, @@ -112,18 +121,21 @@ from app.crud.job import create_batch_job, get_batch_job from app.models import EvaluationRun, EvaluationRunUpdate from app.models.batch_job import BatchJob, BatchJobCreate -from app.models.llm.request import TextLLMParams -from app.services.llm.mappers import map_kaapi_to_openai_params -from app.services.response.response import get_file_search_results +from app.models.llm.request import ( + ConfigBlob, + LLMCallConfig, + QueryParams, + TextContent, + TextInput, +) +from app.models.llm.response import TextOutput +from app.services.llm.chain.types import BlockResult, GuardrailOutcomeEnum +from app.services.llm.jobs import execute_llm_call logger = logging.getLogger(__name__) _retry_openai_call = retry_openai_call(logger) - - -@_retry_openai_call -def _create_response(openai_client: OpenAI, params: dict[str, Any]) -> Any: - return openai_client.responses.create(**params) +_retry_llm_call = retry_llm_call(logger) @_retry_openai_call @@ -151,17 +163,45 @@ def _run_in_pool( return results -def _responses_call_for_item( +@_retry_llm_call +def _execute_llm_call_for_question( *, - openai_client: OpenAI, - base_params: dict[str, Any], + config: LLMCallConfig, + question: str, + project_id: int, + organization_id: int, +) -> BlockResult: + """Generate one answer through the same `/llm/call` path production runs.""" + return execute_llm_call( + config=config, + # Fresh query per attempt: execute_llm_call mutates it in place. + query=QueryParams(input=TextInput(content=TextContent(value=question))), + job_id=uuid4(), + project_id=project_id, + organization_id=organization_id, + request_metadata=None, + langfuse_credentials=None, + # Raw response carries the file_search hits the knowledge_base metric scores. + include_provider_raw_response=True, + include_guardrail_metadata=True, + record_call=False, + ) + + +def _response_text(result: BlockResult) -> str: + """Generated text; empty when blocked or output wasn't text.""" + output = result.response.response.output if result.response else None + return output.content.value if isinstance(output, TextOutput) else "" + + +def _llm_call_for_item( + *, + config: LLMCallConfig, + project_id: int, + organization_id: int, item: dict[str, Any], ) -> ResponseResult: - """Run one Responses call for a dataset item, in the batch path's per-item shape. - - `base_params` is the question-independent OpenAI body produced once by - `map_kaapi_to_openai_params`; only `input` varies per item. - """ + """Generate one item's answer. Never raises: that would abort the whole chunk.""" item_id = item["id"] question = item["input"].get("question", "") if item.get("input") else "" ground_truth = ( @@ -183,27 +223,66 @@ def failed_result(generated_output: str) -> ResponseResult: return failed_result("ERROR: missing question in dataset item") try: - response = _create_response(openai_client, {**base_params, "input": question}) - except openai.OpenAIError as exc: + result = _execute_llm_call_for_question( + config=config, + question=question, + project_id=project_id, + organization_id=organization_id, + ) + except Exception as exc: logger.warning( - f"[_responses_call_for_item] Item failed | item_id={item_id} | error={exc}" + f"[_llm_call_for_item] Item failed | item_id={item_id} | error={exc}", + exc_info=True, ) return failed_result(f"ERROR: {exc}") + # Before error check: a rephrased row has no error and would read as success. + guardrail: str | None = None + input_to_llm: str | None = None + output_from_llm: str | None = None + if result.guardrail_outcome == GuardrailOutcomeEnum.BLOCKED: + generated_output = "" + guardrail = f"{GuardrailOutcomeEnum.BLOCKED}: {result.error}" + elif result.guardrail_outcome == GuardrailOutcomeEnum.REPHRASED: + generated_output = _response_text(result) + guardrail = GuardrailOutcomeEnum.REPHRASED + elif result.error is not None: + logger.warning( + f"[_llm_call_for_item] Item failed | item_id={item_id} | " + f"error={result.error}" + ) + return failed_result(f"ERROR: {result.error}") + else: + generated_output = _response_text(result) + metadata = result.metadata or {} + if any(key in metadata for key in GUARDRAIL_METADATA_KEYS): + guardrail = GUARDRAIL_APPLIED + # Rephrased rows never reached the LLM; blocked rows carry no metadata. + input_to_llm = (metadata.get(INPUT_GUARDRAIL_METADATA_KEY) or {}).get( + "input_to_llm" + ) + output_from_llm = (metadata.get(OUTPUT_GUARDRAIL_METADATA_KEY) or {}).get( + "output_from_llm" + ) + + # Tokens are billed even on an output block, so usage is read off every outcome. return build_response_result( item_id=item_id, question=question, ground_truth=ground_truth, question_id=question_id, - generated_output=extract_response_text(response), - response_id=getattr(response, "id", None), - usage=extract_usage(getattr(response, "usage", None), RESPONSE_USAGE_KEYS), + generated_output=generated_output, + response_id=( + result.response.response.provider_response_id if result.response else None + ), + usage=extract_usage(result.usage, RESPONSE_USAGE_KEYS), failed=False, - # Plain dicts (not FileResultChunk) so the unit stays JSON-serializable for S3. - retrieved_chunks=[ - {"score": c.score, "text": c.text, "filename": c.filename} - for c in get_file_search_results(response) - ], + guardrail=guardrail, + input_to_llm=input_to_llm, + output_from_llm=output_from_llm, + retrieved_chunks=extract_file_search_chunks( + result.response.provider_raw_response if result.response else None + ), ) @@ -317,9 +396,8 @@ def _cleanup_response_chunks(*, session: Session, eval_run: EvaluationRun) -> No def run_response_chunk( *, session: Session, - openai_client: OpenAI, eval_run: EvaluationRun, - config: TextLLMParams, + config_blob: ConfigBlob, dataset_items_slice: list[dict[str, Any]], chunk_index: int, ) -> None: @@ -339,20 +417,22 @@ def run_response_chunk( if existing and existing.raw_output_url: return - base_params, mapper_warnings = map_kaapi_to_openai_params( - session=session, kaapi_params=config - ) + # Resolved blob passed ad-hoc skips a per-row config fetch; shared read-only. + config = LLMCallConfig(blob=config_blob) + # Read off the session-bound row here: the workers must not touch it in threads. + project_id = eval_run.project_id + organization_id = eval_run.organization_id - # Ask OpenAI to return the file_search hits so knowledge_base can judge them. - # tool_choice stays at the model default (auto) — consistent with normal calls; - # a row where the model doesn't query the KB is scored N/A, not forced to search. - if any(t.get("type") == "file_search" for t in base_params.get("tools", [])): - base_params["include"] = ["file_search_call.results"] + # Native params are a dict, Kaapi params a typed model; field_value reads either. + model = field_value(config_blob.completion.params, "model") results = _run_in_pool( items=dataset_items_slice, - worker=lambda item: _responses_call_for_item( - openai_client=openai_client, base_params=base_params, item=item + worker=lambda item: _llm_call_for_item( + config=config, + project_id=project_id, + organization_id=organization_id, + item=item, ), max_workers=settings.EVAL_FAST_API_CONCURRENCY, ) @@ -372,7 +452,7 @@ def run_response_chunk( config={ "run_mode": "fast", "endpoint": RESPONSES_ENDPOINT, - "model": config.model, + "model": model, "usage": sum_usage(results, RESPONSE_USAGE_KEYS), CHUNK_CONFIG_RUN_ID: eval_run.id, CHUNK_CONFIG_INDEX: chunk_index, @@ -476,8 +556,12 @@ def _stage2_embeddings( if cached is not None: return eval_run, cached - # Only embed items that succeeded in Stage 1. - embed_candidates = [r for r in response_results if not r.get("failed")] + # Blocked rows are unscoreable, not failed; embedding them trips the threshold. + embed_candidates = [ + r + for r in response_results + if not r.get("failed") and not is_guardrail_blocked(r) + ] embedding_results = _run_in_pool( items=embed_candidates, diff --git a/backend/app/crud/evaluations/fast_cosine.py b/backend/app/crud/evaluations/fast_cosine.py index 5a349b80a..85ecebdfe 100644 --- a/backend/app/crud/evaluations/fast_cosine.py +++ b/backend/app/crud/evaluations/fast_cosine.py @@ -11,12 +11,14 @@ import numpy as np from app.crud.evaluations.embeddings import calculate_cosine_similarity +from app.crud.evaluations.fast_results import is_guardrail_blocked from app.crud.evaluations.merge import apply_cosine_breakdown from app.crud.evaluations.score import ( COSINE_SCORE_NAME, UNSCOREABLE_EMBEDDING_FAILED, UNSCOREABLE_EMPTY_GROUND_TRUTH, UNSCOREABLE_EMPTY_OUTPUT, + UNSCOREABLE_GUARDRAIL_BLOCKED, SummaryScore, ) @@ -24,7 +26,10 @@ def classify_empty_side(response: dict[str, Any]) -> str | None: - """Why a row can't be scored from its own text, or None if both sides are present.""" + """Why a row can't be scored, or None. Shared by cosine and judge paths.""" + # Before empty-output check: a blocked row is empty because of guardrails. + if is_guardrail_blocked(response): + return UNSCOREABLE_GUARDRAIL_BLOCKED if not response.get("generated_output"): return UNSCOREABLE_EMPTY_OUTPUT if not response.get("ground_truth"): diff --git a/backend/app/crud/evaluations/fast_results.py b/backend/app/crud/evaluations/fast_results.py index 507e46950..44664860b 100644 --- a/backend/app/crud/evaluations/fast_results.py +++ b/backend/app/crud/evaluations/fast_results.py @@ -5,10 +5,23 @@ JSON-serializable and match the batch path's shape. """ +from collections.abc import Mapping from typing import Any, TypedDict from app.core.config import settings from app.crud.evaluations.response_parsing import field_value +from app.services.llm.chain.types import GuardrailOutcomeEnum + +# Guardrails passed content through. Blocked rows store "blocked: {error}". +GUARDRAIL_APPLIED: str = "applied" + +# `BlockResult.metadata` keys present when guardrails passed content through. +INPUT_GUARDRAIL_METADATA_KEY: str = "input_guardrail" +OUTPUT_GUARDRAIL_METADATA_KEY: str = "output_guardrail" +GUARDRAIL_METADATA_KEYS: tuple[str, ...] = ( + INPUT_GUARDRAIL_METADATA_KEY, + OUTPUT_GUARDRAIL_METADATA_KEY, +) class ResponseResult(TypedDict, total=False): @@ -23,6 +36,9 @@ class ResponseResult(TypedDict, total=False): question_id: int | None failed: bool retrieved_chunks: list[dict[str, Any]] + guardrail: str | None + input_to_llm: str | None + output_from_llm: str | None class EmbeddingResult(TypedDict, total=False): @@ -51,6 +67,9 @@ def build_response_result( response_id: str | None = None, usage: dict[str, int] | None = None, retrieved_chunks: list[dict[str, Any]] | None = None, + guardrail: str | None = None, + input_to_llm: str | None = None, + output_from_llm: str | None = None, ) -> ResponseResult: """One Stage-1 per-item result, in the batch path's shape.""" return { @@ -63,9 +82,18 @@ def build_response_result( "question_id": question_id, "failed": failed, "retrieved_chunks": retrieved_chunks, + "guardrail": guardrail, + "input_to_llm": input_to_llm, + "output_from_llm": output_from_llm, } +def is_guardrail_blocked(row: Mapping[str, Any]) -> bool: + """True when guardrails hard-blocked this row, leaving it with no output to score.""" + # `.get` not `[...]`: response units written before guardrails landed have no key. + return (row.get("guardrail") or "").startswith(f"{GuardrailOutcomeEnum.BLOCKED}:") + + def build_embedding_failure(item_id: str, error: str) -> EmbeddingResult: """One failed Stage-2 per-pair result.""" return { diff --git a/backend/app/crud/evaluations/fast_traces.py b/backend/app/crud/evaluations/fast_traces.py index 0ec6d7e5b..0b2a752fa 100644 --- a/backend/app/crud/evaluations/fast_traces.py +++ b/backend/app/crud/evaluations/fast_traces.py @@ -160,6 +160,10 @@ def build_trace_records( "ground_truth_answer": response.get("ground_truth", ""), "question_id": response.get("question_id"), "category": response.get("category") or DEFAULT_CATEGORY, + # Always emitted: None = didn't fire, vs. absent key on non-fast traces. + "guardrail": response.get("guardrail"), + "input_to_llm": response.get("input_to_llm"), + "output_from_llm": response.get("output_from_llm"), "scores": trace_scores, } traces.append(trace) diff --git a/backend/app/crud/evaluations/merge.py b/backend/app/crud/evaluations/merge.py index fa22d314c..626e8851b 100644 --- a/backend/app/crud/evaluations/merge.py +++ b/backend/app/crud/evaluations/merge.py @@ -17,6 +17,7 @@ COSINE_SCORE_COMMENT, COSINE_SCORE_NAME, DEFAULT_CATEGORY, + GUARDRAIL_TRACE_KEYS, EvaluationScore, NumericSummaryScore, SummaryScore, @@ -196,6 +197,13 @@ def _merge_single_trace(existing: TraceData, fresh: TraceData) -> TraceData: fresh.get("category") or existing.get("category") or DEFAULT_CATEGORY ) + # Present key wins even if None (no `or`), else a stale "blocked" sticks. + for key in GUARDRAIL_TRACE_KEYS: + if key in fresh: + merged[key] = fresh[key] + elif key in existing: + merged[key] = existing[key] + return merged diff --git a/backend/app/crud/evaluations/response_parsing.py b/backend/app/crud/evaluations/response_parsing.py index 86179ecfe..bbb4e9d2b 100644 --- a/backend/app/crud/evaluations/response_parsing.py +++ b/backend/app/crud/evaluations/response_parsing.py @@ -7,6 +7,8 @@ from typing import Any +FILE_SEARCH_CALL_TYPE = "file_search_call" + def field_value(obj: Any, name: str, default: Any = None) -> Any: """Read a field from an object or dict (SDK object vs test dict), with a default.""" @@ -36,3 +38,20 @@ def extract_response_text(response: Any) -> str: if text: return text return "" + + +def extract_file_search_chunks(raw: dict[str, Any] | None) -> list[dict[str, Any]]: + """Flatten file_search hits from a `model_dump()` Responses payload.""" + chunks: list[dict[str, Any]] = [] + for item in field_value(raw, "output") or []: + if field_value(item, "type") != FILE_SEARCH_CALL_TYPE: + continue + for hit in field_value(item, "results") or []: + chunks.append( + { + "score": field_value(hit, "score"), + "text": field_value(hit, "text"), + "filename": field_value(hit, "filename"), + } + ) + return chunks diff --git a/backend/app/crud/evaluations/retry.py b/backend/app/crud/evaluations/retry.py index 16cd136a6..cb5066d45 100644 --- a/backend/app/crud/evaluations/retry.py +++ b/backend/app/crud/evaluations/retry.py @@ -1,8 +1,7 @@ -"""Shared OpenAI retry policy for the synchronous evaluation stages. +"""Retry policies for synchronous evaluation stages. -Responses, embeddings and the judge all issue single-row OpenAI calls from a -worker thread pool, so they share one transient-error policy rather than each -declaring its own. +Embeddings/judge retry on exceptions; generation retries on `retryable`, since +`execute_llm_call` returns failures instead of raising. """ import logging @@ -11,13 +10,17 @@ import openai from tenacity import ( + RetryCallState, before_sleep_log, retry, retry_if_exception_type, + retry_if_result, stop_after_attempt, wait_random_exponential, ) +from app.services.llm.chain.types import BlockResult + RETRY_MAX_ATTEMPTS = 3 RETRY_BASE_DELAY_SECONDS = 1.0 RETRY_MAX_DELAY_SECONDS = 30.0 @@ -50,3 +53,31 @@ def retry_openai_call( before_sleep=before_sleep_log(logger, logging.INFO), reraise=True, ) + + +def _is_retryable_llm_result(result: BlockResult) -> bool: + """True only for failures `execute_llm_call` flagged retryable.""" + return result.error is not None and result.retryable + + +def _last_llm_result(retry_state: RetryCallState) -> BlockResult: + """Return the final result on exhaustion; raising would kill the whole chunk.""" + outcome = retry_state.outcome + if outcome is None: # unreachable: tenacity records an attempt before calling back + raise RuntimeError("retry_error_callback fired with no attempt outcome") + return outcome.result() + + +def retry_llm_call( + logger: logging.Logger, +) -> Callable[[Callable[P, BlockResult]], Callable[P, BlockResult]]: + """Retry `execute_llm_call` results flagged `retryable`; same limits as OpenAI.""" + return retry( + retry=retry_if_result(_is_retryable_llm_result), + wait=wait_random_exponential( + multiplier=RETRY_BASE_DELAY_SECONDS, max=RETRY_MAX_DELAY_SECONDS + ), + stop=stop_after_attempt(RETRY_MAX_ATTEMPTS), + before_sleep=before_sleep_log(logger, logging.INFO), + retry_error_callback=_last_llm_result, + ) diff --git a/backend/app/crud/evaluations/score.py b/backend/app/crud/evaluations/score.py index c04d6d055..ef9be18de 100644 --- a/backend/app/crud/evaluations/score.py +++ b/backend/app/crud/evaluations/score.py @@ -1,7 +1,7 @@ """Score types, verdict banding and the run-level overall rollup.""" from enum import Enum -from typing import NotRequired, TypedDict +from typing import Literal, NotRequired, TypedDict DEFAULT_CATEGORY: str = "Other" @@ -48,6 +48,7 @@ def verdict_from_score(score: float) -> VerdictEnum: UNSCOREABLE_EMPTY_GROUND_TRUTH: str = "empty_ground_truth" UNSCOREABLE_EMBEDDING_FAILED: str = "embedding_failed" UNSCOREABLE_MISSING_TRACE_ID: str = "missing_trace_id" +UNSCOREABLE_GUARDRAIL_BLOCKED: str = "guardrail_blocked" JUDGE_FAILED_REASON: str = "judge_failed" UNSCOREABLE_REASONS: tuple[str, ...] = ( @@ -55,6 +56,7 @@ def verdict_from_score(score: float) -> VerdictEnum: UNSCOREABLE_EMPTY_GROUND_TRUTH, UNSCOREABLE_EMBEDDING_FAILED, UNSCOREABLE_MISSING_TRACE_ID, + UNSCOREABLE_GUARDRAIL_BLOCKED, JUDGE_FAILED_REASON, ) @@ -80,9 +82,20 @@ class TraceData(TypedDict): question_id: int | None ground_truth_answer: str category: NotRequired[str] + # "blocked: ..." | "rephrased" | "applied"; None if guardrails didn't fire. + guardrail: NotRequired[str | None] + # LLM input after input guardrails / output before output guardrails. + input_to_llm: NotRequired[str | None] + output_from_llm: NotRequired[str | None] scores: list[TraceScore] +# Merged key-by-key since None is meaningful. +GUARDRAIL_TRACE_KEYS: tuple[ + Literal["guardrail", "input_to_llm", "output_from_llm"], ... +] = ("guardrail", "input_to_llm", "output_from_llm") + + class CategoryMetrics(TypedDict): """Aggregated per-category metrics across an eval run. diff --git a/backend/app/services/evaluations/fast.py b/backend/app/services/evaluations/fast.py index fd5646cb9..57ed337f3 100644 --- a/backend/app/services/evaluations/fast.py +++ b/backend/app/services/evaluations/fast.py @@ -14,7 +14,6 @@ from fastapi import HTTPException from langfuse import Langfuse -from openai import OpenAI from sqlmodel import Session from app.celery.utils import start_fast_evaluation_chunk @@ -41,7 +40,7 @@ EvaluationRunUpdate, RunModeEnum, ) -from app.models.llm.request import TextLLMParams +from app.models.llm.request import ConfigBlob from app.services.evaluations.evaluation import create_evaluation_run from app.services.evaluations.validators import parse_csv_items from app.services.llm.providers import LLMProvider @@ -54,6 +53,9 @@ ERR_CONFIG_TYPE_UNSUPPORTED = "config_type_unsupported" ERR_DATASET_TOO_LARGE_FOR_FAST = "dataset_too_large_for_fast" ERR_DUPLICATION_FACTOR_NOT_SUPPORTED = "duplication_factor_override_not_supported" +ERR_CONFIG_TEMPLATE_MISSING_INPUT = "config_template_missing_input" + +PROMPT_TEMPLATE_INPUT_PLACEHOLDER = "{{input}}" def is_dataset_fast_eligible(*, original_items_count: int) -> bool: @@ -159,7 +161,8 @@ def validate_fast_evaluation_inputs( 1. Dataset exists; v1 runs also require a Langfuse id, v2 judged runs don't (they load items from S3). 2. Config resolves to a text-type OpenAI config. - 3. Dataset's original_items_count <= EVAL_FAST_MAX_UNIQUE_ROWS. + 3. Config's prompt_template, when set, carries the {{input}} placeholder. + 4. Dataset's original_items_count <= EVAL_FAST_MAX_UNIQUE_ROWS. `duplication_factor`, when provided, overrides the dataset's stored factor for this run only and is supported for S3-only datasets exclusively; it is rejected @@ -220,6 +223,22 @@ def validate_fast_evaluation_inputs( detail=ERR_CONFIG_TYPE_UNSUPPORTED, ) + # execute_llm_call always interpolates prompt_template; no placeholder drops input. + prompt_template = config_blob.prompt_template + if ( + prompt_template is not None + and PROMPT_TEMPLATE_INPUT_PLACEHOLDER not in prompt_template.template + ): + logger.warning( + f"[validate_fast_evaluation_inputs] Config prompt_template has no " + f"{PROMPT_TEMPLATE_INPUT_PLACEHOLDER} placeholder, so every dataset " + f"question would be dropped | config_id={config_id}" + ) + raise HTTPException( + status_code=422, + detail=ERR_CONFIG_TEMPLATE_MISSING_INPUT, + ) + original_items_count = (dataset.dataset_metadata or {}).get( DATASET_META_ORIGINAL_ITEMS ) @@ -367,12 +386,8 @@ def _get_fast_run(*, session: Session, eval_run_id: int) -> EvaluationRun: def _resolve_config_and_clients( *, session: Session, eval_run: EvaluationRun, dataset: EvaluationDataset -) -> tuple[TextLLMParams, OpenAI, Langfuse | None]: - """Resolve the run's text config and build its OpenAI + (optional) Langfuse clients. - - Only a Langfuse-backed (v1) dataset needs a Langfuse client — its items live in - Langfuse. A v2 dataset loads from S3, so we skip the client (and its credential - requirement) rather than fail a Langfuse-free run. Mirrors the fan-out sizing.""" +) -> tuple[ConfigBlob, Langfuse | None]: + """Resolve the run's config blob, plus a Langfuse client for v1 datasets only.""" config_blob, error = resolve_evaluation_config( session=session, config_id=eval_run.config_id, @@ -382,12 +397,6 @@ def _resolve_config_and_clients( if error or config_blob is None: raise ValueError(f"Failed to resolve config: {error}") - text_params = TextLLMParams.model_validate(config_blob.completion.params) - openai_client = get_openai_client( - session=session, - org_id=eval_run.organization_id, - project_id=eval_run.project_id, - ) langfuse_client = ( get_langfuse_client( session=session, @@ -397,7 +406,7 @@ def _resolve_config_and_clients( if dataset.langfuse_dataset_id else None ) - return text_params, openai_client, langfuse_client + return config_blob, langfuse_client def execute_fast_evaluation_chunk(*, eval_run_id: int, chunk_index: int) -> None: @@ -433,7 +442,7 @@ def execute_fast_evaluation_chunk(*, eval_run_id: int, chunk_index: int) -> None raise ValueError( f"Dataset {eval_run.dataset_id} not found for run {eval_run_id}" ) - text_params, openai_client, langfuse_client = _resolve_config_and_clients( + config_blob, langfuse_client = _resolve_config_and_clients( session=session, eval_run=eval_run, dataset=dataset ) dataset_items = load_run_dataset_items( @@ -449,9 +458,8 @@ def execute_fast_evaluation_chunk(*, eval_run_id: int, chunk_index: int) -> None run_response_chunk( session=session, - openai_client=openai_client, eval_run=eval_run, - config=text_params, + config_blob=config_blob, dataset_items_slice=items_slice, chunk_index=chunk_index, ) diff --git a/backend/app/services/llm/chain/types.py b/backend/app/services/llm/chain/types.py index 7fa0f39d8..5a4f6e5f2 100644 --- a/backend/app/services/llm/chain/types.py +++ b/backend/app/services/llm/chain/types.py @@ -1,9 +1,15 @@ from dataclasses import dataclass +from enum import StrEnum from uuid import UUID from app.models.llm.response import LLMCallResponse, Usage +class GuardrailOutcomeEnum(StrEnum): + BLOCKED = "blocked" + REPHRASED = "rephrased" + + @dataclass class BlockResult: """Result of a single block/LLM call execution.""" @@ -13,6 +19,11 @@ class BlockResult: usage: Usage | None = None error: str | None = None metadata: dict | None = None + guardrail_outcome: GuardrailOutcomeEnum | None = None + """Set when guardrails decided the outcome, vs. a provider failure.""" + + retryable: bool = False + """Whether retrying could succeed. False by default so new failures fail fast.""" @property def success(self) -> bool: diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index a57fcb9d6..3adcf1970 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -144,6 +144,11 @@ class GuardrailsOutcome: def applied(self) -> bool: return bool(self.raw) and not self.bypassed + @property + def blocked(self) -> bool: + """True for a rejecting verdict, not a fail-closed auth/transport error.""" + return self.error is not None and not self.raw.get("auth_error") + def apply_guardrails( *, @@ -345,6 +350,7 @@ def run_guardrails_validation( return { "success": False, "bypassed": False, + "auth_error": True, # Status only — str(e) embeds the internal service URL and this # string is client-visible via job.error_message. "error": f"Guardrails service rejected the request (HTTP {status_code})", diff --git a/backend/app/services/llm/jobs.py b/backend/app/services/llm/jobs.py index c07084d34..e90fa6b71 100644 --- a/backend/app/services/llm/jobs.py +++ b/backend/app/services/llm/jobs.py @@ -66,7 +66,7 @@ TextOutput, Usage, ) -from app.services.llm.chain.types import BlockResult +from app.services.llm.chain.types import BlockResult, GuardrailOutcomeEnum from app.services.llm.guardrails import apply_guardrails, summarize_validator_results from app.services.llm.mappers import ( resolve_default_audio_provider, @@ -372,13 +372,19 @@ def apply_input_guardrails( project_id: int, organization_id: int, include_guardrail_metadata: bool = False, -) -> tuple[QueryParams, str | None, str | None, dict[str, Any] | None]: +) -> tuple[ + QueryParams, + str | None, + str | None, + dict[str, Any] | None, + GuardrailOutcomeEnum | None, +]: """Apply input guardrails from a config_blob. Shared with llm-call and llm-chain. - Returns (query, error, guardrail_direct_response, metadata). + Returns (query, error, guardrail_direct_response, metadata, guardrail_outcome). """ if not config_blob or not config_blob.input_guardrails: - return query, None, None, None + return query, None, None, None, None if not isinstance(query.input, TextInput): logger.info( @@ -386,7 +392,7 @@ def apply_input_guardrails( f"job_id={job_id}, " f"input_type={getattr(query.input, 'type', type(query.input).__name__)}" ) - return query, None, None, None + return query, None, None, None, None original_input_text = query.input.content.value outcome = apply_guardrails( @@ -408,13 +414,19 @@ def apply_input_guardrails( } if outcome.error is not None: - return query, outcome.error, None, metadata + return ( + query, + outcome.error, + None, + metadata, + GuardrailOutcomeEnum.BLOCKED if outcome.blocked else None, + ) if outcome.rephrase_needed: logger.info( f"[apply_input_guardrails] rephrase_needed=True, returning safe_text directly | job_id={job_id}" ) - return query, None, outcome.safe_text, metadata + return query, None, outcome.safe_text, metadata, GuardrailOutcomeEnum.REPHRASED # No-op paths (no validators, bypassed) leave the query untouched. if outcome.applied and outcome.safe_text is not None: @@ -428,9 +440,10 @@ def apply_input_guardrails( "Input guardrails rejected the request and left no usable content.", None, metadata, + GuardrailOutcomeEnum.BLOCKED, ) query.input.content.value = outcome.safe_text - return query, None, None, metadata + return query, None, None, metadata, None def apply_output_guardrails( @@ -442,13 +455,14 @@ def apply_output_guardrails( organization_id: int, input_text: str | None = None, include_guardrail_metadata: bool = False, -) -> tuple[BlockResult, str | None]: +) -> tuple[BlockResult, str | None, GuardrailOutcomeEnum | None]: """Apply output guardrails from a config_blob. Shared by /llm/call and /llm/chain. - Returns (modified_result, None) on success, or (result, error_string) on failure. + Returns (modified_result, None, None) on success, or + (result, error_string, guardrail_outcome) on failure. """ if not config_blob or not config_blob.output_guardrails: - return result, None + return result, None, None if not isinstance(result.response.response.output, TextOutput): logger.info( @@ -456,7 +470,7 @@ def apply_output_guardrails( f"job_id={job_id}, " f"output_type={getattr(result.response.response.output, 'type', type(result.response.response.output).__name__)}" ) - return result, None + return result, None, None original_output_text = result.response.response.output.content.value outcome = apply_guardrails( @@ -478,7 +492,11 @@ def apply_output_guardrails( result.metadata = existing_metadata if outcome.error is not None: - return result, outcome.error + return ( + result, + outcome.error, + GuardrailOutcomeEnum.BLOCKED if outcome.blocked else None, + ) if outcome.applied and outcome.safe_text is not None: if not outcome.safe_text.strip(): @@ -489,9 +507,10 @@ def apply_output_guardrails( return ( result, "Output guardrails rejected the response and left no usable content.", + GuardrailOutcomeEnum.BLOCKED, ) result.response.response.output.content.value = outcome.safe_text - return result, None + return result, None, None def persist_output_guardrail_result( @@ -564,6 +583,7 @@ def execute_llm_call( include_guardrail_metadata: bool = False, chain_id: UUID | None = None, detected_language: str | None = None, + record_call: bool = True, ) -> BlockResult: """Execute a single LLM call. Shared by /llm/call and /llm/chain. @@ -571,15 +591,19 @@ def execute_llm_call( Args: detected_language: Language code detected by STT (used to replace {{detected}} marker in TTS) + record_call: When False the call is not production traffic — no `LlmCall` + row is written, no AI spans are emitted and no LLM metrics are + recorded. Used by evaluation runs. """ config_blob: ConfigBlob | None = None llm_call_id: UUID | None = None trace_id = correlation_id.get() + _tracer = tracer if record_call else trace.NoOpTracer() try: with Session(engine) as session: - with tracer.start_as_current_span("llm.resolve_config") as cfg_span: + with _tracer.start_as_current_span("llm.resolve_config") as cfg_span: _set_traceability_attributes( cfg_span, job_id=job_id, @@ -634,7 +658,7 @@ def execute_llm_call( interpolated = template.replace("{{input}}", query.input.content.value) query.input.content.value = interpolated - with tracer.start_as_current_span("llm.guardrails.input") as guard_span: + with _tracer.start_as_current_span("llm.guardrails.input") as guard_span: _set_traceability_attributes( guard_span, job_id=job_id, @@ -648,6 +672,7 @@ def execute_llm_call( input_error, guardrail_direct_response, input_guardrail_metadata, + input_guardrail_outcome, ) = apply_input_guardrails( config_blob=config_blob, query=query, @@ -687,29 +712,34 @@ def execute_llm_call( ) if original_input_value is not None: query.input.content.value = original_input_value - llm_call_id = save_rephrase_guardrail_call( - session=session, - query=query, - config=config, - request_metadata=request_metadata, - config_blob=config_blob, - guardrail_direct_response=guardrail_direct_response, - job_id=job_id, - project_id=project_id, - organization_id=organization_id, - chain_id=chain_id, - ) + if record_call: + llm_call_id = save_rephrase_guardrail_call( + session=session, + query=query, + config=config, + request_metadata=request_metadata, + config_blob=config_blob, + guardrail_direct_response=guardrail_direct_response, + job_id=job_id, + project_id=project_id, + organization_id=organization_id, + chain_id=chain_id, + ) return BlockResult( response=llm_response, usage=guardrail_usage, metadata=request_metadata, llm_call_id=llm_call_id, + guardrail_outcome=input_guardrail_outcome, ) if input_error: guard_span.set_status( trace.Status(trace.StatusCode.ERROR, input_error) ) - return BlockResult(error=input_error) + return BlockResult( + error=input_error, + guardrail_outcome=input_guardrail_outcome, + ) # proxy branch, bypass execution of API CAll if config_blob.completion.type == Provider.PROXY.value: if not isinstance(query.input, TextInput): @@ -749,23 +779,24 @@ def execute_llm_call( ) try: - llm_call_request = LLMCallRequest( - query=query, - config=config, - request_metadata=request_metadata, - ) - llm_call = create_llm_call( - session, - request=llm_call_request, - job_id=job_id, - project_id=project_id, - organization_id=organization_id, - resolved_config=config_blob, - original_provider=Provider.PROXY.value, - chain_id=chain_id, - metadata=request_metadata, - ) - llm_call_id = llm_call.id + if record_call: + llm_call_request = LLMCallRequest( + query=query, + config=config, + request_metadata=request_metadata, + ) + llm_call = create_llm_call( + session, + request=llm_call_request, + job_id=job_id, + project_id=project_id, + organization_id=organization_id, + resolved_config=config_blob, + original_provider=Provider.PROXY.value, + chain_id=chain_id, + metadata=request_metadata, + ) + llm_call_id = llm_call.id except Exception as e: logger.error( f"[execute_llm_call] Failed to create proxy LLM call record: {e} | job_id={job_id}", @@ -775,17 +806,18 @@ def execute_llm_call( error=f"Failed to create LLM call record: {str(e)}" ) - record_llm_call_started( - provider=Provider.PROXY.value, - model="", - operation="chat", - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_started( + provider=Provider.PROXY.value, + model="", + operation="chat", + organization_id=organization_id, + project_id=project_id, + ) proxy_started_at = time.perf_counter() proxy_data: dict | None = None - with tracer.start_as_current_span("llm.proxy.execute") as proxy_span: + with _tracer.start_as_current_span("llm.proxy.execute") as proxy_span: _set_traceability_attributes( proxy_span, job_id=job_id, @@ -817,15 +849,17 @@ def execute_llm_call( proxy_span.set_status( trace.Status(trace.StatusCode.ERROR, str(e)) ) - record_llm_call_finished( - provider=Provider.PROXY.value, - model="", - operation="chat", - duration_ms=(time.perf_counter() - proxy_started_at) * 1000, - error=True, - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_finished( + provider=Provider.PROXY.value, + model="", + operation="chat", + duration_ms=(time.perf_counter() - proxy_started_at) + * 1000, + error=True, + organization_id=organization_id, + project_id=project_id, + ) logger.error( f"[execute_llm_call] Proxy call failed: {e} | job_id={job_id}, url={client_llm_url}", exc_info=True, @@ -833,6 +867,7 @@ def execute_llm_call( return BlockResult( error=f"Proxy call failed: {str(e)}", llm_call_id=llm_call_id, + retryable=True, ) try: @@ -865,33 +900,35 @@ def execute_llm_call( usage=proxy_usage, ) - try: - update_llm_call_response( - session, - llm_call_id=llm_call_id, - provider_response_id=provider_response_id, - content=proxy_response.response.output.model_dump(), - usage=proxy_usage.model_dump(), - conversation_id=None, - ) - except Exception as e: - logger.error( - f"[execute_llm_call] Failed to update proxy LLM call record: {e} | llm_call_id={llm_call_id}", - exc_info=True, - ) + if llm_call_id: + try: + update_llm_call_response( + session, + llm_call_id=llm_call_id, + provider_response_id=provider_response_id, + content=proxy_response.response.output.model_dump(), + usage=proxy_usage.model_dump(), + conversation_id=None, + ) + except Exception as e: + logger.error( + f"[execute_llm_call] Failed to update proxy LLM call record: {e} | llm_call_id={llm_call_id}", + exc_info=True, + ) # sentry emit metrics - record_llm_call_finished( - provider=Provider.PROXY.value, - model=proxy_model, - operation="chat", - duration_ms=(time.perf_counter() - proxy_started_at) * 1000, - input_tokens=proxy_usage.input_tokens, - output_tokens=proxy_usage.output_tokens, - total_tokens=proxy_usage.total_tokens, - error=False, - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_finished( + provider=Provider.PROXY.value, + model=proxy_model, + operation="chat", + duration_ms=(time.perf_counter() - proxy_started_at) * 1000, + input_tokens=proxy_usage.input_tokens, + output_tokens=proxy_usage.output_tokens, + total_tokens=proxy_usage.total_tokens, + error=False, + organization_id=organization_id, + project_id=project_id, + ) result = BlockResult( response=proxy_response, @@ -900,7 +937,7 @@ def execute_llm_call( metadata=request_metadata, ) - with tracer.start_as_current_span( + with _tracer.start_as_current_span( "llm.guardrails.output" ) as out_guard_span: _set_traceability_attributes( @@ -912,7 +949,11 @@ def execute_llm_call( project_id=project_id, organization_id=organization_id, ) - result, output_error = apply_output_guardrails( + ( + result, + output_error, + output_guardrail_outcome, + ) = apply_output_guardrails( config_blob=config_blob, result=result, job_id=job_id, @@ -925,7 +966,13 @@ def execute_llm_call( out_guard_span.set_status( trace.Status(trace.StatusCode.ERROR, output_error) ) - return BlockResult(error=output_error, llm_call_id=llm_call_id) + # Proxy already billed these tokens; keep them on the result. + return BlockResult( + error=output_error, + llm_call_id=llm_call_id, + usage=proxy_usage, + guardrail_outcome=output_guardrail_outcome, + ) if config_blob.output_guardrails: updated_content = None if isinstance(result.response.response.output, TextOutput): @@ -952,7 +999,10 @@ def execute_llm_call( ), ): completion_config, warnings = transform_kaapi_config_to_native( - session=session, kaapi_config=completion_config + session=session, + kaapi_config=completion_config, + # Hits only come via the raw response; skip unless requested. + include_file_search_results=include_provider_raw_response, ) existing = request_metadata or {} existing_warnings = list(existing.get("warnings") or []) @@ -977,7 +1027,7 @@ def execute_llm_call( output_guardrails=config_blob.output_guardrails, ) - with tracer.start_as_current_span("llm.create_call_record") as create_span: + with _tracer.start_as_current_span("llm.create_call_record") as create_span: _set_traceability_attributes( create_span, job_id=job_id, @@ -992,28 +1042,31 @@ def execute_llm_call( if model_name: create_span.set_attribute("llm.request.model", model_name) try: - llm_call_request = LLMCallRequest( - query=query, - config=config, - request_metadata=request_metadata, - ) - llm_call = create_llm_call( - session, - request=llm_call_request, - job_id=job_id, - project_id=project_id, - organization_id=organization_id, - resolved_config=resolved_config_blob, - original_provider=original_provider, - chain_id=chain_id, - metadata=request_metadata, - ) - llm_call_id = llm_call.id - _set_traceability_attributes(create_span, llm_call_id=llm_call_id) - logger.info( - f"[execute_llm_call] Created LLM call record | " - f"llm_call_id={llm_call_id}, job_id={job_id}" - ) + if record_call: + llm_call_request = LLMCallRequest( + query=query, + config=config, + request_metadata=request_metadata, + ) + llm_call = create_llm_call( + session, + request=llm_call_request, + job_id=job_id, + project_id=project_id, + organization_id=organization_id, + resolved_config=resolved_config_blob, + original_provider=original_provider, + chain_id=chain_id, + metadata=request_metadata, + ) + llm_call_id = llm_call.id + _set_traceability_attributes( + create_span, llm_call_id=llm_call_id + ) + logger.info( + f"[execute_llm_call] Created LLM call record | " + f"llm_call_id={llm_call_id}, job_id={job_id}" + ) except Exception as e: create_span.record_exception(e) create_span.set_status(trace.Status(trace.StatusCode.ERROR, str(e))) @@ -1099,19 +1152,20 @@ def execute_llm_call( model_name = str(completion_config.params.get("model") or "") completion_type = str(completion_config.type or "") # sentry emit - record_llm_call_started( - provider=provider_name, - model=model_name, - operation=operation, - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_started( + provider=provider_name, + model=model_name, + operation=operation, + organization_id=organization_id, + project_id=project_id, + ) provider_started_at = time.perf_counter() response = None error = None ai_span_name = f"chat {model_name}" if model_name else f"chat {provider_name}" - with tracer.start_as_current_span(ai_span_name) as ai_span: + with _tracer.start_as_current_span(ai_span_name) as ai_span: ai_span.set_attribute("sentry.op", "gen_ai.chat") _set_traceability_attributes( ai_span, @@ -1136,7 +1190,7 @@ def execute_llm_call( try: with resolved_input_context(query.input) as resolved_input: - with tracer.start_as_current_span( + with _tracer.start_as_current_span( "llm.provider.execute" ) as provider_span: _set_traceability_attributes( @@ -1173,15 +1227,16 @@ def execute_llm_call( ) except ValueError as ve: ai_span.set_status(trace.Status(trace.StatusCode.ERROR, str(ve))) - record_llm_call_finished( - provider=provider_name, - model=model_name, - operation=operation, - duration_ms=(time.perf_counter() - provider_started_at) * 1000, - error=True, - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_finished( + provider=provider_name, + model=model_name, + operation=operation, + duration_ms=(time.perf_counter() - provider_started_at) * 1000, + error=True, + organization_id=organization_id, + project_id=project_id, + ) return BlockResult(error=str(ve), llm_call_id=llm_call_id) if response: @@ -1241,7 +1296,7 @@ def execute_llm_call( with Session(engine) as session: if llm_call_id: - with tracer.start_as_current_span( + with _tracer.start_as_current_span( "llm.update_call_record" ) as update_span: _set_traceability_attributes( @@ -1273,19 +1328,19 @@ def execute_llm_call( exc_info=True, ) - duration_ms = (time.perf_counter() - provider_started_at) * 1000 - record_llm_call_finished( - provider=provider_name, - model=model_name, - operation=operation, - duration_ms=duration_ms, - input_tokens=response.usage.input_tokens, - output_tokens=response.usage.output_tokens, - total_tokens=response.usage.total_tokens, - error=False, - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_finished( + provider=provider_name, + model=model_name, + operation=operation, + duration_ms=(time.perf_counter() - provider_started_at) * 1000, + input_tokens=response.usage.input_tokens, + output_tokens=response.usage.output_tokens, + total_tokens=response.usage.total_tokens, + error=False, + organization_id=organization_id, + project_id=project_id, + ) result = BlockResult( response=response, @@ -1294,7 +1349,7 @@ def execute_llm_call( metadata=request_metadata, ) - with tracer.start_as_current_span( + with _tracer.start_as_current_span( "llm.guardrails.output" ) as out_guard_span: _set_traceability_attributes( @@ -1306,7 +1361,11 @@ def execute_llm_call( project_id=project_id, organization_id=organization_id, ) - result, output_error = apply_output_guardrails( + ( + result, + output_error, + output_guardrail_outcome, + ) = apply_output_guardrails( config_blob=config_blob, result=result, job_id=job_id, @@ -1319,7 +1378,13 @@ def execute_llm_call( out_guard_span.set_status( trace.Status(trace.StatusCode.ERROR, output_error) ) - return BlockResult(error=output_error, llm_call_id=llm_call_id) + # Provider already billed these tokens; keep them on the result. + return BlockResult( + error=output_error, + llm_call_id=llm_call_id, + usage=response.usage, + guardrail_outcome=output_guardrail_outcome, + ) if config_blob.output_guardrails: updated_content = None if isinstance(result.response.response.output, TextOutput): @@ -1332,18 +1397,20 @@ def execute_llm_call( return result - duration_ms = (time.perf_counter() - provider_started_at) * 1000 - record_llm_call_finished( - provider=provider_name, - model=model_name, - operation=operation, - duration_ms=duration_ms, - error=True, - organization_id=organization_id, - project_id=project_id, - ) + if record_call: + record_llm_call_finished( + provider=provider_name, + model=model_name, + operation=operation, + duration_ms=(time.perf_counter() - provider_started_at) * 1000, + error=True, + organization_id=organization_id, + project_id=project_id, + ) error_message = error or "Unknown error occurred" - return BlockResult(error=error_message, llm_call_id=llm_call_id) + # ponytail: every provider failure retries, even deterministic 4xx (unbilled). + # Upgrade: have providers return the error category. + return BlockResult(error=error_message, llm_call_id=llm_call_id, retryable=True) except (Timeout, SoftTimeLimitExceeded): raise diff --git a/backend/app/services/llm/mappers.py b/backend/app/services/llm/mappers.py index 88c26cade..0235a9c5a 100644 --- a/backend/app/services/llm/mappers.py +++ b/backend/app/services/llm/mappers.py @@ -726,6 +726,7 @@ def resolve_default_audio_provider( def transform_kaapi_config_to_native( session: Session, kaapi_config: KaapiCompletionConfig, + include_file_search_results: bool = False, ) -> tuple[NativeCompletionConfig, list[str]]: """Transform Kaapi completion config to native provider config with mapped parameters. @@ -734,6 +735,11 @@ def transform_kaapi_config_to_native( Args: session: Database session used to look up model-specific config (e.g. reasoning support) kaapi_config: KaapiCompletionConfig with abstracted parameters + include_file_search_results: Ask OpenAI to return the text of the file_search + hits, not just the fact that a search ran. Off by default: it enlarges + every response, and the chunks are only reachable by a caller that also + asked for the raw provider response. Evaluation turns it on so the + `knowledge_base` metric has chunks to score. Returns: Tuple of: @@ -746,6 +752,11 @@ def transform_kaapi_config_to_native( mapped_params, warnings = map_kaapi_to_openai_params( session=session, kaapi_params=kaapi_params ) + # Not in the mapper: Batch API bodies reject `include`. + if include_file_search_results and any( + t.get("type") == "file_search" for t in mapped_params.get("tools", []) + ): + mapped_params["include"] = ["file_search_call.results"] return ( NativeCompletionConfig( provider="openai-native", params=mapped_params, type=kaapi_config.type diff --git a/backend/app/tests/api/routes/test_evaluation_fast.py b/backend/app/tests/api/routes/test_evaluation_fast.py index 3f2a8eee0..3fdf49d36 100644 --- a/backend/app/tests/api/routes/test_evaluation_fast.py +++ b/backend/app/tests/api/routes/test_evaluation_fast.py @@ -12,7 +12,6 @@ from typing import Any from unittest.mock import MagicMock, patch -import openai import pytest from fastapi import HTTPException from fastapi.testclient import TestClient @@ -22,10 +21,8 @@ from app.core.util import now from app.crud.evaluations.cron import dispatch_fast_evaluation_barriers from app.crud.evaluations.fast import ( - _create_response, _merge_response_chunks, _stage2_embeddings, - _stage3_score_and_trace, run_fast_evaluation, run_response_chunk, ) @@ -42,16 +39,19 @@ from app.models import Config, EvaluationDataset, EvaluationRun from app.models.batch_job import BatchJob from app.models.evaluation import RunModeEnum +from app.models.llm import Usage from app.models.llm.request import ( ConfigBlob, - TextLLMParams, + NativeCompletionConfig, build_kaapi_completion_config, ) from app.services.evaluations.fast import ( execute_fast_evaluation_chunk, validate_and_start_fast_evaluation, ) +from app.services.llm.chain.types import BlockResult from app.tests.utils.auth import TestAuthContext +from app.tests.utils.llm import text_llm_call_response from app.tests.utils.test_data import ( create_test_config, create_test_evaluation_dataset, @@ -89,48 +89,6 @@ def test_returns_false_at_threshold(self) -> None: assert is_failure_threshold_breached(failed_rows=5, total_rows=10) is False -class TestCallWithRetry: - """FR-8: transient OpenAI errors retry; permanent ones do not.""" - - def test_returns_immediately_on_success(self) -> None: - client = MagicMock() - client.responses.create.return_value = "ok" - - result = _create_response(client, {"model": "gpt-4o"}) - - assert result == "ok" - assert client.responses.create.call_count == 1 - - def test_retries_on_transient_then_succeeds(self, monkeypatch) -> None: - # tenacity sleeps via tenacity.nap.sleep — make backoff a no-op. - monkeypatch.setattr("tenacity.nap.sleep", lambda *_: None) - - client = MagicMock() - client.responses.create.side_effect = [ - # APIConnectionError needs a request; pass a minimal object. - openai.APIConnectionError(request=MagicMock()), - openai.APIConnectionError(request=MagicMock()), - "ok", - ] - - result = _create_response(client, {"model": "gpt-4o"}) - - assert result == "ok" - assert client.responses.create.call_count == 3 - - def test_does_not_retry_on_permanent_error(self) -> None: - client = MagicMock() - # AuthenticationError is a non-retryable OpenAIError subclass. - client.responses.create.side_effect = openai.AuthenticationError( - message="bad key", response=MagicMock(), body=None - ) - - with pytest.raises(openai.AuthenticationError): - _create_response(client, {"model": "gpt-4o"}) - - assert client.responses.create.call_count == 1 - - # Shared factories + mock boundaries @@ -236,6 +194,15 @@ def _resp_result( } +def _blocked_resp_result(item_id: str, question: str = "Q") -> dict[str, Any]: + """A Stage-1 row guardrails hard-blocked: empty output, but `failed=False`.""" + return { + **_resp_result(item_id, question), + "generated_output": "", + "guardrail": "blocked: input flagged as abusive", + } + + @pytest.fixture def _s3_store() -> Iterator[dict[str, list[dict[str, Any]]]]: """Back the S3 upload/load edge with an in-memory dict keyed by url. @@ -494,16 +461,6 @@ def test_fr5_filters_to_fast_eligible_only( # Response fixtures for the OpenAI SDK shapes -def _fake_openai_response(text: str = "answer", item_id: str = "item-1"): - """Mimic the SDK's response.responses.create return shape.""" - return SimpleNamespace( - id=f"resp_{item_id}", - output_text=text, - output=[], - usage=SimpleNamespace(input_tokens=10, output_tokens=20, total_tokens=30), - ) - - def _fake_embedding_response(): """Mimic openai.embeddings.create return shape (2 identical vectors).""" return SimpleNamespace( @@ -518,6 +475,31 @@ def _fake_embedding_response(): # run_response_chunk: one parallel responses chunk + idempotency +def _kaapi_config_blob(model: str = "gpt-4o") -> ConfigBlob: + return ConfigBlob( + completion=build_kaapi_completion_config( + provider="openai", + type="text", + params={"model": model, "temperature": 0.7}, + ) + ) + + +def _native_config_blob(model: str = "gpt-5-native") -> ConfigBlob: + return ConfigBlob( + completion=NativeCompletionConfig( + provider="openai-native", type="text", params={"model": model} + ) + ) + + +def _ok_block_result() -> BlockResult: + return BlockResult( + response=text_llm_call_response(), + usage=Usage(input_tokens=5, output_tokens=7, total_tokens=12), + ) + + class TestRunResponseChunk: def test_writes_chunk_job_and_partial_unit( self, @@ -528,62 +510,111 @@ def test_writes_chunk_job_and_partial_unit( eval_run = _make_fast_run(db=db, user_api_key=user_api_key) items = [_dataset_item("item-1", "Q1"), _dataset_item("item-2", "Q2")] - fake_openai = MagicMock() - fake_openai.responses.create.side_effect = lambda **_: _fake_openai_response() - with patch( - "app.crud.evaluations.fast.map_kaapi_to_openai_params", - return_value=({"model": "gpt-4o"}, []), - ): + "app.crud.evaluations.fast.execute_llm_call", + return_value=_ok_block_result(), + ) as mock_execute: run_response_chunk( session=db, - openai_client=fake_openai, eval_run=eval_run, - config=TextLLMParams(model="gpt-4o", instructions="x"), + config_blob=_kaapi_config_blob(), dataset_items_slice=items, chunk_index=0, ) - assert fake_openai.responses.create.call_count == 2 + assert mock_execute.call_count == 2 job = get_chunk_job(session=db, eval_run_id=eval_run.id, chunk_index=0) assert job is not None assert job.job_type == JOB_TYPE_EVALUATION_FAST_CHUNK assert job.config[CHUNK_CONFIG_RUN_ID] == eval_run.id assert job.config[CHUNK_CONFIG_INDEX] == 0 + assert job.config["model"] == "gpt-4o" + assert job.config["usage"]["total_tokens"] == 24 assert job.raw_output_url == f"s3://bucket/responses_{eval_run.id}_0.json" assert len(_s3_store[job.raw_output_url]) == 2 - def test_idempotent_skips_openai_when_chunk_already_done( + def test_reads_the_batch_job_model_from_native_dict_params( self, db: Session, user_api_key: TestAuthContext, _s3_store, ): eval_run = _make_fast_run(db=db, user_api_key=user_api_key) - items = [_dataset_item("item-1", "Q1"), _dataset_item("item-2", "Q2")] - fake_openai = MagicMock() - fake_openai.responses.create.side_effect = lambda **_: _fake_openai_response() + with patch( + "app.crud.evaluations.fast.execute_llm_call", + return_value=_ok_block_result(), + ): + run_response_chunk( + session=db, + eval_run=eval_run, + config_blob=_native_config_blob("gpt-5-native"), + dataset_items_slice=[_dataset_item("item-1", "Q1")], + chunk_index=0, + ) + + job = get_chunk_job(session=db, eval_run_id=eval_run.id, chunk_index=0) + assert job.config["model"] == "gpt-5-native" + + def test_worker_gets_the_resolved_config_blob_and_tenant_ids( + self, + db: Session, + user_api_key: TestAuthContext, + _s3_store, + ): + eval_run = _make_fast_run(db=db, user_api_key=user_api_key) + config_blob = _kaapi_config_blob() + captured: dict[str, Any] = {} + + def _capture(*, config, project_id, organization_id, item): + captured.update( + config=config, project_id=project_id, organization_id=organization_id + ) + return {"item_id": item["id"], "failed": False, "usage": {}} + + with patch( + "app.crud.evaluations.fast._llm_call_for_item", side_effect=_capture + ): + run_response_chunk( + session=db, + eval_run=eval_run, + config_blob=config_blob, + dataset_items_slice=[_dataset_item("item-1", "Q1")], + chunk_index=0, + ) + assert captured["config"].is_stored_config is False + assert captured["config"].blob is config_blob + assert captured["project_id"] == eval_run.project_id + assert captured["organization_id"] == eval_run.organization_id + + def test_idempotent_skips_generation_when_chunk_already_done( + self, + db: Session, + user_api_key: TestAuthContext, + _s3_store, + ): + eval_run = _make_fast_run(db=db, user_api_key=user_api_key) + items = [_dataset_item("item-1", "Q1"), _dataset_item("item-2", "Q2")] kwargs = { "session": db, - "openai_client": fake_openai, "eval_run": eval_run, - "config": TextLLMParams(model="gpt-4o", instructions="x"), + "config_blob": _kaapi_config_blob(), "dataset_items_slice": items, "chunk_index": 0, } + with patch( - "app.crud.evaluations.fast.map_kaapi_to_openai_params", - return_value=({"model": "gpt-4o"}, []), - ): + "app.crud.evaluations.fast.execute_llm_call", + return_value=_ok_block_result(), + ) as mock_execute: run_response_chunk(**kwargs) - assert fake_openai.responses.create.call_count == 2 + assert mock_execute.call_count == 2 - # Second run for the same (run, index) must not re-charge OpenAI. + # Second run for the same (run, index) must not re-charge the provider. run_response_chunk(**kwargs) - assert fake_openai.responses.create.call_count == 2 + assert mock_execute.call_count == 2 jobs = list_response_chunk_jobs(session=db, eval_run_id=eval_run.id) assert len([j for j in jobs if j.config[CHUNK_CONFIG_INDEX] == 0]) == 1 @@ -737,6 +768,103 @@ def test_fr7_stage2_skips_when_embedding_batch_job_id_already_set( fake_openai.embeddings.create.assert_not_called() +class TestStage2GuardrailBlocked: + def test_blocked_rows_are_neither_embedded_nor_counted_as_failures( + self, + db: Session, + user_api_key: TestAuthContext, + _s3_store, + ): + eval_run = _make_fast_run(db=db, user_api_key=user_api_key, total_items=4) + response_results = [_resp_result("item-1", "Q1")] + [ + _blocked_resp_result(f"item-{n}", f"Q{n}") for n in (2, 3, 4) + ] + fake_openai = MagicMock() + fake_openai.embeddings.create.return_value = _fake_embedding_response() + + _, results = _stage2_embeddings( + session=db, + openai_client=fake_openai, + eval_run=eval_run, + response_results=response_results, + ) + + # 3/4 blocked would exceed the 0.5 failure threshold if counted as failed. + assert [r["item_id"] for r in results] == ["item-1"] + assert fake_openai.embeddings.create.call_count == 1 + assert fake_openai.embeddings.create.call_args.kwargs["input"] == [ + "answer to Q1", + "A", + ] + + def test_a_rephrased_row_is_embedded_like_any_answered_row( + self, + db: Session, + user_api_key: TestAuthContext, + _s3_store, + ): + eval_run = _make_fast_run(db=db, user_api_key=user_api_key, total_items=2) + rephrased = { + **_resp_result("item-1", "Q1"), + "generated_output": "I can't help with that, but here's what I can do.", + "guardrail": "rephrased", + } + fake_openai = MagicMock() + fake_openai.embeddings.create.return_value = _fake_embedding_response() + + _, results = _stage2_embeddings( + session=db, + openai_client=fake_openai, + eval_run=eval_run, + response_results=[rephrased, _blocked_resp_result("item-2", "Q2")], + ) + + assert [r["item_id"] for r in results] == ["item-1"] + assert fake_openai.embeddings.create.call_args.kwargs["input"] == [ + "I can't help with that, but here's what I can do.", + "A", + ] + + def test_every_row_blocked_still_writes_the_retry_skip_marker( + self, + db: Session, + user_api_key: TestAuthContext, + _s3_store, + ): + eval_run = _make_fast_run(db=db, user_api_key=user_api_key, total_items=2) + response_results = [_blocked_resp_result("item-1"), _blocked_resp_result("i-2")] + fake_openai = MagicMock() + + updated_run, results = _stage2_embeddings( + session=db, + openai_client=fake_openai, + eval_run=eval_run, + response_results=response_results, + ) + + assert results == [] + fake_openai.embeddings.create.assert_not_called() + marker = db.get(BatchJob, updated_run.embedding_batch_job_id) + assert marker.job_type == JOB_TYPE_EMBEDDING_FAST + assert _s3_store[marker.raw_output_url] == [] + + # The marker is what makes the stage skippable, so a rerun must reload it. + _, rerun_results = _stage2_embeddings( + session=db, + openai_client=fake_openai, + eval_run=updated_run, + response_results=response_results, + ) + assert rerun_results == [] + embedding_jobs = db.exec( + select(BatchJob).where( + BatchJob.project_id == eval_run.project_id, + BatchJob.job_type == JOB_TYPE_EMBEDDING_FAST, + ) + ).all() + assert len(embedding_jobs) == 1 + + # End-to-end aggregate pipeline with mocked externals (FR-9..FR-14) @@ -1073,11 +1201,7 @@ def test_chunk_failure_reraises_and_leaves_run_processing( ), patch( "app.services.evaluations.fast._resolve_config_and_clients", - return_value=( - TextLLMParams(model="gpt-4o", instructions="x"), - MagicMock(), - MagicMock(), - ), + return_value=(_kaapi_config_blob(), MagicMock()), ), patch( "app.services.evaluations.fast.fetch_dataset_items", @@ -1163,11 +1287,7 @@ def _capture(*, dataset_items_slice, **_): ), patch( "app.services.evaluations.fast._resolve_config_and_clients", - return_value=( - TextLLMParams(model="gpt-4o", instructions="x"), - MagicMock(), - MagicMock(), - ), + return_value=(_kaapi_config_blob(), MagicMock()), ), patch( "app.services.evaluations.fast.fetch_dataset_items", diff --git a/backend/app/tests/crud/evaluations/test_fast_cosine.py b/backend/app/tests/crud/evaluations/test_fast_cosine.py index 9bd6de8df..1c62b348a 100644 --- a/backend/app/tests/crud/evaluations/test_fast_cosine.py +++ b/backend/app/tests/crud/evaluations/test_fast_cosine.py @@ -148,3 +148,51 @@ def test_per_item_scores_are_keyed_by_ref_not_item_id(self) -> None: total_items=1, ) assert result.per_item_scores == {"trace-a": 1.0} + + +class TestGuardrailBlockedScoring: + def test_blocked_row_is_guardrail_blocked_not_empty_output(self) -> None: + response = _response("i", output="") + response["guardrail"] = "blocked: input flagged as abusive" + assert classify_empty_side(response) == "guardrail_blocked" + + def test_blocked_rows_land_in_their_own_summary_bucket(self) -> None: + responses = [_response("a")] + for item_id in ("b", "c"): + blocked = _response(item_id, output="") + blocked["guardrail"] = "blocked: input flagged as abusive" + responses.append(blocked) + + result = score_cosine_run( + response_results=responses, + embedding_results=[_embedding("a", [1.0, 0.0], [1.0, 0.0])], + item_refs=build_item_refs(responses, {}), + trace_id_mapping={}, + total_items=3, + ) + + assert result.unscoreable == { + "b": "guardrail_blocked", + "c": "guardrail_blocked", + } + summary = next( + s for s in result.summary_scores if s["name"] == COSINE_SCORE_NAME + ) + # "other" = reason missing from UNSCOREABLE_REASONS. + assert summary["unscoreable"] == {"guardrail_blocked": 2} + + def test_rephrased_row_is_scored_like_any_other(self) -> None: + response = _response("a", output="the canned safe answer") + response["guardrail"] = "rephrased" + assert classify_empty_side(response) is None + + result = score_cosine_run( + response_results=[response], + embedding_results=[_embedding("a", [1.0, 0.0], [1.0, 0.0])], + item_refs=build_item_refs([response], {}), + trace_id_mapping={}, + total_items=1, + ) + + assert result.unscoreable == {} + assert result.item_id_to_score == {"a": 1.0} diff --git a/backend/app/tests/crud/evaluations/test_fast_judge.py b/backend/app/tests/crud/evaluations/test_fast_judge.py index 763a5e960..fb7dcb728 100644 --- a/backend/app/tests/crud/evaluations/test_fast_judge.py +++ b/backend/app/tests/crud/evaluations/test_fast_judge.py @@ -23,16 +23,16 @@ from types import SimpleNamespace from typing import Any from unittest.mock import MagicMock, patch +from uuid import uuid4 -import openai import pytest from sqlmodel import Session from app.core.config import settings from app.crud.evaluations.fast import ( - _responses_call_for_item, + _llm_call_for_item, + _score_judge_path, run_fast_evaluation, - run_response_chunk, ) from app.crud.evaluations.fast_chunks import ( CHUNK_CONFIG_INDEX, @@ -53,13 +53,14 @@ from app.models.evaluation import RunModeEnum from app.models.llm.request import ( ConfigBlob, + LLMCallConfig, PromptTemplate, - TextLLMParams, build_kaapi_completion_config, ) -from app.models.response import FileResultChunk +from app.services.llm.chain.types import BlockResult from app.services.llm.providers.claude import STOP_REASON_COMPLETE from app.tests.utils.auth import TestAuthContext +from app.tests.utils.llm import text_llm_call_response from app.tests.utils.test_data import ( create_test_config, create_test_evaluation_dataset, @@ -68,6 +69,9 @@ COSINE_SCORE_NAME = "Cosine Similarity" +EVAL_PROJECT_ID = 101 +EVAL_ORG_ID = 202 + def _make_dataset(*, db: Session, user_api_key: TestAuthContext) -> EvaluationDataset: return create_test_evaluation_dataset( @@ -1079,34 +1083,37 @@ def _responses_item(item_id: str = "item-1") -> dict[str, Any]: } -def _openai_response(): - return SimpleNamespace( - output_text="generated answer", - output=[], - id="resp_1", - usage=SimpleNamespace(input_tokens=5, output_tokens=5, total_tokens=10), - ) - - class TestResponsesChunkCapture: - """`_responses_call_for_item` flattens file_search hits into JSON-safe dicts.""" - - def test_success_with_chunks_returns_serializable_plain_dicts(self): - client = MagicMock() - client.responses.create.return_value = _openai_response() + @staticmethod + def _run_item(result: BlockResult) -> dict[str, Any]: with patch( - "app.crud.evaluations.fast.get_file_search_results", - return_value=[ - FileResultChunk(score=0.91, text="chunk A", filename="doc.pdf"), - FileResultChunk(score=0.42, text="chunk B"), - ], + "app.crud.evaluations.fast._execute_llm_call_for_question", + return_value=result, ): - result = _responses_call_for_item( - openai_client=client, - base_params={"model": "gpt-4o"}, + return _llm_call_for_item( + config=LLMCallConfig(id=uuid4(), version=1), + project_id=EVAL_PROJECT_ID, + organization_id=EVAL_ORG_ID, item=_responses_item(), ) + def test_success_with_chunks_returns_serializable_plain_dicts(self): + raw = { + "output": [ + { + "type": "file_search_call", + "results": [ + {"score": 0.91, "text": "chunk A", "filename": "doc.pdf"}, + {"score": 0.42, "text": "chunk B"}, + ], + } + ] + } + + result = self._run_item( + BlockResult(response=text_llm_call_response(provider_raw_response=raw)) + ) + assert result["failed"] is False # filename flows into the persisted unit so knowledge_base can name its matches. assert result["retrieved_chunks"] == [ @@ -1116,33 +1123,18 @@ def test_success_with_chunks_returns_serializable_plain_dicts(self): json.dumps(result) # the S3 unit must stay JSON-serializable def test_success_without_hits_returns_empty_chunks(self): - client = MagicMock() - client.responses.create.return_value = _openai_response() - with patch( - "app.crud.evaluations.fast.get_file_search_results", return_value=[] - ): - result = _responses_call_for_item( - openai_client=client, - base_params={"model": "gpt-4o"}, - item=_responses_item(), - ) + result = self._run_item( + BlockResult(response=text_llm_call_response(provider_raw_response={})) + ) assert result["failed"] is False assert result["retrieved_chunks"] == [] def test_error_path_has_no_chunks(self): - client = MagicMock() - client.responses.create.side_effect = openai.OpenAIError("provider down") - with patch("app.crud.evaluations.fast.get_file_search_results") as fake_search: - result = _responses_call_for_item( - openai_client=client, - base_params={"model": "gpt-4o"}, - item=_responses_item(), - ) + result = self._run_item(BlockResult(error="provider down")) assert result["failed"] is True assert result["retrieved_chunks"] is None - fake_search.assert_not_called() class TestKnowledgeBaseScoring: @@ -1457,60 +1449,34 @@ def test_empty_input_is_empty_string(self) -> None: assert format_top_kb_matches([]) == "" -class TestFileSearchIncludeParam: - """`run_response_chunk` requests file_search hits only when a file_search tool is present, - and never overrides tool_choice (stays at the model default / auto).""" - - def _run_and_capture_base_params( - self, *, db: Session, eval_run: EvaluationRun, tools: list[dict[str, Any]] - ) -> dict[str, Any]: - captured: dict[str, Any] = {} - - def _fake_call(*, openai_client, base_params, item): - captured.update(base_params) - return {"item_id": item["id"], "failed": False, "usage": {}} +class TestJudgePathGuardrailBlocked: + def test_blocked_row_keeps_guardrail_blocked_while_a_judged_row_fails( + self, + db: Session, + user_api_key: TestAuthContext, + ) -> None: + eval_run = _make_run(db=db, user_api_key=user_api_key, is_judge_run=True) + blocked = { + **_resp_result("item-1", "Q1"), + "generated_output": "", + "guardrail": "blocked: input flagged as abusive", + } + item_refs = {"item-1": "trace-1", "item-2": "trace-2"} - with ( - patch( - "app.crud.evaluations.fast.map_kaapi_to_openai_params", - return_value=({"model": "gpt-4o", "tools": tools}, []), - ), - patch( - "app.crud.evaluations.fast._responses_call_for_item", - side_effect=_fake_call, - ), - patch( - "app.crud.evaluations.fast._upload_unit_to_s3", - return_value="s3://bucket/chunk.json", - ), + with patch( + "app.crud.evaluations.judge._create_judge_response", + side_effect=RuntimeError("judge exploded"), ): - run_response_chunk( + outcome = _score_judge_path( session=db, openai_client=MagicMock(), + response_results=[blocked, _resp_result("item-2", "Q2")], + item_refs=item_refs, eval_run=eval_run, - config=TextLLMParams(model="gpt-4o"), - dataset_items_slice=[{"id": "item-1"}], - chunk_index=0, + log_prefix="test", ) - return captured - def test_file_search_present_sets_include_and_leaves_tool_choice_default( - self, db: Session, user_api_key: TestAuthContext - ): - eval_run = _make_run(db=db, user_api_key=user_api_key, is_judge_run=True) - base_params = self._run_and_capture_base_params( - db=db, eval_run=eval_run, tools=[{"type": "file_search"}] - ) - assert base_params["include"] == ["file_search_call.results"] - assert "tool_choice" not in base_params - - def test_no_file_search_leaves_include_and_tool_choice_unset( - self, db: Session, user_api_key: TestAuthContext - ): - eval_run = _make_run(db=db, user_api_key=user_api_key, is_judge_run=True) - base_params = self._run_and_capture_base_params( - db=db, eval_run=eval_run, tools=[] - ) - assert "include" not in base_params - assert "tool_choice" not in base_params - assert "include" not in base_params + assert outcome.unscoreable == { + "trace-1": "guardrail_blocked", + "trace-2": "judge_failed", + } diff --git a/backend/app/tests/crud/evaluations/test_fast_llm_call.py b/backend/app/tests/crud/evaluations/test_fast_llm_call.py new file mode 100644 index 000000000..e82769c6a --- /dev/null +++ b/backend/app/tests/crud/evaluations/test_fast_llm_call.py @@ -0,0 +1,295 @@ +"""Fast-eval generation against `execute_llm_call`.""" + +from collections.abc import Iterator +from typing import Any +from unittest.mock import MagicMock, patch +from uuid import UUID, uuid4 + +import pytest + +from app.crud.evaluations.fast import ( + _execute_llm_call_for_question, + _llm_call_for_item, +) +from app.models.llm import QueryParams, Usage +from app.models.llm.request import LLMCallConfig +from app.services.llm.chain.types import BlockResult +from app.tests.utils.llm import text_llm_call_response + +GUARDRAILS_AUTH_ERROR = "Guardrails service rejected the request (HTTP 401)" + +QUESTION = "What is X?" +PROJECT_ID = 11 +ORG_ID = 22 + +USAGE = Usage(input_tokens=5, output_tokens=7, total_tokens=12) +USAGE_DICT = {"input_tokens": 5, "output_tokens": 7, "total_tokens": 12} + + +@pytest.fixture +def no_backoff(monkeypatch: pytest.MonkeyPatch) -> Iterator[list[float]]: + """Record backoff; tenacity binds nap.sleep at import, so patch time.sleep.""" + recorded: list[float] = [] + monkeypatch.setattr("tenacity.nap.time.sleep", recorded.append) + yield recorded + + +def _config() -> LLMCallConfig: + return LLMCallConfig(id=uuid4(), version=3) + + +def _item( + item_id: str = "item-1", + *, + question: str | None = QUESTION, +) -> dict[str, Any]: + return { + "id": item_id, + "input": {"question": question} if question is not None else {}, + "expected_output": {"answer": "golden"}, + "metadata": {"question_id": 7}, + } + + +def _run_item( + result: BlockResult | Exception, item: dict[str, Any] | None = None +) -> tuple[dict[str, Any], MagicMock]: + kwargs = ( + {"side_effect": result} + if isinstance(result, Exception) + else {"return_value": result} + ) + with patch( + "app.crud.evaluations.fast._execute_llm_call_for_question", **kwargs + ) as mock_call: + return ( + _llm_call_for_item( + config=_config(), + project_id=PROJECT_ID, + organization_id=ORG_ID, + item=item if item is not None else _item(), + ), + mock_call, + ) + + +class TestBlockResultMapping: + def test_blocked_row_is_empty_output_not_failed_and_labelled(self) -> None: + result, _ = _run_item( + BlockResult( + guardrail_outcome="blocked", + error="uli_slur_match", + response=None, + usage=USAGE, + ) + ) + + assert result["generated_output"] == "" + assert result["failed"] is False + assert result["guardrail"] == "blocked: uli_slur_match" + + def test_blocked_row_keeps_the_tokens_the_provider_already_billed(self) -> None: + result, _ = _run_item( + BlockResult(guardrail_outcome="blocked", error="uli", usage=USAGE) + ) + + assert result["response_id"] is None + assert result["retrieved_chunks"] == [] + assert result["usage"] == USAGE_DICT + + def test_rephrased_row_carries_the_rephrase_text(self) -> None: + result, _ = _run_item( + BlockResult( + guardrail_outcome="rephrased", + response=text_llm_call_response("please rephrase your question"), + usage=USAGE, + ) + ) + + assert result["generated_output"] == "please rephrase your question" + assert result["failed"] is False + assert result["guardrail"] == "rephrased" + + def test_provider_error_without_a_verdict_fails_the_row(self) -> None: + result, _ = _run_item(BlockResult(error="provider 503", usage=USAGE)) + + assert result["failed"] is True + assert result["generated_output"] == "ERROR: provider 503" + assert result["guardrail"] is None + + @pytest.mark.parametrize("key", ["input_guardrail", "output_guardrail"]) + def test_success_with_guardrail_metadata_is_labelled_applied( + self, key: str + ) -> None: + result, _ = _run_item( + BlockResult( + response=text_llm_call_response("answer text"), + usage=USAGE, + metadata={key: {"validators": []}}, + ) + ) + + assert result["generated_output"] == "answer text" + assert result["failed"] is False + assert result["guardrail"] == "applied" + + def test_success_without_guardrail_metadata_has_no_label(self) -> None: + result, _ = _run_item( + BlockResult( + response=text_llm_call_response("answer text"), + usage=USAGE, + metadata={"latency_ms": 12}, + ) + ) + + assert result["generated_output"] == "answer text" + assert result["failed"] is False + assert result["guardrail"] is None + assert result["input_to_llm"] is None + assert result["output_from_llm"] is None + + def test_applied_row_carries_the_text_the_llm_saw_and_produced(self) -> None: + result, _ = _run_item( + BlockResult( + response=text_llm_call_response("call [REDACTED]"), + usage=USAGE, + metadata={ + "input_guardrail": { + "input_from_user": "my number is 98765", + "input_to_llm": "my number is [REDACTED]", + "validators": [], + }, + "output_guardrail": { + "output_from_llm": "call 98765", + "output_to_user": "call [REDACTED]", + "validators": [], + }, + }, + ) + ) + + assert result["guardrail"] == "applied" + assert result["input_to_llm"] == "my number is [REDACTED]" + assert result["output_from_llm"] == "call 98765" + # The scored answer stays the post-guardrail text. + assert result["generated_output"] == "call [REDACTED]" + + def test_only_the_side_that_applied_is_filled(self) -> None: + result, _ = _run_item( + BlockResult( + response=text_llm_call_response("answer text"), + usage=USAGE, + metadata={"input_guardrail": {"input_to_llm": "redacted q"}}, + ) + ) + + assert result["input_to_llm"] == "redacted q" + assert result["output_from_llm"] is None + + def test_rephrased_row_does_not_report_the_canned_reply_as_llm_input( + self, + ) -> None: + """On rephrase, input_to_llm is the canned reply, not what the model got.""" + result, _ = _run_item( + BlockResult( + guardrail_outcome="rephrased", + response=text_llm_call_response("please rephrase"), + usage=USAGE, + metadata={"input_guardrail": {"input_to_llm": "please rephrase"}}, + ) + ) + + assert result["input_to_llm"] is None + assert result["output_from_llm"] is None + + +class TestRowIsolation: + def test_missing_question_fails_the_row_without_calling_the_llm(self) -> None: + result, mock_call = _run_item( + BlockResult(response=text_llm_call_response()), item=_item(question=None) + ) + + assert result["failed"] is True + assert result["generated_output"] == "ERROR: missing question in dataset item" + assert mock_call.call_count == 0 + + def test_raised_exception_fails_only_that_row(self) -> None: + result, _ = _run_item(RuntimeError("config lookup exploded")) + + assert result["failed"] is True + assert result["generated_output"] == "ERROR: config lookup exploded" + + +class TestGuardrailsAuthFailsClosed: + def test_auth_rejection_fails_the_row_on_the_first_attempt( + self, no_backoff: list[float] + ) -> None: + # Fail-closed transport error: row fails, no retry (each retry re-bills). + with patch( + "app.crud.evaluations.fast.execute_llm_call", + return_value=BlockResult( + error=GUARDRAILS_AUTH_ERROR, guardrail_outcome=None + ), + ) as mock_execute: + result = _llm_call_for_item( + config=_config(), + project_id=PROJECT_ID, + organization_id=ORG_ID, + item=_item(), + ) + + assert result["failed"] is True + assert result["generated_output"] == f"ERROR: {GUARDRAILS_AUTH_ERROR}" + assert result["guardrail"] is None + assert mock_execute.call_count == 1 + assert no_backoff == [] + + +class TestExecuteLlmCallForQuestion: + @staticmethod + def _capture(outcomes: list[BlockResult]) -> list[dict[str, Any]]: + """Record each attempt's kwargs/input, then mutate the query in place.""" + calls: list[dict[str, Any]] = [] + + def _fake(**kwargs: Any) -> BlockResult: + calls.append({**kwargs, "seen_input": kwargs["query"].input.content.value}) + kwargs["query"].input.content.value = "rewritten in place" + return outcomes[len(calls) - 1] + + with patch("app.crud.evaluations.fast.execute_llm_call", side_effect=_fake): + _execute_llm_call_for_question( + config=_config(), + question=QUESTION, + project_id=PROJECT_ID, + organization_id=ORG_ID, + ) + return calls + + def test_sends_the_eval_specific_flags(self, no_backoff: list[float]) -> None: + (call,) = self._capture([BlockResult(response=text_llm_call_response())]) + + assert call["record_call"] is False + assert call["include_provider_raw_response"] is True + assert call["include_guardrail_metadata"] is True + assert call["langfuse_credentials"] is None + assert call["request_metadata"] is None + assert call["project_id"] == PROJECT_ID + assert call["organization_id"] == ORG_ID + assert isinstance(call["query"], QueryParams) + assert call["seen_input"] == QUESTION + + def test_each_retried_attempt_gets_a_fresh_query_and_job_id( + self, no_backoff: list[float] + ) -> None: + """execute_llm_call mutates the query in place; each attempt needs a new one.""" + first, second = self._capture( + [ + BlockResult(error="provider 503", retryable=True), + BlockResult(response=text_llm_call_response()), + ] + ) + + assert first["query"] is not second["query"] + assert second["seen_input"] == QUESTION + assert isinstance(second["job_id"], UUID) + assert first["job_id"] != second["job_id"] diff --git a/backend/app/tests/crud/evaluations/test_fast_results.py b/backend/app/tests/crud/evaluations/test_fast_results.py index c604dd257..a0fde65c1 100644 --- a/backend/app/tests/crud/evaluations/test_fast_results.py +++ b/backend/app/tests/crud/evaluations/test_fast_results.py @@ -16,6 +16,7 @@ build_response_result, extract_usage, is_failure_threshold_breached, + is_guardrail_blocked, parse_embedding_pair, sum_usage, ) @@ -41,6 +42,9 @@ def test_carries_every_key_the_s3_unit_needs(self) -> None: "question_id", "failed", "retrieved_chunks", + "guardrail", + "input_to_llm", + "output_from_llm", } def test_optional_fields_default_to_none(self) -> None: @@ -55,6 +59,9 @@ def test_optional_fields_default_to_none(self) -> None: assert result["response_id"] is None assert result["usage"] is None assert result["retrieved_chunks"] is None + assert result["guardrail"] is None + assert result["input_to_llm"] is None + assert result["output_from_llm"] is None class TestBuildEmbeddingFailure: @@ -166,3 +173,18 @@ def test_strictly_above_threshold_breaches(self) -> None: ) is True ) + + +class TestIsGuardrailBlocked: + def test_blocked_row_carries_the_provider_reason_after_the_prefix(self) -> None: + assert is_guardrail_blocked({"guardrail": "blocked: input flagged as abusive"}) + + @pytest.mark.parametrize( + "guardrail", ["rephrased", "applied", None, "", "blockedish"] + ) + def test_non_blocking_outcomes_are_not_blocked(self, guardrail: str | None) -> None: + # "blockedish" is the colon check: only the "blocked: " prefix counts. + assert not is_guardrail_blocked({"guardrail": guardrail}) + + def test_response_unit_written_before_guardrails_has_no_key(self) -> None: + assert not is_guardrail_blocked({"item_id": "i", "generated_output": "out"}) diff --git a/backend/app/tests/crud/evaluations/test_fast_traces.py b/backend/app/tests/crud/evaluations/test_fast_traces.py index eee07e7a5..533a6456e 100644 --- a/backend/app/tests/crud/evaluations/test_fast_traces.py +++ b/backend/app/tests/crud/evaluations/test_fast_traces.py @@ -238,3 +238,49 @@ def test_trace_is_keyed_by_ref_and_defaults_its_category(self) -> None: assert traces[0]["question_id"] == 1 assert traces[0]["llm_answer"] == "out" assert traces[0]["ground_truth_answer"] == "gt" + + +class TestBuildTraceRecordsGuardrail: + def test_every_trace_carries_the_key_even_when_guardrails_did_not_fire( + self, + ) -> None: + traces = build_trace_records( + response_results=[ + _response("a", guardrail="blocked: input flagged as abusive"), + _response("b"), + ], + item_refs={"a": "a", "b": "b"}, + is_judge_run=False, + judge_results={}, + metrics=[], + cosine_by_item_id={"b": 0.5}, + unscoreable={"a": "guardrail_blocked"}, + ) + + by_ref = {t["trace_id"]: t for t in traces} + assert by_ref["a"]["guardrail"] == "blocked: input flagged as abusive" + # Subscript, not .get: an absent key would read as None and hide the gap. + assert by_ref["b"]["guardrail"] is None + assert by_ref["b"]["input_to_llm"] is None + assert by_ref["b"]["output_from_llm"] is None + + def test_guardrail_texts_pass_through_to_the_trace(self) -> None: + traces = build_trace_records( + response_results=[ + _response( + "a", + guardrail="applied", + input_to_llm="q [REDACTED]", + output_from_llm="raw out", + ) + ], + item_refs={"a": "a"}, + is_judge_run=False, + judge_results={}, + metrics=[], + cosine_by_item_id={"a": 0.5}, + unscoreable={}, + ) + + assert traces[0]["input_to_llm"] == "q [REDACTED]" + assert traces[0]["output_from_llm"] == "raw out" diff --git a/backend/app/tests/crud/evaluations/test_merge.py b/backend/app/tests/crud/evaluations/test_merge.py index 59aa174c1..46768f72f 100644 --- a/backend/app/tests/crud/evaluations/test_merge.py +++ b/backend/app/tests/crud/evaluations/test_merge.py @@ -362,3 +362,50 @@ def test_resync_backfills_computed_but_unwritten_scores(self): s for s in merged["summary_scores"] if s["name"] == "Cosine Similarity" ) assert cosine["total_pairs"] == 2 # both recovered, not 1 + + +class TestMergeGuardrailOutcome: + def test_fresh_none_clears_a_stale_block(self): + existing = [{**_trace("1"), "guardrail": "blocked: input flagged as abusive"}] + fresh = [{**_trace("1"), "guardrail": None}] + + merged, stats = merge_trace_data(existing, fresh) + + assert merged[0]["guardrail"] is None + assert stats["updated"] == 1 + + def test_langfuse_resync_without_the_key_keeps_the_cached_outcome(self): + """Langfuse traces lack `guardrail`; resync must keep the blocked reason.""" + existing = [{**_trace("1"), "guardrail": "blocked: input flagged as abusive"}] + fresh = [_trace("1", value=2.0)] + + merged, _ = merge_trace_data(existing, fresh) + + assert merged[0]["guardrail"] == "blocked: input flagged as abusive" + + def test_a_trace_that_never_had_the_key_does_not_gain_one(self): + merged, _ = merge_trace_data([_trace("1")], [_trace("1", value=2.0)]) + assert "guardrail" not in merged[0] + assert "input_to_llm" not in merged[0] + assert "output_from_llm" not in merged[0] + + def test_guardrail_texts_follow_the_same_fresh_wins_rule(self): + existing = [ + {**_trace("1"), "input_to_llm": "old q", "output_from_llm": "old out"} + ] + fresh = [{**_trace("1"), "input_to_llm": None, "output_from_llm": "new out"}] + + merged, _ = merge_trace_data(existing, fresh) + + assert merged[0]["input_to_llm"] is None + assert merged[0]["output_from_llm"] == "new out" + + def test_langfuse_resync_keeps_the_cached_guardrail_texts(self): + existing = [ + {**_trace("1"), "input_to_llm": "q [REDACTED]", "output_from_llm": "raw"} + ] + + merged, _ = merge_trace_data(existing, [_trace("1", value=2.0)]) + + assert merged[0]["input_to_llm"] == "q [REDACTED]" + assert merged[0]["output_from_llm"] == "raw" diff --git a/backend/app/tests/crud/evaluations/test_response_parsing.py b/backend/app/tests/crud/evaluations/test_response_parsing.py new file mode 100644 index 000000000..363d7d290 --- /dev/null +++ b/backend/app/tests/crud/evaluations/test_response_parsing.py @@ -0,0 +1,66 @@ +"""`response_parsing.py`: chunk shape is persisted to S3; bad input must not raise.""" + +from typing import Any + +import pytest + +from app.crud.evaluations.response_parsing import extract_file_search_chunks + + +def _file_search_call(results: list[dict[str, Any]] | None) -> dict[str, Any]: + return {"type": "file_search_call", "results": results} + + +class TestExtractFileSearchChunks: + def test_flattens_hits_across_two_calls_in_payload_order(self) -> None: + raw = { + "output": [ + _file_search_call( + [ + {"score": 0.91, "text": "chunk A", "filename": "a.pdf"}, + {"score": 0.42, "text": "chunk B", "filename": "b.pdf"}, + ] + ), + {"type": "message", "content": []}, + _file_search_call([{"score": 0.5, "text": "chunk C", "filename": "c"}]), + ] + } + + assert extract_file_search_chunks(raw) == [ + {"score": 0.91, "text": "chunk A", "filename": "a.pdf"}, + {"score": 0.42, "text": "chunk B", "filename": "b.pdf"}, + {"score": 0.5, "text": "chunk C", "filename": "c"}, + ] + + def test_hit_without_filename_yields_none(self) -> None: + raw = {"output": [_file_search_call([{"score": 0.7, "text": "chunk"}])]} + + assert extract_file_search_chunks(raw) == [ + {"score": 0.7, "text": "chunk", "filename": None} + ] + + def test_non_file_search_items_are_skipped(self) -> None: + raw = { + "output": [ + {"type": "message", "content": [{"type": "output_text", "text": "hi"}]}, + {"type": "reasoning"}, + ] + } + + assert extract_file_search_chunks(raw) == [] + + @pytest.mark.parametrize( + "raw", + [ + None, + {}, + {"output": None}, + {"output": []}, + {"output": [{"type": "file_search_call", "results": None}]}, + {"output": [{"type": "file_search_call"}]}, + ], + ) + def test_malformed_or_empty_payloads_return_no_chunks( + self, raw: dict[str, Any] | None + ) -> None: + assert extract_file_search_chunks(raw) == [] diff --git a/backend/app/tests/crud/evaluations/test_retry.py b/backend/app/tests/crud/evaluations/test_retry.py new file mode 100644 index 000000000..f4d238146 --- /dev/null +++ b/backend/app/tests/crud/evaluations/test_retry.py @@ -0,0 +1,135 @@ +"""`retry_llm_call`, the result-based retry policy for generation.""" + +import logging +from collections.abc import Callable, Iterator + +import pytest + +from app.crud.evaluations.retry import ( + RETRY_MAX_ATTEMPTS, + retry_llm_call, +) +from app.models.llm.response import Usage +from app.services.llm.chain.types import BlockResult, GuardrailOutcomeEnum + +logger = logging.getLogger(__name__) + + +@pytest.fixture +def sleeps(monkeypatch: pytest.MonkeyPatch) -> Iterator[list[float]]: + """Record backoff; tenacity binds nap.sleep at import, so patch time.sleep.""" + recorded: list[float] = [] + monkeypatch.setattr("tenacity.nap.time.sleep", recorded.append) + yield recorded + + +def _decorate(fn: Callable[[], BlockResult]) -> Callable[[], BlockResult]: + return retry_llm_call(logger)(fn) + + +def _failure(error: str = "provider 503") -> BlockResult: + return BlockResult(error=error, retryable=True) + + +def _deterministic_failure(error: str = "config_not_found") -> BlockResult: + """A failure a second identical call cannot clear; `retryable` stays False.""" + return BlockResult(error=error) + + +def _success() -> BlockResult: + return BlockResult(usage=Usage(input_tokens=1, output_tokens=1, total_tokens=2)) + + +def _guardrail(outcome: GuardrailOutcomeEnum) -> BlockResult: + return BlockResult(error="uli_slur_match", guardrail_outcome=outcome) + + +class TestRetryLlmCall: + def test_clean_success_runs_once(self, sleeps: list[float]) -> None: + calls: list[int] = [] + expected = _success() + + @_decorate + def call() -> BlockResult: + calls.append(1) + return expected + + assert call() is expected + assert len(calls) == 1 + assert sleeps == [] + + def test_transient_failure_then_success_runs_twice( + self, sleeps: list[float] + ) -> None: + expected = _success() + outcomes = [_failure(), expected] + + @_decorate + def call() -> BlockResult: + return outcomes.pop(0) + + assert call() is expected + assert outcomes == [] + assert len(sleeps) == 1 + + def test_exhaustion_returns_the_last_result_instead_of_raising( + self, sleeps: list[float] + ) -> None: + attempts: list[BlockResult] = [] + + @_decorate + def call() -> BlockResult: + attempts.append(_failure(f"provider 503 #{len(attempts)}")) + return attempts[-1] + + result = call() + + assert isinstance(result, BlockResult) + assert result is attempts[-1] + assert result.error == "provider 503 #2" + assert len(attempts) == RETRY_MAX_ATTEMPTS == 3 + assert len(sleeps) == RETRY_MAX_ATTEMPTS - 1 + + def test_deterministic_failure_is_not_retried(self, sleeps: list[float]) -> None: + calls: list[int] = [] + expected = _deterministic_failure() + + @_decorate + def call() -> BlockResult: + calls.append(1) + return expected + + assert call() is expected + assert len(calls) == 1 + assert sleeps == [] + + @pytest.mark.parametrize("outcome", ["blocked", "rephrased"]) + def test_guardrail_verdict_is_never_retried( + self, outcome: GuardrailOutcomeEnum, sleeps: list[float] + ) -> None: + calls: list[int] = [] + + @_decorate + def call() -> BlockResult: + calls.append(1) + return _guardrail(outcome) + + assert call().guardrail_outcome == outcome + assert len(calls) == 1 + assert sleeps == [] + + def test_raised_exception_propagates_without_retrying( + self, sleeps: list[float] + ) -> None: + calls: list[int] = [] + + @_decorate + def call() -> BlockResult: + calls.append(1) + raise RuntimeError("config lookup exploded") + + with pytest.raises(RuntimeError, match="config lookup exploded"): + call() + + assert len(calls) == 1 + assert sleeps == [] diff --git a/backend/app/tests/services/evaluations/test_fast_validation.py b/backend/app/tests/services/evaluations/test_fast_validation.py new file mode 100644 index 000000000..59ce37d9b --- /dev/null +++ b/backend/app/tests/services/evaluations/test_fast_validation.py @@ -0,0 +1,89 @@ +"""Fast run config preconditions: prompt_template must contain `{{input}}`.""" + +import pytest +from fastapi import HTTPException +from sqlmodel import Session + +from app.models import Config, EvaluationDataset +from app.models.llm.request import ( + ConfigBlob, + PromptTemplate, + build_kaapi_completion_config, +) +from app.services.evaluations.fast import ( + ERR_CONFIG_TEMPLATE_MISSING_INPUT, + validate_fast_evaluation_inputs, +) +from app.tests.utils.auth import TestAuthContext +from app.tests.utils.test_data import ( + create_test_config, + create_test_evaluation_dataset, +) + + +def _config_with_template(db: Session, project_id: int, template: str | None) -> Config: + """Text-OpenAI Kaapi config; no model row, to dodge a SQLAlchemy ARRAY-enum bug.""" + blob = ConfigBlob( + completion=build_kaapi_completion_config( + provider="openai", + type="text", + params={"model": "gpt-4o-fast-eval-test", "temperature": 0.7}, + ), + prompt_template=PromptTemplate(template=template) if template else None, + ) + return create_test_config( + db=db, project_id=project_id, use_kaapi_schema=True, config_blob=blob + ) + + +@pytest.fixture +def dataset(db: Session, user_api_key: TestAuthContext) -> EvaluationDataset: + return create_test_evaluation_dataset( + db=db, + organization_id=user_api_key.organization_id, + project_id=user_api_key.project_id, + original_items_count=3, + duplication_factor=1, + ) + + +def _validate( + db: Session, + user_api_key: TestAuthContext, + dataset: EvaluationDataset, + template: str | None, +) -> EvaluationDataset: + config = _config_with_template(db, user_api_key.project_id, template) + return validate_fast_evaluation_inputs( + session=db, + dataset_id=dataset.id, + config_id=config.id, + config_version=1, + organization_id=user_api_key.organization_id, + project_id=user_api_key.project_id, + ) + + +class TestPromptTemplatePlaceholder: + def test_template_without_the_placeholder_is_rejected( + self, db: Session, user_api_key: TestAuthContext, dataset: EvaluationDataset + ) -> None: + with pytest.raises(HTTPException) as exc_info: + _validate(db, user_api_key, dataset, "Answer in Hindi. Be concise.") + + assert exc_info.value.status_code == 422 + assert ERR_CONFIG_TEMPLATE_MISSING_INPUT in exc_info.value.detail + + def test_template_with_the_placeholder_passes( + self, db: Session, user_api_key: TestAuthContext, dataset: EvaluationDataset + ) -> None: + validated = _validate(db, user_api_key, dataset, "Answer in Hindi: {{input}}") + + assert validated.id == dataset.id + + def test_config_without_a_template_passes( + self, db: Session, user_api_key: TestAuthContext, dataset: EvaluationDataset + ) -> None: + validated = _validate(db, user_api_key, dataset, None) + + assert validated.id == dataset.id diff --git a/backend/app/tests/services/evaluations/test_load_run_dataset_items.py b/backend/app/tests/services/evaluations/test_load_run_dataset_items.py index 8430569ad..5e061f274 100644 --- a/backend/app/tests/services/evaluations/test_load_run_dataset_items.py +++ b/backend/app/tests/services/evaluations/test_load_run_dataset_items.py @@ -261,7 +261,7 @@ def test_chunk_reload_passes_run_duplication_factor( patch(f"{_FAST}.get_dataset_by_id", return_value=MagicMock()), patch( f"{_FAST}._resolve_config_and_clients", - return_value=(MagicMock(), MagicMock(), None), + return_value=(MagicMock(), None), ), patch( f"{_FAST}.load_run_dataset_items", return_value=loaded_items diff --git a/backend/app/tests/services/llm/test_execute_llm_call.py b/backend/app/tests/services/llm/test_execute_llm_call.py new file mode 100644 index 000000000..dc0443c2d --- /dev/null +++ b/backend/app/tests/services/llm/test_execute_llm_call.py @@ -0,0 +1,511 @@ +"""Direct `execute_llm_call` tests for what `execute_job` tests can't reach.""" + +from collections.abc import Iterator +from contextlib import contextmanager +from typing import Any +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +import httpx +import pytest +from sqlmodel import Session, func, select + +from app.models.llm import ( + LLMCallResponse, + LLMResponse, + QueryParams, + TextContent, + TextOutput, + Usage, +) +from app.models.llm.request import ConfigBlob, LLMCallConfig, LlmCall +from app.services.llm.chain.types import BlockResult +from app.services.llm.jobs import execute_llm_call +from app.tests.utils.utils import get_project + +VALIDATOR_CONFIG_ID = "00000000-0000-0000-0000-000000000001" +PROXY_URL = "https://api.tap.example/v1/predictions" + +TEXT_COMPLETION = { + "type": "text", + "provider": "openai-native", + "params": {"model": "gpt-4o"}, +} +PROXY_COMPLETION = { + "type": "proxy", + "provider": None, + "params": {"client_llm_url": PROXY_URL}, +} +# Kaapi-shaped so it goes through the mapper; the KB adds a file_search tool. +KAAPI_KB_COMPLETION = { + "type": "text", + "provider": "openai", + "params": {"model": "gpt-4o", "knowledge_base_ids": ["vs_abc123"]}, +} +PROXY_PAYLOAD = { + "id": "resp_abc", + "model": "gpt-5", + "output": [ + { + "type": "message", + "content": [{"type": "output_text", "text": "Proxy answer."}], + } + ], + "usage": {"input_tokens": 11, "output_tokens": 7, "total_tokens": 18}, +} + + +def build_blob( + completion: dict[str, Any], + *, + input_guardrails: bool = False, + output_guardrails: bool = False, +) -> ConfigBlob: + return ConfigBlob.model_validate( + { + "completion": completion, + "input_guardrails": [{"validator_config_id": VALIDATOR_CONFIG_ID}] + if input_guardrails + else [], + "output_guardrails": [{"validator_config_id": VALIDATOR_CONFIG_ID}] + if output_guardrails + else [], + } + ) + + +def llm_call_count(db: Session) -> int: + return db.exec(select(func.count()).select_from(LlmCall)).one() + + +@contextmanager +def guardrails_http(*verdicts: dict[str, Any] | Exception) -> Iterator[MagicMock]: + """Mock guardrails HTTP: GET returns one validator, POSTs return `verdicts`.""" + config_response = MagicMock() + config_response.raise_for_status.return_value = None + config_response.json.return_value = { + "success": True, + "data": [{"type": "uli_slur_match"}], + } + + responses = [] + for verdict in verdicts: + response = MagicMock() + if isinstance(verdict, Exception): + response.raise_for_status.side_effect = verdict + else: + response.raise_for_status.return_value = None + response.json.return_value = verdict + responses.append(response) + + client = MagicMock() + client.get.return_value = config_response + client.post.side_effect = responses + + with patch("app.services.llm.guardrails.httpx.Client") as client_cls: + client_cls.return_value.__enter__.return_value = client + yield client + + +def http_status_error(status_code: int) -> httpx.HTTPStatusError: + response = MagicMock() + response.status_code = status_code + return httpx.HTTPStatusError("rejected", request=MagicMock(), response=response) + + +@pytest.fixture +def provider(db: Session): + with ( + patch("app.services.llm.jobs.Session") as session_cls, + patch("app.services.llm.jobs.get_llm_provider") as get_provider, + ): + session_cls.return_value.__enter__.return_value = db + session_cls.return_value.__exit__.return_value = None + instance = MagicMock() + get_provider.return_value = instance + yield instance + + +@pytest.fixture +def metrics(): + with ( + patch("app.services.llm.jobs.record_llm_call_started") as started, + patch("app.services.llm.jobs.record_llm_call_finished") as finished, + ): + yield started, finished + + +@pytest.fixture +def provider_response() -> LLMCallResponse: + return LLMCallResponse( + response=LLMResponse( + provider_response_id="resp-123", + conversation_id=None, + model="gpt-4o", + provider="openai", + output=TextOutput(content=TextContent(value="Provider answer.")), + ), + usage=Usage(input_tokens=10, output_tokens=20, total_tokens=30), + provider_raw_response=None, + ) + + +@pytest.fixture +def proxy_http(): + response = MagicMock() + response.raise_for_status.return_value = None + response.json.return_value = PROXY_PAYLOAD + + client = MagicMock() + client.__enter__.return_value = client + client.__exit__.return_value = None + client.post.return_value = response + + with ( + patch( + "app.services.llm.jobs.get_provider_credential", + return_value={"api_key": "tap-token"}, + ), + patch("app.services.llm.jobs.httpx.Client", return_value=client), + ): + yield client + + +def call( + db: Session, + blob: ConfigBlob, + *, + text: str = "what are my land rights", + **kw: Any, +) -> BlockResult: + project = get_project(db) + return execute_llm_call( + config=LLMCallConfig(blob=blob), + query=QueryParams(input=text), + job_id=uuid4(), + project_id=project.id, + organization_id=project.organization_id, + request_metadata=None, + langfuse_credentials=None, + **kw, + ) + + +class TestRecordCallFalse: + def test_provider_path_writes_no_llm_call_and_no_metrics( + self, db: Session, provider, metrics, provider_response + ): + started, finished = metrics + provider.execute.return_value = (provider_response, None) + before = llm_call_count(db) + + result = call(db, build_blob(TEXT_COMPLETION), record_call=False) + + assert result.error is None + assert result.response.response.output.content.value == "Provider answer." + assert result.usage.total_tokens == 30 + assert result.llm_call_id is None + assert llm_call_count(db) == before + started.assert_not_called() + finished.assert_not_called() + + def test_proxy_path_writes_no_llm_call_and_no_metrics( + self, db: Session, provider, metrics, proxy_http + ): + started, finished = metrics + before = llm_call_count(db) + + with patch( + "app.services.llm.jobs.update_llm_call_response" + ) as update_llm_call_response: + result = call(db, build_blob(PROXY_COMPLETION), record_call=False) + + assert result.error is None + assert result.response.response.output.content.value == "Proxy answer." + assert result.usage.total_tokens == 18 + assert result.llm_call_id is None + assert llm_call_count(db) == before + # Only observable via the mock: the surrounding try/except swallows failures. + update_llm_call_response.assert_not_called() + started.assert_not_called() + finished.assert_not_called() + + def test_rephrase_path_writes_no_llm_call(self, db: Session, provider): + rephrase_text = "Please rephrase without unsafe content." + before = llm_call_count(db) + with guardrails_http( + { + "success": True, + "bypassed": False, + "data": {"safe_text": rephrase_text, "rephrase_needed": True}, + } + ): + result = call( + db, + build_blob(TEXT_COMPLETION, input_guardrails=True), + record_call=False, + ) + + assert result.error is None + assert result.response.response.output.content.value == rephrase_text + assert result.guardrail_outcome == "rephrased" + assert result.llm_call_id is None + assert llm_call_count(db) == before + provider.execute.assert_not_called() + + +class TestRecordCallDefault: + """`record_call` defaults True: audit row and metrics still fire.""" + + @pytest.fixture + def llm_call_crud(self): + with ( + patch("app.services.llm.jobs.create_llm_call") as create_llm_call, + patch("app.services.llm.jobs.update_llm_call_response"), + ): + create_llm_call.return_value = MagicMock(id=uuid4()) + yield create_llm_call + + def test_provider_path_records_call_and_metrics( + self, db: Session, provider, metrics, provider_response, llm_call_crud + ): + started, finished = metrics + provider.execute.return_value = (provider_response, None) + + result = call(db, build_blob(TEXT_COMPLETION)) + + assert result.error is None + assert result.llm_call_id == llm_call_crud.return_value.id + started.assert_called_once() + finished.assert_called_once() + + def test_proxy_path_records_call_and_metrics( + self, db: Session, provider, metrics, proxy_http, llm_call_crud + ): + started, finished = metrics + + result = call(db, build_blob(PROXY_COMPLETION)) + + assert result.error is None + assert result.llm_call_id == llm_call_crud.return_value.id + started.assert_called_once() + finished.assert_called_once() + + +class TestGuardrailOutcome: + def test_input_hard_block(self, db: Session, provider): + with guardrails_http({"success": False, "error": "Unsafe content detected"}): + result = call( + db, + build_blob(TEXT_COMPLETION, input_guardrails=True), + record_call=False, + ) + + assert result.error == "Unsafe content detected" + assert result.guardrail_outcome == "blocked" + provider.execute.assert_not_called() + + def test_input_stripped_to_empty_is_a_block(self, db: Session, provider): + with guardrails_http( + { + "success": True, + "bypassed": False, + "data": {"safe_text": " ", "rephrase_needed": False}, + } + ): + result = call( + db, + build_blob(TEXT_COMPLETION, input_guardrails=True), + record_call=False, + ) + + assert ( + result.error + == "Input guardrails rejected the request and left no usable content." + ) + assert result.guardrail_outcome == "blocked" + provider.execute.assert_not_called() + + def test_output_hard_block_on_provider_path_keeps_usage( + self, db: Session, provider, provider_response + ): + provider.execute.return_value = (provider_response, None) + with guardrails_http({"success": False, "error": "Output blocked"}): + result = call( + db, + build_blob(TEXT_COMPLETION, output_guardrails=True), + record_call=False, + ) + + assert result.error == "Output blocked" + assert result.guardrail_outcome == "blocked" + assert result.usage.total_tokens == 30 + + def test_output_hard_block_on_proxy_path_keeps_usage( + self, db: Session, provider, proxy_http + ): + with ( + patch( + "app.services.llm.guardrails.list_validators_config", + return_value=([], [{"type": "pii_remover"}]), + ), + patch( + "app.services.llm.guardrails.run_guardrails_validation", + return_value={ + "success": False, + "bypassed": False, + "error": "Output blocked", + }, + ), + ): + result = call( + db, + build_blob(PROXY_COMPLETION, output_guardrails=True), + record_call=False, + ) + + assert result.error == "Output blocked" + assert result.guardrail_outcome == "blocked" + assert result.usage.total_tokens == 18 + + def test_provider_error_has_no_guardrail_outcome(self, db: Session, provider): + provider.execute.return_value = (None, "API rate limit exceeded") + + result = call(db, build_blob(TEXT_COMPLETION), record_call=False) + + assert result.error == "API rate limit exceeded" + assert result.guardrail_outcome is None + + @pytest.mark.parametrize("status_code", [401, 403, 422]) + def test_guardrails_auth_failure_has_no_guardrail_outcome( + self, db: Session, provider, status_code: int + ): + with guardrails_http(http_status_error(status_code)): + result = call( + db, + build_blob(TEXT_COMPLETION, input_guardrails=True), + record_call=False, + ) + + assert result.error == ( + f"Guardrails service rejected the request (HTTP {status_code})" + ) + assert result.guardrail_outcome is None + provider.execute.assert_not_called() + + +class TestRetryableFlag: + """Pin `BlockResult.retryable` where it is set.""" + + def test_provider_failure_is_retryable(self, db: Session, provider): + provider.execute.return_value = (None, "API rate limit exceeded") + + result = call(db, build_blob(TEXT_COMPLETION), record_call=False) + + assert result.retryable is True + + def test_proxy_transport_failure_is_retryable( + self, db: Session, provider, proxy_http + ): + proxy_http.post.side_effect = httpx.ConnectError("connection refused") + + result = call(db, build_blob(PROXY_COMPLETION), record_call=False) + + assert result.error.startswith("Proxy call failed:") + assert result.retryable is True + + def test_guardrail_block_is_not_retryable(self, db: Session, provider): + with guardrails_http({"success": False, "error": "Unsafe content detected"}): + result = call( + db, + build_blob(TEXT_COMPLETION, input_guardrails=True), + record_call=False, + ) + + assert result.guardrail_outcome == "blocked" + assert result.retryable is False + + @pytest.mark.parametrize("status_code", [401, 403, 422]) + def test_input_guardrails_auth_failure_is_not_retryable( + self, db: Session, provider, status_code: int + ): + with guardrails_http(http_status_error(status_code)): + result = call( + db, + build_blob(TEXT_COMPLETION, input_guardrails=True), + record_call=False, + ) + + assert result.retryable is False + provider.execute.assert_not_called() + + def test_output_guardrails_auth_failure_is_not_retryable( + self, db: Session, provider, provider_response + ): + """Output-side auth failure must not retry: each retry re-bills.""" + provider.execute.return_value = (provider_response, None) + with guardrails_http(http_status_error(401)): + result = call( + db, + build_blob(TEXT_COMPLETION, output_guardrails=True), + record_call=False, + ) + + assert result.error == "Guardrails service rejected the request (HTTP 401)" + assert result.guardrail_outcome is None + assert result.retryable is False + assert result.usage.total_tokens == 30 + + def test_unresolvable_stored_config_is_not_retryable(self, db: Session, provider): + project = get_project(db) + + result = execute_llm_call( + config=LLMCallConfig(id=uuid4(), version=1), + query=QueryParams(input="what are my land rights"), + job_id=uuid4(), + project_id=project.id, + organization_id=project.organization_id, + request_metadata=None, + langfuse_credentials=None, + record_call=False, + ) + + assert result.error is not None + assert result.retryable is False + provider.execute.assert_not_called() + + +class TestFileSearchResultsAreOptIn: + """`include=file_search_call.results` is opt-in with the raw response.""" + + @staticmethod + def _params_sent_to_provider(provider) -> dict[str, Any]: + completion_config = provider.execute.call_args.args[0] + return completion_config.params + + def test_raw_response_requested_asks_for_the_hits( + self, db: Session, provider, provider_response + ): + provider.execute.return_value = (provider_response, None) + + call( + db, + build_blob(KAAPI_KB_COMPLETION), + record_call=False, + include_provider_raw_response=True, + ) + + params = self._params_sent_to_provider(provider) + assert params["tools"][0]["type"] == "file_search" + assert params["include"] == ["file_search_call.results"] + + def test_default_call_does_not_ask_for_the_hits( + self, db: Session, provider, provider_response + ): + provider.execute.return_value = (provider_response, None) + + call(db, build_blob(KAAPI_KB_COMPLETION), record_call=False) + + params = self._params_sent_to_provider(provider) + assert params["tools"][0]["type"] == "file_search" + assert "include" not in params diff --git a/backend/app/tests/services/llm/test_guardrails.py b/backend/app/tests/services/llm/test_guardrails.py index b1ccabcdb..eaf84e5b1 100644 --- a/backend/app/tests/services/llm/test_guardrails.py +++ b/backend/app/tests/services/llm/test_guardrails.py @@ -21,6 +21,7 @@ ) from app.services.llm.guardrails import ( GuardrailsOutcome, + apply_guardrails, list_validators_config, run_guardrails_validation, summarize_validator_results, @@ -532,3 +533,76 @@ def test_request_metadata_forwarded_to_llm_call_request( self._call(db, job, request_metadata=metadata) _, kwargs = mock_create.call_args assert kwargs["request"].request_metadata == metadata + + +class TestGuardrailsOutcomeBlocked: + """`blocked` separates a content verdict from a fail-closed auth error.""" + + VALIDATORS = [Validator(validator_config_id=uuid.uuid4())] + + def _apply(self, post_result: Any) -> GuardrailsOutcome: + config_response = MagicMock() + config_response.raise_for_status.return_value = None + config_response.json.return_value = { + "success": True, + "data": [{"type": "uli_slur_match", "stage": "input"}], + } + + client = MagicMock() + client.get.return_value = config_response + if isinstance(post_result, Exception): + client.post.side_effect = post_result + else: + post_response = MagicMock() + post_response.raise_for_status.return_value = None + post_response.json.return_value = post_result + client.post.return_value = post_response + + with patch("app.services.llm.guardrails.httpx.Client") as client_cls: + client_cls.return_value.__enter__.return_value = client + return apply_guardrails( + text=TEST_TEXT, + validators=self.VALIDATORS, + job_id=TEST_JOB_ID, + project_id=TEST_PROJECT_ID, + organization_id=TEST_ORGANIZATION_ID, + ) + + @staticmethod + def _http_status_error(status_code: int) -> httpx.HTTPStatusError: + response = MagicMock() + response.status_code = status_code + return httpx.HTTPStatusError("rejected", request=MagicMock(), response=response) + + def test_content_verdict_is_blocked(self) -> None: + outcome = self._apply({"success": False, "error": "Unsafe content detected"}) + + assert outcome.error == "Unsafe content detected" + assert outcome.blocked is True + + @pytest.mark.parametrize("status_code", [401, 403, 422]) + def test_auth_failure_is_not_blocked(self, status_code: int) -> None: + outcome = self._apply(self._http_status_error(status_code)) + + assert outcome.error == ( + f"Guardrails service rejected the request (HTTP {status_code})" + ) + assert outcome.blocked is False + + def test_success_is_not_blocked(self) -> None: + outcome = self._apply( + { + "success": True, + "bypassed": False, + "data": {"safe_text": TEST_TEXT, "rephrase_needed": False}, + } + ) + + assert outcome.error is None + assert outcome.blocked is False + + def test_bypassed_is_not_blocked(self) -> None: + outcome = self._apply(httpx.ConnectError("guardrails unreachable")) + + assert outcome.bypassed is True + assert outcome.blocked is False diff --git a/backend/app/tests/services/llm/test_mappers.py b/backend/app/tests/services/llm/test_mappers.py index c69832c35..fa3aa843e 100644 --- a/backend/app/tests/services/llm/test_mappers.py +++ b/backend/app/tests/services/llm/test_mappers.py @@ -95,6 +95,17 @@ def test_knowledge_base_ids_mapping(self, db: Session): assert result["tools"][0]["max_num_results"] == 50 assert warnings == [] + def test_knowledge_base_ids_do_not_add_include(self, db: Session): + # Batch API bodies reject `include`. + kaapi_params = TextLLMParams(model="gpt-4o", knowledge_base_ids=["vs_abc123"]) + + result, warnings = map_kaapi_to_openai_params( + session=db, kaapi_params=kaapi_params.model_dump(exclude_none=True) + ) + + assert result["tools"][0]["type"] == "file_search" + assert "include" not in result + def test_temperature_suppressed_for_reasoning_models(self, db: Session): """Test that temperature is suppressed with warning for reasoning models when reasoning is set.""" kaapi_params = TextLLMParams( @@ -1455,3 +1466,50 @@ def test_transform_google_tts_completion(self, db: Session): assert result.params["language"] == "hi" # Mapped from hi-IN assert result.params["response_format"] == "wav" # Default assert warnings == [] + + def test_transform_openai_text_with_knowledge_base_requests_file_search_results( + self, db: Session + ): + kaapi_config = build_kaapi_completion_config( + provider="openai", + type="text", + params={"model": "gpt-4o", "knowledge_base_ids": ["vs_abc123"]}, + ) + + result, warnings = transform_kaapi_config_to_native( + session=db, kaapi_config=kaapi_config, include_file_search_results=True + ) + + assert result.params["tools"][0]["type"] == "file_search" + assert result.params["include"] == ["file_search_call.results"] + + def test_transform_openai_text_omits_include_by_default(self, db: Session): + # Default off: production /llm/call traffic must not get the extra payload. + kaapi_config = build_kaapi_completion_config( + provider="openai", + type="text", + params={"model": "gpt-4o", "knowledge_base_ids": ["vs_abc123"]}, + ) + + result, warnings = transform_kaapi_config_to_native( + session=db, kaapi_config=kaapi_config + ) + + assert result.params["tools"][0]["type"] == "file_search" + assert "include" not in result.params + + def test_transform_openai_text_without_knowledge_base_omits_include( + self, db: Session + ): + kaapi_config = build_kaapi_completion_config( + provider="openai", + type="text", + params={"model": "gpt-4o"}, + ) + + result, warnings = transform_kaapi_config_to_native( + session=db, kaapi_config=kaapi_config, include_file_search_results=True + ) + + assert "tools" not in result.params + assert "include" not in result.params diff --git a/backend/app/tests/utils/llm.py b/backend/app/tests/utils/llm.py index a60059b87..f16a883df 100644 --- a/backend/app/tests/utils/llm.py +++ b/backend/app/tests/utils/llm.py @@ -1,17 +1,25 @@ +from typing import Any + from sqlmodel import Session from app.crud import JobCrud from app.crud.llm import create_llm_call, update_llm_call_response -from app.models import JobType, Job -from app.models.llm.response import LLMCallResponse +from app.models import Job, JobType +from app.models.llm import LLMCallRequest from app.models.llm.request import ( ConfigBlob, LLMCallConfig, QueryParams, build_kaapi_completion_config, ) +from app.models.llm.response import ( + LLMCallResponse, + LLMResponse, + TextContent, + TextOutput, + Usage, +) from app.tests.utils.utils import get_project -from app.models.llm import LLMCallRequest def create_llm_job(db: Session) -> Job: @@ -134,3 +142,23 @@ def create_llm_call_with_audio_uri_response( ) return llm_call + + +def text_llm_call_response( + text: str = "generated answer", + *, + provider_response_id: str = "resp_1", + model: str = "gpt-4o", + usage: Usage | None = None, + provider_raw_response: dict[str, Any] | None = None, +) -> LLMCallResponse: + return LLMCallResponse( + response=LLMResponse( + provider_response_id=provider_response_id, + provider="openai", + model=model, + output=TextOutput(content=TextContent(value=text)), + ), + usage=usage or Usage(input_tokens=5, output_tokens=7, total_tokens=12), + provider_raw_response=provider_raw_response, + ) diff --git a/docs/architecture/kaapi-evaluations-ARCHITECTURE.md b/docs/architecture/kaapi-evaluations-ARCHITECTURE.md index 08f5f5ae1..51d85f13b 100644 --- a/docs/architecture/kaapi-evaluations-ARCHITECTURE.md +++ b/docs/architecture/kaapi-evaluations-ARCHITECTURE.md @@ -713,6 +713,14 @@ built for processing many requests at once). The accepted downsides: ### 12.2 Fast evals (planned — design not finalised) +> **Status update.** Fast mode now shares `/llm/call`'s code: it calls +> `services/llm/jobs.py::execute_llm_call` in-process with `record_call=False`, not the HTTP +> endpoint, so it gets the same config resolution, mappers, provider routing and guardrails +> without touching the `high_priority` queue or writing `LlmCall` rows. The rest of this +> section is the original design note and is stale in places; see +> `docs/wiki/modules/evaluations.md` for what is built. (§3.4's claim that eval does not reuse +> the mappers has been false for a while too.) Batch mode is unchanged and still bypasses it. + The planned fix for the slow feedback loop in 12.1: a **"fast evals"** mode for _small_ golden sets that runs over the live **`/llm/call`** endpoint instead of a batch. Fast evals are **still fully scored** (cosine similarity _and_ diff --git a/docs/plans/evaluation-guardrail.md b/docs/plans/evaluation-guardrail.md new file mode 100644 index 000000000..a3fb6d08e --- /dev/null +++ b/docs/plans/evaluation-guardrail.md @@ -0,0 +1,634 @@ +# Plan: share `/llm/call`'s code with evaluation, and run eval rows through guardrails + +**Branch:** `evaluation-guardrail` +**Status:** Phases 1, 2 and 3 done (uncommitted). Clean-DB suite green, `/pr-review` run, and +end-to-end exercised against a dev project (2026-09-27) — see "Verification run" below. Review nits +fixed and `TestRetryOpenAICall` dropped on the user's call. Still open: a real *blocking* +guardrail run (local guardrails service is down), Verification item 5 (Sentry/OTel spans, never +run), and whether the untracked `docs/plans/` ships in the PR. +**Scope decided with the user.** Do not re-litigate the choices in "Scope" — they are answers, not suggestions. + +## How to use this file + +This is a cold-start brief. Read, in order: + +1. This file, top to bottom. +2. `docs/wiki/modules/guardrails.md` — §4 failure-semantics matrix, §6 inline hooks, §7 metadata, §9 invariants. +3. `docs/wiki/modules/evaluations.md` and `docs/wiki/modules/llm-call.md`. + +Then work the phases in order (Phase 2 depends on Phase 1). Per root `CLAUDE.md`: layer edits go +through the `senior-engineer` subagent, tests through `test-writer`, then `/pr-review` on the full +diff before committing. + +--- + +## In plain terms + +Evaluation has its own copy of "call the model". We delete that copy and have it call the same +code production uses. Because guardrails already live inside that shared code, evaluation gets +guardrails for free the moment it switches over. + +Three steps: + +1. **Make the shared call usable by evaluation.** It currently insists on writing an audit row + and reporting itself to production monitoring. Add a switch that turns both off, and give it + a way to say "guardrails stopped this" instead of just "something went wrong". +2. **Point evaluation at it.** Swap evaluation's private model call for the shared one, with the + switch turned off. Evaluation keeps its own result format, scoring and storage — only the + invocation changes. +3. **Decide what a guardrail-stopped row means for a score.** It is not a crash, so it must not + count as a failure; it just has nothing to score. Make both scoring paths treat it that way + and show the reason. + +Scoring, judging, embeddings, batch runs, the summary and prompt improvement are untouched. + +--- + +## Context + +Today `crud/evaluations/fast.py::_responses_call_for_item` calls +`openai_client.responses.create(...)` directly, with its own param assembly, its own retry +wrapper and its own result shape. `/llm/call` runs a completely separate path through +`services/llm/jobs.py::execute_llm_call` → `providers/*.execute`. + +Two costs: + +1. **Two functions to maintain.** Anything added to the `/llm/call` pipeline has to be + re-implemented in eval or it silently doesn't apply there. + `docs/architecture/kaapi-evaluations-ARCHITECTURE.md` §12.1 records this as an accepted + downside — "an eval does not run the same code that production will run". §12.2 records + reusing `/llm/call` as the intended fix; it was never built. +2. **Evals don't run guardrails.** `resolve_evaluation_config` returns a full `ConfigBlob` + carrying `input_guardrails` / `output_guardrails`, and evaluation reads only + `completion.params`. Evaluating a guardrailed config measures the bare model, not what + production serves. Verified: zero `guardrail` references anywhere under + `services/evaluations/`, `crud/evaluations/`, `api/routes/evaluations/`. + +--- + +## Scope + +- **Fast mode only** (`run_mode=fast`, which is also all of v2). The batch path stays on provider + Batch APIs — a batch submission is a JSONL upload and structurally cannot route through a + per-call function. Batch runs keep having no guardrails. +- **A guardrail-blocked row is unscoreable, not failed.** It must not count toward + `EVAL_FAST_FAILURE_THRESHOLD` in **either** stage — the v1 embedding stage has a second + threshold check that would otherwise catch these rows (see Phase 3). +- **Latency/throughput tuning is deferred.** Ship, measure, tune. See Risks. +- **Assumption to confirm at review:** a **rephrased** row (guardrails answered directly, no LLM + call) is scored normally, because that canned text is exactly what production would have + returned. It will drag scores down when compared against ground truth. Treating rephrase as + unscoreable like a block is a one-line change either way. + +Unchanged and explicitly out of scope: embeddings (v1 cosine), the LLM judge, the AI summary and +prompt improvement all keep their direct SDK calls and never get guardrails. + +--- + +## Phase 1 — make `execute_llm_call` usable without persistence or telemetry + +**DONE.** Built as specced, with two corrections found while reading the code — keep both in mind for +Phase 2: + +- `outcome.error` is **not** always a content block: `run_guardrails_validation` also sets it when + the guardrails service fail-closes on HTTP 401/403/422. Tagging that `"blocked"` would turn broken + guardrail credentials into a "completed" eval where every row is silently unscoreable. The auth + dict now carries `auth_error: True` and `GuardrailsOutcome.blocked` is the discriminator, so + `guardrail_outcome` stays `None` on an auth failure and the row fails normally. **Phase 2 must key + unscoreable off `guardrail_outcome`, never off `error`.** +- The proxy-branch output block carries `usage=proxy_usage`, not `response.usage` — `response` is + not bound until after that branch returns, so the plan's original text would have raised + `UnboundLocalError` into the outer handler and lost the block entirely. + +Tests: `app/tests/services/llm/test_execute_llm_call.py` (new, 13), plus 6 in `test_guardrails.py` +and 3 in `test_mappers.py`. `uv run pytest app/tests/services/llm/ -q` → 528 passed, 6 failed; those +6 are pre-existing local-DB drift (`column "metadata" of relation "llm_call" does not exist`, +migration 083 unapplied) and fail identically with the change stashed. + +Carry into Phase 2: `test_fast_judge.py::TestFileSearchIncludeParam` covers eval's current manual +`include` injection and must be updated when `run_response_chunk` stops setting it by hand. + +### Original spec + + +### `backend/app/services/llm/chain/types.py` + +Add `guardrail_outcome: Literal["blocked", "rephrased"] | None = None` to `BlockResult`. +`BlockResult.error` is a bare string today, so a caller cannot tell a guardrail block from a +provider failure — eval needs that distinction to decide unscoreable vs failed. + +### `backend/app/services/llm/jobs.py::execute_llm_call` + +- Add `record_call: bool = True` (keyword-only, like every other param). One flag covering the + `LlmCall` row **and** telemetry: "this call is not production traffic — keep it out of the + audit table, the AI spans and the LLM metrics." +- Skip the three `LlmCall` **creators** when it is `False`, leaving `llm_call_id = None`: + - `create_llm_call` on the main path (inside the `llm.create_call_record` span) + - `create_llm_call` in the proxy branch + - `save_rephrase_guardrail_call` in the rephrase short-circuit +- Add the missing `if llm_call_id:` guard around the proxy branch's `update_llm_call_response`. + Every other downstream write is already gated on `llm_call_id` (STT S3 upload + + `update_llm_call_input`, TTS S3 upload, the main-path `update_llm_call_response`, and + `persist_output_guardrail_result`, which early-returns on a falsy id). The proxy branch's + update is the one unguarded site and would raise once the id can be `None`. +- Set `guardrail_outcome` at the three guardrail return sites: the input hard block, the rephrase + short-circuit, and the output hard block. The **output** guardrail has two call sites (proxy + branch + main provider path) — both need it, same as every other output-guardrail change + (`guardrails.md` §10). +- At the two **output**-block sites, also set `usage=response.usage` on the returned + `BlockResult`. The LLM was called and the tokens were spent; today the block returns a bare + `BlockResult(error=...)` and the cost vanishes. Eval sums usage per chunk, so this would + under-report run cost. +- Skip `record_llm_call_started` / `record_llm_call_finished` when `record_call` is `False`. +- Select the tracer per call: `_tracer = tracer if record_call else trace.NoOpTracer()`, and use + `_tracer` for the eight `start_as_current_span` blocks in this function. **Verify first** that + `NoOpTracer().start_as_current_span(...)` works as a drop-in context manager and that the + yielded span tolerates `set_attribute` / `set_status` / `record_exception` (it yields + `INVALID_SPAN`, a `NonRecordingSpan`, which should no-op all three). + +Why silence telemetry rather than "spans are cheap": the main span is tagged +`sentry.op = "gen_ai.chat"` and `record_llm_call_*` emits per-provider/model/project metrics. A +100-row eval of a broken config would otherwise land in the production Sentry AI dashboards and +error-rate metrics — exactly the prod/eval mixing arch §12.2 flagged. + +**Still emitted with `record_call=False`,** and out of scope to silence: the three spans from +`services/llm/guardrails.py`'s own module tracer, and the generic HTTP client spans from the +global `HTTPXClientInstrumentor` / `RequestsInstrumentor` (eval's direct SDK calls already emit +those today, so this is not a regression). + +**Langfuse** needs no code change: pass `langfuse_credentials=None` and +`core/langfuse/langfuse.py::observe_llm_execution` returns the undecorated function. Eval also +skips the credential DB read entirely. + +Net diff is small because the `llm_call_id` gating already exists. Do **not** restructure +`execute_llm_call` into extracted phases — it is the hottest path in the service and +`ChainBlock.execute` is a second live caller. + +### `backend/app/services/llm/mappers.py::transform_kaapi_config_to_native` + +OpenAI branch only — when the mapped params carry a `file_search` tool, set +`mapped_params["include"] = ["file_search_call.results"]`. + +Eval sets this by hand today in `run_response_chunk` because the v2 `knowledge_base` judge metric +scores the retrieved chunks. Once generation goes through `execute_llm_call`, eval has no place +to inject it — an undeclared key on `TextLLMParams` is dropped silently at validation +(`llm-call.md`, "Key pydantic/SQLModel schemas"). + +Put it in `transform_kaapi_config_to_native`, **not** in `map_kaapi_to_openai_params`. The mapper +has five other callers — batch eval JSONL (`crud/evaluations/batch.py`), the judge +(`crud/evaluations/judge.py`), and two assessment batch paths (`crud/assessment/batch.py`, +`services/assessment/api/batch.py`) — and three of those build **Batch API** bodies. The +transform is only reached from `execute_llm_call`, so `/llm/call` and eval fast get it and +nothing else changes. + +--- + +## Phase 2 — eval's generation stage calls `execute_llm_call` + +**DONE.** Built as specced. Deviations and things Phase 3 inherits: + +- **BLOCKER: Phase 3 must land in the same PR.** For **v1 (cosine)** a blocked row now has + `failed=False, generated_output=""`, so it enters `_stage2_embeddings`'s `embed_candidates`, gets + `build_embedding_failure`, and counts toward that stage's own threshold check — a guardrailed v1 + run blocking more than `EVAL_FAST_FAILURE_THRESHOLD` (0.5) of rows will **fail**, the exact + behaviour this plan rejects. v2 is already safe (`classify_empty_side` / `select_judgeable_rows` + skip empty output). `_stage2_embeddings` was deliberately not touched here. +- The retry had to become **result-based**, not exception-based: `execute_llm_call` converts + provider transients into `BlockResult(error=...)` and never raises. New + `retry.py::retry_llm_call` uses `retry_if_result`. Its `retry_error_callback` is load-bearing — + `reraise` does **not** apply to `retry_if_result`, so without it an exhausted retry raises + `RetryError` through `_run_in_pool`'s `future.result()`, killing the whole chunk; the cron healer + then re-enqueues it and the provider is re-charged for every row already generated. + `retry_openai_call` stays for embeddings and the judge. +- `QueryParams` **and** `job_id` are built inside the retried function, not per row. + `execute_llm_call` mutates `query.input.content.value` twice in place (template interpolation, + then the guardrails' `safe_text`), so a retried attempt reusing the object would double-apply the + template and resend already-sanitised text. +- A **blocked** `BlockResult` has `response=None` (both the input block and the output block), so + every read of `result.response.*` is guarded — `_response_text()` and + `... if result.response else None`. Unguarded, the `AttributeError` would have been swallowed by + the worker's `except Exception` into `failed=True`, silently defeating the whole change. +- The mapping checks `guardrail_outcome` **before** `error`: a rephrased row carries no error and a + real response, so checked later it reads as a plain success. +- `openai_client` was dropped from `_resolve_config_and_clients`' return tuple rather than threaded + through unused — it had exactly one consumer, `run_response_chunk`. Embeddings and the judge build + their own clients in the aggregate. +- The `batch_job` config's `"model"` is read off `config_blob.completion.params` with the existing + dict-or-typed-model idiom from `core.py::resolve_model_from_config`, avoiding a second config + resolution round trip. +- **Convention deviations to flag at `/pr-review`:** `crud/evaluations/{fast,retry}.py` now import + from `app/services/llm/` (`.claude/conventions/crud.md` forbids crud → service), verified free of + import cycles; and `_llm_call_for_item` catches bare `except Exception` (crud.md wants concrete + types) so one bad row cannot abort the pool. +- Tests: `test-writer` added 36, deleted 5, rewrote 5. Suite delta against the pre-change baseline is + clean — no new assertion failures; the 46 failures are the same pre-existing local DB drift + (`sqlalchemy.exc.ProgrammingError`). A clean-DB run is still owed before the PR. + +### `backend/app/services/evaluations/fast.py` + +- `_resolve_config_and_clients` currently narrows to + `TextLLMParams.model_validate(config_blob.completion.params)` and throws the blob away. Return + the `ConfigBlob` instead (the `OpenAI` client is still needed for embeddings + judge, the + Langfuse client for v1 datasets). +- `execute_fast_evaluation_chunk` passes the blob down to `run_response_chunk`. +- `validate_fast_evaluation_inputs`: **reject a config whose `prompt_template` is set but does not + contain `{{input}}`.** See Risks — this is the one silent-breakage path in the change. + +### `backend/app/crud/evaluations/fast.py` + +- `run_response_chunk(..., config: TextLLMParams, ...)` → takes `config_blob: ConfigBlob` and the + ids it needs off `eval_run`. Drop the `map_kaapi_to_openai_params` call and the manual + `base_params["include"]` line — `execute_llm_call` owns both now. +- Replace `_responses_call_for_item` with `_llm_call_for_item`, still the `_run_in_pool` worker at + `EVAL_FAST_API_CONCURRENCY`: + +```python +result = execute_llm_call( + config=LLMCallConfig(id=eval_run.config_id, version=eval_run.config_version), + query=QueryParams(input=TextInput(content=TextContent(value=question))), + job_id=uuid4(), # synthetic, never persisted; see note below + project_id=eval_run.project_id, + organization_id=eval_run.organization_id, + request_metadata=None, + langfuse_credentials=None, + include_provider_raw_response=True, # file_search chunks for the knowledge_base metric + include_guardrail_metadata=True, # per-row guardrail record, gated on outcome.applied + record_call=False, # no LlmCall row, no AI spans, no LLM metrics +) +``` + +- `job_id`: a fresh `uuid4()` per row. It is never persisted with `record_call=False`; its only + live uses are log lines, the guardrails `request_id`, and the rephrase fallback + `provider_response_id`. Per-row makes a guardrails-service log line traceable back to a single + eval row for free. (Chains already reuse one `job_id` across blocks, so upstream tolerates + either.) +- **Build a fresh `QueryParams` per row.** `execute_llm_call` mutates `query.input.content.value` + in place, twice — once for `prompt_template` interpolation, once for the guardrails' + `safe_text`. A reused object gets the template applied repeatedly. +- Pass the blob already resolved from the run's pinned `config_id`/`config_version` as an ad-hoc + `LLMCallConfig(blob=...)`, so each row skips a config-version fetch. Guardrails and + `prompt_template` ride on the blob, so they still apply. + +### Result mapping + +Add one field, `guardrail: str | None`, to the `ResponseResult` TypedDict and to +`fast_results.build_response_result`. + +| `BlockResult` | `generated_output` | `failed` | `guardrail` | +|---|---|---|---| +| success | response text | `False` | `None`, or `"applied"` when `result.metadata` carries guardrail keys | +| `guardrail_outcome="blocked"` | `""` | **`False`** | `f"blocked: {result.error}"` | +| `guardrail_outcome="rephrased"` | the rephrase text | `False` | `"rephrased"` | +| any other `error` | `f"ERROR: {result.error}"` | `True` | `None` | + +- `usage`: `extract_usage(result.usage, RESPONSE_USAGE_KEYS)` works unchanged — + `RESPONSE_USAGE_KEYS` is exactly `("input_tokens", "output_tokens", "total_tokens")`, which is + `models/llm/response.py::Usage`, and `field_value` reads attributes or dict keys. +- `response_id`: `result.response.response.provider_response_id`. +- `retrieved_chunks`: new `extract_file_search_chunks(raw: dict)` in + `backend/app/crud/evaluations/response_parsing.py`, reading + `result.response.provider_raw_response`. The existing + `services/response/response.py::get_file_search_results` only works on the live SDK object; + `include_provider_raw_response` hands back `response.model_dump()`, a plain dict. Keep emitting + the same `{score, text, filename}` dicts so the S3 unit stays JSON-serializable. + +### Retry + +`_responses_call_for_item` is wrapped in `crud/evaluations/retry.py::retry_openai_call` (tenacity, +3 attempts, on `RateLimitError` / `APITimeoutError` / `APIConnectionError` / +`InternalServerError`). `execute_llm_call` **never raises** on provider failure — it returns +`BlockResult(error=...)` — so that retry is lost on the swap. On a 100-row burst against a +rate-limited project that is a real regression. + +Retry `_llm_call_for_item` up to 3 times with exponential backoff whenever +`result.error is not None and result.retryable`. `error` is one string for every failure kind, +so the classification has to travel on its own field: `execute_llm_call` sets +`BlockResult.retryable` only where the provider, the proxy or the `LlmCall` write actually +failed. A guardrail block is never retried (content decision), and neither is a deterministic +failure — unresolvable config, rejected model, revoked key, guardrails auth fail-closed — since +three attempts and their backoff only reach the same answer more slowly. Known ceiling: a +provider 4xx is still flagged retryable, because the provider flattens its exception into a +string before `jobs.py` sees it; the waste is bounded (no tokens are billed on a rejected +request). + +--- + +## Phase 3 — guardrail outcomes in scoring + +**DONE.** Built as specced, with two deliberate deviations from the text below: + +- **The unscoreable reason is a stable key, not the row's raw string.** The plan asked for the + breakdown to read `{"blocked: ...": N}`. `merge.py::summarize_unscoreable` buckets any reason + outside `UNSCOREABLE_REASONS` as `"other"`, so that would have produced exactly the + undifferentiated bucket the plan wanted to avoid. New + `score.py::UNSCOREABLE_GUARDRAIL_BLOCKED = "guardrail_blocked"`, added to that tuple. The provider + message stays on the row's `guardrail` field and reaches the reviewer through the trace. + (`test_blocked_rows_land_in_their_own_summary_bucket` is that assert; it produces `{"other": 2}` + when the constant is patched out of the tuple.) +- **"Verify the v2 path records dropped rows" needed no v2 code, and `judge_stage.py` is untouched.** + Both scoring paths already route through `fast_cosine.py::classify_empty_side` — v1 via + `score_cosine_run`, v2 via `_score_judge_path` — so putting the guardrail check in as its **first** + branch (ahead of `empty_output`) covers both. The v1 threshold fix is then one clause on + `_stage2_embeddings`'s `embed_candidates`; `total_failures` and the `len(response_results)` + denominator are untouched, since excluding the rows from the candidates already keeps them out of + the numerator. + +Other things worth knowing: + +- Blocked/rephrased labels come from `GuardrailOutcomeEnum` (`services/llm/chain/types.py`), the + same `StrEnum` `execute_llm_call` sets, so the two sides cannot drift. Only `GUARDRAIL_APPLIED` + is a crud-local label, in `fast_results.py`, beside the shared `is_guardrail_blocked()` + predicate `fast_cosine.py` uses. +- The predicate matches the prefix `f"{GuardrailOutcomeEnum.BLOCKED}:"` **with the colon**, and reads + `.get("guardrail")` — S3 response units written before Phase 2 have no such key at all. +- `merge.py::_merge_single_trace` had to name `guardrail` explicitly: it rebuilds the merged dict + from a fixed key list, so an unnamed key is dropped on **every** read, including a plain cache + serve (which runs a step-forward merge with an empty fresh side). The carry is + fresh-authoritative-when-present, not the `or` chain `category` uses — an absent/`None` guardrail + means "did not fire", so an `or` would pin a stale `"blocked"` on a row that passes in a later + run. A Langfuse-fetched trace (`langfuse.py:518`) omits the key entirely, so a resync preserves + the cached value. Neither an `or` chain nor an unconditional `fresh` satisfies both directions; + `TestMergeGuardrailOutcome` holds that pair. +- An all-blocked v1 run now reaches Stage 2 with `embed_candidates == []`. Verified safe: + `_run_in_pool([])` returns `[]` and the empty unit uploads as `"[]"`, so the stage completes and + writes its retry-skip marker normally. +- The v2 `setdefault` clause that would stop `judge_failed` overwriting `guardrail_blocked` is + **unreachable for a blocked row** — `select_judgeable_rows` drops empty-output rows before + judging, so a blocked row never enters `judge_failed_refs`. Tested as the real scenario instead: + the two reasons coexist across different rows. +- Tests: 14 functions / 18 IDs added, 0 deleted, 0 rewritten. The primary regression red is real — + removing the `is_guardrail_blocked` clause from `embed_candidates` raises + `RuntimeError: Fast eval Stage 2 exceeded failure threshold | failed=3/4 | threshold=0.5`. +- **Known stale, deliberately not touched:** the `unscoreable` JSONB column comment at + `models/evaluation.py:375` still enumerates the four old reasons. Alembic autogenerate diffs + column comments, so fixing it needs a migration this change does not otherwise want. +- **Not in scope, worth a follow-up:** `crud/evaluations/embeddings.py:119` (the **batch** path) + hardcodes `"empty_output"`/`"empty_ground_truth"` instead of calling `classify_empty_side`. + Harmless today — batch rows never carry `guardrail` — but the two paths can now drift. + + +A blocked row lands with `generated_output=""`. On **v2** that is already enough: +`judge_stage.select_judgeable_rows` keeps only rows with a non-empty `generated_output`, and +Stage-1's failure threshold counts `failed` only, which a blocked row is not. + +**v1 is not safe as-is — this is the one real gap.** `_stage2_embeddings` has its **own** +threshold check: + +```python +embed_candidates = [r for r in response_results if not r.get("failed")] +... +total_failures = failed_count + sum(1 for r in response_results if r.get("failed")) +if is_failure_threshold_breached(failed_rows=total_failures, total_rows=len(response_results)): + raise RuntimeError("Fast eval Stage 2 exceeded failure threshold | ...") +``` + +A blocked row has `failed=False`, so it enters `embed_candidates`, `_embedding_call_for_pair` +returns `build_embedding_failure` on the empty output, and it counts toward `total_failures`. A v1 +run where guardrails block more than `EVAL_FAST_FAILURE_THRESHOLD` (0.5) of rows would **fail** — +contradicting the chosen behaviour. + +Fix: exclude guardrail-blocked rows from `embed_candidates` and record them straight into +`unscoreable` with their guardrail reason, so they are neither embedded nor counted. + +Also: + +- **Carry the guardrail reason into the unscoreable entry.** `fast_cosine` writes the generic + `"empty output or ground_truth"` reason today; prefer the row's `guardrail` value when set, so + `merge.summarize_unscoreable` reports `{"blocked: ...": N}` instead of an undifferentiated + "empty output" bucket. +- **Verify the v2 path records dropped rows.** Confirm rows filtered out by + `select_judgeable_rows` end up in `eval_run.unscoreable` with a reason; add it in + `_stage3_score_and_trace` if they currently vanish silently. +- **Surface the guardrail field on the trace** in `fast_traces.py`, so a reviewer can see per row + whether guardrails fired. + +### Known gap, deliberately not closed + +A **bypassed** run (guardrails service unreachable → fail open, `guardrails.md` §4) is invisible to +the caller: metadata emission is gated on `outcome.applied`, and `apply_input_guardrails` / +`apply_output_guardrails` don't propagate the bypass flag. So an eval row can silently skip +guardrails and look identical to one that passed. Carrying a `"bypassed"` value through both +adapters is a follow-up; note it in the wiki rather than widening this change. + +--- + +## Files + +| File | Change | +|---|---| +| `backend/app/services/llm/chain/types.py` | `BlockResult.guardrail_outcome` | +| `backend/app/services/llm/jobs.py` | `record_call` flag (row + spans + metrics); gate the 3 creators; add the missing proxy `if llm_call_id:`; set `guardrail_outcome` at the 3 guardrail returns; carry `usage` on output blocks | +| `backend/app/services/llm/mappers.py` | `include=["file_search_call.results"]` in `transform_kaapi_config_to_native`'s OpenAI branch when a `file_search` tool is present **and** the new `include_file_search_results` flag is set; `execute_llm_call` drives it off `include_provider_raw_response` so production traffic is unchanged | +| `backend/app/crud/evaluations/response_parsing.py` | `extract_file_search_chunks(raw: dict)` | +| `backend/app/crud/evaluations/fast_results.py` | `guardrail` on `ResponseResult` + `build_response_result` | +| `backend/app/crud/evaluations/fast.py` | `_llm_call_for_item` replaces `_responses_call_for_item`; `run_response_chunk` takes `ConfigBlob`; `_stage2_embeddings` excludes blocked rows from `embed_candidates` and the threshold | +| `backend/app/services/evaluations/fast.py` | `_resolve_config_and_clients` returns the blob; `{{input}}` template guard in `validate_fast_evaluation_inputs` | +| `backend/app/crud/evaluations/fast_cosine.py`, `fast_traces.py`, `judge_stage.py` | guardrail reason into unscoreable + traces | + +No migration — no schema change. + +--- + +## Risks + +1. **`prompt_template` starts being applied to eval rows.** Today eval sends the raw question and + never interpolates; `execute_llm_call` does `template.replace("{{input}}", value)`. A config + whose template omits `{{input}}` would send the template alone and **drop the question**, + silently producing a run of answers to nothing. Hence the validation gate in Phase 2. + (Fixing the interpolation gap is itself a win: `judge_stage` already shows the template to the + judge as "the prompt wrapped around each user input", so the judge has been grading against a + prompt the model was never given.) The iteration loop is safe: `prompt_improvement` only + rewrites `params["instructions"]`, never `prompt_template`, so a config that passes the gate in + round 1 keeps passing it. +2. **Latency.** Guardrails add up to two HTTP hops per direction (config fetch 10s timeout, + validation 45s). Chunk budget today is 50 rows / 4 workers / 300s + `CELERY_TASK_SOFT_TIME_LIMIT`. A guardrailed run can trip the soft limit; the cron healer then + re-enqueues the chunk, and the `raw_output_url` skip guard means it re-charges the provider for + the rows it hadn't written. Deferred by decision — watch `run_evaluation_fast_chunk` durations + after rollout and tune `EVAL_FAST_CHUNK_SIZE` down first. +3. **DB reads per row.** `execute_llm_call` does a config-version read, a model-config validation + read, `transform_kaapi_config_to_native`, and `get_llm_provider` (credentials) on every call — + roughly four round trips per row, from four concurrent greenlets each holding a `Session`. + Watch the connection pool. Caching the resolved provider per chunk is the obvious follow-up if + it bites; don't pre-build it. +4. **`ChainBlock.execute` is a second caller** of `execute_llm_call` and must keep writing its + `LlmCall` rows and its spans. `record_call` defaults to `True`, so it is untouched — the chain + tests are the regression net. +5. **Fast mode is still gated to OpenAI + text** by `validate_fast_evaluation_inputs`. + `execute_llm_call` supports every provider and input type, so that gate can be widened + afterwards for free. Not in this change. + +--- + +## Verification + +1. `uv run bash scripts/tests-start.sh` — whole suite. +2. **Regression net for Phase 1:** `app/tests/services/llm/test_jobs.py`, `test_chain.py`, + `test_chain_executor.py`, `test_guardrails.py` must pass **unchanged**. `record_call` defaults + to `True`, so any diff there means the flag leaked into the default path. +3. New tests (`test-writer`): + - `execute_llm_call(record_call=False)` creates no `LlmCall` row, returns `llm_call_id=None`, + emits no `record_llm_call_started/finished`, and still returns the provider response — + including down the rephrase and proxy branches. + - An output-guardrail block still returns `usage`. + - `guardrail_outcome` is `"blocked"` on an input hard block, `"blocked"` on an output hard block + (assert **both** output call sites), `"rephrased"` on the rephrase path, `None` on a provider + error. + - `_llm_call_for_item` maps each `BlockResult` shape to the `ResponseResult` row in the table + above; a blocked row has `failed=False` and an empty `generated_output`. + - A chunk where every row is blocked completes without tripping `EVAL_FAST_FAILURE_THRESHOLD` — + assert for **both** v1 (Stage-2 embeddings) and v2 (Stage-1 merge) — and the rows land in + `eval_run.unscoreable` with the guardrail reason. + - `transform_kaapi_config_to_native` emits `include` only when a `file_search` tool is present, + and `map_kaapi_to_openai_params` output is unchanged for the batch/judge/assessment callers. + - `validate_fast_evaluation_inputs` rejects a `prompt_template` without `{{input}}`. + - Mock the guardrails HTTP boundary with `unittest.mock.patch` on + `app.services.llm.guardrails.httpx.Client` — the module's established style, not `respx` + (`guardrails.md` §13). +4. **End-to-end, against a dev project:** save a config with `input_guardrails` + + `output_guardrails` set, `POST /api/v2/evaluations` with a small dataset, and check: the run + reaches `completed`; **no new `llm_call` rows** — snapshot + `select count(*) from llm_call where project_id = :p` before and after, since the synthetic + `job_id` is never persisted and can't be queried; the S3 responses unit carries `guardrail` per + row; blocked rows appear in `unscoreable` and not in the failure count; a knowledge-base config + still produces `retrieved_chunks` so the `knowledge_base` judge metric scores. +5. Confirm no Langfuse generation is created for the run (v2 already passes `langfuse=None`), and + that no `gen_ai.chat` span or LLM metric appears for the eval run in Sentry/OTel. + +--- + +## Wiki updates (same PR — `CLAUDE.md` maintenance rule) + +- `docs/wiki/modules/evaluations.md` — the "Eval traffic deliberately bypasses `/llm/call`" gotcha + is now false for fast mode; record the guardrails behaviour and the unscoreable rule. +- `docs/wiki/modules/guardrails.md` — §1 and §6: eval fast runs are a third caller of the inline + hooks. Add the bypass-invisibility gap to §12. +- `docs/wiki/modules/llm-call.md` — `record_call` and `guardrail_outcome` on the + `execute_llm_call` contract; the new `include` behaviour in `transform_kaapi_config_to_native`. +- `docs/architecture/kaapi-evaluations-ARCHITECTURE.md` §12.1/§12.2 describe this as unbuilt — + worth a note that fast mode now does it. (Those docs are already stale in places: §3.4 claims + eval doesn't reuse the mappers; it does.) + +--- + +## Verification run — 2026-09-27 + +### 1. Clean-DB suite — PASS + +The local `ai_platform_test` on Homebrew pg14 (`localhost:5432`, *not* the docker pg17) was +stamped `083` with 083's column missing, which is what produced every `UndefinedColumn` failure +in earlier runs. It was left untouched — it may belong to another branch. A fresh database was +used instead: + +```bash +createdb -h localhost -U postgres ai_platform_test_eg +cd backend && POSTGRES_DB=ai_platform_test_eg uv run bash scripts/tests-start.sh +``` + +Result: **3570 passed, 6 skipped, 0 failed** in 177s. All 8 previously drift-blocked IDs are +green, including `TestStage2GuardrailBlocked` (the Phase-3 regression test, which until now had +only ever passed under an in-transaction `ALTER TABLE` shim). The new DB reads `085`; the old one +still reads `083`. + +Regression contract (Verification item 2) holds: `test_guardrails.py` is +76 lines with zero +deletions, so no existing assertion was bent. The one changed line in +`test_load_run_dataset_items.py` tracks `_resolve_config_and_clients`' new return arity, which is +a deliberate Phase-2 contract change. + +### 2. `/pr-review` — approve with nits + +Run against `git diff ccaebf7f` (working tree; nothing is committed, so `$BASE...HEAD` is empty +and the command's pull step was skipped — no upstream). 29 tracked files + 7 untracked, +1079+/410-. **No blocking issues.** Findings: + +*Suggestions — fixed* + +- `crud/evaluations/fast.py::run_response_chunk` read the model with an inline + `isinstance(dict) / getattr` dance → now `field_value(config_blob.completion.params, "model")`, + the helper already sitting one module over. +- `services/evaluations/fast.py` raised + `detail=f"{ERR_CONFIG_TEMPLATE_MISSING_INPUT}: "` while every sibling in the same + function raises the bare code → now `detail=ERR_CONFIG_TEMPLATE_MISSING_INPUT`, with the prose + moved into a `logger.warning`, so a client matching `detail == ""` matches. +- A guardrails auth fail-closed is no longer retried at all. It still fails the row, but the + credentials are as broken on the third attempt as on the first, and on the *output* side each + attempt would re-charge the provider, since the completion is generated before output + guardrails run. This is what `BlockResult.retryable` exists for. + +*Suggestion — deferred* + +- `crud/evaluations/retry.py` imports `app.services.llm.chain.types`, deepening the existing + crud → services dependency (`fast.py` already imported `services.llm.mappers` at HEAD). + `[follow-up]`, not this PR. + +*Nits — fixed* + +- `GUARDRAIL_METADATA_KEYS` moved from `fast.py` to `fast_results.py`, beside the other + `GUARDRAIL_*` constants. +- `_last_llm_result`'s bare `assert` replaced with an explicit `RuntimeError` guard. + +*Checked and clean* + +- `GuardrailsOutcome.blocked` (`error is not None and not raw.get("auth_error")`) was audited + against every return in `run_guardrails_validation`: a 2xx body (the service's own verdict), + the 401/403/422 branch (sets `auth_error`), and every other exception (returns + `bypassed: True`, which `apply_guardrails` converts to `error=None`). There is no third + fail-closed shape, so nothing that isn't a content verdict can be labelled `blocked`. A + `list_validators_config` auth failure raises `ValueError` and surfaces as a plain row failure, + which is the intended behaviour. +- `test_guardrails.py` is +76 lines with zero deletions; no existing assertion was bent. +- `TestRetryOpenAICall` (3 tests) dropped on the user's call: `retry_openai_call` is tenacity's + stock exception-based behaviour and this branch does not change it. `test_retry.py` now covers + `retry_llm_call` only; its `httpx` / `openai` / `retry_openai_call` imports went with it. +- Wiki maintenance rule satisfied: `evaluations.md`, `llm-call.md`, `domain-map.md`, `INDEX.md`, + a new `guardrails.md`, the architecture doc and the `get_evaluation.md` API doc all move with + the code. +- No migration, no schema change, no secrets in the diff. + +### 3. End-to-end — two runs against a dev project + +The docker stack was rebuilt from this working tree (`backend:latest`) and `backend` + +`celery-worker` recreated; both were verified to be running branch code before either run. No +Celery beat runs locally, so the cron barrier never enqueues the fan-in — `execute_fast_evaluation_aggregate` +was called directly inside the worker container for both runs. That code is untouched by this +branch, so it is a disclosure, not a gap. + +**Run 18** — dataset `aicohort` (10 rows), config `cohort` **v3** (no guardrails): + +- Reached `completed`; 10 rows generated, judged and scored; `unscoreable` empty. +- **No new `llm_call` rows.** `count(*) where project_id=1` was 1 before and 1 after, while a + plain `POST /llm/call` on the same config moved it 0 → 1 as the positive control. + `record_call=False` holds across 10 real provider calls. +- The S3 responses unit carries `guardrail` on **every** row (`None` here), with `failed=False` + and real `usage`. +- `GET /evaluations/18?get_trace_info=true&export_format=row` returns `guardrail` on all 10 + traces, so the key survives `_merge_single_trace` and reaches the client. + +**Run 19** — same dataset, config **v4** = v3 plus `input_guardrails` + `output_guardrails` +pointing at a synthetic validator id. Config-version save does **not** resolve validator ids +against the guardrails service, so a guardrailed blob is buildable even while that service is +down. + +- The worker logged `[list_validators_config] Guardrails service unavailable ... Proceeding + without input/output guardrails` and `[apply_guardrails] No validator configs resolved + upstream` **per row, both directions** — live proof that fast eval now reaches the inline + guardrail hooks. +- Fail-open behaved as `guardrails.md` §4 describes: run `completed`, 10/10 rows answered, + `failed=False`, `unscoreable` empty, `llm_call` still 1. +- Every row's `guardrail` is `None`, **indistinguishable from a run where guardrails passed.** + That is the bypass-invisibility gap the plan records under "Known gap, deliberately not + closed", now observed rather than reasoned about. + +**Still not covered:** + +- A real *block*. The local `kaapi-guardrails-backend` container never finishes booting — its + Guardrails Hub token has expired, so `hub://guardrails/ban_list` fails to install and nothing + listens on its port (`curl` returns `000` from the host and from inside the worker network). + Proving `guardrail="blocked: ..."` and a `guardrail_blocked` unscoreable entry needs a fresh + hub token in that repo, not a change here. +- The v1 Stage-2 embedding threshold fix. `/api/v2/evaluations` always takes the judge path. + `TestStage2GuardrailBlocked` is its only proof, and it passes. +- Verification item 5 (no `gen_ai.chat` span or LLM metric in Sentry/OTel for the run). + +**Leftovers, not cleaned up:** the `ai_platform_test_eg` database, config `cohort` v4, runs 18 +and 19, and the positive-control `llm_call` row plus its job. diff --git a/docs/wiki/INDEX.md b/docs/wiki/INDEX.md index 9bc299d34..3eabe09de 100644 --- a/docs/wiki/INDEX.md +++ b/docs/wiki/INDEX.md @@ -17,6 +17,7 @@ Deep design narrative lives in `docs/architecture/*.md`; open those only for des ### Modules - [modules/llm-call.md](modules/llm-call.md) — `POST /llm/call` pipeline, configs (`Config`/`ConfigVersion`, `LLMCallConfig`), guardrails, chains. Deep dive: `docs/architecture/kaapi-llm-call-ARCHITECTURE.md` +- [modules/guardrails.md](modules/guardrails.md) — guardrails service integration: `POST /guardrails` async job, inline input/output hooks, management proxies, fail-open/fail-closed matrix. Deep dive: `docs/architecture/kaapi-llm-call-ARCHITECTURE.md` §7 (stale on auth failures). - [modules/evaluations.md](modules/evaluations.md) — text/STT/TTS evals, datasets, runs, batch + cron scoring, fast evals. Deep dive: `docs/architecture/kaapi-evaluations-ARCHITECTURE.md` - [modules/knowledge-base.md](modules/knowledge-base.md) — documents, collections, transforms, vector-store providers. Deep dive: `docs/architecture/kaapi-knowledge-base-ARCHITECTURE.md` - [modules/responses.md](modules/responses.md) — OpenAI Responses API integration, conversations, threads, assistants. No deep-dive doc yet. diff --git a/docs/wiki/domain-map.md b/docs/wiki/domain-map.md index 073d65540..788e2c42f 100644 --- a/docs/wiki/domain-map.md +++ b/docs/wiki/domain-map.md @@ -56,6 +56,7 @@ APIKey → Organization, Project, User # programmatic access - **Langfuse** — every LLM call and evaluation run writes traces/scores. A change to run scoring or trace shape ripples here. - **kaapi-frontend console** — reads run results, annotation queues, config CRUD. A response-shape change ripples here. - **Provider Batch APIs** (OpenAI, Gemini, Anthropic in `core/batch/`) — eval/assessment payload shape changes ripple here. +- **kaapi-guardrails service** — `/llm/call`, `/llm/chain` **and fast evaluation runs** call it inline (`services/llm/guardrails.py`). A validator or ban-list change therefore moves eval scores, not just live traffic; see [modules/guardrails.md](modules/guardrails.md). - **Object storage** (`core/cloud/storage.py` S3; `services/buckets/` GCS signed URLs) — files, dataset artifacts, gs:// attachments. ## Blast-radius procedure diff --git a/docs/wiki/modules/evaluations.md b/docs/wiki/modules/evaluations.md index 00669c4b8..c39f05de9 100644 --- a/docs/wiki/modules/evaluations.md +++ b/docs/wiki/modules/evaluations.md @@ -49,6 +49,8 @@ v2 judge field on `EvaluationRun`: `is_judge_run` (bool marker gating native jud - LangGraph (`langgraph`, `langgraph-checkpoint-postgres`) for the iteration loop — its `PostgresSaver` checkpointer connects via its own `psycopg` (v3) connection pool against the same database, separate from the app's SQLAlchemy engine; owns its own tables (`checkpoints`, `checkpoint_blobs`, `checkpoint_writes`, `checkpoint_migrations`), not Alembic-managed. ## Gotchas -- Eval traffic deliberately bypasses `/llm/call` (separate code path from production). +- **Fast text runs go through `/llm/call`'s own code.** `crud/evaluations/fast.py::_llm_call_for_item` calls `services/llm/jobs.py::execute_llm_call` with `record_call=False` (no `LlmCall` row, no AI spans, no LLM metrics) and `langfuse_credentials=None`, passing the blob `_resolve_config_and_clients` already resolved from the run's pinned `config_id`/`config_version` as an ad-hoc `LLMCallConfig(blob=...)`, so each row skips a config-version fetch while guardrails and `prompt_template` (carried on the blob) still apply. The blob is shared read-only across workers — `execute_llm_call` only writes into it for STT/TTS, which fast eval rejects. Consequences: a fast run now honours the config's `input_guardrails`/`output_guardrails` and its `prompt_template` (which eval never interpolated before — hence the `{{input}}` gate in `validate_fast_evaluation_inputs`, `422 config_template_missing_input`), and `transform_kaapi_config_to_native` owns the `include=["file_search_call.results"]` injection the chunk used to do by hand — opt-in, reached because eval passes `include_provider_raw_response=True`, so production traffic does not pay for the chunks. Provider failures no longer raise, so the tenacity policy is result-based (`retry.py::retry_llm_call`, 3 attempts on `error` when `BlockResult.retryable` is set); exhaustion **returns** the last `BlockResult` rather than raising, because a raise would abort the whole chunk through `_run_in_pool` and the cron healer would re-enqueue it, re-charging the provider. +- **Batch runs still bypass `/llm/call`** (a Batch submission is a JSONL upload; it structurally cannot route through a per-call function) and therefore still have no guardrails. +- **A guardrail-blocked row is unscoreable, not failed.** `ResponseResult.guardrail` carries `"blocked: ..."` / `"rephrased"` / `"applied"` / `None`, and a blocked row has `failed=False` with an empty `generated_output`, so it must not count toward `EVAL_FAST_FAILURE_THRESHOLD`. A guardrails **auth** failure (HTTP 401/403/422) is the opposite: it sets `error` with `guardrail_outcome=None` and fails the row normally — see [guardrails.md](guardrails.md) §9 invariant 8. Key unscoreable off `guardrail_outcome`, never off `error`. Scoring reflects that in one place: `fast_cosine.py::classify_empty_side` returns `UNSCOREABLE_GUARDRAIL_BLOCKED` (`"guardrail_blocked"`) ahead of the `empty_output` check, and both paths route through it — v1 via `score_cosine_run`, v2 via `_score_judge_path`. The reason is a stable key, not the raw `"blocked: ..."` string, because `merge.py::summarize_unscoreable` buckets anything outside `UNSCOREABLE_REASONS` as `"other"`. v1 needs one extra guard: `_stage2_embeddings` excludes blocked rows from `embed_candidates`, since embedding an empty output manufactures a `build_embedding_failure` that counts toward **that stage's own** threshold check. A **rephrased** row has real text and is scored normally — that canned answer is what production would have returned. Per-row visibility: `TraceData.guardrail` is emitted on every trace `build_trace_records` writes, and `merge.py::_merge_single_trace` has to name it explicitly or it is dropped on every read; the `grouped` export and Langfuse-fetched traces still do not carry it. On `"applied"` rows the row also carries `input_to_llm` (prompt after input guardrails, post-template) and `output_from_llm` (answer before output guardrails), read from `BlockResult.metadata`'s `input_guardrail`/`output_guardrail`; both are `None` on blocked rows (the block paths return no metadata) and on rephrased rows (the LLM never ran — the metadata's `input_to_llm` there is the canned reply). They ride `ResponseResult` → `TraceData` and merge under the same fresh-wins rule as `guardrail` (`score.py::GUARDRAIL_TRACE_KEYS`). `output_from_llm` is pre-redaction text, so it can hold what an output PII validator removed. - Scores sync to Langfuse; durable per-row maps on `evaluation_run` are the resync source of truth. - v2 (`is_judge_run`) is fully Kaapi-native: no Langfuse dataset/trace/score sync; one combined `responses.create` per row scores every applicable metric, structured via `crud/evaluations/judge.py` `METRIC_REGISTRY`. Live metrics (all trace-only — score + reasoning land in `score_trace_url`, no backup columns): `ground_truth`, `prompt` (obedience to the assistant's configured instructions; applies only when the run resolves a config prompt), and `knowledge_base` (groundedness; judges the file_search chunks captured during response generation, applies only to rows that retrieved chunks). Every run judges the whole registry; `_applicable_metrics` drops, per row, any metric whose required inputs that row cannot supply — including `prompt` when the run resolved no config prompt (it is passed as `""` for every row). Each metric's prompt fragment states its own CONSIDER/IGNORE input scope, so `_INPUT_LABELS` and those fragments must be edited together. v1 path is unchanged. diff --git a/docs/wiki/modules/guardrails.md b/docs/wiki/modules/guardrails.md new file mode 100644 index 000000000..ab265bac4 --- /dev/null +++ b/docs/wiki/modules/guardrails.md @@ -0,0 +1,176 @@ +# Module: Guardrails + +Handoff/context page for changing anything guardrails-related. Guardrails themselves run in a **separate service** (`kaapi-guardrails`); this backend only proxies to it and orchestrates jobs around it. Nothing in this repo evaluates text. + +All paths relative to `backend/app/`. Related: [llm-call.md](llm-call.md) (guardrails are part of the `/llm/call` pipeline), [../cross-cutting/auth.md](../cross-cutting/auth.md). Deep dive: `docs/architecture/kaapi-llm-call-ARCHITECTURE.md` §7 — but see §12, it predates the fail-closed-on-auth change. + +## 1. Two entry points, one transport + +| Entry point | Shape | Where | +|---|---|---| +| `POST /guardrails` | Standalone async job. Caller supplies text + validator IDs, gets `job_id`, result via webhook or poll. For callers running their own LLM workflow. | `api/routes/guardrails.py`, `services/guardrails/jobs.py` | +| `/llm/call` + `/llm/chain` + **fast evaluation** | Inline hooks around the provider call, driven by `config_blob.input_guardrails` / `output_guardrails`. | `services/llm/jobs.py` (`apply_input_guardrails`, `apply_output_guardrails`) | + +Both funnel into `apply_guardrails()` in `services/llm/guardrails.py`. That module is the **only** place that talks HTTP to the guardrails service. + +## 2. File map + +| Concern | File | +|---|---| +| Transport, outcome type, proxy helper | `services/llm/guardrails.py` | +| Standalone job orchestration (dedupe, warnings, callbacks) | `services/guardrails/jobs.py` | +| Routes: async job + poll + management proxies | `api/routes/guardrails.py` | +| Public endpoint contract (rendered into OpenAPI) | `api/docs/guardrails/apply_guardrails.md` | +| Request/response schemas | `models/guardrails/request.py`, `models/guardrails/response.py` | +| Inline adapters + call sites | `services/llm/jobs.py`, `services/llm/chain/chain.py` | +| Celery task + enqueue helper | `celery/tasks/job_execution.py` (`run_guardrails_job`), `celery/utils.py` (`start_guardrails_job`) | +| `JobType.LLM_GUARDRAILS`, `job.meta` | `models/job.py` | +| Settings | `core/config.py` — `KAAPI_GUARDRAILS_URL`, `KAAPI_GUARDRAILS_AUTH` | +| Sentry scrubbing | `core/sentry_filters.py` (`before_send_filter`) | +| Migrations | `alembic/versions/070_add_meta_and_guardrails_jobtype.py` (`job.meta` + enum value), `alembic/versions/083_add_llm_call_metadata.py` (`llm_call.metadata`) | + +Timeouts, all hardcoded in `services/llm/guardrails.py`: validator-config fetch 10s, validation POST 45s, management proxy `GUARDRAILS_PROXY_TIMEOUT_SECONDS` 30s. + +## 3. Transport layer (`services/llm/guardrails.py`) + +`_guardrails_headers()` builds every outbound request: `Authorization: Bearer {KAAPI_GUARDRAILS_AUTH}` plus tenant in `X-ORGANIZATION-ID` / `X-PROJECT-ID`. Tenant values come from the auth context only — never from request body or query. + +`apply_guardrails(text, validators, job_id, project_id, organization_id, output_text=None)` makes two upstream hops: + +1. `GET /validators/configs/?ids=...` via `list_validators_config()` — resolves config IDs to full validator configs. `output_text is not None` routes the IDs through the output slot instead of the input slot. +2. `POST /` via `run_guardrails_validation()` with `{request_id, input, validators}`, plus `output` when output guardrails are running (for validators that judge an input/output pair). Query param `suppress_pass_logs=false`. + +Returns `GuardrailsOutcome(safe_text, error, bypassed, rephrase_needed, raw)`. Property `.applied` = "ran and was not bypassed" — it is the gate for metadata emission. + +`summarize_validator_results(outcome)` flattens `raw.data.validator_results` into per-validator `{name, outcome, error, input_text, output_text}`. + +`proxy_guardrails_request(method, path, ...)` is a separate, dumber path used only by the management CRUD routes: forwards verbatim, returns `(status_code, parsed_json_or_None)`. + +## 4. Failure semantics (the thing most changes get wrong) + +| Upstream condition | Behaviour | +|---|---| +| Network error / timeout / 5xx during validation | **Fail open.** `bypassed=True`, original text passes through. | +| 401 / 403 / 422 on validation or on config fetch | **Fail closed.** Job FAILED. `_AUTH_ERROR_STATUS_CODES` includes 422 deliberately: a missing/invalid tenant header is a broken deploy, not a transient outage, so it must not fall open like one. | +| Config fetch fails for any other reason | Returns `([], [])` → `apply_guardrails` short-circuits with a no-op outcome where `bypassed=False`. See §9. | +| Upstream responds `success=false` | **Hard block.** `outcome.error` set; job FAILED. | +| Sanitised text is empty/whitespace | Blocked by the inline adapters ("left no usable content") — an empty prompt would otherwise produce a confusing provider-side error. | + +`outcome.error` therefore covers two very different situations, and `GuardrailsOutcome.blocked` is the discriminator: it is `True` only for a content verdict. The fail-closed auth dict carries `auth_error: True`, so `blocked` stays `False` there and callers keep treating it as a plain job failure rather than "the text was rejected". + +Error strings handed back to clients are status-only (`"Guardrails service rejected the request (HTTP 403)"`). Never put `str(e)` in them: it embeds the internal service URL and the string surfaces publicly via `job.error_message`. + +## 5. Standalone `POST /guardrails` + +Route validates `callback_url` (SSRF guard, `utils.validate_callback_url`), then `services/guardrails/jobs.py::start_job` creates a `JobType.LLM_GUARDRAILS` row with `meta={"request": ...}` and enqueues `run_guardrails_job`. HTTP 200 returns `GuardrailsJobImmediatePublic` immediately. + +Worker (`execute_job`): PROCESSING → `_dedupe_validators` → `apply_guardrails` → hard block means FAILED + failure callback, otherwise SUCCESS + success callback. + +Endpoint-specific behaviour, none of which exists on the `/llm/call` path: + +- **Dedupe.** `_dedupe_validators` drops repeated `validator_config_id`s (order-preserving) so callers are not double-billed upstream. +- **Warnings channel.** Bypass, dedupe, and "no validators resolved" become human-readable strings. Delivered as `metadata.warnings` in the callback and `warnings[]` on the poll response. A caller-supplied `warnings` key in `request_metadata` is overwritten (documented in the model docstring). +- **`job.meta`** holds `{request, response, callback}`. Raw unsafe text is persisted on purpose — this endpoint exists to inspect unsafe content. +- **`response_id` is server-minted** (`uuid4`), so callers always have a stable correlation handle even when upstream returns none. +- `rephrase_needed` is ignored here. Per-validator results are not exposed here. + +`GET /guardrails/{job_id}` **rebuilds** the payload from `job.meta` rather than replaying the stored callback body — the two representations can drift if only one is changed. + +## 6. Inline guardrails in `/llm/call` and `/llm/chain` + +`execute_llm_call` in `services/llm/jobs.py` is shared by three callers: `/llm/call`, `/llm/chain` (through `ChainBlock.execute` in `services/llm/chain/chain.py`), and fast evaluation (`crud/evaluations/fast.py::_llm_call_for_item`, with `record_call=False`). A guardrails change therefore changes what an eval measures, not just what production serves. + +Order inside `execute_llm_call`: resolve config → capture `original_input_value` → interpolate `prompt_template` → **input guardrails** → create the `LlmCall` row → provider call → **output guardrails** → update that row. + +- `apply_input_guardrails` returns a **5-tuple** `(query, error, guardrail_direct_response, metadata, guardrail_outcome)`. The last element is `"blocked"` / `"rephrased"` / `None` and lands on `BlockResult.guardrail_outcome`, so a caller can tell a content verdict from a provider failure — both of which arrive as a bare string in `error`. It is `None` on a fail-closed auth error (see §4). + - Hard block → job error. + - `rephrase_needed=True` → `safe_text` is returned to the user **without calling the LLM**; the call is recorded via `save_rephrase_guardrail_call`, and `query.input` is restored to `original_input_value` first. + - Otherwise `safe_text` overwrites `query.input.content.value`. + - Non-text input is skipped entirely. +- `apply_output_guardrails` returns `(BlockResult, error, guardrail_outcome)`. An output block now also carries the `usage` actually spent — the provider was called and billed before the verdict arrived. It passes `text=input_text` (the pre-template user input) and `output_text=`the LLM output. On success, sanitised text overwrites the response content, then `persist_output_guardrail_result` re-writes the already-created `LlmCall` row **in its own session** (the caller's session may already be closed). +- **There are two output-guardrail call sites** inside `execute_llm_call`: one in the proxy-provider branch, one in the main provider path. Any change to output guardrails must touch both. + +## 7. Guardrail metadata (`include_guardrail_metadata`, PR #1195) + +Opt-in flag on `LLMCallRequest`, default `False`. Gated on `outcome.applied`, so bypassed and no-op runs emit **nothing** — absence of metadata does not mean "guardrails passed". + +- Input side produces `{"input_guardrail": {input_from_user, input_to_llm, validators}}`, merged into `request_metadata`, which is then stored on the new `llm_call.metadata` column (model field `metadata_`, renamed to dodge SQLAlchemy's reserved `metadata`; migration 083). +- Output side produces `{"output_guardrail": {output_from_llm, output_to_user, validators}}` on `result.metadata`, persisted through `update_llm_call_response` (merge, not replace). +- `GET /llm/call/{job_id}` returns `llm_call.metadata_` as the response envelope's `metadata`. +- **`/llm/call` only.** `LLMChainRequest` has no such field and `ChainBlock.execute` does not forward the flag, so chains never emit guardrail metadata. + +## 8. Management proxy routes (PR #1135) + +Thin pass-throughs over the upstream management API, all under `require_permission(Permission.REQUIRE_PROJECT)`: + +- `GET /guardrails` — validator catalogue +- `/guardrails/ban_lists` — POST, GET (`domain`, `offset`, `limit`); `/{id}` GET/PATCH/DELETE +- `/guardrails/llm_prompt_configs` — POST, GET (`validator_name`, `offset`, `limit`); `/{id}` GET/PATCH/DELETE +- `/guardrails/validators/configs` — POST, GET (`ids`, `stage`, `type`); `/{id}` GET/PATCH/DELETE + +These do **not** fail open: upstream status codes and bodies (including its 422s) are returned unchanged. `_upstream_response` sets `Cache-Control: no-store` (tenant-scoped data must not sit in a shared cache, CWE-525) and keeps an empty upstream body empty, since a 204 cannot carry one. A non-JSON upstream body becomes a 502. + +## 9. Invariants — do not break these + +1. Tenant identity travels in headers derived from the auth context. Never accept `organization_id`/`project_id` from a request body or query string. +2. 401/403/422 fail closed; network/5xx fail open. Changing either side changes the security posture — say so explicitly in the PR. +3. Client-visible error strings stay status-only, never `str(e)`. +4. Fixed `/guardrails/*` paths stay declared **above** `GET /guardrails/{job_id}`. FastAPI matches in declaration order and will not fall through on a UUID parse failure. +5. Management proxies return upstream status/body verbatim with `no-store`. +6. Metadata emission stays gated on `outcome.applied`. +7. Raw text persistence is intentional on the `/guardrails` job, not an oversight. +8. `guardrail_outcome` marks content verdicts only. Never label a fail-closed auth/transport error `"blocked"` — a caller that treats blocks as benign (evaluation does) would silently swallow a broken deploy. It must not be retried either: the credentials are as broken on the third attempt as on the first, and an *output*-side failure re-charges the provider each time, because the completion is generated before output guardrails run. Hence `retryable` stays `False` on both guardrail error paths. + +## 10. Touch-map: common changes + +| Change | Files, in order | +|---|---| +| Add/modify an output guardrail behaviour | `apply_output_guardrails`, then **both** call sites in `execute_llm_call` (proxy branch + main provider path) | +| Change `apply_input_guardrails`' return | It is a 4-tuple — update the unpack in `execute_llm_call` | +| Add a field to the `/guardrails` result | `models/guardrails/response.py` → `_build_callback_payload` (callback path) → `get_guardrails_job_status` (poll path, rebuilds independently) → `api/docs/guardrails/apply_guardrails.md` | +| Add a management proxy route | `api/routes/guardrails.py` using `proxy_guardrails_request` + `_upstream_response`, declared above `/{job_id}` | +| Change upstream request/response shape | `run_guardrails_validation` and/or `list_validators_config`; check `summarize_validator_results` still finds `data.validator_results` | +| Add a new warning | `_dedupe_validators` or `_outcome_warnings` in `services/guardrails/jobs.py`; document it in the api doc | +| Change timeouts | `services/llm/guardrails.py` (three separate values, see §2) | +| Forward `include_guardrail_metadata` to chains | `LLMChainRequest`, `ChainBlock.__init__`/`execute`, `LLMChain`, `execute_chain_job`. **Hazard:** blocks share one `context.request_metadata`, and `apply_input_guardrails`' result is merged with an in-place `.update()` — every block would overwrite the previous block's `input_guardrail`. Key it per block before wiring this up. | + +## 11. Inferred upstream contract + +Not authoritative — reconstructed from call sites in this repo. The real contract lives in `kaapi-guardrails`. + +``` +POST / -> {success, bypassed?, error, data: {safe_text, rephrase_needed, validator_results[], usage{input_tokens,output_tokens,total_tokens,reasoning_tokens}}} +GET /validators/configs/?ids=... -> {success, data: [ {...validator config...} ]} +``` + +`validator_results[]` entries are read for `name`, `outcome`, `error`, `input_text`, `output_text`. + +## 12. Known issues / open questions + +- **`input_from_user` is post-template.** `prompt_template` interpolation runs before input guardrails inside `execute_llm_call`, so the metadata field holds the templated prompt, not the raw user text (`original_input_value`, captured before interpolation, is what output guardrails receive). +- **The architecture doc is stale on failure semantics.** `docs/architecture/kaapi-llm-call-ARCHITECTURE.md` §5, §7 and §10 state guardrails are fail-open unconditionally. That predates PR #1135, which made 401/403/422 fail closed. Trust the §4 matrix here. +- **Guardrail bypass is invisible to the caller.** When the service is unreachable the outcome is `bypassed=True` (§4), metadata emission is gated on `outcome.applied`, and neither adapter propagates the bypass flag. A row that silently skipped guardrails looks identical to one that passed them. This matters most for evaluation, where a whole run can quietly measure the bare model. Carrying a `"bypassed"` value through both adapters is the fix; it was deliberately left out of the eval-guardrail change to keep that diff contained. +- **Guardrail metadata reaches failure callbacks only by accident.** A hard block returns `BlockResult(error=..., llm_call_id=...)` with no `metadata`, and the failure callback in `execute_job` is built from `request.request_metadata`. Input metadata still shows up there when the caller supplied a `request_metadata` dict (even `{}`), because `apply_input_guardrails`' result is merged with an in-place `.update()` on that same object; it is dropped when the caller sent none. The alias also breaks on the main provider path whenever `transform_kaapi_config_to_native` rebinds `request_metadata` to a new dict to attach warnings. Do not rely on either behaviour — make it explicit if failure payloads need the metadata. +- **Silent no-op when the config fetch fails non-auth.** `list_validators_config` returns `[]` and the request proceeds with no guardrails at all, signalled only by a log line on the `/llm/call` path. On `/guardrails` it surfaces as a "none resolved" warning, indistinguishable from genuinely bad validator IDs. +- **Unverified:** in output mode the success branch does `data.get("safe_text", text)` where `text` is the *user input*. If upstream ever omits `safe_text`, the LLM output would be replaced by the user's input. Depends on the upstream contract, which is not visible from this repo. +- **Sentry redaction does not cover `/guardrails`.** `_LLM_JOB_TASK_NAMES` lists `run_llm_job`, `run_llm_chain_job`, `run_response_job` — not `run_guardrails_job` — and `_SENSITIVE_REQUEST_DATA_KEYS` does not include `text`. So the standalone endpoint's raw text and `callback_url` reach Sentry unredacted via the Celery integration. + +## 13. Tests + +| File | Covers | +|---|---| +| `tests/api/routes/test_guardrails.py` | Routes, proxy behaviour, route ordering, poll | +| `tests/services/llm/test_guardrails.py` | Transport, fail-open/fail-closed matrix | +| `tests/services/guardrails/test_jobs.py` | Standalone job worker, dedupe, warnings, callbacks | +| `tests/services/llm/test_jobs.py` | Inline adapters, metadata emission | +| `tests/core/test_sentry_filters.py` | Redaction | + +Mocking style is `unittest.mock.patch`, not `respx`. Transport tests patch `app.services.llm.guardrails.httpx.Client`; route tests patch `app.api.routes.guardrails.start_job` or stub upstream via a local `_mock_upstream` helper; worker tests patch `Session` and `JobCrud` inside `app.services.guardrails.jobs` (the real ones would escape the test fixture's savepoint rollback). + +Run: `uv run bash scripts/tests-start.sh`. + +## 14. Workflow + +- Layer edits (model/crud/service/route/migration/celery) go through the `senior-engineer` subagent; tests through `test-writer`. See root `CLAUDE.md`. +- Run `/pr-review` on the full diff before committing. +- Maintenance rule: a change to guardrails routes/models/services updates **this page** in the same PR (and `domain-map.md` if entities or edges changed). diff --git a/docs/wiki/modules/llm-call.md b/docs/wiki/modules/llm-call.md index de5749b90..1d065148e 100644 --- a/docs/wiki/modules/llm-call.md +++ b/docs/wiki/modules/llm-call.md @@ -15,6 +15,7 @@ All paths relative to `backend/app/`. - `/guardrails/ban_lists` — POST, GET (`offset`, `limit`); `/{id}` GET/PATCH/DELETE - `/guardrails/llm_prompt_configs` — POST, GET (`validator_name`, `offset`, `limit`); `/{id}` GET/PATCH/DELETE - `/guardrails/validators/configs` — POST, GET (`ids`, `stage`, `type`); `/{id}` GET/PATCH/DELETE + - Full guardrails context (transport, failure semantics, touch-map): [guardrails.md](guardrails.md) - Gotcha: the fixed `/guardrails/*` paths must stay declared above `GET /guardrails/{job_id}` — FastAPI matches in declaration order and won't fall through on a UUID parse failure. ## Tables (SQLModel) @@ -26,7 +27,7 @@ All paths relative to `backend/app/`. ## Key pydantic/SQLModel schemas (`models/llm/request.py`) - `LLMCallConfig` — one-of: saved reference (`id` + `version`) XOR ad-hoc `blob` (validator-enforced) -- `ConfigBlob` — `completion` + optional `prompt_template` (`PromptTemplate.template`, plain string; `{{input}}` interpolation is llm-chain-only) + `input_guardrails`/`output_guardrails` +- `ConfigBlob` — `completion` + optional `prompt_template` (`PromptTemplate.template`, plain string) + `input_guardrails`/`output_guardrails` - `CompletionConfig` — discriminated union on `provider`: `KaapiCompletionConfig` (standardized params: `TextLLMParams`/`STTLLMParams`/`TTSLLMParams`), `NativeCompletionConfig` (pass-through), `ProxyCompletionConfig` (client's own endpoint) - `TextLLMParams` reasoning knobs: `reasoning` + `effort` (OpenAI-style), `thinking` (Anthropic adaptive-thinking container, forwarded as-is) and `thinking_level` (Gemini). A knob must be **declared here** to survive config save — pydantic's default extra policy is ignore, so an undeclared key is dropped silently at validation with no error. - `QueryParams` — per-call input + `ConversationConfig` @@ -34,6 +35,7 @@ All paths relative to `backend/app/`. ## Services / CRUD - `services/llm/` — `mappers.py` (Kaapi params → provider API), `providers/`, `chain/`, `guardrails.py`, `jobs.py` +- `jobs.py::execute_llm_call` is the single shared invocation, reached from `/llm/call`, from `ChainBlock.execute`, and from fast evaluation runs. Two knobs shape what it records: `record_call` (default `True`) turns off the `LlmCall` row, the AI spans and the LLM metrics in one switch, for traffic that is not production; Langfuse is turned off separately by passing `langfuse_credentials=None`. `BlockResult.guardrail_outcome` (`GuardrailOutcomeEnum.BLOCKED` / `.REPHRASED` / `None`, a `StrEnum` in `chain/types.py`) tells a caller a guardrail verdict apart from a provider failure, since both otherwise arrive as a bare string in `error` — see [guardrails.md](guardrails.md) §4, §6. `BlockResult.retryable` answers the other question that string cannot: whether re-running the identical call could succeed. It defaults to `False` and is set only where the provider or the proxy call actually failed, so a failure path added later fails fast instead of inheriting three attempts. - `mappers.py` is the **only** param mapper in the codebase; the assessment fork was merged back into it. Callers: `crud/evaluations/{batch,fast,judge}.py`, `crud/assessment/batch.py`, `services/assessment/api/batch.py`, `services/assessment/prefilter/request_builder.py`. Structured output is keyed `output_schema` for every provider (OpenAI `text.format`, Anthropic `output_config.format`, Gemini `output_schema`), and Anthropic reads `effort` into `output_config.effort`. Every param that only assessment set (`top_p`, `max_output_tokens`, `thinking_level`, `thinking`, `output_schema`) defaults to `None` on `TextLLMParams` and is dropped by `ParamSerialization._dump_compact`, so a config that does not set it maps exactly as before. Anthropic never receives `temperature`/`top_p`: the Messages API returns 400 for a non-default sampling value on every Claude model Kaapi serves, so the mapper drops both, warning only when the value was not Anthropic's own default of 1.0. `normalize_llm_text` no longer lives here; it moved to `services/assessment/validators.py`, the only place that calls it. - `services/guardrails/` — validator execution - `crud/llm.py`, `crud/llm_chain.py`, `crud/config/` — persistence @@ -49,4 +51,5 @@ All paths relative to `backend/app/`. - `type=proxy` auto-injects `provider="proxy"` (ConfigBlob validator). - Missing project credentials raise, except `google-gcp`/`google-gcp-native`, which fall back to platform-shared credentials (`services/llm/providers/registry.py`). - Feature needs an LLM config? Spec `LLMCallConfig` whole (never a bespoke params + prompt pair), and prefer an optional per-request field over a per-project binding table — saved references already give durable versioned config via `config`/`config_version`. -- `PromptTemplate.template` is a plain prompt string; `{{input}}` interpolation is llm-chain-only — features that assemble their own inputs don't use it. +- `PromptTemplate.template` is a plain prompt string, and its `{{input}}` interpolation is **not** llm-chain-only: `execute_llm_call` does `template.replace("{{input}}", value)` unconditionally for every caller, before input guardrails run. A template that omits the placeholder therefore sends the template alone and silently drops the user input — which is why fast evaluation gates on it (`422 config_template_missing_input`). A feature that assembles its own inputs must leave `prompt_template` unset rather than assume it is ignored. +- `transform_kaapi_config_to_native` adds `include=["file_search_call.results"]` on the OpenAI branch when the mapped params carry a `file_search` tool **and** the caller passed `include_file_search_results=True`, so a knowledge-base call gets its retrieved chunks back. It lives in the transform and not in `map_kaapi_to_openai_params` because that mapper's other callers build Batch API bodies, where `include` is invalid. It is opt-in, and `execute_llm_call` drives it off `include_provider_raw_response`: the hits are only reachable through the raw provider response, so for a caller that did not ask for one they are extra payload on every request. Evaluation is the caller that asks; ordinary `/llm/call` and `/llm/chain` traffic is unaffected unless the client sets the flag.