Custom flag for validator log table

[rkritika1508]

Summary

Target issue is #28.
Explain the motivation for making this change. What existing problem does the pull request solve?
We don't want to populate the validator_log each time a validator has parsed a message. There will be a flag include_all_validator_logs which if disabled will populate only failed validator logs. Otherwise, we will populate all logs to the validator_log table.

Checklist

Before submitting a pull request, please ensure that you mark these task.

• Ran fastapi run --reload app/main.py or docker compose up in the repository root and test.
• If you've fixed a bug or added code that is tested and has test cases.

Notes

Please add here if any other information is required for the reviewer.

Summary by CodeRabbit

• New Features

  • Configurable validator logging: optionally include all validation results (including successful checks) in persisted logs.

• Bug Fixes / Tests

  • Adjusted test expectations for guardrail responses (minor punctuation) and updated test API path references to match the new endpoint layout.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an include_all_validator_logs: bool = False flag to the guardrails validation flow so caller can opt to persist all validator logs (including passing validations); threads the flag through run_guardrails, _validate_with_guard, and add_validator_logs, and updates tests and a test constant path.

Changes

Cohort / File(s)	Summary
Guardrails Logging Control backend/app/api/routes/guardrails.py	Added include_all_validator_logs: bool = False parameter to run_guardrails, _validate_with_guard, and add_validator_logs. Imported PassResult and implemented filtering so passing validator results are omitted from persisted per-iteration logs unless the flag is true. Propagated flag through finalization/log persistence.
Tests — Integration backend/app/tests/test_guardrails_api_integration.py	Updated expected assertion strings to include a trailing period after the redacted slur in two test cases.
Tests — Constants backend/app/tests/utils/constants.py	Changed VALIDATE_API_PATH from "/api/v1/guardrails/validate/" to "/api/v1/guardrails/" (removed validate segment).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• Updated endpoint and added "rephrase" on_fail action #20 — Modifies the same guardrails route handlers and helper functions (run_guardrails, _validate_with_guard, add_validator_logs).
• Added validator logs and organized code #18 — Extends validator-logging flow and conditional filtering for validator results in the same module.

Suggested reviewers

• nishika26

Poem

🐰 A little flag that hops and stays,
Keeps every pass or hides the praise.
Logs grow tidy, paths grow short,
Tests aligned for the new report.
Hooray — the rabbit stamps its paws! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Custom flag for validator log table' is related to the main change—adding an include_all_validator_logs flag to control validator log persistence—but it's somewhat vague and doesn't clearly convey the primary purpose or functionality of the flag.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/validator-log-flag

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

backend/app/api/routes/guardrails.py (3)

24-28: Consider moving include_all_validator_logs into the request body (GuardrailRequest).

Since this is a POST endpoint, include_all_validator_logs as a bare function parameter will be interpreted by FastAPI as a query parameter, mixing concerns (body for validation payload, query string for logging control). If consumers always send JSON, embedding this flag in GuardrailRequest (or a wrapper model) keeps the API surface consistent and avoids query-string leakage of operational flags into URLs/logs.

If a query parameter is intentional (e.g., to keep the validation payload model clean), adding Query(default=False) explicitly would make the intent clearer:

Explicit Query annotation

+from fastapi import APIRouter, Query
 ...
 async def run_guardrails(
     payload: GuardrailRequest,
     session: SessionDep,
     _: AuthDep,
-    include_all_validator_logs: bool = False,
+    include_all_validator_logs: bool = Query(default=False, description="When true, persist all validator logs including passes"),
 ):

---

163-196: Function signature is growing — consider grouping parameters.

add_validator_logs now takes four positional arguments. This is still manageable, but as a minor readability improvement you could use keyword-only arguments after the first positional param to prevent accidental mis-ordering at call sites:

Suggested signature

-def add_validator_logs(guard: Guard, request_log_id: UUID, validator_log_crud: ValidatorLogCrud, include_all_validator_logs: bool = False):
+def add_validator_logs(
+    guard: Guard,
+    request_log_id: UUID,
+    validator_log_crud: ValidatorLogCrud,
+    *,
+    include_all_validator_logs: bool = False,
+):

This also requires updating the call site on line 118 to pass the flag as a keyword argument:

add_validator_logs(guard, request_log_id, validator_log_crud, include_all_validator_logs=include_all_validator_logs)

---

1-2: Nit: uuid is imported twice — once as a type (UUID from uuid) and once as the module.

Both are used (UUID for type hints, uuid.uuid4() on line 87), so this is functionally fine. A minor consistency improvement would be to use uuid.UUID everywhere or import uuid4 directly:

Option: import uuid4 directly

-from uuid import UUID
-import uuid
+from uuid import UUID, uuid4

Then replace uuid.uuid4() on line 87 with uuid4().

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

backend/app/tests/test_guardrails_api_integration.py (1)

1-196: 🛠️ Refactor suggestion | 🟠 Major

No tests for the new include_all_validator_logs feature.

The main feature of this PR — the include_all_validator_logs flag — has no test coverage. Consider adding at least:

1. A test with include_all_validator_logs=True that verifies all validator logs (including passes) are persisted.
2. A test with the default (False) that verifies only failure logs are persisted.

This would confirm the filtering logic works end-to-end.

🤖 Fix all issues with AI agents

In `@backend/app/api/routes/guardrails.py`:
- Around line 180-181: Replace the explicit equality comparison to False in the
guardrails check with a boolean negation: in the if statement that currently
reads "if include_all_validator_logs == False and isinstance(result,
PassResult):", use "if not include_all_validator_logs and isinstance(result,
PassResult):" so the code uses idiomatic Python and avoids issues when
include_all_validator_logs is a truthy/falsy non-bool; update the condition
where include_all_validator_logs and PassResult are checked.

🧹 Nitpick comments (2)

backend/app/api/routes/guardrails.py (2)

24-28: include_all_validator_logs is a query parameter on a POST endpoint — consider moving it into the request body.

Since payload is a Pydantic model parsed from the JSON body, FastAPI will treat include_all_validator_logs as a query parameter (e.g., ?include_all_validator_logs=true). Mixing body and query params on a POST can be surprising to API consumers. Consider adding this field to GuardrailRequest instead for a cleaner contract.

---

163-163: Long signature — consider grouping parameters or wrapping across lines for readability.

This is a minor readability nit. The function signature is quite long on a single line.

Suggested formatting

-def add_validator_logs(guard: Guard, request_log_id: UUID, validator_log_crud: ValidatorLogCrud, include_all_validator_logs: bool = False):
+def add_validator_logs(
+    guard: Guard,
+    request_log_id: UUID,
+    validator_log_crud: ValidatorLogCrud,
+    include_all_validator_logs: bool = False,
+):