-
Notifications
You must be signed in to change notification settings - Fork 10
feat(guardrails): Add intermediate information fields to guardrails execution in the callback payload #1195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ffed94c
707cfa5
27c9216
a332c24
319c0ea
0cc41d8
9dcea43
ecc5396
4554c12
0a110bb
6858906
4712e63
4eb178e
834d8a3
1fa2756
c6fe07d
418d458
c2916b7
fce8e4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,7 +106,7 @@ def _upstream_response(status_code: int, payload: Any) -> Response: | |
| return JSONResponse(status_code=status_code, content=payload, headers=headers) | ||
|
|
||
|
|
||
| # ROUTE ORDERING: these fixed paths must stay above GET /guardrails/{job_id} — FastAPI matches in declaration order. | ||
| # ROUTE ORDERING: these fixed paths must stay above GET /guardrails/{job_id}. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needed?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed after resolving merge conflicts |
||
|
|
||
|
|
||
| @router.get( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -589,6 +589,10 @@ class LLMCallRequest(SQLModel): | |
| default=False, | ||
| description="Whether to include the raw LLM provider response in the output", | ||
| ) | ||
| include_guardrail_metadata: bool = Field( | ||
| default=False, | ||
| description="Include per-validator guardrail metadata (input/output text, pass/fail) in the response", | ||
| ) | ||
| request_metadata: dict[str, Any] | None = Field( | ||
| default=None, | ||
| description=( | ||
|
|
@@ -764,6 +768,16 @@ class LlmCall(SQLModel, table=True): | |
| ), | ||
| ) | ||
|
|
||
| metadata_: dict[str, Any] | None = Field( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any type?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's an extensibility catch-all per the field comment, shape isn't fixed by design. |
||
| 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,11 +56,6 @@ def proxy_guardrails_request( | |
| # Unset query params must be omitted, not sent as empty values. | ||
| query = {k: v for k, v in (params or {}).items() if v is not None} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we have better variable names here |
||
|
|
||
| logger.info( | ||
| f"[proxy_guardrails_request] Forwarding to guardrails | method: {method}, " | ||
| f"url: {url}, organization_id: {organization_id}, project_id: {project_id}" | ||
| ) | ||
|
|
||
| try: | ||
| with ( | ||
| tracer.start_as_current_span( | ||
|
|
@@ -111,13 +106,15 @@ def proxy_guardrails_request( | |
| ) from None | ||
|
|
||
|
|
||
| # 422 included: a missing/invalid tenant header is a backend bug, not a | ||
| # transient outage, so it must not fall open like one. | ||
| _AUTH_ERROR_STATUS_CODES = (401, 403, 422) | ||
|
|
||
|
|
||
| def _is_auth_error(e: Exception) -> TypeGuard[httpx.HTTPStatusError]: | ||
| # 422 included: a missing/invalid tenant header is a backend bug, not a | ||
| # transient outage, so it must not fall open like one. | ||
| return isinstance(e, httpx.HTTPStatusError) and e.response.status_code in ( | ||
| 401, | ||
| 403, | ||
| 422, | ||
| return ( | ||
| isinstance(e, httpx.HTTPStatusError) | ||
| and e.response.status_code in _AUTH_ERROR_STATUS_CODES | ||
| ) | ||
|
Comment on lines
+114
to
+118
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think instead of making this as function, create the one array constant and then directly check from that constant variable.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pulled the codes into |
||
|
|
||
|
|
||
|
|
@@ -240,6 +237,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]], | ||
|
|
@@ -324,10 +341,6 @@ def run_guardrails_validation( | |
| if _is_auth_error(e): | ||
| # Auth failure means a broken deploy (token/IP mismatch), not a | ||
| # transient outage — fail the job instead of silently bypassing. | ||
| logger.error( | ||
| f"[run_guardrails_validation] Guardrails auth failed. " | ||
| f"job_id={job_id}, elapsed_ms={elapsed_ms}, error={e}" | ||
| ) | ||
| status_code = e.response.status_code | ||
| return { | ||
| "success": False, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need history as well of this that it was dropped and now readded again