From 4554c1297acfe082b3f7c18183c104f28a67133a Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Wed, 9 Sep 2026 11:33:19 +0530 Subject: [PATCH 1/3] feat:add metadata field to store and send guardrails intermediate results --- .../versions/083_add_llm_call_metadata.py | 40 +++++ backend/app/api/routes/llm.py | 4 +- backend/app/celery/celery_app.py | 3 +- backend/app/core/sentry_filters.py | 56 +++++++ backend/app/crud/llm.py | 10 ++ backend/app/main.py | 3 +- backend/app/models/llm/request.py | 10 ++ backend/app/services/llm/guardrails.py | 20 +++ backend/app/services/llm/jobs.py | 109 +++++++++--- backend/app/tests/core/test_sentry_filters.py | 79 +++++++++ .../app/tests/services/llm/test_guardrails.py | 53 ++++++ backend/app/tests/services/llm/test_jobs.py | 158 ++++++++++++++++++ 12 files changed, 522 insertions(+), 23 deletions(-) create mode 100644 backend/app/alembic/versions/083_add_llm_call_metadata.py create mode 100644 backend/app/tests/core/test_sentry_filters.py diff --git a/backend/app/alembic/versions/083_add_llm_call_metadata.py b/backend/app/alembic/versions/083_add_llm_call_metadata.py new file mode 100644 index 000000000..53e4e9f0f --- /dev/null +++ b/backend/app/alembic/versions/083_add_llm_call_metadata.py @@ -0,0 +1,40 @@ +"""Add llm_call.metadata (generic extensibility catch-all) + +Revision ID: 083 +Revises: 082 +Create Date: 2026-09-08 00:00:00.000000 + +Re-adds a `metadata` JSONB column on `llm_call`, previously dropped by 079 +(as collateral of an unrelated feature revert, not because the column was a +bad idea). This time it's a generic catch-all, mirroring `llm_chain.metadata` +(added via `metadata_` in the model to dodge SQLAlchemy's reserved +`Base.metadata` attribute) — first use case: persisting input/output +guardrail results so /llm/call polling (GET /llm/call/{job_id}) can surface +them, matching what's already sent on the callback payload's `metadata` +field. +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "083" +down_revision = "082" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column( + "llm_call", + sa.Column( + "metadata", + postgresql.JSONB(astext_type=sa.Text()), + nullable=True, + comment="Future-proof extensibility catch-all (e.g. guardrail results)", + ), + ) + + +def downgrade(): + op.drop_column("llm_call", "metadata") diff --git a/backend/app/api/routes/llm.py b/backend/app/api/routes/llm.py index 94d3b341e..91ee0d67e 100644 --- a/backend/app/api/routes/llm.py +++ b/backend/app/api/routes/llm.py @@ -189,6 +189,7 @@ def get_llm_call_status( raise HTTPException(status_code=404, detail="Job not found") llm_call_response = None + call_metadata: dict | None = None if job.status.value == JobStatus.SUCCESS: llm_calls = get_llm_calls_by_job_id( session=session, job_id=job_id, project_id=project_id @@ -229,6 +230,7 @@ def get_llm_call_status( usage=Usage(**usage_payload), provider_raw_response=None, ) + call_metadata = llm_call.metadata_ job_response = LLMJobPublic( job_id=job.id, @@ -237,4 +239,4 @@ def get_llm_call_status( error_message=job.error_message, ) - return APIResponse.success_response(data=job_response) + return APIResponse.success_response(data=job_response, metadata=call_metadata) diff --git a/backend/app/celery/celery_app.py b/backend/app/celery/celery_app.py index c2cba4cfb..0859f0c1c 100644 --- a/backend/app/celery/celery_app.py +++ b/backend/app/celery/celery_app.py @@ -13,7 +13,7 @@ from app.core.config import settings from app.core.logger import configure_logging -from app.core.sentry_filters import before_send_transaction_filter +from app.core.sentry_filters import before_send_filter, before_send_transaction_filter logger = logging.getLogger(__name__) _telemetry_initialized = False @@ -48,6 +48,7 @@ def _initialize_worker_observability() -> None: # bodies to error events or trace transactions. max_request_body_size="never", enable_logs=True, + before_send=before_send_filter, before_send_transaction=before_send_transaction_filter, integrations=[ LoggingIntegration( diff --git a/backend/app/core/sentry_filters.py b/backend/app/core/sentry_filters.py index 6712803bd..13dc0a393 100644 --- a/backend/app/core/sentry_filters.py +++ b/backend/app/core/sentry_filters.py @@ -2,6 +2,20 @@ from typing import Any +_REDACTED = "[REDACTED]" + +# sentry_sdk's CeleryIntegration attaches the task's raw args/kwargs to every +# event captured during that task as extra["celery-job"]. For these tasks, +# kwargs["request_data"] carries the caller's raw query text and (for chain +# jobs) prior block responses -- none of that should reach Sentry. +_LLM_JOB_TASK_NAMES = { + "app.celery.tasks.job_execution.run_llm_job", + "app.celery.tasks.job_execution.run_llm_chain_job", + "app.celery.tasks.job_execution.run_response_job", +} +_SENSITIVE_REQUEST_DATA_KEYS = ("query", "request_metadata", "response", "output") + + _SQL_OR_CONNECT = re.compile(r"^(select|insert|update|delete|connect)\b", re.IGNORECASE) _HTTP_SEND_RECEIVE = re.compile(r"http (send|receive)$", re.IGNORECASE) _DB_QUERY_SPAN = re.compile(r"^db\.query$", re.IGNORECASE) @@ -85,3 +99,45 @@ def before_send_transaction_filter( event["spans"] = filtered return event + + +def _redact_llm_job_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]: + """Replaces end-user text fields in an LLM job's request_data with a + placeholder, leaving non-text fields (config, ids, callback_url) intact. + """ + request_data = kwargs.get("request_data") + if not isinstance(request_data, dict): + return kwargs + + redacted_request_data = dict(request_data) + for key in _SENSITIVE_REQUEST_DATA_KEYS: + if key in redacted_request_data: + redacted_request_data[key] = _REDACTED + + redacted_kwargs = dict(kwargs) + redacted_kwargs["request_data"] = redacted_request_data + return redacted_kwargs + + +def before_send_filter( + event: dict[str, Any], hint: dict[str, Any] +) -> dict[str, Any] | None: + """Strips end-user query/response text from LLM job celery-job context + before an event reaches Sentry. + """ + extra = event.get("extra") + if not isinstance(extra, dict): + return event + + celery_job = extra.get("celery-job") + if not isinstance(celery_job, dict): + return event + + if celery_job.get("task_name") not in _LLM_JOB_TASK_NAMES: + return event + + kwargs = celery_job.get("kwargs") + if isinstance(kwargs, dict): + celery_job["kwargs"] = _redact_llm_job_kwargs(kwargs) + + return event diff --git a/backend/app/crud/llm.py b/backend/app/crud/llm.py index 44e605fb7..a0c6448e1 100644 --- a/backend/app/crud/llm.py +++ b/backend/app/crud/llm.py @@ -65,6 +65,7 @@ def create_llm_call( organization_id: int, resolved_config: ConfigBlob, original_provider: str, + metadata: dict[str, Any] | None = None, ) -> LlmCall: """ Create a new LLM call record in the database. @@ -76,6 +77,7 @@ def create_llm_call( project_id: Project this LLM call belongs to organization_id: Organization this LLM call belongs to resolved_config: The resolved configuration blob (either from stored config or ad-hoc) + metadata: Extensibility catch-all dict (e.g. input guardrail results) Returns: LlmCall: The created LLM call record @@ -150,6 +152,7 @@ def create_llm_call( conversation_id=conversation_id, auto_create=auto_create, config=config_dict, + metadata_=metadata, ) session.add(db_llm_call) @@ -172,6 +175,7 @@ def update_llm_call_response( content: dict[str, Any] | None = None, usage: dict[str, Any] | None = None, conversation_id: str | None = None, + metadata: dict[str, Any] | None = None, ) -> LlmCall: """ Update an LLM call record with response data. @@ -183,6 +187,7 @@ def update_llm_call_response( content: Response content dict usage: Token usage dict conversation_id: Conversation ID if created/updated + metadata: Extensibility catch-all dict, merged into any existing value Returns: LlmCall: The updated LLM call record @@ -218,6 +223,10 @@ def update_llm_call_response( db_llm_call.usage = usage if conversation_id is not None: db_llm_call.conversation_id = conversation_id + if metadata is not None: + existing_metadata = db_llm_call.metadata_ or {} + existing_metadata.update(metadata) + db_llm_call.metadata_ = existing_metadata db_llm_call.updated_at = now() @@ -283,6 +292,7 @@ def save_rephrase_guardrail_call( resolved_config=config_blob, original_provider=str(config_blob.completion.provider), chain_id=chain_id, + metadata=request_metadata, ) try: update_llm_call_response( diff --git a/backend/app/main.py b/backend/app/main.py index 5ad2ad6e0..fd97c03da 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -18,7 +18,7 @@ from app.core.exception_handlers import register_exception_handlers from app.core.logger import configure_logging from app.core.middleware import StripTrailingSlashMiddleware, http_request_logger -from app.core.sentry_filters import before_send_transaction_filter +from app.core.sentry_filters import before_send_filter, before_send_transaction_filter from app.core.telemetry import instrument_app, setup_telemetry from app.load_env import load_environment @@ -38,6 +38,7 @@ # bodies to error events or trace transactions. max_request_body_size="never", enable_logs=True, + before_send=before_send_filter, before_send_transaction=before_send_transaction_filter, integrations=[ LoggingIntegration( diff --git a/backend/app/models/llm/request.py b/backend/app/models/llm/request.py index c028a978a..63b2e7fbb 100644 --- a/backend/app/models/llm/request.py +++ b/backend/app/models/llm/request.py @@ -764,6 +764,16 @@ class LlmCall(SQLModel, table=True): ), ) + metadata_: dict[str, Any] | None = Field( + default=None, + sa_column=sa.Column( + "metadata", + JSONB, + nullable=True, + comment="Future-proof extensibility catch-all (e.g. guardrail results)", + ), + ) + # Timestamps inserted_at: datetime = Field( default_factory=now, diff --git a/backend/app/services/llm/guardrails.py b/backend/app/services/llm/guardrails.py index b2364c118..bf12512e7 100644 --- a/backend/app/services/llm/guardrails.py +++ b/backend/app/services/llm/guardrails.py @@ -240,6 +240,26 @@ def apply_guardrails( ) +def summarize_validator_results(outcome: GuardrailsOutcome) -> list[dict[str, Any]]: + """Per-validator name/outcome/text summary, extracted from the raw + guardrails service response, for use in llm_call metadata.""" + data = outcome.raw.get("data") or {} + validator_results = data.get("validator_results") or [] + + summaries = [] + for validator_result in validator_results: + summaries.append( + { + "name": validator_result.get("name"), + "outcome": validator_result.get("outcome"), + "error": validator_result.get("error"), + "input_text": validator_result.get("input_text"), + "output_text": validator_result.get("output_text"), + } + ) + return summaries + + def run_guardrails_validation( input_text: str, guardrail_config: Sequence[Validator | dict[str, Any]], diff --git a/backend/app/services/llm/jobs.py b/backend/app/services/llm/jobs.py index 00cdea402..62f5baf34 100644 --- a/backend/app/services/llm/jobs.py +++ b/backend/app/services/llm/jobs.py @@ -67,7 +67,7 @@ Usage, ) from app.services.llm.chain.types import BlockResult -from app.services.llm.guardrails import apply_guardrails +from app.services.llm.guardrails import apply_guardrails, summarize_validator_results from app.services.llm.mappers import ( resolve_default_audio_provider, transform_kaapi_config_to_native, @@ -371,19 +371,13 @@ def apply_input_guardrails( job_id: UUID, project_id: int, organization_id: int, -) -> tuple[QueryParams, str | None, str | None]: +) -> tuple[QueryParams, str | None, str | None, dict[str, Any] | None]: """Apply input guardrails from a config_blob. Shared with llm-call and llm-chain. - Thin adapter over ``apply_guardrails`` that maps the outcome onto a - ``QueryParams`` payload. - - Returns (query, error, guardrail_direct_response) where: - - error is set when guardrails hard-block the request - - guardrail_direct_response is set when rephrase_needed=True and the safe_text - should be returned directly to the user without hitting the LLM + Returns (query, error, guardrail_direct_response, metadata). """ if not config_blob or not config_blob.input_guardrails: - return query, None, None + return query, None, None, None if not isinstance(query.input, TextInput): logger.info( @@ -391,24 +385,34 @@ def apply_input_guardrails( f"job_id={job_id}, " f"input_type={getattr(query.input, 'type', type(query.input).__name__)}" ) - return query, None, None + return query, None, None, None + original_input_text = query.input.content.value outcome = apply_guardrails( - text=query.input.content.value, + text=original_input_text, validators=config_blob.input_guardrails, job_id=job_id, project_id=project_id, organization_id=organization_id, ) + metadata = None + if outcome.applied: + metadata = { + "input_guardrail": { + "user_input": original_input_text, + "input_sent_to_llm": outcome.safe_text, + "validators": summarize_validator_results(outcome), + } + } if outcome.error is not None: - return query, outcome.error, None + return query, outcome.error, None, metadata 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 + return query, None, outcome.safe_text, metadata # No-op paths (no validators, bypassed) leave the query untouched. if outcome.applied and outcome.safe_text is not None: @@ -425,9 +429,10 @@ def apply_input_guardrails( query, "Input guardrails rejected the request and left no usable content.", None, + metadata, ) query.input.content.value = outcome.safe_text - return query, None, None + return query, None, None, metadata def apply_output_guardrails( @@ -441,9 +446,6 @@ def apply_output_guardrails( ) -> tuple[BlockResult, str | None]: """Apply output guardrails from a config_blob. Shared by /llm/call and /llm/chain. - Thin adapter over ``apply_guardrails`` that maps the outcome onto a - ``BlockResult``. - Returns (modified_result, None) on success, or (result, error_string) on failure. """ if not config_blob or not config_blob.output_guardrails: @@ -457,15 +459,25 @@ def apply_output_guardrails( ) return result, None + original_output_text = result.response.response.output.content.value outcome = apply_guardrails( text=input_text or "", validators=config_blob.output_guardrails, job_id=job_id, project_id=project_id, organization_id=organization_id, - output_text=result.response.response.output.content.value, + output_text=original_output_text, ) + if outcome.applied: + existing_metadata = result.metadata or {} + existing_metadata["output_guardrail"] = { + "llm_output_pre_guardrail": original_output_text, + "response_to_user": outcome.safe_text, + "validators": summarize_validator_results(outcome), + } + result.metadata = existing_metadata + if outcome.error is not None: return result, outcome.error @@ -483,6 +495,32 @@ def apply_output_guardrails( return result, None +def persist_output_guardrail_result( + *, + llm_call_id: UUID | None, + content: dict[str, Any] | None, + metadata: dict[str, Any] | None, +) -> None: + """Re-persists content/metadata onto the already-created LlmCall row + after output guardrails run. Uses its own session since the caller's + session may already be closed by this point.""" + if not llm_call_id: + return + if content is None and metadata is None: + return + try: + with Session(engine) as session: + update_llm_call_response( + session, llm_call_id=llm_call_id, content=content, metadata=metadata + ) + except Exception as e: + logger.error( + f"[persist_output_guardrail_result] Failed to persist guardrail " + f"result: {e} | llm_call_id={llm_call_id}", + exc_info=True, + ) + + DETECTED_LANGUAGE_FALLBACK = "en-IN" _TTS_LANGUAGE_KEYS = ("target_language_code", "language_code") @@ -605,13 +643,22 @@ def execute_llm_call( project_id=project_id, organization_id=organization_id, ) - query, input_error, guardrail_direct_response = apply_input_guardrails( + ( + query, + input_error, + guardrail_direct_response, + input_guardrail_metadata, + ) = apply_input_guardrails( config_blob=config_blob, query=query, job_id=job_id, project_id=project_id, organization_id=organization_id, ) + if input_guardrail_metadata: + if request_metadata is None: + request_metadata = {} + request_metadata.update(input_guardrail_metadata) if guardrail_direct_response is not None: # Runs before the Kaapi->native transform, so params may be # a typed model (Kaapi/proxy variants) rather than a dict. @@ -715,6 +762,7 @@ def execute_llm_call( 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: @@ -876,6 +924,17 @@ def execute_llm_call( trace.Status(trace.StatusCode.ERROR, output_error) ) return BlockResult(error=output_error, llm_call_id=llm_call_id) + if config_blob.output_guardrails: + updated_content = None + if isinstance(result.response.response.output, TextOutput): + updated_content = ( + result.response.response.output.model_dump() + ) + persist_output_guardrail_result( + llm_call_id=llm_call_id, + content=updated_content, + metadata=result.metadata, + ) return result @@ -945,6 +1004,7 @@ def execute_llm_call( 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) @@ -1257,6 +1317,15 @@ def execute_llm_call( trace.Status(trace.StatusCode.ERROR, output_error) ) return BlockResult(error=output_error, llm_call_id=llm_call_id) + if config_blob.output_guardrails: + updated_content = None + if isinstance(result.response.response.output, TextOutput): + updated_content = result.response.response.output.model_dump() + persist_output_guardrail_result( + llm_call_id=llm_call_id, + content=updated_content, + metadata=result.metadata, + ) return result diff --git a/backend/app/tests/core/test_sentry_filters.py b/backend/app/tests/core/test_sentry_filters.py new file mode 100644 index 000000000..8f750a9d7 --- /dev/null +++ b/backend/app/tests/core/test_sentry_filters.py @@ -0,0 +1,79 @@ +from app.core.sentry_filters import before_send_filter + + +def _event_with_celery_job(task_name: str, kwargs: dict) -> dict: + return { + "extra": { + "celery-job": { + "task_name": task_name, + "args": [], + "kwargs": kwargs, + } + } + } + + +def test_before_send_filter_redacts_query_for_llm_job(): + event = _event_with_celery_job( + "app.celery.tasks.job_execution.run_llm_job", + { + "job_id": "b9ce6621-9b5a-45c4-969f-df7613ff7dc4", + "organization_id": 1, + "project_id": 1, + "request_data": { + "callback_url": "https://webhooksite.net/some-id", + "config": {"blob": {"completion": {"provider": "openai"}}}, + "query": { + "input": { + "type": "text", + "content": {"value": "Amit Gupta phone number is 919611188278"}, + } + }, + "request_metadata": None, + }, + }, + ) + + result = before_send_filter(event, {}) + + request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] + assert request_data["query"] == "[REDACTED]" + # Non-sensitive fields are left untouched so the trace stays useful. + assert request_data["callback_url"] == "https://webhooksite.net/some-id" + assert request_data["config"] == {"blob": {"completion": {"provider": "openai"}}} + + +def test_before_send_filter_redacts_query_for_chain_and_response_jobs(): + for task_name in ( + "app.celery.tasks.job_execution.run_llm_chain_job", + "app.celery.tasks.job_execution.run_response_job", + ): + event = _event_with_celery_job( + task_name, + {"request_data": {"query": {"input": "sensitive text"}}}, + ) + + result = before_send_filter(event, {}) + + request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] + assert request_data["query"] == "[REDACTED]" + + +def test_before_send_filter_ignores_unrelated_tasks(): + event = _event_with_celery_job( + "app.celery.tasks.job_execution.run_doctransform_job", + {"request_data": {"query": {"input": "not an llm job"}}}, + ) + + result = before_send_filter(event, {}) + + request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] + assert request_data["query"] == {"input": "not an llm job"} + + +def test_before_send_filter_passes_through_events_without_celery_job(): + event = {"message": "some unrelated error"} + + result = before_send_filter(event, {}) + + assert result == event diff --git a/backend/app/tests/services/llm/test_guardrails.py b/backend/app/tests/services/llm/test_guardrails.py index 8e357aaf3..b1ccabcdb 100644 --- a/backend/app/tests/services/llm/test_guardrails.py +++ b/backend/app/tests/services/llm/test_guardrails.py @@ -20,8 +20,10 @@ Validator, ) from app.services.llm.guardrails import ( + GuardrailsOutcome, list_validators_config, run_guardrails_validation, + summarize_validator_results, ) from app.tests.utils.utils import get_project @@ -374,6 +376,57 @@ def test_list_validators_config_network_error_fails_open(mock_client_cls) -> Non assert output_guardrails == [] +def test_summarize_validator_results_extracts_per_validator_fields() -> None: + outcome = GuardrailsOutcome( + safe_text="My credit card is [REDACTED]", + error=None, + bypassed=False, + rephrase_needed=False, + raw={ + "success": True, + "data": { + "safe_text": "My credit card is [REDACTED]", + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "input", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": "My credit card is 4111 1111 1111 1111", + "output_text": "My credit card is [REDACTED]", + } + ], + }, + }, + ) + + summaries = summarize_validator_results(outcome) + + assert summaries == [ + { + "name": "PIIRemover", + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": "My credit card is 4111 1111 1111 1111", + "output_text": "My credit card is [REDACTED]", + } + ] + + +def test_summarize_validator_results_empty_when_raw_has_no_validator_results() -> None: + outcome = GuardrailsOutcome( + safe_text="hello", + error=None, + bypassed=True, + rephrase_needed=False, + raw={}, + ) + + assert summarize_validator_results(outcome) == [] + + _SAFE_TEXT = "Please rephrase: content not allowed." _CONFIG_BLOB = ConfigBlob( completion=NativeCompletionConfig( diff --git a/backend/app/tests/services/llm/test_jobs.py b/backend/app/tests/services/llm/test_jobs.py index 9ce03f2ca..fd8bdf898 100644 --- a/backend/app/tests/services/llm/test_jobs.py +++ b/backend/app/tests/services/llm/test_jobs.py @@ -1344,6 +1344,85 @@ def test_guardrails_sanitize_input_before_provider( assert result["success"] + def test_guardrails_metadata_reports_per_validator_text_for_input_guardrail( + self, db, job_env, job_for_execution + ): + """metadata.input_guardrail should surface the original text, what + was sent to the LLM, and each validator's own before/after text -- + not just the raw kaapi-guardrails response wrapper. + """ + env = job_env + env["provider"].execute.return_value = (env["mock_llm_response"], None) + + unsafe_input = "My credit card is 4111 1111 1111 1111" + sanitized_input = "My credit card is [REDACTED]" + + with ( + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + ): + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": sanitized_input, + "rephrase_needed": False, + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "input", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": unsafe_input, + "output_text": sanitized_input, + } + ], + }, + } + mock_fetch_configs.return_value = ( + [{"type": "pii_remover", "stage": "input"}], + [], + ) + + request_data = { + "query": {"input": unsafe_input}, + "config": { + "blob": { + "completion": { + "provider": "openai-native", + "type": "text", + "params": {"model": "gpt-4o"}, + }, + "input_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_1} + ], + "output_guardrails": [], + } + }, + "include_provider_raw_response": False, + "callback_url": None, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + + input_guardrail_metadata = result["metadata"]["input_guardrail"] + assert input_guardrail_metadata["user_input"] == unsafe_input + assert input_guardrail_metadata["input_sent_to_llm"] == sanitized_input + + validators = input_guardrail_metadata["validators"] + assert len(validators) == 1 + assert validators[0]["name"] == "PIIRemover" + assert validators[0]["outcome"] == "FAIL" + assert validators[0]["input_text"] == unsafe_input + assert validators[0]["output_text"] == sanitized_input + def test_guardrails_skip_input_validation_for_audio_input( self, db, job_env, job_for_execution ): @@ -1443,6 +1522,85 @@ def test_guardrails_sanitize_output_after_provider( assert "REDACTED" in result["data"]["response"]["output"]["content"]["value"] + def test_guardrails_metadata_reports_per_validator_text_for_output_guardrail( + self, db, job_env, job_for_execution + ): + """metadata.output_guardrail should surface the raw LLM output + (pre-guardrail), the final response text, and each validator's own + before/after text -- not just the raw kaapi-guardrails response + wrapper. + """ + env = job_env + + raw_llm_output = "Aadhar no 123-45-6789" + sanitized_output = "Aadhar [REDACTED]" + env["mock_llm_response"].response.output.content.value = raw_llm_output + env["provider"].execute.return_value = (env["mock_llm_response"], None) + + with ( + patch( + "app.services.llm.guardrails.run_guardrails_validation" + ) as mock_guardrails, + patch( + "app.services.llm.guardrails.list_validators_config" + ) as mock_fetch_configs, + ): + mock_guardrails.return_value = { + "success": True, + "bypassed": False, + "data": { + "safe_text": sanitized_output, + "rephrase_needed": False, + "validator_results": [ + { + "name": "PIIRemover", + "type": "pii_remover", + "stage": "output", + "order": 1, + "outcome": "FAIL", + "error": "PII detected in the text.", + "input_text": raw_llm_output, + "output_text": sanitized_output, + } + ], + }, + } + mock_fetch_configs.return_value = ( + [], + [{"type": "pii_remover", "stage": "output"}], + ) + + request_data = { + "query": {"input": "hello"}, + "config": { + "blob": { + "completion": { + "provider": "openai-native", + "type": "text", + "params": {"model": "gpt-4o"}, + }, + "input_guardrails": [], + "output_guardrails": [ + {"validator_config_id": VALIDATOR_CONFIG_ID_2} + ], + } + }, + } + result = self._execute_job(job_for_execution, db, request_data) + + assert result["success"] + + output_guardrail_metadata = result["metadata"]["output_guardrail"] + assert output_guardrail_metadata["llm_output_pre_guardrail"] == raw_llm_output + assert output_guardrail_metadata["response_to_user"] == sanitized_output + + validators = output_guardrail_metadata["validators"] + assert len(validators) == 1 + assert validators[0]["name"] == "PIIRemover" + assert validators[0]["outcome"] == "FAIL" + assert validators[0]["input_text"] == raw_llm_output + assert validators[0]["output_text"] == sanitized_output + def test_guardrails_output_validation_sends_input_output_pair( self, db, job_env, job_for_execution ): From 6858906c6fd1a163c832a037ac943bd2db0f2517 Mon Sep 17 00:00:00 2001 From: Prajna1999 Date: Wed, 9 Sep 2026 11:53:45 +0530 Subject: [PATCH 2/3] fix: bugs --- backend/app/core/sentry_filters.py | 13 ++++++++++--- backend/app/crud/llm.py | 2 +- backend/app/tests/core/test_sentry_filters.py | 3 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/backend/app/core/sentry_filters.py b/backend/app/core/sentry_filters.py index 13dc0a393..6ac3720c7 100644 --- a/backend/app/core/sentry_filters.py +++ b/backend/app/core/sentry_filters.py @@ -13,7 +13,13 @@ "app.celery.tasks.job_execution.run_llm_chain_job", "app.celery.tasks.job_execution.run_response_job", } -_SENSITIVE_REQUEST_DATA_KEYS = ("query", "request_metadata", "response", "output") +_SENSITIVE_REQUEST_DATA_KEYS = ( + "query", + "request_metadata", + "response", + "output", + "callback_url", +) _SQL_OR_CONNECT = re.compile(r"^(select|insert|update|delete|connect)\b", re.IGNORECASE) @@ -102,8 +108,9 @@ def before_send_transaction_filter( def _redact_llm_job_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]: - """Replaces end-user text fields in an LLM job's request_data with a - placeholder, leaving non-text fields (config, ids, callback_url) intact. + """Replaces end-user text fields and identifying URLs in an LLM job's + request_data with a placeholder, leaving non-sensitive fields (config, ids) + intact. """ request_data = kwargs.get("request_data") if not isinstance(request_data, dict): diff --git a/backend/app/crud/llm.py b/backend/app/crud/llm.py index a0c6448e1..f96b598a1 100644 --- a/backend/app/crud/llm.py +++ b/backend/app/crud/llm.py @@ -224,7 +224,7 @@ def update_llm_call_response( if conversation_id is not None: db_llm_call.conversation_id = conversation_id if metadata is not None: - existing_metadata = db_llm_call.metadata_ or {} + existing_metadata = dict(db_llm_call.metadata_ or {}) existing_metadata.update(metadata) db_llm_call.metadata_ = existing_metadata diff --git a/backend/app/tests/core/test_sentry_filters.py b/backend/app/tests/core/test_sentry_filters.py index 8f750a9d7..1873b7368 100644 --- a/backend/app/tests/core/test_sentry_filters.py +++ b/backend/app/tests/core/test_sentry_filters.py @@ -38,8 +38,9 @@ def test_before_send_filter_redacts_query_for_llm_job(): request_data = result["extra"]["celery-job"]["kwargs"]["request_data"] assert request_data["query"] == "[REDACTED]" + # callback_url can carry identifying/credential data and is redacted too. + assert request_data["callback_url"] == "[REDACTED]" # Non-sensitive fields are left untouched so the trace stays useful. - assert request_data["callback_url"] == "https://webhooksite.net/some-id" assert request_data["config"] == {"blob": {"completion": {"provider": "openai"}}} From 506d8b6d01d1f2668a80ca3bc8a42051aa270ff8 Mon Sep 17 00:00:00 2001 From: Akhilesh Negi Date: Wed, 9 Sep 2026 15:29:57 +0530 Subject: [PATCH 3/3] fix(collections): Normalize file extension case (#1193) --- .../services/collections/providers/openai.py | 8 +++++++- .../providers/test_openai_provider.py | 20 ++++++++++++++----- docs/wiki/modules/knowledge-base.md | 1 + 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/backend/app/services/collections/providers/openai.py b/backend/app/services/collections/providers/openai.py index 770e0773f..84993268c 100644 --- a/backend/app/services/collections/providers/openai.py +++ b/backend/app/services/collections/providers/openai.py @@ -47,8 +47,14 @@ def upload_files( if doc.file_size_kb is None: doc.file_size_kb = round(tmp.tell() / 1024, 2) tmp.seek(0) + if "." in doc.fname: + filename, extension = doc.fname.rsplit(".", 1) + normalized_fname = f"{filename}.{extension.lower()}" + else: + normalized_fname = doc.fname uploaded = self.client.files.create( - file=(doc.fname, tmp), purpose="assistants" + file=(normalized_fname, tmp), + purpose="assistants", ) except Exception as err: logger.error( diff --git a/backend/app/tests/services/collections/providers/test_openai_provider.py b/backend/app/tests/services/collections/providers/test_openai_provider.py index d31fb0ae3..8d8cf42c0 100644 --- a/backend/app/tests/services/collections/providers/test_openai_provider.py +++ b/backend/app/tests/services/collections/providers/test_openai_provider.py @@ -296,15 +296,25 @@ def test_upload_files_empty_docs_is_noop() -> None: client.files.create.assert_not_called() -def test_upload_files_file_name_matches_doc_fname() -> None: - """The file tuple passed to OpenAI must carry the original filename.""" +@pytest.mark.parametrize( + ("fname", "expected"), + [ + ("report.pdf", "report.pdf"), + ("Report.PDF", "Report.pdf"), + ("README", "README"), + ], +) +def test_upload_files_file_name_lowercases_only_extension( + fname: str, expected: str +) -> None: + """The file tuple passed to OpenAI keeps the stem's case, lowercases the extension.""" client = MagicMock() client.files.create.return_value = MagicMock(id="file-abc") provider = OpenAIProvider(client=client) storage = _make_storage(b"data") doc = _make_doc(file_size_kb=1.0) - doc.fname = "report.pdf" + doc.fname = fname session_p, crud_p = _patch_session_and_crud() with session_p as MockSession, crud_p: @@ -313,8 +323,8 @@ def test_upload_files_file_name_matches_doc_fname() -> None: provider.upload_files(storage, [doc], project_id=1) _, kwargs = client.files.create.call_args - fname, _ = kwargs["file"] - assert fname == "report.pdf" + uploaded_fname, _ = kwargs["file"] + assert uploaded_fname == expected def test_upload_files_raises_on_db_update_failure() -> None: diff --git a/docs/wiki/modules/knowledge-base.md b/docs/wiki/modules/knowledge-base.md index 4cefd5719..179b0c69d 100644 --- a/docs/wiki/modules/knowledge-base.md +++ b/docs/wiki/modules/knowledge-base.md @@ -35,6 +35,7 @@ All paths relative to `backend/app/`. ## Gotchas - `signed_url` behaviour is chosen per request by the `download` query param (default false = inline, as before). `put()` stores the upload's `Content-Type`, so a bare presigned URL for a PDF opens in a tab (CSV/XLSX datasets happen to download, which is why only KB docs looked broken). `get_signed_url(..., filename=...)` adds `ResponseContentDisposition: attachment`; `_signed_url()` in `services/documents/helpers.py` passes `fname` only when `download` is set, and the flag threads through `build_document_schema(s)` / `build_job_schema(s)` from `api/routes/documents.py`, `doc_transformation_job.py` and `collections.py`. An attachment URL will not render in an `