Skip to content

feat(traceloop-sdk): Add guardrails - #3649

Merged
nina-kollman merged 92 commits into
mainfrom
nk/guardrail_v2
Apr 12, 2026
Merged

nina-kollman merged 92 commits into
mainfrom
nk/guardrail_v2

Conversation

@nina-kollman

@nina-kollman nina-kollman commented Feb 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixes: TLP-1692
Fixes: TLP-1691
Fixes: TLP-1438

  • I have added tests that cover my changes.
  • If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
  • PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
  • (If applicable) I have updated the documentation accordingly.

Summary by CodeRabbit

  • New Features

    • Revamped Guardrails system: built-in guards, conditions, on-failure handlers, default input mapping, and a guardrail decorator for sync/async functions.
    • New client API to create guardrails and a standalone evaluator "run" interface.
    • New interactive/sample guardrail examples demonstrating evaluators, custom functions, multi-guard flows, and validation.
  • Refactor / Deprecation

    • Legacy guardrails module removed and marked deprecated; older demo scripts deleted.
  • Tests

    • Extensive unit and integration tests with recorded API interactions.
  • Chores

    • Pytest asyncio mode configured.

@galkleinman

Copy link
Copy Markdown
Contributor

Code review

Found 2 issues:

  1. Condition.is_true() uses identity check (value is True) instead of truthiness. Python developers will expect is_true() to mean "truthy" (bool(value)), but it rejects 1, "yes", non-empty lists, and other truthy values. The docstring warns about this, but the fact that extensive documentation is needed suggests the naming is misleading. is_exact_true() or similar would be clearer, or is_true() should check truthiness and is_exactly_true() should check identity.

conditions, so conditions receive the field value directly.
"""
from typing import Any, Callable
class Condition:
"""Built-in conditions for common evaluator result patterns."""
@staticmethod
def is_true() -> Callable[[Any], bool]:
"""
Pass if value is exactly True (must be a bool).

  1. guardrail is not exported in __all__ in decorators/__init__.py. The list still only contains the original 9 decorators, so from traceloop.sdk.decorators import * will silently omit guardrail.

from traceloop.sdk.guardrail.on_failure import OnFailure
F = TypeVar("F", bound=Callable[..., Any])
_P = ParamSpec("_P")
_R = TypeVar("_R")
__all__ = [
"conversation",
"task",
"workflow",
"agent",

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@galkleinman

Copy link
Copy Markdown
Contributor

DX feedback: API patterns are not Pythonic

The current guardrails API leans heavily on Java/C#-style static factory classes (OnFailure.return_value(...), Condition.is_true()) and imperative factory methods (client.create_guardrail(...)). These work, but they fight how Python developers naturally express things. Below is a pattern-by-pattern breakdown with suggested alternatives.


1. OnFailure.* — static namespace class should be plain strings / callables / chaining

Current (appears ~40+ times across production, examples, and tests):

on_failure=OnFailure.raise_exception("PII detected in response")
on_failure=OnFailure.return_value(value="Sorry, I can't help you with that.")
on_failure=OnFailure.log(level=logging.WARNING, message="Guard failed")
on_failure=OnFailure.noop()

Suggested — strings for common cases, callables for custom:

# Common cases as strings (like pytest markers, SQLAlchemy cascade, etc.)
on_failure="raise"
on_failure="log"
on_failure="ignore"  # noop

# With parameters — just pass the value directly
on_failure="Sorry, I can't help you with that."  # string = return this value

# Custom behavior — plain callable
on_failure=lambda result: log_and_alert(result)

Or — fluent chaining on the result:

result = await guardrail.run(my_func).on_failure("raise")
result = await guardrail.run(my_func).on_failure(return_value="fallback")

The OnFailure class is just 4 @staticmethod methods returning closures. Module-level functions or string literals achieve the same with less ceremony.


2. Condition.* — static namespace class should be plain Python expressions

Current (appears ~60+ times across condition.py, guards.py, test_conditions.py):

condition=Condition.is_true()
condition=Condition.is_false()
condition=Condition.greater_than(0.8)
condition=Condition.less_than(1000)
condition=Condition.between(0.3, 0.7)
condition=Condition.equals("approved")
condition=Condition.greater_than_or_equal(0.8)

Suggested — plain Python, since these are just predicates:

# Python already has a vocabulary for this
condition=bool                          # instead of Condition.is_truthy()
condition=lambda v: not v               # instead of Condition.is_falsy()
condition=lambda v: v > 0.8             # instead of Condition.greater_than(0.8)
condition=lambda v: 0.3 <= v <= 0.7     # instead of Condition.between(0.3, 0.7)
condition=lambda v: v == "approved"     # instead of Condition.equals("approved")

Or if you want named helpers for discoverability, use module-level functions with operator:

from traceloop.sdk.guardrail.conditions import gt, lt, between, eq

condition=gt(0.8)       # reads naturally, no class namespace
condition=between(0.3, 0.7)

Every Python developer already knows >, <, ==, lambda. Wrapping these in Condition.greater_than_or_equal() adds 30 characters for zero expressiveness.


3. client.create_guardrail(...) — factory method should be direct construction or fluent

Current (appears in every example and all tests):

client = Traceloop.init(app_name="my-app", ...)
guardrail = client.create_guardrail(
    guards=[pii_guard(), toxicity_guard()],
    on_failure=OnFailure.raise_exception("Content policy violation"),
    name="safety-check",
)
result = await guardrail.run(my_func)

Suggested — direct construction, Python-style:

guardrail = Guardrail(
    pii_guard(),
    toxicity_guard(),
    on_failure="raise",
    name="safety-check",
)
result = await guardrail.run(my_func)

Or with chaining:

result = await Guardrail(pii_guard(), toxicity_guard()).run(my_func).on_failure("raise")

In Python, if a class can be instantiated directly, it should be. The client.create_guardrail() indirection forces users to manage a client object just to create a guardrail. The HTTP client dependency can be resolved internally (from the initialized Traceloop singleton, which the guards already do).


4. guardrail.run(lambda: func(...)) — lambda wrapping should be transparent

Current (in decorator internals and many examples):

# Decorator wraps in lambda internally:
return await g.run(
    lambda: func(*args, **kwargs),
    input_mapper=input_mapper,
)

# Examples force users to define zero-arg wrappers:
async def generate_summary() -> str:
    return await openai.chat(...)

result = await guardrail.run(generate_summary)

Suggested — accept func + args directly:

result = await guardrail.run(generate_summary, prompt, model="gpt-4")

# or as a decorator that's transparent:
@guardrail(pii_guard(), on_failure="raise")
async def generate_summary(prompt: str) -> str:
    return await openai.chat(prompt)

# just call it normally:
result = await generate_summary("tell me about X")

This is how asyncio.create_task(coro), functools.partial(fn, *args), and concurrent.futures.submit(fn, *args) work in the stdlib. Python developers expect run(fn, *args, **kwargs), not run(lambda: fn(*args, **kwargs)).


5. input_mapper=lambda text: [PIIDetectorInput(text=text)] — positional list protocol is fragile

Current (appears in all examples using Traceloop evaluator guards):

input_mapper=lambda response: [
    AnswerRelevancyInput(answer=response, question=prompt),
    SexismDetectorInput(text=response),
    ToxicityDetectorInput(text=response),
]

The user must return a list whose elements are position-matched to the guards list. If guards are reordered, the mapper silently sends wrong inputs to wrong guards.

Suggested — dict keyed by guard name or guard reference:

input_mapper=lambda response: {
    "answer_relevancy": AnswerRelevancyInput(answer=response, question=prompt),
    "sexism": SexismDetectorInput(text=response),
    "toxicity": ToxicityDetectorInput(text=response),
}

Or let each guard declare its own input mapping:

pii_guard(input=lambda r: PIIDetectorInput(text=r))
toxicity_guard(input=lambda r: ToxicityDetectorInput(text=r))

This is self-documenting and order-independent.


6. Guard factory functions should compose more naturally

Current:

guardrail = client.create_guardrail(
    guards=[
        toxicity_guard(condition=Condition.is_true()),
        pii_guard(condition=Condition.is_false()),
        custom_evaluator_guard(
            evaluator_slug="medicaladvice",
            condition=Condition.greater_than_or_equal(0.8),
        ),
    ],
    on_failure=OnFailure.raise_exception("Content failed safety checks"),
)

Suggested — more composable, reads like English:

guardrail = Guardrail(
    toxicity_guard(),           # sensible defaults, no Condition needed
    no_pii(),                   # name implies the condition
    custom_evaluator("medicaladvice", threshold=0.8),
    on_failure="raise",
)

The Condition parameter is needed because the guard names don't encode their polarity (pii_guard + Condition.is_false() means "pass when no PII"). Renaming to no_pii() or pii_free() eliminates the need for an explicit condition in the common case.


TL;DR

The current API requires users to learn OnFailure, Condition, create_guardrail, input_mapper list protocol, and lambda-wrapping patterns. A Pythonic API would use strings, plain callables, direct construction, and *args/**kwargs — things every Python developer already knows. The goal should be that a developer can read a guardrail definition and understand it without reading any docs.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@galkleinman galkleinman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general it looks good, i think in terms of the DX, we can do better imo and introduce a DSL which is more functional. Maybe it is worth that we'll merge, start experimenting with it and than improve if we'll find it necessary.

The only thing which is a must change here is those hardcoded string which is used to decide the action to be taken upon failure and so on (like the "raise") - it is really a bad practice.


g = Guardrails(
custom_evaluator_guard(evaluator_slug="diagnosis-blocker", condition_field="pass"),
on_failure="raise",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't be something strong/structured than hardcoded string?

Comment thread packages/sample-app/sample_app/guardrails/multiple_guards_example.py Outdated
Comment thread packages/sample-app/sample_app/guardrails/custom_evaluator_guard.py Fixed
@nina-kollman
nina-kollman merged commit c4602f6 into main Apr 12, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants