Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds tenant-scoped Topic Relevance: DB migration and model, schemas, CRUD, API routes and router registration, LLM-based TopicRelevance validator and validator configs, runtime resolution in run_guardrails, prompt templates, docs, tests, and minor X-API-KEY header wording updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route as "API Route"
participant Guardrails as "Guardrails Executor"
participant TR_CRUD as "TopicRelevance CRUD"
participant DB as "Database"
participant LLM as "LLM Service"
Client->>API_Route: POST /guardrails/run (payload with topic_relevance_config_id)
API_Route->>Guardrails: run_guardrails(payload, session, auth)
Guardrails->>TR_CRUD: get(id, organization_id, project_id)
TR_CRUD->>DB: SELECT topic_relevance WHERE id & tenant scope
DB-->>TR_CRUD: TopicRelevance record
TR_CRUD-->>Guardrails: configuration + prompt_schema_version
Guardrails->>LLM: validate(input, metric_prompt built from configuration)
LLM-->>Guardrails: scope_score / PassResult or FailResult
Guardrails-->>API_Route: aggregated validation result (with scope_score metadata)
API_Route-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
dennyabrain
reviewed
Mar 5, 2026
backend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.md
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
backend/app/api/routes/topic_relevance_configs.py (1)
21-28: Optional REST polish: return201 Createdfor successful POST.Line 21 can explicitly set
status_code=201to better reflect resource creation semantics.Small route metadata tweak
`@router.post`( "/", + status_code=201, description=load_description( "topic_relevance_configs/create_topic_relevance_config.md" ), response_model=APIResponse[TopicRelevanceResponse], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/topic_relevance_configs.py` around lines 21 - 28, The POST route decorator for create_topic_relevance_config should explicitly signal resource creation by adding status_code=201 to the `@router.post`(...) declaration; update the `@router.post`(...) for the create_topic_relevance_config function to include status_code=201 while keeping the existing description and response_model parameters.backend/app/tests/test_topic_relevance_configs_api.py (1)
63-76: Assert tenant-scoped CRUD arguments in the create test.Line 69 currently validates only the returned payload. Add a strict
crud.create.assert_called_once_with(...)assertion so org/project scoping regressions are caught early.Suggested test tightening
result = create_topic_relevance_config( payload=create_payload, session=mock_session, auth=auth_context, ) + crud.create.assert_called_once_with( + mock_session, + create_payload, + TOPIC_RELEVANCE_TEST_ORGANIZATION_ID, + TOPIC_RELEVANCE_TEST_PROJECT_ID, + ) assert result.data == sample_topic_relevance🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_topic_relevance_configs_api.py` around lines 63 - 76, The test currently only asserts the returned payload; add a strict call assertion to ensure tenant/org/project scoping is passed to the CRUD layer by adding crud.create.assert_called_once_with(...) after the call to create_topic_relevance_config in test_create_calls_crud — pass the same payload and session and the auth/tenant info (e.g. payload=create_payload, session=mock_session, auth=auth_context; if topic_relevance_crud.create expects explicit tenant/org/project ids, assert those are passed from auth_context like org_id/auth_context.org_id or project_id/auth_context.project_id).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/alembic/versions/006_added_topic_relevance_config.py`:
- Around line 38-43: The unique constraint uq_topic_relevance_name_org_project
currently enforces uniqueness on ("name", "organization_id", "project_id") but
must also include "configuration" and "prompt_version" to enforce
configuration-level deduplication as documented; update the UniqueConstraint
declaration in the migration 006_added_topic_relevance_config.py (the
sa.UniqueConstraint call that defines
name="uq_topic_relevance_name_org_project") to include "configuration" and
"prompt_version" in the column list so rows with identical
configuration/prompt_version are rejected.
In `@backend/app/api/API_USAGE.md`:
- Line 186: Replace all occurrences of the field name scope_definitions with
configuration across the docs that reference topic_relevance and related
validator configs (e.g., the API usage examples for topic_relevance, the runtime
validators README, and the run_guardrails docs); update the sentence that
currently reads "you can pass either inline scope_definitions or a
topic_relevance_config_id" to mention inline configuration or
topic_relevance_config_id and ensure any JSON/YAML examples, headings, and
references use configuration consistently.
In `@backend/app/api/routes/guardrails.py`:
- Around line 204-217: In _resolve_topic_relevance_scope, avoid calling
topic_relevance_crud.get for inline configs by first checking the
TopicRelevanceSafetyValidatorConfig instance's topic_relevance_config_id and
skipping DB resolution when it's None/empty; i.e., inside the loop over
payload.validators, after confirming isinstance(...,
TopicRelevanceSafetyValidatorConfig), if not
validator.topic_relevance_config_id: continue (or leave existing
validator.configuration/prompt_version as-is), otherwise call
topic_relevance_crud.get and assign validator.configuration and
validator.prompt_version from the returned config.
In
`@backend/app/core/validators/config/topic_relevance_safety_validator_config.py`:
- Around line 15-21: The build() method may pass an empty dict into
TopicRelevance causing a delayed constructor error; add an explicit precondition
in build() that checks self.configuration (and/or
self.configuration.get("topics") if relevant) is present and non-empty and raise
a clear ValueError (e.g., "TopicRelevance configuration is required") before
constructing TopicRelevance; reference the build() method and TopicRelevance
instantiation to locate where to add the guard and keep the existing use of
prompt_version, llm_callable and on_fail unchanged.
In `@backend/app/crud/topic_relevance.py`:
- Around line 30-39: The current except IntegrityError block in the
session.commit() handling maps every IntegrityError to a "duplicate
configuration" HTTPException; change it to inspect the IntegrityError (e.g., the
caught exception variable from the commit try/except, using e.orig,
e.orig.pgcode or the DB-driver error text) and only raise the 400 "duplicate
configuration" when the error is a unique/duplicate-constraint violation; for
other integrity errors (nullability, check constraints, bad data) either
re-raise or convert to a more specific HTTPException that surfaces the
underlying error message; apply the same change to the other commit block (the
second try/except around session.commit() in this file) so only true
unique-constraint errors get the duplicate message and other IntegrityErrors are
propagated or reported with their real details.
- Around line 65-75: The query lacks a deterministic order before applying
offset/limit, causing unstable pagination; update the query built from
select(TopicRelevance) to include a stable order_by (e.g., query =
query.order_by(TopicRelevance.id)) before the offset/limit calls so
session.exec(query).all() returns consistently ordered results; use
TopicRelevance.id (or TopicRelevance.created_at if you prefer chronological
ordering) as the stable sort key in the query construction.
In `@backend/app/models/config/topic_relevance.py`:
- Around line 67-80: The timestamp Field comments in topic_relevance.py are
stale references to “ban list entry”; update the sa_column_kwargs "comment"
values for created_at and updated_at to refer to a "topic relevance entry"
instead—i.e., change created_at's comment to "Timestamp when the topic relevance
entry was created" and updated_at's comment to "Timestamp when the topic
relevance entry was last updated" (the updated_at block already includes
onupdate: now).
In `@backend/app/schemas/topic_relevance.py`:
- Around line 20-24: The TopicRelevanceBase Pydantic schema currently allows
description to be None but the DB model
backend/app/models/config/topic_relevance.py treats description as non-nullable;
update TopicRelevanceBase so description is required and validated (e.g., change
description: Optional[str] to a non-optional constrained str with a sensible max
length) to match the DB contract and avoid insert failures; adjust any
callers/tests that relied on a missing description accordingly.
- Around line 23-36: The create/update models currently allow non-positive
prompt_version and let update bypass the same constraints as TopicRelevanceBase;
add the same input guards to enforce prompt_version > 0 and validate
configuration on updates: add a constrained type or a Pydantic validator for
prompt_version (e.g., conint(gt=0) or `@validator`) and reuse the same
configuration validation logic from TopicRelevanceBase in TopicRelevanceUpdate,
so TopicRelevanceCreate (which inherits TopicRelevanceBase) and
TopicRelevanceUpdate both reject invalid versions/configs and preserve
consistent constraints.
---
Nitpick comments:
In `@backend/app/api/routes/topic_relevance_configs.py`:
- Around line 21-28: The POST route decorator for create_topic_relevance_config
should explicitly signal resource creation by adding status_code=201 to the
`@router.post`(...) declaration; update the `@router.post`(...) for the
create_topic_relevance_config function to include status_code=201 while keeping
the existing description and response_model parameters.
In `@backend/app/tests/test_topic_relevance_configs_api.py`:
- Around line 63-76: The test currently only asserts the returned payload; add a
strict call assertion to ensure tenant/org/project scoping is passed to the CRUD
layer by adding crud.create.assert_called_once_with(...) after the call to
create_topic_relevance_config in test_create_calls_crud — pass the same payload
and session and the auth/tenant info (e.g. payload=create_payload,
session=mock_session, auth=auth_context; if topic_relevance_crud.create expects
explicit tenant/org/project ids, assert those are passed from auth_context like
org_id/auth_context.org_id or project_id/auth_context.project_id).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65aafd52-9176-42e2-a56c-81225ae5796f
📒 Files selected for processing (25)
backend/README.mdbackend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/API_USAGE.mdbackend/app/api/docs/guardrails/run_guardrails.mdbackend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.mdbackend/app/api/docs/topic_relevance_configs/delete_topic_relevance_config.mdbackend/app/api/docs/topic_relevance_configs/get_topic_relevance_config.mdbackend/app/api/docs/topic_relevance_configs/list_topic_relevance_configs.mdbackend/app/api/docs/topic_relevance_configs/update_topic_relevance_config.mdbackend/app/api/main.pybackend/app/api/routes/guardrails.pybackend/app/api/routes/topic_relevance_configs.pybackend/app/core/enum.pybackend/app/core/validators/README.mdbackend/app/core/validators/config/llm_critic_safety_validator_config.pybackend/app/core/validators/config/topic_relevance_safety_validator_config.pybackend/app/core/validators/prompts/topic_relevance/v1.mdbackend/app/core/validators/topic_relevance.pybackend/app/core/validators/validators.jsonbackend/app/crud/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/guardrail_config.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api.pybackend/app/tests/test_topic_relevance_configs_api_integration.py
backend/app/core/validators/config/topic_relevance_safety_validator_config.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
backend/app/core/validators/config/topic_relevance_safety_validator_config.py (1)
17-23:⚠️ Potential issue | 🟠 MajorFail fast before constructing
TopicRelevance.At Line 19, falling back to
" "guarantees a downstreamValueErrorand obscures the real cause (missing unresolved configuration). Validate explicitly and passself.configurationdirectly.Suggested guard in `build()`
def build(self): + if not self.configuration or not self.configuration.strip(): + raise ValueError( + "Topic relevance configuration is missing. Resolve " + "`topic_relevance_config_id` or provide `configuration` before build()." + ) return TopicRelevance( - topic_config=self.configuration or " ", + topic_config=self.configuration, prompt_version=self.prompt_version or 1, llm_callable=self.llm_callable, on_fail=self.resolve_on_fail(), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/config/topic_relevance_safety_validator_config.py` around lines 17 - 23, The build() method currently falls back to a blank string which hides missing configuration; change build() to validate that self.configuration is present (e.g., not None and not empty) and raise a clear ValueError if it is missing, then construct TopicRelevance with topic_config=self.configuration (no " " fallback), preserving prompt_version (self.prompt_version or 1), llm_callable (self.llm_callable) and on_fail (self.resolve_on_fail()) so the downstream error is explicit and easier to debug.backend/app/schemas/topic_relevance.py (2)
39-44:⚠️ Potential issue | 🟠 MajorUpdate schema bypasses create-time field constraints.
At Line 40-Line 43, update currently permits unconstrained
nameand unconstrainedprompt_versionvalues. This can persist invalid data shapes that create rejects.Suggested update-schema parity
class TopicRelevanceUpdate(BaseModel): - name: Optional[str] = None - description: Optional[str] = None - prompt_version: Optional[int] = None + name: Optional[TopicsName] = None + description: Optional[TopicDescription] = None + prompt_version: Optional[int] = Field(default=None, ge=1) configuration: Optional[TopicConfiguration] = None is_active: Optional[bool] = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/schemas/topic_relevance.py` around lines 39 - 44, The update schema TopicRelevanceUpdate currently allows unconstrained name and prompt_version which can let invalid values be persisted; modify TopicRelevanceUpdate so its name and prompt_version use the same constrained types/validators as the create-time schema (e.g., reuse the same Pydantic field types or validators used in TopicRelevanceCreate or the existing constrained types for name and prompt_version) to enforce identical constraints on update operations (apply the same length/regex/None-allowed logic and integer bounds as the create schema).
28-33:⚠️ Potential issue | 🟠 Major
descriptionschema is looser than DB contract.At Line 30,
descriptionis optional, but the persistence model stores it as non-null. This allows create payloads that fail at commit time.Suggested contract alignment
-from pydantic import BaseModel, StringConstraints +from pydantic import BaseModel, Field, StringConstraints @@ +TopicDescription = Annotated[ + str, + StringConstraints( + strip_whitespace=True, + min_length=1, + max_length=MAX_TOPIC_RELEVANCE_DESCRIPTION_LENGTH, + ), +] + class TopicRelevanceBase(BaseModel): name: TopicsName - description: Optional[str] = None - prompt_version: int + description: TopicDescription + prompt_version: int = Field(ge=1) configuration: TopicConfiguration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/schemas/topic_relevance.py` around lines 28 - 33, The TopicRelevanceBase Pydantic model allows description to be Optional which diverges from the persistence model that requires a non-null description; update the TopicRelevanceBase definition (the description field) to be a non-optional str (e.g., description: str) or provide a non-null default value to match the DB contract, and ensure any create/update code that constructs TopicRelevanceBase (or uses its validation) conforms to this non-null requirement.backend/app/alembic/versions/006_added_topic_relevance_config.py (1)
35-40:⚠️ Potential issue | 🟠 MajorUnique key does not enforce documented duplicate-configuration policy.
At Line 35-Line 40, uniqueness is currently
name + organization_id + project_id, so the sameconfiguration/prompt_versioncan still be inserted under a different name in the same tenant scope.Suggested migration fix
- sa.UniqueConstraint( - "name", - "organization_id", - "project_id", - name="uq_topic_relevance_name_org_project", - ), + sa.UniqueConstraint( + "organization_id", + "project_id", + "prompt_version", + "configuration", + name="uq_topic_relevance_config_org_project_prompt", + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/alembic/versions/006_added_topic_relevance_config.py` around lines 35 - 40, The UniqueConstraint currently named "uq_topic_relevance_name_org_project" only enforces uniqueness on ("name","organization_id","project_id"), which allows duplicate configurations with different names; update the migration in 006_added_topic_relevance_config.py to enforce uniqueness on the configuration identity instead by including the configuration identifiers (e.g., "configuration" and/or "prompt_version") together with "organization_id" and "project_id" in the UniqueConstraint (or replace the existing constraint), and rename the constraint to reflect its purpose (e.g., "uq_topic_relevance_config_org_project") so the same configuration/prompt_version cannot be inserted under a different name within the same tenant scope.
🧹 Nitpick comments (1)
backend/app/tests/test_topic_relevance_configs_api.py (1)
61-74: Strengthen assertions on CRUD call contracts in create/get tests.
test_create_calls_crudandtest_get_successcurrently validate only returned data. Addassert_called_once_with(...)checks to lock tenant-scoped argument propagation (organization_id,project_id) and reduce regression risk.Suggested test hardening
def test_create_calls_crud( mock_session, create_payload, sample_topic_relevance, auth_context ): with patch("app.api.routes.topic_relevance_configs.topic_relevance_crud") as crud: crud.create.return_value = sample_topic_relevance result = create_topic_relevance_config( payload=create_payload, session=mock_session, auth=auth_context, ) + crud.create.assert_called_once_with( + mock_session, + create_payload, + TOPIC_RELEVANCE_TEST_ORGANIZATION_ID, + TOPIC_RELEVANCE_TEST_PROJECT_ID, + ) assert result.data == sample_topic_relevance @@ def test_get_success(mock_session, sample_topic_relevance, auth_context): with patch("app.api.routes.topic_relevance_configs.topic_relevance_crud") as crud: crud.get.return_value = sample_topic_relevance result = get_topic_relevance_config( id=TOPIC_RELEVANCE_TEST_ID, session=mock_session, auth=auth_context, ) + crud.get.assert_called_once_with( + mock_session, + TOPIC_RELEVANCE_TEST_ID, + TOPIC_RELEVANCE_TEST_ORGANIZATION_ID, + TOPIC_RELEVANCE_TEST_PROJECT_ID, + ) assert result.data == sample_topic_relevanceAlso applies to: 95-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_topic_relevance_configs_api.py` around lines 61 - 74, Add call-contract assertions to the tests so the CRUD layer receives tenant-scoped IDs: in test_create_calls_crud, after calling create_topic_relevance_config assert that the patched topic_relevance_crud.create was called once with the payload and propagated tenant args (e.g. assert_called_once_with(payload=create_payload, session=mock_session, auth=auth_context, organization_id=auth_context.organization_id, project_id=auth_context.project_id)); likewise in the test_get_success (the test around get_topic_relevance_config) assert topic_relevance_crud.get.assert_called_once_with(id=sample_topic_relevance.id, session=mock_session, auth=auth_context, organization_id=auth_context.organization_id, project_id=auth_context.project_id) so tenant scoping is locked into the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/tests/test_topic_relevance_configs_api_integration.py`:
- Around line 110-128: The pagination test is flaky because the list method in
backend/app/crud/topic_relevance.py applies offset and limit without a
deterministic ordering; update the list(query) implementation to add an explicit
order_by clause (e.g., query.order_by(TopicRelevance.created_at,
TopicRelevance.id)) so the list method returns results in a consistent order and
the test comparing full_data[2:4] to the paged results becomes deterministic.
---
Duplicate comments:
In `@backend/app/alembic/versions/006_added_topic_relevance_config.py`:
- Around line 35-40: The UniqueConstraint currently named
"uq_topic_relevance_name_org_project" only enforces uniqueness on
("name","organization_id","project_id"), which allows duplicate configurations
with different names; update the migration in
006_added_topic_relevance_config.py to enforce uniqueness on the configuration
identity instead by including the configuration identifiers (e.g.,
"configuration" and/or "prompt_version") together with "organization_id" and
"project_id" in the UniqueConstraint (or replace the existing constraint), and
rename the constraint to reflect its purpose (e.g.,
"uq_topic_relevance_config_org_project") so the same
configuration/prompt_version cannot be inserted under a different name within
the same tenant scope.
In
`@backend/app/core/validators/config/topic_relevance_safety_validator_config.py`:
- Around line 17-23: The build() method currently falls back to a blank string
which hides missing configuration; change build() to validate that
self.configuration is present (e.g., not None and not empty) and raise a clear
ValueError if it is missing, then construct TopicRelevance with
topic_config=self.configuration (no " " fallback), preserving prompt_version
(self.prompt_version or 1), llm_callable (self.llm_callable) and on_fail
(self.resolve_on_fail()) so the downstream error is explicit and easier to
debug.
In `@backend/app/schemas/topic_relevance.py`:
- Around line 39-44: The update schema TopicRelevanceUpdate currently allows
unconstrained name and prompt_version which can let invalid values be persisted;
modify TopicRelevanceUpdate so its name and prompt_version use the same
constrained types/validators as the create-time schema (e.g., reuse the same
Pydantic field types or validators used in TopicRelevanceCreate or the existing
constrained types for name and prompt_version) to enforce identical constraints
on update operations (apply the same length/regex/None-allowed logic and integer
bounds as the create schema).
- Around line 28-33: The TopicRelevanceBase Pydantic model allows description to
be Optional which diverges from the persistence model that requires a non-null
description; update the TopicRelevanceBase definition (the description field) to
be a non-optional str (e.g., description: str) or provide a non-null default
value to match the DB contract, and ensure any create/update code that
constructs TopicRelevanceBase (or uses its validation) conforms to this non-null
requirement.
---
Nitpick comments:
In `@backend/app/tests/test_topic_relevance_configs_api.py`:
- Around line 61-74: Add call-contract assertions to the tests so the CRUD layer
receives tenant-scoped IDs: in test_create_calls_crud, after calling
create_topic_relevance_config assert that the patched
topic_relevance_crud.create was called once with the payload and propagated
tenant args (e.g. assert_called_once_with(payload=create_payload,
session=mock_session, auth=auth_context,
organization_id=auth_context.organization_id,
project_id=auth_context.project_id)); likewise in the test_get_success (the test
around get_topic_relevance_config) assert
topic_relevance_crud.get.assert_called_once_with(id=sample_topic_relevance.id,
session=mock_session, auth=auth_context,
organization_id=auth_context.organization_id,
project_id=auth_context.project_id) so tenant scoping is locked into the
contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 849d8736-1f88-46a0-be21-db00df993c96
📒 Files selected for processing (13)
backend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/API_USAGE.mdbackend/app/api/docs/guardrails/run_guardrails.mdbackend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.mdbackend/app/api/docs/topic_relevance_configs/update_topic_relevance_config.mdbackend/app/core/validators/README.mdbackend/app/core/validators/config/topic_relevance_safety_validator_config.pybackend/app/core/validators/prompts/topic_relevance/v1.mdbackend/app/core/validators/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api.pybackend/app/tests/test_topic_relevance_configs_api_integration.py
✅ Files skipped from review due to trivial changes (1)
- backend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.md
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/core/validators/prompts/topic_relevance/v1.md
- backend/app/models/config/topic_relevance.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/continuous_integration.yml:
- Around line 61-63: The workflow uses the Guardrails CLI with an invalid
boolean argument ("--enable-metrics false"); update the "guardrails configure"
invocation to use the CLI's flag-pair form by replacing the boolean-style flag
with the correct flag name (e.g., use "--disable-metrics" instead of
"--enable-metrics false"), ensuring the command remains "guardrails configure
--token \"$GUARDRAILS_HUB_API_KEY\" --disable-metrics" so it matches the
installer/CLI expectations.
- Around line 57-63: The workflow step attempts to run `guardrails configure`
using the `GUARDRAILS_HUB_API_KEY` env var which may be empty for forked PRs;
add a guard in the `run:` block to check `GUARDRAILS_HUB_API_KEY` (e.g., test
for non-empty or use `[ -z "$GUARDRAILS_HUB_API_KEY" ]`) and skip/echo a message
instead of running `guardrails configure` when the key is absent so the job does
not fail; keep `source .venv/bin/activate` but conditionally execute `guardrails
configure --token "$GUARDRAILS_HUB_API_KEY" --enable-metrics false` only when
the variable is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb2f3d2e-fe50-4c38-b93a-c3afeba31cb9
📒 Files selected for processing (1)
.github/workflows/continuous_integration.yml
rkritika1508
changed the title
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/app/crud/topic_relevance.py (1)
30-36:⚠️ Potential issue | 🟠 MajorDon’t collapse every
IntegrityErrorinto “duplicate configuration.”Both
createandupdatecurrently report all integrity failures as duplicates, which misreports non-unique violations and obscures real errors.🔧 Proposed fix pattern
- except IntegrityError: + except IntegrityError as exc: session.rollback() - raise HTTPException( - 400, "Topic relevance with the same configuration already exists" - ) + message = "Invalid topic relevance data" + if "uq_topic_relevance_config_org_project_prompt" in str(getattr(exc, "orig", exc)): + message = "Topic relevance with the same configuration already exists" + raise HTTPException(status_code=400, detail=message) from excAlso applies to: 97-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/crud/topic_relevance.py` around lines 30 - 36, The current except IntegrityError block around session.commit() in topic_relevance.py (and the similar block at lines 97-103) always returns "duplicate configuration"; change the error handling to inspect the IntegrityError's underlying exception (e.g., IntegrityError.orig or the DB-specific message) and only map known unique-constraint/duplicate messages to a 400 "Topic relevance with the same configuration already exists" HTTPException; for any other integrity violations, rollback and raise a 500 (or re-raise) that includes the original error detail (and log it) so non-unique integrity errors are not misclassified.
🧹 Nitpick comments (1)
backend/app/models/config/topic_relevance.py (1)
9-11: Mirror the migration’s unique constraint in the SQLModel metadata.The migration defines
uq_topic_relevance_config_org_project_prompt, but the model omits__table_args__. Adding it keeps ORM metadata aligned and avoids avoidable schema-drift noise in autogenerate workflows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/models/config/topic_relevance.py` around lines 9 - 11, The model class TopicRelevance (SQLModel, table=True) is missing __table_args__ to mirror the migration's unique constraint; add a __table_args__ attribute to the TopicRelevance class that includes sqlalchemy.schema.UniqueConstraint("config_id", "org_id", "project_id", "prompt_id", name="uq_topic_relevance_config_org_project_prompt") and ensure UniqueConstraint is imported from sqlalchemy so the ORM metadata matches the migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/API_USAGE.md`:
- Around line 186-187: Update the documentation for topic_relevance to reflect
current behavior: instead of saying "pass topic_relevance_config_id only",
clarify that the API accepts either a config id (topic_relevance_config_id) or
an inline configuration, because guardrails.py resolves configuration and
prompt_schema_version before validator execution; reference the topic_relevance
validator and the tests in backend/app/tests/test_validate_with_guard.py (topic
relevance scope tests) to justify the change and add a short note explaining the
pre-resolution behavior in guardrails.py.
In `@backend/app/models/config/topic_relevance.py`:
- Around line 35-38: The Field "description" in topic_relevance.py is declared
nullable at the DB level but typed as a non‑optional str; change the model so
the "description" attribute is Optional[str] and provide a None default to match
nullable=True (e.g., update the annotation to Optional[str] and set default to
None via Field or a default value) to keep typing and DB contract consistent;
locate the "description" Field declaration in the TopicRelevance model/class to
apply this change.
In `@backend/app/tests/test_topic_relevance_configs_api_integration.py`:
- Around line 19-30: The test fixture always posts an identical payload (same
prompt_schema_version + configuration) causing unique constraint collisions on
repeated creates; modify the create() helper so the default payload is unique
each call by appending a random or time-based suffix to either the
"configuration" or "name" field (or include a generated UID) before posting to
BASE_URL; ensure you update the payload construction in the function that
returns client.post(BASE_URL, json=payload, headers=self._headers(api_key)) so
subsequent calls produce distinct prompt_schema_version+configuration combos and
avoid collisions.
---
Duplicate comments:
In `@backend/app/crud/topic_relevance.py`:
- Around line 30-36: The current except IntegrityError block around
session.commit() in topic_relevance.py (and the similar block at lines 97-103)
always returns "duplicate configuration"; change the error handling to inspect
the IntegrityError's underlying exception (e.g., IntegrityError.orig or the
DB-specific message) and only map known unique-constraint/duplicate messages to
a 400 "Topic relevance with the same configuration already exists"
HTTPException; for any other integrity violations, rollback and raise a 500 (or
re-raise) that includes the original error detail (and log it) so non-unique
integrity errors are not misclassified.
---
Nitpick comments:
In `@backend/app/models/config/topic_relevance.py`:
- Around line 9-11: The model class TopicRelevance (SQLModel, table=True) is
missing __table_args__ to mirror the migration's unique constraint; add a
__table_args__ attribute to the TopicRelevance class that includes
sqlalchemy.schema.UniqueConstraint("config_id", "org_id", "project_id",
"prompt_id", name="uq_topic_relevance_config_org_project_prompt") and ensure
UniqueConstraint is imported from sqlalchemy so the ORM metadata matches the
migration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e972c96-3d84-496d-9f0b-55f561a81453
📒 Files selected for processing (14)
backend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/API_USAGE.mdbackend/app/api/docs/guardrails/run_guardrails.mdbackend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.mdbackend/app/api/routes/guardrails.pybackend/app/core/validators/README.mdbackend/app/core/validators/config/topic_relevance_safety_validator_config.pybackend/app/core/validators/topic_relevance.pybackend/app/crud/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api.pybackend/app/tests/test_topic_relevance_configs_api_integration.pybackend/app/tests/test_validate_with_guard.py
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/app/api/docs/guardrails/run_guardrails.md
- backend/app/api/routes/guardrails.py
- backend/app/tests/test_topic_relevance_configs_api.py
- backend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.md
- backend/app/core/validators/config/topic_relevance_safety_validator_config.py
- backend/app/schemas/topic_relevance.py
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/app/api/API_USAGE.md (1)
186-187:⚠️ Potential issue | 🟡 MinorTopic relevance runtime docs are still too strict (
config_idis not the only supported input).These lines still imply
topic_relevance_config_idis mandatory, but runtime behavior also allows inlineconfigurationwhen no preset lookup is needed. Please update this guidance to “prefertopic_relevance_config_idfor tenant presets; inlineconfigurationis also supported,” and keep the resolver note for the config-id path. This same wording issue also appears at Line 415.Proposed doc patch
-- For `topic_relevance`, pass `topic_relevance_config_id` only. -- The API resolves `configuration` + `prompt_schema_version` in `guardrails.py` before validator execution, so the validator always executes with both values. +- For `topic_relevance`, prefer `topic_relevance_config_id` when using a tenant-scoped preset. +- Inline `configuration` can also be provided directly in the runtime validator object. +- When `topic_relevance_config_id` is provided, the API resolves `configuration` + `prompt_schema_version` in `guardrails.py` before validator execution. ... -7. For `topic_relevance`, create/manage a topic relevance config and pass `topic_relevance_config_id` at runtime. The server resolves the configuration string internally. +7. For `topic_relevance`, either pass inline `configuration` at runtime or create/manage a topic relevance config and pass `topic_relevance_config_id`. For config-id flows, the server resolves configuration internally.Also applies to: 415-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/API_USAGE.md` around lines 186 - 187, Update the API_USAGE.md wording around `topic_relevance` to say prefer `topic_relevance_config_id` for tenant presets but also allow inline `configuration` when presets are not used, and retain the note about `guardrails.py` resolving `configuration` + `prompt_schema_version` before validator execution; apply this change for both occurrences (the current paragraph referencing `topic_relevance` and the duplicate at the later occurrence). Reference the `topic_relevance` field name and `guardrails.py` resolver behavior in the updated sentence so readers know the config-id path still resolves `configuration` + `prompt_schema_version` before validation.
🧹 Nitpick comments (3)
backend/app/tests/test_validate_with_guard.py (1)
169-205: Good edge case coverage for missing/inline config.The tests properly verify that no database lookup occurs when
topic_relevance_config_idis missing or when inline configuration is already provided.Consider adding a test for the error path when
topic_relevance_crud.getraises a 404 (config not found). This would ensure the error propagates correctly to the caller.Would you like me to generate a test case for the 404 scenario?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_validate_with_guard.py` around lines 169 - 205, Add a test that patches app.api.routes.guardrails.topic_relevance_crud.get to raise a 404-style exception and asserts that _resolve_topic_relevance_scope propagates that error: create a GuardrailRequest similar to the other tests (with a validator that references a topic_relevance config id), patch topic_relevance_crud.get to raise an HTTPException or custom NotFound error, call _resolve_topic_relevance_scope with a mock session inside pytest.raises(expected_exception), and verify the exception is raised and that topic_relevance_crud.get was called with the expected id.backend/app/crud/topic_relevance.py (1)
1-1: Use built-inlistinstead oftyping.List.
typing.Listis deprecated in Python 3.9+. Use the built-inlisttype annotation instead.Proposed fix
-from typing import List +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + passAnd on line 64:
- ) -> List[TopicRelevance]: + ) -> list[TopicRelevance]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/crud/topic_relevance.py` at line 1, Replace the deprecated typing.List usage: remove the import "from typing import List" and update any function/class annotations that use "List[...]" (e.g., the annotation referenced around line 64) to use the built-in "list[...]" or plain "list" as appropriate (preserve element type inside the brackets). Ensure all occurrences in topic_relevance.py (functions/methods and return types) are updated consistently and run type checks.backend/app/alembic/versions/006_added_topic_relevance_config.py (1)
56-57: Downgrade drops table but not indexes explicitly.The
op.drop_table()cascades index drops in most databases, so this is functionally correct. However, for symmetry and explicitness, you could drop indexes first. This is optional since PostgreSQL handles it automatically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/alembic/versions/006_added_topic_relevance_config.py` around lines 56 - 57, The downgrade() currently calls op.drop_table("topic_relevance") but does not explicitly drop the indexes; update downgrade() to first call op.drop_index(...) for each index created in the corresponding upgrade (use the same index names used when creating them) and then call op.drop_table("topic_relevance"); specifically reference op.drop_index and the existing downgrade() / op.drop_table("topic_relevance") lines and ensure the index names match those used in the migration (replace with the actual index names if they differ).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/routes/guardrails.py`:
- Around line 193-197: The current code assumes history.last.iterations is
non-empty and directly indexes [-1], which can raise if iterations is empty;
modify the guard/history access in the block that reads history = getattr(guard,
"history", None) so you first verify history and history.last exist and that
getattr(history.last, "iterations", None) is a non-empty sequence before
assigning iteration = history.last.iterations[-1]; if iterations is
missing/empty, skip indexing and treat as a validator failure path (e.g., set
logs = [] or return the existing validator failure message) while still safely
reading outputs via getattr(iteration, "outputs", None) and getattr(...,
"validator_logs", []) when iteration is present.
In `@backend/app/core/validators/topic_relevance.py`:
- Around line 111-112: The except block currently binds an unused variable
(`except Exception as e:`) which triggers Ruff F841; change it to a bare
exception handler (use `except Exception:`) in the fallback handler where
`FailResult(error_message="LLM critic returned an invalid response.")` is
returned so the unused `e` is removed while preserving behavior (look for the
except block that returns FailResult in topic_relevance.py).
---
Duplicate comments:
In `@backend/app/api/API_USAGE.md`:
- Around line 186-187: Update the API_USAGE.md wording around `topic_relevance`
to say prefer `topic_relevance_config_id` for tenant presets but also allow
inline `configuration` when presets are not used, and retain the note about
`guardrails.py` resolving `configuration` + `prompt_schema_version` before
validator execution; apply this change for both occurrences (the current
paragraph referencing `topic_relevance` and the duplicate at the later
occurrence). Reference the `topic_relevance` field name and `guardrails.py`
resolver behavior in the updated sentence so readers know the config-id path
still resolves `configuration` + `prompt_schema_version` before validation.
---
Nitpick comments:
In `@backend/app/alembic/versions/006_added_topic_relevance_config.py`:
- Around line 56-57: The downgrade() currently calls
op.drop_table("topic_relevance") but does not explicitly drop the indexes;
update downgrade() to first call op.drop_index(...) for each index created in
the corresponding upgrade (use the same index names used when creating them) and
then call op.drop_table("topic_relevance"); specifically reference op.drop_index
and the existing downgrade() / op.drop_table("topic_relevance") lines and ensure
the index names match those used in the migration (replace with the actual index
names if they differ).
In `@backend/app/crud/topic_relevance.py`:
- Line 1: Replace the deprecated typing.List usage: remove the import "from
typing import List" and update any function/class annotations that use
"List[...]" (e.g., the annotation referenced around line 64) to use the built-in
"list[...]" or plain "list" as appropriate (preserve element type inside the
brackets). Ensure all occurrences in topic_relevance.py (functions/methods and
return types) are updated consistently and run type checks.
In `@backend/app/tests/test_validate_with_guard.py`:
- Around line 169-205: Add a test that patches
app.api.routes.guardrails.topic_relevance_crud.get to raise a 404-style
exception and asserts that _resolve_topic_relevance_scope propagates that error:
create a GuardrailRequest similar to the other tests (with a validator that
references a topic_relevance config id), patch topic_relevance_crud.get to raise
an HTTPException or custom NotFound error, call _resolve_topic_relevance_scope
with a mock session inside pytest.raises(expected_exception), and verify the
exception is raised and that topic_relevance_crud.get was called with the
expected id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a656d3c-ea60-4a9f-bbb0-52d8800d9a6f
📒 Files selected for processing (15)
backend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/API_USAGE.mdbackend/app/api/docs/guardrails/run_guardrails.mdbackend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.mdbackend/app/api/routes/guardrails.pybackend/app/core/validators/README.mdbackend/app/core/validators/config/topic_relevance_safety_validator_config.pybackend/app/core/validators/prompts/topic_relevance/v1.mdbackend/app/core/validators/topic_relevance.pybackend/app/crud/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api.pybackend/app/tests/test_topic_relevance_configs_api_integration.pybackend/app/tests/test_validate_with_guard.py
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/core/validators/config/topic_relevance_safety_validator_config.py
- backend/app/tests/test_topic_relevance_configs_api.py
- backend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.md
nishika26
requested changes
Mar 10, 2026
backend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.md
Outdated
Show resolved
Hide resolved
backend/app/api/docs/topic_relevance_configs/create_topic_relevance_config.md
Outdated
Show resolved
Hide resolved
nishika26
requested changes
Mar 10, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (2)
backend/app/core/validators/README.md (2)
275-275:⚠️ Potential issue | 🟡 MinorDefine
llm_callableand explain when to customize it.The
llm_callableparameter is documented but not explained. Users need to understand:
- What this parameter controls (which LLM model is called for validation)
- What values are valid (e.g., other OpenAI models, model providers)
- When and why to customize it (cost vs. quality tradeoffs, latency considerations)
📝 Suggested addition
Parameters / customization: - `topic_relevance_config_id: UUID` (required at runtime; resolves configuration and prompt version from tenant config) - `prompt_schema_version: int` (optional; defaults to `1`) -- `llm_callable: str` (default: `gpt-4o-mini`) +- `llm_callable: str` (default: `gpt-4o-mini`) - Specifies which LLM model to use as the judge for topic relevance validation. Customize this to balance cost, latency, and validation quality. Supported values include OpenAI models like `gpt-4o`, `gpt-4o-mini`, `gpt-3.5-turbo`. - `on_fail`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/README.md` at line 275, Update the README entry for the llm_callable parameter (currently shown as `llm_callable: str (default: gpt-4o-mini)`) to explain what it controls (the LLM model invoked by validation routines), list valid value types (example OpenAI model names like gpt-4o-mini, gpt-4, gpt-3.5-turbo and note support for other providers or custom model identifiers if applicable), and describe when to customize it (tradeoffs: cost vs. quality, latency, token limits, and safety/temperature differences) including brief guidance/examples for typical choices (e.g., use gpt-4 for highest quality, gpt-3.5/gpt-4o-mini for cheaper/faster validation) so users know how to pick the right model for their needs.
12-12:⚠️ Potential issue | 🟡 MinorAdd a dedicated section documenting the
llm_criticvalidator.The
llm_criticvalidator is added to the manifest but lacks a dedicated documentation section. All other validators (1-4) have detailed sections covering code paths, purpose, recommendations, parameters, and limitations. Without this section, users won't understand when to usellm_criticdirectly or how to configure it.📝 Suggested documentation structure
Add a section following the pattern of existing validators:
### 5) LLM Critic Validator (`llm_critic`) Code: - Config: `backend/app/core/validators/config/llm_critic_safety_validator_config.py` - Runtime validator: Guardrails Hub (`hub://guardrails/llm_critic`) What it does: - [Brief description of what llm_critic validates] - Uses an LLM-as-judge approach to evaluate text against custom criteria. Why this is used: - [Explanation of use cases] Recommendation: - [Stage recommendations: input/output] Parameters / customization: - [List parameters and their defaults] Notes / limitations: - [Known limitations or considerations]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/README.md` at line 12, Add a new dedicated README section for the llm_critic validator titled "5) LLM Critic Validator (`llm_critic`)" that follows the same structure as the other validators: list the code paths (reference backend/app/core/validators/config/llm_critic_safety_validator_config.py and the runtime source hub://guardrails/llm_critic), a concise "What it does" description explaining the LLM-as-judge approach, "Why this is used" use cases, "Recommendation" for when to run it (input/output stage), "Parameters / customization" with defaults, and "Notes / limitations" with known caveats — ensure content mirrors tone and detail level of validators 1–4.
🧹 Nitpick comments (2)
backend/app/core/validators/README.md (2)
272-281: Clarify the relationship betweentopic_relevance_config_idand auto-resolved parameters.The documentation mentions that
configurationandprompt_schema_versionare "resolved from tenant config" but doesn't explicitly state that whentopic_relevance_config_idis provided, those parameters are automatically populated and override any explicitly set values. This could confuse users trying to understand the parameter precedence.📝 Suggested clarification
Parameters / customization: -- `topic_relevance_config_id: UUID` (required at runtime; resolves configuration and prompt version from tenant config) +- `topic_relevance_config_id: UUID` (required at runtime; when provided, automatically populates `configuration` and `prompt_schema_version` from the tenant's Topic Relevance Config) - `prompt_schema_version: int` (optional; defaults to `1`) - `llm_callable: str` (default: `gpt-4o-mini`) - `on_fail` Notes / limitations: - Runtime validation requires `topic_relevance_config_id`. -- Configuration is resolved in `backend/app/api/routes/guardrails.py` from tenant Topic Relevance Config APIs. +- The `configuration` text and `prompt_schema_version` are fetched from the Topic Relevance Config API at runtime by `backend/app/api/routes/guardrails.py`. - Prompt templates must include the `{{TOPIC_CONFIGURATION}}` placeholder. +- The `{{TOPIC_CONFIGURATION}}` placeholder is replaced with the `configuration` text from the resolved config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/README.md` around lines 272 - 281, Update the README to explicitly state that when topic_relevance_config_id is provided the system will auto-resolve and populate the related configuration and prompt_schema_version from tenant config (via backend/app/api/routes/guardrails.py) and that these auto-resolved values take precedence and will override any explicitly supplied configuration or prompt_schema_version at runtime; also mention that prompt templates must include the {{TOPIC_CONFIGURATION}} placeholder and note the precedence behavior so users know they cannot override the auto-resolved settings by passing their own values.
258-258: Fix hyphenation for consistency.The term should be "LLM-critic" (hyphenated) for consistency with compound adjective style.
✏️ Grammar fix
What it does: -- Checks whether the user message is in scope using an LLM-critic style metric. +- Checks whether the user message is in scope using an LLM-critic-style metric. - Builds the final prompt from:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/README.md` at line 258, Update the README sentence that currently reads "Checks whether the user message is in scope using an LLM-critic style metric" to use the hyphenated compound term; change the phrase to "an LLM-critic-style metric" so "LLM-critic" is hyphenated and properly connected to "style" (locate the sentence in backend/app/core/validators/README.md containing "LLM-critic style metric" and replace it accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/app/core/validators/README.md`:
- Line 275: Update the README entry for the llm_callable parameter (currently
shown as `llm_callable: str (default: gpt-4o-mini)`) to explain what it controls
(the LLM model invoked by validation routines), list valid value types (example
OpenAI model names like gpt-4o-mini, gpt-4, gpt-3.5-turbo and note support for
other providers or custom model identifiers if applicable), and describe when to
customize it (tradeoffs: cost vs. quality, latency, token limits, and
safety/temperature differences) including brief guidance/examples for typical
choices (e.g., use gpt-4 for highest quality, gpt-3.5/gpt-4o-mini for
cheaper/faster validation) so users know how to pick the right model for their
needs.
- Line 12: Add a new dedicated README section for the llm_critic validator
titled "5) LLM Critic Validator (`llm_critic`)" that follows the same structure
as the other validators: list the code paths (reference
backend/app/core/validators/config/llm_critic_safety_validator_config.py and the
runtime source hub://guardrails/llm_critic), a concise "What it does"
description explaining the LLM-as-judge approach, "Why this is used" use cases,
"Recommendation" for when to run it (input/output stage), "Parameters /
customization" with defaults, and "Notes / limitations" with known caveats —
ensure content mirrors tone and detail level of validators 1–4.
---
Nitpick comments:
In `@backend/app/core/validators/README.md`:
- Around line 272-281: Update the README to explicitly state that when
topic_relevance_config_id is provided the system will auto-resolve and populate
the related configuration and prompt_schema_version from tenant config (via
backend/app/api/routes/guardrails.py) and that these auto-resolved values take
precedence and will override any explicitly supplied configuration or
prompt_schema_version at runtime; also mention that prompt templates must
include the {{TOPIC_CONFIGURATION}} placeholder and note the precedence behavior
so users know they cannot override the auto-resolved settings by passing their
own values.
- Line 258: Update the README sentence that currently reads "Checks whether the
user message is in scope using an LLM-critic style metric" to use the hyphenated
compound term; change the phrase to "an LLM-critic-style metric" so "LLM-critic"
is hyphenated and properly connected to "style" (locate the sentence in
backend/app/core/validators/README.md containing "LLM-critic style metric" and
replace it accordingly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07063d8f-16b2-43b1-ba20-a740425c73bf
📒 Files selected for processing (2)
backend/README.mdbackend/app/core/validators/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/README.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/api/routes/guardrails.py (1)
48-58:⚠️ Potential issue | 🟠 MajorResolve config-backed validators before creating the request log, or finalize failures here.
The new lookup on Line 57 can raise before
_validate_with_guard()ever runs. Because the log is already created on Line 52, those requests never get a terminal status or response text.💡 One simple fix
- try: - request_log = request_log_crud.create(payload) - except ValueError: - return APIResponse.failure_response(error="Invalid request_id") - _resolve_ban_list_banned_words(payload, session) _resolve_topic_relevance_scope(payload, session) + try: + request_log = request_log_crud.create(payload) + except ValueError: + return APIResponse.failure_response(error="Invalid request_id") + return _validate_with_guard( payload, request_log_crud,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/guardrails.py` around lines 48 - 58, The request log is being created by RequestLogCrud.create before calling the config-backed resolvers (_resolve_ban_list_banned_words and _resolve_topic_relevance_scope), so if those resolvers raise an exception the created request never gets a terminal status or response text; fix by either moving the calls to _resolve_ban_list_banned_words and _resolve_topic_relevance_scope to run before RequestLogCrud.create, or wrap those resolver calls in a try/except that catches failures and uses RequestLogCrud (e.g., an update/mark_failed method on the RequestLogCrud instance) to set a terminal failure status and response_text before re-raising/returning, ensuring _validate_with_guard always runs with a properly finalized log.
♻️ Duplicate comments (2)
backend/app/api/routes/guardrails.py (1)
202-205:⚠️ Potential issue | 🟠 MajorGuard history access still assumes at least one iteration.
Line 204 indexes
history.last.iterations[-1]without the emptiness check thatadd_validator_logs()already uses. If Guard returns no iterations, this path raises and overwrites the real validator failure with the generic exception response.🛡️ Minimal hardening
- history = getattr(guard, "history", None) - if history and getattr(history, "last", None): - iteration = history.last.iterations[-1] - logs = getattr(iteration.outputs, "validator_logs", []) + history = getattr(guard, "history", None) + last_call = getattr(history, "last", None) if history else None + iterations = getattr(last_call, "iterations", None) + logs = [] + if iterations: + iteration = iterations[-1] + outputs = getattr(iteration, "outputs", None) + logs = getattr(outputs, "validator_logs", []) if outputs else []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/guardrails.py` around lines 202 - 205, The guard history access assumes there is at least one iteration; change the block that reads history = getattr(guard, "history", None) and then uses history.last.iterations[-1] so it first checks that history and history.last exist and that history.last.iterations is non-empty (similar to add_validator_logs()), and only then assign iteration = history.last.iterations[-1] and extract validator_logs; otherwise set logs to an empty list or skip processing to avoid raising an IndexError and losing the real validator failure.backend/app/core/validators/README.md (1)
272-280:⚠️ Potential issue | 🟡 MinorDon’t mark preset IDs as universally required.
_resolve_topic_relevance_scope()only does a DB lookup whentopic_relevance_config_idis present, so inlineconfigurationis still a supported runtime path. Documenting the ID as mandatory narrows the contract more than the code does.📝 Suggested wording
- - `topic_relevance_config_id: UUID` (required at runtime; resolves configuration and prompt version from tenant config) - - `prompt_schema_version: int` (optional; defaults to `1`) + - `configuration: str` (optional when `topic_relevance_config_id` is provided) + - `topic_relevance_config_id: UUID` (optional when `configuration` is provided; resolves a stored tenant preset) + - `prompt_schema_version: int` (optional with inline `configuration`; defaults to `1`) ... - - Runtime validation requires `topic_relevance_config_id`. + - Runtime validation requires either inline `configuration` or `topic_relevance_config_id`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/README.md` around lines 272 - 280, Update the README so `topic_relevance_config_id` is not described as universally required: remove "required at runtime" from the parameter list for `topic_relevance_config_id` and instead state that the implementation supports either passing `topic_relevance_config_id` (which triggers a DB lookup in _resolve_topic_relevance_scope()) or supplying an inline `configuration` at runtime; also adjust the Notes to say runtime validation requires either `topic_relevance_config_id` or an inline `configuration` (rather than only the ID) and keep the existing reference to configuration resolution in the guardrails route.
🧹 Nitpick comments (2)
backend/app/api/routes/topic_relevance_configs.py (2)
21-25: Set explicit201 Createdfor the create endpoint.This is a create operation; setting
status_code=201improves API semantics and client expectations.Suggested patch
`@router.post`( "/", description=load_description("topic_relevance_configs/create_config.md"), response_model=APIResponse[TopicRelevanceResponse], + status_code=201, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/topic_relevance_configs.py` around lines 21 - 25, The create endpoint decorator for the topic relevance config is missing an explicit 201 status; update the `@router.post`(...) for the create route (the decorator where description=load_description("topic_relevance_configs/create_config.md") and response_model=APIResponse[TopicRelevanceResponse]) to include status_code=201 so the endpoint returns HTTP 201 Created for successful creates.
48-50: Consider a bounded default forlimitto avoid unbounded list reads.
limit=Nonecan result in large result sets; using a safe default (e.g., 50/100) is a better operational default.Suggested patch
- limit: Annotated[int | None, Query(ge=1, le=100)] = None, + limit: Annotated[int, Query(ge=1, le=100)] = 100,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/topic_relevance_configs.py` around lines 48 - 50, Change the unbounded default for the query parameter limit from None to a safe bounded default (e.g., 50) to avoid large uncontrolled reads: replace the signature using limit: Annotated[int | None, Query(ge=1, le=100)] = None with limit: Annotated[int, Query(ge=1, le=100)] = 50 (keep offset: Annotated[int, Query(ge=0)] = 0 unchanged); update any logic that checks for None to assume a concrete int if present and adjust tests or docs that relied on None as the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.test.example:
- Line 24: The OPENAI_API_KEY placeholder in .env.test.example uses surrounding
quotes which triggers dotenv-linter's QuoteCharacter rule; remove the quotes so
the line reads OPENAI_API_KEY=<ADD-KEY> (i.e., update the OPENAI_API_KEY entry
to have no surrounding " characters) to satisfy the linter.
In `@backend/app/core/validators/topic_relevance.py`:
- Around line 59-89: The LLMCritic is being initialized with an OpenAI-specific
llm_kwargs (response_format={"type":"json_object"}) while llm_callable is an
unconstrained string; update TopicRelevanceValidator.__init__ to either (A)
restrict/validate llm_callable to a known set of OpenAI-compatible models (e.g.,
require values like "gpt-4o-mini") before creating self._critic, or (B) query
LiteLLM’s capabilities via get_supported_openai_params() for the given
llm_callable and only pass llm_kwargs when the model supports response_format;
if unsupported, remove llm_kwargs or fall back to a non-JSON parsing approach
(or implement a custom LLMCritic that forwards llm_kwargs). Ensure changes
reference the llm_callable parameter, the LLMCritic instantiation
(self._critic), and the llm_kwargs handling so the init does not silently pass
unsupported OpenAI-style params.
---
Outside diff comments:
In `@backend/app/api/routes/guardrails.py`:
- Around line 48-58: The request log is being created by RequestLogCrud.create
before calling the config-backed resolvers (_resolve_ban_list_banned_words and
_resolve_topic_relevance_scope), so if those resolvers raise an exception the
created request never gets a terminal status or response text; fix by either
moving the calls to _resolve_ban_list_banned_words and
_resolve_topic_relevance_scope to run before RequestLogCrud.create, or wrap
those resolver calls in a try/except that catches failures and uses
RequestLogCrud (e.g., an update/mark_failed method on the RequestLogCrud
instance) to set a terminal failure status and response_text before
re-raising/returning, ensuring _validate_with_guard always runs with a properly
finalized log.
---
Duplicate comments:
In `@backend/app/api/routes/guardrails.py`:
- Around line 202-205: The guard history access assumes there is at least one
iteration; change the block that reads history = getattr(guard, "history", None)
and then uses history.last.iterations[-1] so it first checks that history and
history.last exist and that history.last.iterations is non-empty (similar to
add_validator_logs()), and only then assign iteration =
history.last.iterations[-1] and extract validator_logs; otherwise set logs to an
empty list or skip processing to avoid raising an IndexError and losing the real
validator failure.
In `@backend/app/core/validators/README.md`:
- Around line 272-280: Update the README so `topic_relevance_config_id` is not
described as universally required: remove "required at runtime" from the
parameter list for `topic_relevance_config_id` and instead state that the
implementation supports either passing `topic_relevance_config_id` (which
triggers a DB lookup in _resolve_topic_relevance_scope()) or supplying an inline
`configuration` at runtime; also adjust the Notes to say runtime validation
requires either `topic_relevance_config_id` or an inline `configuration` (rather
than only the ID) and keep the existing reference to configuration resolution in
the guardrails route.
---
Nitpick comments:
In `@backend/app/api/routes/topic_relevance_configs.py`:
- Around line 21-25: The create endpoint decorator for the topic relevance
config is missing an explicit 201 status; update the `@router.post`(...) for the
create route (the decorator where
description=load_description("topic_relevance_configs/create_config.md") and
response_model=APIResponse[TopicRelevanceResponse]) to include status_code=201
so the endpoint returns HTTP 201 Created for successful creates.
- Around line 48-50: Change the unbounded default for the query parameter limit
from None to a safe bounded default (e.g., 50) to avoid large uncontrolled
reads: replace the signature using limit: Annotated[int | None, Query(ge=1,
le=100)] = None with limit: Annotated[int, Query(ge=1, le=100)] = 50 (keep
offset: Annotated[int, Query(ge=0)] = 0 unchanged); update any logic that checks
for None to assume a concrete int if present and adjust tests or docs that
relied on None as the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2331fb1c-fc70-4948-ba91-08f341b7806d
📒 Files selected for processing (12)
.env.example.env.test.examplebackend/app/api/docs/topic_relevance_configs/create_config.mdbackend/app/api/docs/topic_relevance_configs/delete_config.mdbackend/app/api/docs/topic_relevance_configs/get_config.mdbackend/app/api/docs/topic_relevance_configs/list_configs.mdbackend/app/api/docs/topic_relevance_configs/update_config.mdbackend/app/api/routes/guardrails.pybackend/app/api/routes/topic_relevance_configs.pybackend/app/core/validators/README.mdbackend/app/core/validators/topic_relevance.pybackend/app/schemas/guardrail_config.py
✅ Files skipped from review due to trivial changes (1)
- backend/app/api/docs/topic_relevance_configs/list_configs.md
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/schemas/guardrail_config.py
nishika26
requested changes
Mar 17, 2026
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
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/api/routes/guardrails.py (1)
43-56:⚠️ Potential issue | 🟠 MajorResolve stored configs before creating the request log.
This adds another DB-backed resolver after
request_log_crud.create(payload). If either resolver raises, the request log row is left in its initial state because_validate_with_guard()never gets a chance to finalize it.💡 Minimal change
- try: - request_log = request_log_crud.create(payload) - except ValueError: - return APIResponse.failure_response(error="Invalid request_id") - _resolve_ban_list_banned_words(payload, session) _resolve_topic_relevance_scope(payload, session) + + try: + request_log = request_log_crud.create(payload) + except ValueError: + return APIResponse.failure_response(error="Invalid request_id")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/guardrails.py` around lines 43 - 56, Move the DB-backed config resolution calls before creating the request log so partial rows aren't left behind: call _resolve_ban_list_banned_words(payload, session) and _resolve_topic_relevance_scope(payload, session) before invoking RequestLogCrud(session).create(payload), preserving the existing ValueError handling for create and ensuring any exceptions from the resolvers are raised/handled prior to creating the RequestLog record; update the order in the guardrail handler that currently calls RequestLogCrud.create, _resolve_ban_list_banned_words, _resolve_topic_relevance_scope and ensure _validate_with_guard still runs after creation.
♻️ Duplicate comments (1)
backend/app/core/validators/README.md (1)
300-309:⚠️ Potential issue | 🟡 MinorDocument both supported
topic_relevanceruntime shapes.The runtime now supports either a stored
topic_relevance_config_idor inlineconfiguration, but this section still says the ID is required. That will mislead API consumers into treating the inline path as invalid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/README.md` around lines 300 - 309, Update the README section describing parameters to document both supported runtime shapes for topic_relevance: the stored reference path using topic_relevance_config_id: UUID and the inline path using configuration (object) — make clear that prompt_schema_version (int) and llm_callable (str) apply to both, and that runtime validation requires either topic_relevance_config_id or an inline configuration (not strictly the ID). Also update the Notes to mention that configuration resolution still happens in guardrails.py via tenant Topic Relevance Config APIs when using an ID, and remind authors that prompt templates must include the {{TOPIC_CONFIGURATION}} placeholder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/routes/topic_relevance_configs.py`:
- Around line 26-30: The create/update endpoints accept payloads with
TopicRelevanceCreate/TopicRelevanceUpdate that allow description=None while the
DB model (TopicRelevance) now requires description non-nullable; update the
request validation to make description required (non-optional) or
coerce/validate it before persisting so the handlers
create_topic_relevance_config and the corresponding update method reject or fill
missing descriptions early; specifically modify the TopicRelevanceCreate and
TopicRelevanceUpdate schemas to mark description as non-optional (or add
explicit validation/Default logic in the create_topic_relevance_config and
update handler) so invalid payloads fail at validation rather than at CRUD
persistence.
---
Outside diff comments:
In `@backend/app/api/routes/guardrails.py`:
- Around line 43-56: Move the DB-backed config resolution calls before creating
the request log so partial rows aren't left behind: call
_resolve_ban_list_banned_words(payload, session) and
_resolve_topic_relevance_scope(payload, session) before invoking
RequestLogCrud(session).create(payload), preserving the existing ValueError
handling for create and ensuring any exceptions from the resolvers are
raised/handled prior to creating the RequestLog record; update the order in the
guardrail handler that currently calls RequestLogCrud.create,
_resolve_ban_list_banned_words, _resolve_topic_relevance_scope and ensure
_validate_with_guard still runs after creation.
---
Duplicate comments:
In `@backend/app/core/validators/README.md`:
- Around line 300-309: Update the README section describing parameters to
document both supported runtime shapes for topic_relevance: the stored reference
path using topic_relevance_config_id: UUID and the inline path using
configuration (object) — make clear that prompt_schema_version (int) and
llm_callable (str) apply to both, and that runtime validation requires either
topic_relevance_config_id or an inline configuration (not strictly the ID). Also
update the Notes to mention that configuration resolution still happens in
guardrails.py via tenant Topic Relevance Config APIs when using an ID, and
remind authors that prompt templates must include the {{TOPIC_CONFIGURATION}}
placeholder.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbd4db67-cf38-4a23-af40-8212fa0b95fd
📒 Files selected for processing (8)
backend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/routes/guardrails.pybackend/app/api/routes/topic_relevance_configs.pybackend/app/core/validators/README.mdbackend/app/core/validators/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_validate_with_guard.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/schemas/topic_relevance.py
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
.env.test.example (1)
24-24:⚠️ Potential issue | 🟡 MinorRemove quotes from
OPENAI_API_KEYplaceholder.Line 24 still uses a quoted
.envvalue and triggers dotenv-linter’sQuoteCharacterwarning.Proposed fix
-OPENAI_API_KEY="<ADD-KEY>" +OPENAI_API_KEY=<ADD-KEY>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.test.example at line 24, The OPENAI_API_KEY placeholder currently includes surrounding quotes which triggers dotenv-linter's QuoteCharacter warning; edit the .env test example to remove the quotation marks so the line reads OPENAI_API_KEY=<ADD-KEY> (i.e., update the OPENAI_API_KEY entry to be unquoted) and save the file so the linter no longer flags QuoteCharacter for that variable.backend/app/core/validators/config/topic_relevance_safety_validator_config.py (1)
17-23:⚠️ Potential issue | 🟠 MajorReject unresolved topic relevance configs instead of passing a blank prompt.
A payload like
{"type": "topic_relevance"}now skips preset lookup and then buildsTopicRelevance(topic_config=" "). That hides a caller error and lets the validator run with an empty scope. Please fail fast before constructingTopicRelevance.🛡️ Suggested fix
def build(self): + if not self.configuration or not self.configuration.strip(): + raise ValueError( + "Topic relevance configuration is missing. " + "Resolve `topic_relevance_config_id` or provide `configuration` before build()." + ) return TopicRelevance( - topic_config=self.configuration or " ", + topic_config=self.configuration, prompt_schema_version=self.prompt_schema_version or 1, llm_callable=self.llm_callable, on_fail=self.resolve_on_fail(), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/config/topic_relevance_safety_validator_config.py` around lines 17 - 23, The build() currently substitutes a blank string when topic config is missing which hides caller errors; instead, check that self.configuration (and any preset lookup result) is present and raise a clear exception (e.g., ValueError or a Config/ValidationError) if not, before constructing TopicRelevance. Update the build method referenced here (build, TopicRelevance, resolve_on_fail, self.configuration, self.prompt_schema_version) to validate presence of topic_config and fail fast rather than passing " " into TopicRelevance.backend/app/api/API_USAGE.md (1)
186-187:⚠️ Potential issue | 🟡 MinorDocument both supported
topic_relevanceruntime shapes.The route and tests currently allow either inline
configurationor atopic_relevance_config_idlookup. Saying “config id only” here is still inaccurate and will mislead integrators; please update the end-to-end note too, and mirror the same contract inbackend/app/core/validators/README.md.📝 Suggested wording
-- For `topic_relevance`, pass `topic_relevance_config_id` only. -- The API resolves `configuration` + `prompt_schema_version` in `guardrails.py` before validator execution, so the validator always executes with both values. +- For `topic_relevance`, you can either pass inline `configuration` (optionally with `prompt_schema_version`) or pass `topic_relevance_config_id`. +- When `topic_relevance_config_id` is provided, `guardrails.py` resolves `configuration` + `prompt_schema_version` before validator execution. -7. For `topic_relevance`, create/manage a topic relevance config and pass `topic_relevance_config_id` at runtime. The server resolves the configuration string internally. +7. For `topic_relevance`, either pass inline `configuration` or reference a stored topic relevance config with `topic_relevance_config_id`. The server resolves stored configuration before validation.Also applies to: 415-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/API_USAGE.md` around lines 186 - 187, Update the API_USAGE.md entry that currently states "For `topic_relevance`, pass `topic_relevance_config_id` only" to document both supported runtime shapes: (1) passing `topic_relevance_config_id` for a lookup and (2) providing an inline `configuration` object; note that `guardrails.py` still resolves `configuration` + `prompt_schema_version` before validator execution so validators always see both values. Also update the end-to-end note in API_USAGE.md to reflect both options and mirror this same contract and wording in backend/app/core/validators/README.md so the route, tests, and validator documentation are consistent.backend/app/crud/topic_relevance.py (1)
30-36:⚠️ Potential issue | 🟠 MajorOnly map unique-key failures to the duplicate-config response.
Both commit blocks collapse every
IntegrityErrorinto “same configuration already exists”. That misreports nullability/check/other constraint failures and makes debugging much harder. Please inspect the underlying DB error and only use this message for actual unique violations.🧩 Safer error mapping
- except IntegrityError: + except IntegrityError as exc: session.rollback() - raise HTTPException( - 400, "Topic relevance with the same configuration already exists" - ) + pgcode = getattr(getattr(exc, "orig", None), "pgcode", None) + detail = ( + "Topic relevance with the same configuration already exists" + if pgcode == "23505" + else "Invalid topic relevance configuration" + ) + raise HTTPException(status_code=400, detail=detail) from excAlso applies to: 97-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/crud/topic_relevance.py` around lines 30 - 36, The IntegrityError except blocks around the session.commit calls should only map true unique-key violations to the “same configuration already exists” HTTPException; modify both handlers (the session.commit try/except blocks shown and the similar block at lines 97-103) to catch IntegrityError as e, inspect the DB-specific error (e.g. e.orig or e.orig.pgcode for Postgres) or check for unique-violation indicators (Postgres code '23505' or "unique" in the error string) and only raise the duplicate-config 400 when it is a unique-key violation; otherwise re-raise or raise a different error with the original error details so non-unique integrity failures (nullability/check constraints) are not misreported.
🧹 Nitpick comments (3)
backend/app/alembic/versions/006_added_topic_relevance_config.py (1)
9-18: Consider modernizing type annotations (optional).Ruff suggests using
collections.abc.Sequenceinstead oftyping.Sequenceand usingX | Yunion syntax. This is minor and doesn't affect migration behavior.♻️ Optional modernization
-from typing import Sequence, Union +from collections.abc import Sequence from alembic import op import sqlalchemy as sa # revision identifiers, used by Alembic. revision: str = "006" down_revision = "005" -branch_labels: Union[str, Sequence[str], None] = None -depends_on: Union[str, Sequence[str], None] = None +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/alembic/versions/006_added_topic_relevance_config.py` around lines 9 - 18, Update the type annotations to use modern imports/syntax: replace typing.Sequence with collections.abc.Sequence and use the pipe union syntax for branch_labels and depends_on (e.g., str | Sequence[str] | None) so the module-level annotations for revision/down_revision/branch_labels/depends_on in this Alembic migration reflect current Python typing style; adjust the imports accordingly (import collections.abc.Sequence) and update the annotations on branch_labels and depends_on.backend/app/api/routes/topic_relevance_configs.py (1)
106-118: Consider combining get-then-delete into a single operation.The delete endpoint fetches the object first (
crud.get) and then deletes it. This pattern is acceptable for ensuring the object exists and belongs to the tenant, but could be combined into a single CRUD method that returns a boolean or raises an exception if not found.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/topic_relevance_configs.py` around lines 106 - 118, The delete endpoint delete_topic_relevance_config currently calls topic_relevance_crud.get then topic_relevance_crud.delete; implement a single CRUD operation (e.g., topic_relevance_crud.delete_by_id or delete_for_tenant) that accepts session, id, organization_id and project_id, and either returns a boolean (deleted/not found) or raises the existing NotFound/Authorization error, then update delete_topic_relevance_config to call that new method and handle its return/exception to return the same APIResponse.success_response or an appropriate error; this centralizes the existence/ownership check and deletion into one atomic method and removes the redundant get-then-delete pattern.backend/app/core/validators/topic_relevance.py (1)
5-5: Consider importingCallablefromcollections.abc.The
typing.Callablealias is deprecated in favor ofcollections.abc.Callableas of Python 3.9.🔧 Suggested fix
-from typing import Callable, Optional +from collections.abc import Callable +from typing import Optional🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/topic_relevance.py` at line 5, Update the import in topic_relevance.py to use Callable from collections.abc instead of typing: replace the typing import of Callable with collections.abc.Callable (keep Optional from typing as needed) so code uses the modern Callable reference; modify the import statement that currently mentions Callable to import Callable from collections.abc while leaving other imports like Optional unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 24: Remove the surrounding quotes around the OPENAI_API_KEY placeholder
in .env.example so the line reads an unquoted key assignment (i.e., change
OPENAI_API_KEY="<ADD-KEY>" to OPENAI_API_KEY=<ADD-KEY>) to satisfy dotenv-linter
and keep example formatting consistent; update the OPENAI_API_KEY entry
accordingly while preserving the placeholder text.
In `@backend/app/api/docs/topic_relevance_configs/delete_config.md`:
- Line 8: Replace the inconsistent term "preset" with "configuration" in the API
message — specifically change the string "Topic relevance preset not found in
tenant's scope." to "Topic relevance configuration not found in tenant's scope."
and scan the same documentation for any other occurrences of "preset" to align
all messaging to "configuration" (look for that exact sentence and similar
phrases).
In `@backend/app/api/docs/topic_relevance_configs/get_config.md`:
- Line 8: Replace the inconsistent resource name in the error message string
"Topic relevance preset not found in tenant's scope." with "configuration" so it
matches the endpoint/model naming; update the message in the docs entry for the
topic_relevance_configs get_config response (the string in get_config.md) to
read something like "Topic relevance configuration not found in tenant's scope."
to keep naming consistent across the API.
In `@backend/app/schemas/topic_relevance.py`:
- Around line 29-33: The schema class TopicRelevanceBase declares description:
Optional[str] = None but the DB model has description: str with nullable=False,
causing persistence errors; choose one: either make the schema non-optional by
changing TopicRelevanceBase.description to description: str (remove Optional and
the default None) so it matches the DB model, or update the DB model's
description column to allow NULL (set nullable=True / Optional[str] in the DB
model class) if None should be permitted—apply the change to the symbol
TopicRelevanceBase.description and the corresponding DB model description field
so both types and nullability match.
In `@backend/app/tests/test_topic_relevance_configs_api_integration.py`:
- Around line 172-186: The test
TestUpdateTopicRelevanceConfig.test_update_success sets prompt_schema_version to
2 but the validator function _load_prompt_template only accepts versions with an
existing template (only v1.md exists), causing runtime failure; fix by updating
the test to use prompt_schema_version: 1 (or alternatively add a v2.md prompt
template or add API-level validation that checks _load_prompt_template for the
requested version on update), and ensure the test asserts the expected
prompt_schema_version matches the available template version.
In `@backend/app/tests/test_validate_with_guard.py`:
- Around line 97-102: The mock for GRFailResult used by the test is missing the
outcome attribute which _validate_with_guard() (via add_validator_logs() in
_finalize()) expects and calls .upper() on; update the test's
mock_log.validation_result to include an outcome string (e.g., "fail") on the
mocked FailResult so result.outcome.upper() is valid—adjust the mock creation of
mock_log.validation_result (the MagicMock/spec=GRFailResult) to provide an
outcome attribute.
---
Duplicate comments:
In @.env.test.example:
- Line 24: The OPENAI_API_KEY placeholder currently includes surrounding quotes
which triggers dotenv-linter's QuoteCharacter warning; edit the .env test
example to remove the quotation marks so the line reads OPENAI_API_KEY=<ADD-KEY>
(i.e., update the OPENAI_API_KEY entry to be unquoted) and save the file so the
linter no longer flags QuoteCharacter for that variable.
In `@backend/app/api/API_USAGE.md`:
- Around line 186-187: Update the API_USAGE.md entry that currently states "For
`topic_relevance`, pass `topic_relevance_config_id` only" to document both
supported runtime shapes: (1) passing `topic_relevance_config_id` for a lookup
and (2) providing an inline `configuration` object; note that `guardrails.py`
still resolves `configuration` + `prompt_schema_version` before validator
execution so validators always see both values. Also update the end-to-end note
in API_USAGE.md to reflect both options and mirror this same contract and
wording in backend/app/core/validators/README.md so the route, tests, and
validator documentation are consistent.
In
`@backend/app/core/validators/config/topic_relevance_safety_validator_config.py`:
- Around line 17-23: The build() currently substitutes a blank string when topic
config is missing which hides caller errors; instead, check that
self.configuration (and any preset lookup result) is present and raise a clear
exception (e.g., ValueError or a Config/ValidationError) if not, before
constructing TopicRelevance. Update the build method referenced here (build,
TopicRelevance, resolve_on_fail, self.configuration, self.prompt_schema_version)
to validate presence of topic_config and fail fast rather than passing " " into
TopicRelevance.
In `@backend/app/crud/topic_relevance.py`:
- Around line 30-36: The IntegrityError except blocks around the session.commit
calls should only map true unique-key violations to the “same configuration
already exists” HTTPException; modify both handlers (the session.commit
try/except blocks shown and the similar block at lines 97-103) to catch
IntegrityError as e, inspect the DB-specific error (e.g. e.orig or e.orig.pgcode
for Postgres) or check for unique-violation indicators (Postgres code '23505' or
"unique" in the error string) and only raise the duplicate-config 400 when it is
a unique-key violation; otherwise re-raise or raise a different error with the
original error details so non-unique integrity failures (nullability/check
constraints) are not misreported.
---
Nitpick comments:
In `@backend/app/alembic/versions/006_added_topic_relevance_config.py`:
- Around line 9-18: Update the type annotations to use modern imports/syntax:
replace typing.Sequence with collections.abc.Sequence and use the pipe union
syntax for branch_labels and depends_on (e.g., str | Sequence[str] | None) so
the module-level annotations for revision/down_revision/branch_labels/depends_on
in this Alembic migration reflect current Python typing style; adjust the
imports accordingly (import collections.abc.Sequence) and update the annotations
on branch_labels and depends_on.
In `@backend/app/api/routes/topic_relevance_configs.py`:
- Around line 106-118: The delete endpoint delete_topic_relevance_config
currently calls topic_relevance_crud.get then topic_relevance_crud.delete;
implement a single CRUD operation (e.g., topic_relevance_crud.delete_by_id or
delete_for_tenant) that accepts session, id, organization_id and project_id, and
either returns a boolean (deleted/not found) or raises the existing
NotFound/Authorization error, then update delete_topic_relevance_config to call
that new method and handle its return/exception to return the same
APIResponse.success_response or an appropriate error; this centralizes the
existence/ownership check and deletion into one atomic method and removes the
redundant get-then-delete pattern.
In `@backend/app/core/validators/topic_relevance.py`:
- Line 5: Update the import in topic_relevance.py to use Callable from
collections.abc instead of typing: replace the typing import of Callable with
collections.abc.Callable (keep Optional from typing as needed) so code uses the
modern Callable reference; modify the import statement that currently mentions
Callable to import Callable from collections.abc while leaving other imports
like Optional unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41040bc6-5d90-4137-8de2-3a0a64c99fe2
📒 Files selected for processing (28)
.env.example.env.test.examplebackend/README.mdbackend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/API_USAGE.mdbackend/app/api/docs/guardrails/run_guardrails.mdbackend/app/api/docs/topic_relevance_configs/create_config.mdbackend/app/api/docs/topic_relevance_configs/delete_config.mdbackend/app/api/docs/topic_relevance_configs/get_config.mdbackend/app/api/docs/topic_relevance_configs/list_configs.mdbackend/app/api/docs/topic_relevance_configs/update_config.mdbackend/app/api/main.pybackend/app/api/routes/guardrails.pybackend/app/api/routes/topic_relevance_configs.pybackend/app/core/enum.pybackend/app/core/validators/README.mdbackend/app/core/validators/config/llm_critic_safety_validator_config.pybackend/app/core/validators/config/topic_relevance_safety_validator_config.pybackend/app/core/validators/prompts/topic_relevance/v1.mdbackend/app/core/validators/topic_relevance.pybackend/app/core/validators/validators.jsonbackend/app/crud/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/guardrail_config.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api.pybackend/app/tests/test_topic_relevance_configs_api_integration.pybackend/app/tests/test_validate_with_guard.py
backend/app/tests/test_topic_relevance_configs_api_integration.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/app/tests/test_validate_with_guard.py (2)
250-270: Consider adding a test for precedence when both config_id and inline configuration are provided.The test name suggests inline configuration prevents lookup, but the payload only includes
configuration—it doesn't includetopic_relevance_config_id. To fully verify the precedence behavior (that inline config takes priority over config lookup), consider adding a test where both are provided:def test_topic_relevance_inline_config_takes_precedence_over_config_id(): payload = GuardrailRequest( request_id=str(uuid4()), organization_id=VALIDATOR_TEST_ORGANIZATION_ID, project_id=VALIDATOR_TEST_PROJECT_ID, input="test", validators=[ { "type": "topic_relevance", "topic_relevance_config_id": str(uuid4()), "configuration": "inline config", } ], ) mock_session = MagicMock() with patch("app.api.routes.guardrails.topic_relevance_crud.get") as mock_get: _resolve_topic_relevance_scope(payload, mock_session) assert payload.validators[0].configuration == "inline config" mock_get.assert_not_called()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_validate_with_guard.py` around lines 250 - 270, Add a new unit test to assert precedence when both topic_relevance_config_id and configuration are present: create a GuardrailRequest with validators[0] containing both "topic_relevance_config_id" (UUID) and "configuration" ("inline config"), call _resolve_topic_relevance_scope(payload, mock_session) while patching app.api.routes.guardrails.topic_relevance_crud.get, then assert validators[0].configuration remains "inline config" and that topic_relevance_crud.get was not called; reference _resolve_topic_relevance_scope, payload.validators[0].configuration, and topic_relevance_config_id to locate the relevant logic.
226-231: Minor inconsistency in mock assertion style.This test uses
session=mock_sessionas a keyword argument, while the analogoustest_resolve_ban_list_banned_words_from_ban_list_idtest at line 177 passesmock_sessionas a positional argument. Consider aligning the calling convention for consistency across tests—if the actualtopic_relevance_crud.getandban_list_crud.getfunctions have the same signature, the assertions should match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_validate_with_guard.py` around lines 226 - 231, Update the mock assertion to match the positional-argument style used in the analogous test: call mock_get with mock_session as the first (positional) argument instead of as session=..., keeping the same id and org/project keyword args; locate the assertion in test_validate_with_guard.py that references mock_get, mock_session, validator.topic_relevance_config_id, VALIDATOR_TEST_ORGANIZATION_ID and VALIDATOR_TEST_PROJECT_ID and make it consistent with how test_resolve_ban_list_banned_words_from_ban_list_id asserts ban_list_crud.get to avoid mixed calling conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/tests/test_validate_with_guard.py`:
- Around line 250-270: Add a new unit test to assert precedence when both
topic_relevance_config_id and configuration are present: create a
GuardrailRequest with validators[0] containing both "topic_relevance_config_id"
(UUID) and "configuration" ("inline config"), call
_resolve_topic_relevance_scope(payload, mock_session) while patching
app.api.routes.guardrails.topic_relevance_crud.get, then assert
validators[0].configuration remains "inline config" and that
topic_relevance_crud.get was not called; reference
_resolve_topic_relevance_scope, payload.validators[0].configuration, and
topic_relevance_config_id to locate the relevant logic.
- Around line 226-231: Update the mock assertion to match the
positional-argument style used in the analogous test: call mock_get with
mock_session as the first (positional) argument instead of as session=...,
keeping the same id and org/project keyword args; locate the assertion in
test_validate_with_guard.py that references mock_get, mock_session,
validator.topic_relevance_config_id, VALIDATOR_TEST_ORGANIZATION_ID and
VALIDATOR_TEST_PROJECT_ID and make it consistent with how
test_resolve_ban_list_banned_words_from_ban_list_id asserts ban_list_crud.get to
avoid mixed calling conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4378e59-8d98-443b-885a-1ab45da2dc26
📒 Files selected for processing (1)
backend/app/tests/test_validate_with_guard.py
nishika26
reviewed
Mar 18, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/core/validators/topic_relevance.py`:
- Around line 71-72: The constructor in topic_relevance.py currently raises
ValueError when topic_config is blank, which causes a 500; instead treat a blank
config as a client error by not crashing in construction: replace the raise
ValueError in the topic_config guard with logic that records the invalid state
(e.g., set a flag like self._config_blank or self._invalid_config_reason) and
allow the validator to be constructed; then have the validator's
validate/execute method return a FailResult (or raise a mapped client exception)
describing the missing/blank config so upstream callers (including
topic_relevance_safety_validator_config and the guardrails route) can produce a
4xx response. Mention and update references to topic_config, the constructor in
topic_relevance.py, and the validator's validate/execute method to implement
this controlled FailResult path.
In `@backend/app/schemas/topic_relevance.py`:
- Around line 39-44: The TopicRelevanceUpdate model currently allows explicit
nulls which then get included by model_dump(exclude_unset=True) and cause DB
integrity errors; add validators on TopicRelevanceUpdate for the fields name,
description, prompt_schema_version, configuration and is_active to reject None
(i.e. if the field is present but None, raise ValueError) so that callers cannot
submit explicit nulls—implement per-field `@validator` (or `@field_validator` for
SQLModel/Pydantic v2) functions on the TopicRelevanceUpdate class that check
value is not None when provided and raise a clear error, ensuring the update
path in backend/app/crud/topic_relevance.py continues to only receive omitted
fields or valid values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55ed93df-4b46-482c-9167-202c2fc0a33b
📒 Files selected for processing (3)
backend/app/core/validators/topic_relevance.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api_integration.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/tests/test_topic_relevance_configs_api_integration.py
Collaborator
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (6)
backend/app/api/docs/topic_relevance_configs/delete_config.md (1)
8-8:⚠️ Potential issue | 🟡 MinorUse consistent terminology: "configuration" instead of "preset".
This API is documented as a topic relevance configuration API, but Line 8 says "preset not found". Align the term to avoid confusion.
Proposed fix
-- Topic relevance preset not found in tenant's scope. +- Topic relevance configuration not found in tenant's scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/docs/topic_relevance_configs/delete_config.md` at line 8, Change the inconsistent term in the API docs by replacing the phrase "Topic relevance preset not found in tenant's scope." with "Topic relevance configuration not found in tenant's scope." so the documentation consistently uses "configuration" (locate the string in delete_config.md).backend/app/api/docs/topic_relevance_configs/get_config.md (1)
8-8:⚠️ Potential issue | 🟡 MinorUse consistent resource naming in the not-found message.
Line [8] still uses “preset” while this API surface is “configuration”, which can confuse clients.
Proposed wording fix
-- Topic relevance preset not found in tenant's scope. +- Topic relevance configuration not found in tenant's scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/docs/topic_relevance_configs/get_config.md` at line 8, Update the not-found message to use consistent resource naming by replacing the word "preset" with "configuration" in the string "Topic relevance preset not found in tenant's scope." so it reads "Topic relevance configuration not found in tenant's scope."; locate and update the exact message text in get_config.md (search for the original string) to ensure all occurrences use "configuration" instead of "preset".backend/app/api/docs/topic_relevance_configs/update_config.md (1)
11-11:⚠️ Potential issue | 🟡 MinorAlign not-found wording with the configuration terminology.
Line [11] says “preset”, but this endpoint and payload terminology are “configuration”.
Proposed wording fix
-- Topic relevance preset not found in tenant's scope. +- Topic relevance configuration not found in tenant's scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/docs/topic_relevance_configs/update_config.md` at line 11, Replace the wording "Topic relevance preset not found in tenant's scope." with "Topic relevance configuration not found in tenant's scope." in the update_config.md documentation (and any matching error text for the topic_relevance_configs endpoint/payload) so the term "configuration" is used consistently with the endpoint and payload terminology; search for the exact string "Topic relevance preset not found in tenant's scope." and update it to use "configuration" wherever it appears.backend/app/api/API_USAGE.md (1)
186-187:⚠️ Potential issue | 🟡 MinorDocument the supported inline
configurationpath fortopic_relevance.Line 186 and Line 415 still describe runtime usage as config-id-only, but
backend/app/api/routes/guardrails.pyonly does a preset lookup whentopic_relevance_config_idis present. Inlineconfiguration(and optionalprompt_schema_version) is also accepted, so the current wording makes a supported request shape look invalid.Also applies to: 415-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/API_USAGE.md` around lines 186 - 187, Update the API_USAGE.md wording for the topic_relevance endpoint to document that the endpoint accepts either a preset lookup via topic_relevance_config_id or an inline configuration object (and optional prompt_schema_version) because routes/guardrails.py resolves configuration + prompt_schema_version before validator execution; specifically mention topic_relevance accepts an inline "configuration" field and optional "prompt_schema_version" as alternatives to topic_relevance_config_id so consumers know both request shapes are supported.backend/app/core/validators/config/topic_relevance_safety_validator_config.py (1)
17-23:⚠️ Potential issue | 🟠 MajorReject missing topic scope before constructing
TopicRelevance.Line 19 converts a missing configuration into
" ", which only fails later insideTopicRelevance. Since the resolver now skips lookup whentopic_relevance_config_idis absent, a payload with neither field reaches guard construction before failing. Fail fast here with a clear message instead of passing a whitespace sentinel.💡 Minimal hardening
def build(self): + if not self.configuration or not self.configuration.strip(): + raise ValueError( + "Topic relevance configuration is required. " + "Provide inline `configuration` or resolve `topic_relevance_config_id` first." + ) return TopicRelevance( - topic_config=self.configuration or " ", + topic_config=self.configuration, prompt_schema_version=self.prompt_schema_version or 1, llm_callable=self.llm_callable, on_fail=self.resolve_on_fail(), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/config/topic_relevance_safety_validator_config.py` around lines 17 - 23, In build(), do not substitute a whitespace sentinel for a missing topic scope; instead validate that self.configuration (or the topic scope field) is present and raise a clear exception (e.g. ValueError) before constructing TopicRelevance. Update the build() method to check self.configuration (and/or the absence of topic_relevance_config_id if relevant) and fail fast with a descriptive error message rather than passing " " into TopicRelevance; keep the existing prompt_schema_version fallback and existing llm_callable and on_fail usage (resolve_on_fail()).backend/app/crud/topic_relevance.py (1)
30-36:⚠️ Potential issue | 🟠 MajorOnly use the duplicate-config message for the actual unique-constraint case.
Line 32 and Line 99 currently map every
IntegrityErrorto"Topic relevance with the same configuration already exists". That will misreport unrelated integrity failures and make real data issues much harder to diagnose; inspect the underlying constraint/error code before rewriting the exception.Also applies to: 97-103
🧹 Nitpick comments (4)
backend/app/core/validators/config/llm_critic_safety_validator_config.py (1)
10-10: Consider using a more specific type formetrics.The
dicttype is loosely typed. If LLMCritic expects a specific structure (e.g.,Dict[str, str]or a dedicated schema), using a more precise type would improve validation and developer experience.Example improvement
- metrics: dict + metrics: Dict[str, Any] # or a more specific type if knownAdd the import:
from typing import Any, Dict, Literal🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/validators/config/llm_critic_safety_validator_config.py` at line 10, Change the loose "metrics: dict" field to a more specific typing to improve validation and IDE support; update the type of the metrics attribute (e.g., to Dict[str, Any] or Dict[str, str] or a custom schema type) and add the necessary imports from typing (e.g., Dict, Any, Literal if using fixed keys), then adjust any usages in LLMCritic or related validation code to match the new signature so consumers and validators expect the precise structure.backend/app/tests/test_topic_relevance_configs_api.py (1)
61-74: Strengthen create test by asserting tenant-scoped CRUD call arguments.Right now this test validates only the returned data. Please also assert the
createcall includesorganization_idandproject_idso tenant isolation regressions are caught.Proposed test hardening
def test_create_calls_crud( mock_session, create_payload, sample_topic_relevance, auth_context ): with patch("app.api.routes.topic_relevance_configs.topic_relevance_crud") as crud: crud.create.return_value = sample_topic_relevance result = create_topic_relevance_config( payload=create_payload, session=mock_session, auth=auth_context, ) + crud.create.assert_called_once_with( + mock_session, + create_payload, + TOPIC_RELEVANCE_TEST_ORGANIZATION_ID, + TOPIC_RELEVANCE_TEST_PROJECT_ID, + ) assert result.data == sample_topic_relevance🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/test_topic_relevance_configs_api.py` around lines 61 - 74, The test test_create_calls_crud should also assert that topic_relevance_crud.create was called with tenant-scoped IDs; update the test that patches topic_relevance_crud in test_create_calls_crud to assert crud.create.assert_called_with (..., organization_id=auth_context.organization_id, project_id=auth_context.project_id, session=mock_session) (include the original payload arguments like create_payload and any other kwargs used by create_topic_relevance_config) so the call from create_topic_relevance_config is verified to include organization_id and project_id from auth_context; reference create_topic_relevance_config, topic_relevance_crud.create, create_payload, and auth_context to locate and add the assertion.backend/app/schemas/topic_relevance.py (1)
20-26: Consider addingmax_lengthtoTopicConfiguration.The
TopicConfigurationtype alias enforcesmin_length=1but has no upper bound. Given the DB migration stores this in aTextcolumn, extremely large configurations could cause performance issues or exceed memory limits during validation/processing. Consider adding a reasonablemax_lengthconstraint.Suggested change
TopicConfiguration = Annotated[ str, StringConstraints( strip_whitespace=True, min_length=1, + max_length=10000, # Adjust based on expected configuration size ), ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/schemas/topic_relevance.py` around lines 20 - 26, TopicConfiguration currently allows arbitrarily large strings (only min_length=1); add a sensible upper bound by updating the StringConstraints for TopicConfiguration to include max_length (e.g., max_length=10000 or another agreed limit) so very large configs are rejected during validation; locate the TopicConfiguration alias in topic_relevance.py and add the max_length param to the existing StringConstraints tuple and adjust any tests/validators that rely on the previous unlimited size.backend/app/api/routes/topic_relevance_configs.py (1)
101-118: Consider a typed response model for delete.The delete endpoint uses
APIResponse[dict]which is loose. A small typed model (e.g.,DeleteResponsewith amessage: strfield) would provide better API documentation and type safety.Optional: typed delete response
# In schemas/topic_relevance.py or a shared module class DeleteResponse(SQLModel): message: str # In routes `@router.delete`( "/{id}", description=load_description("topic_relevance_configs/delete_config.md"), response_model=APIResponse[DeleteResponse], ) def delete_topic_relevance_config(...) -> APIResponse[DeleteResponse]: ... return APIResponse.success_response(data=DeleteResponse(message="Config deleted successfully"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/topic_relevance_configs.py` around lines 101 - 118, The delete endpoint uses a loose APIResponse[dict]; create a small typed response model (e.g., class DeleteResponse(SQLModel) with message: str) in your schemas (e.g., schemas/topic_relevance.py or a shared schemas module), then update the route decorator and function signature in delete_topic_relevance_config to use APIResponse[DeleteResponse] and return APIResponse.success_response(data=DeleteResponse(message="Config deleted successfully")); keep the existing call to topic_relevance_crud.delete(session, obj) untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 24: Add an OPENAI_API_KEY field to the Settings pydantic model so the
environment variable is read and validated at startup: inside the Settings class
(the same class referenced by load_environment()), declare a field like
OPENAI_API_KEY: Optional[str] = None (or str without a default if you want it
required) and/or use pydantic Field(..., env="OPENAI_API_KEY") so pydantic picks
up the .env value; if your app requires the key when using LLM features, add a
simple validator in Settings (or conditional check in its __post_init__ /
root_validator) that raises a clear error when OPENAI_API_KEY is missing while
LLM usage is enabled.
In @.env.test.example:
- Line 24: The CI is leaving OPENAI_API_KEY set to the placeholder "<ADD-KEY>"
which can break validators like TopicRelevance and LLMCritic that call
llm_callable; either update the CI workflow step that copies .env.test.example
to actually replace OPENAI_API_KEY from the CI secret (add a sed/replace using
secrets.OPENAI_API_KEY) or add a test fixture in conftest.py that mocks OpenAI
client initialization and/or intercepts llm_callable so TopicRelevance and
LLMCritic never attempt real network calls; modify whichever you choose
consistently so tests never run with the "<ADD-KEY>" placeholder.
In `@backend/app/alembic/versions/006_added_topic_relevance_config.py`:
- Line 28: The description field currently lacks the 500-character constraint
despite the MAX_TOPIC_RELEVANCE_DESCRIPTION_LENGTH constant; update all three
layers to enforce it: in the migration (006_added_topic_relevance_config.py)
change sa.Column("description", sa.String(), nullable=False) to use
sa.String(length=MAX_TOPIC_RELEVANCE_DESCRIPTION_LENGTH) (or hardcode 500 if
constants unavailable in migration), in the SQLModel/Pydantic model where
description is declared (description: str = Field(...)) add
max_length=MAX_TOPIC_RELEVANCE_DESCRIPTION_LENGTH, and in the request/response
schema replace the plain str type with a constrained Annotated type (similar to
TopicsName, e.g., TopicsDescription) that enforces max_length; if you prefer not
to enforce, remove the unused MAX_TOPIC_RELEVANCE_DESCRIPTION_LENGTH constant
instead.
In `@backend/app/api/docs/topic_relevance_configs/create_config.md`:
- Around line 20-22: The fenced code block containing "This assistant only
answers questions about maternal health and pregnancy care..." is missing a
language identifier; update the triple-backtick fence to include a language
(e.g., add ```text) so the block becomes ```text ... ```, which will satisfy
markdownlint MD040 and keep create_config.md lint-clean; locate the fenced block
by searching for that exact sentence and add the language token to the opening
fence.
In `@backend/app/api/routes/guardrails.py`:
- Line 56: The request log is being created before the preset-backed lookup in
_resolve_topic_relevance_scope, so if that lookup fails the created row is never
finalized by _validate_with_guard; move the preset resolution earlier (call
_resolve_topic_relevance_scope(payload, session) before you call create()) so
all validations run before the request log is inserted, or wrap the existing
_resolve_topic_relevance_scope call in a try/except that catches lookup errors
and updates the created request log (the row produced by create()) to a
finalized/error state before returning; reference
_resolve_topic_relevance_scope, _validate_with_guard, and the create() call when
making the change.
In `@backend/app/core/validators/README.md`:
- Around line 300-308: Update the README to stop saying
topic_relevance_config_id is required: clarify that the runtime accepts either a
topic_relevance_config_id OR an inline configuration object (the code will only
perform a DB lookup when topic_relevance_config_id is present), mark presets as
the preferred path (not required), and change the Notes section to state runtime
validation requires either topic_relevance_config_id or inline configuration;
reference the guardrails route/handler and the tenant Topic Relevance Config
APIs when noting where config resolution occurs.
In `@backend/app/core/validators/topic_relevance.py`:
- Around line 128-131: The current except block returns a FailResult that
includes the raw exception text (f"LLM critic returned an invalid response:
{e}"), which leaks provider/internal details; update the except handler in
topic_relevance validator to return a stable, generic user-facing error_message
(e.g., "LLM critic returned an invalid response") and move the raw exception to
server logs instead (use the module logger and logger.exception or logger.error
with the exception) while keeping the FailResult type and field names
(FailResult, error_message) unchanged.
In `@backend/app/models/config/topic_relevance.py`:
- Around line 9-10: The ORM model TopicRelevance is missing the DB-level
uniqueness rule; add a __table_args__ on class TopicRelevance that includes a
sqlalchemy UniqueConstraint matching the migration:
UniqueConstraint("organization_id", "project_id", "prompt_schema_version",
"configuration") so the model enforces the same uniqueness as the migration;
update imports if needed to import UniqueConstraint from sqlalchemy and ensure
the constraint is placed on the TopicRelevance class definition.
---
Duplicate comments:
In `@backend/app/api/API_USAGE.md`:
- Around line 186-187: Update the API_USAGE.md wording for the topic_relevance
endpoint to document that the endpoint accepts either a preset lookup via
topic_relevance_config_id or an inline configuration object (and optional
prompt_schema_version) because routes/guardrails.py resolves configuration +
prompt_schema_version before validator execution; specifically mention
topic_relevance accepts an inline "configuration" field and optional
"prompt_schema_version" as alternatives to topic_relevance_config_id so
consumers know both request shapes are supported.
In `@backend/app/api/docs/topic_relevance_configs/delete_config.md`:
- Line 8: Change the inconsistent term in the API docs by replacing the phrase
"Topic relevance preset not found in tenant's scope." with "Topic relevance
configuration not found in tenant's scope." so the documentation consistently
uses "configuration" (locate the string in delete_config.md).
In `@backend/app/api/docs/topic_relevance_configs/get_config.md`:
- Line 8: Update the not-found message to use consistent resource naming by
replacing the word "preset" with "configuration" in the string "Topic relevance
preset not found in tenant's scope." so it reads "Topic relevance configuration
not found in tenant's scope."; locate and update the exact message text in
get_config.md (search for the original string) to ensure all occurrences use
"configuration" instead of "preset".
In `@backend/app/api/docs/topic_relevance_configs/update_config.md`:
- Line 11: Replace the wording "Topic relevance preset not found in tenant's
scope." with "Topic relevance configuration not found in tenant's scope." in the
update_config.md documentation (and any matching error text for the
topic_relevance_configs endpoint/payload) so the term "configuration" is used
consistently with the endpoint and payload terminology; search for the exact
string "Topic relevance preset not found in tenant's scope." and update it to
use "configuration" wherever it appears.
In
`@backend/app/core/validators/config/topic_relevance_safety_validator_config.py`:
- Around line 17-23: In build(), do not substitute a whitespace sentinel for a
missing topic scope; instead validate that self.configuration (or the topic
scope field) is present and raise a clear exception (e.g. ValueError) before
constructing TopicRelevance. Update the build() method to check
self.configuration (and/or the absence of topic_relevance_config_id if relevant)
and fail fast with a descriptive error message rather than passing " " into
TopicRelevance; keep the existing prompt_schema_version fallback and existing
llm_callable and on_fail usage (resolve_on_fail()).
---
Nitpick comments:
In `@backend/app/api/routes/topic_relevance_configs.py`:
- Around line 101-118: The delete endpoint uses a loose APIResponse[dict];
create a small typed response model (e.g., class DeleteResponse(SQLModel) with
message: str) in your schemas (e.g., schemas/topic_relevance.py or a shared
schemas module), then update the route decorator and function signature in
delete_topic_relevance_config to use APIResponse[DeleteResponse] and return
APIResponse.success_response(data=DeleteResponse(message="Config deleted
successfully")); keep the existing call to topic_relevance_crud.delete(session,
obj) untouched.
In `@backend/app/core/validators/config/llm_critic_safety_validator_config.py`:
- Line 10: Change the loose "metrics: dict" field to a more specific typing to
improve validation and IDE support; update the type of the metrics attribute
(e.g., to Dict[str, Any] or Dict[str, str] or a custom schema type) and add the
necessary imports from typing (e.g., Dict, Any, Literal if using fixed keys),
then adjust any usages in LLMCritic or related validation code to match the new
signature so consumers and validators expect the precise structure.
In `@backend/app/schemas/topic_relevance.py`:
- Around line 20-26: TopicConfiguration currently allows arbitrarily large
strings (only min_length=1); add a sensible upper bound by updating the
StringConstraints for TopicConfiguration to include max_length (e.g.,
max_length=10000 or another agreed limit) so very large configs are rejected
during validation; locate the TopicConfiguration alias in topic_relevance.py and
add the max_length param to the existing StringConstraints tuple and adjust any
tests/validators that rely on the previous unlimited size.
In `@backend/app/tests/test_topic_relevance_configs_api.py`:
- Around line 61-74: The test test_create_calls_crud should also assert that
topic_relevance_crud.create was called with tenant-scoped IDs; update the test
that patches topic_relevance_crud in test_create_calls_crud to assert
crud.create.assert_called_with (...,
organization_id=auth_context.organization_id,
project_id=auth_context.project_id, session=mock_session) (include the original
payload arguments like create_payload and any other kwargs used by
create_topic_relevance_config) so the call from create_topic_relevance_config is
verified to include organization_id and project_id from auth_context; reference
create_topic_relevance_config, topic_relevance_crud.create, create_payload, and
auth_context to locate and add the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3895028c-5754-4365-908d-456797508ddd
📒 Files selected for processing (28)
.env.example.env.test.examplebackend/README.mdbackend/app/alembic/versions/006_added_topic_relevance_config.pybackend/app/api/API_USAGE.mdbackend/app/api/docs/guardrails/run_guardrails.mdbackend/app/api/docs/topic_relevance_configs/create_config.mdbackend/app/api/docs/topic_relevance_configs/delete_config.mdbackend/app/api/docs/topic_relevance_configs/get_config.mdbackend/app/api/docs/topic_relevance_configs/list_configs.mdbackend/app/api/docs/topic_relevance_configs/update_config.mdbackend/app/api/main.pybackend/app/api/routes/guardrails.pybackend/app/api/routes/topic_relevance_configs.pybackend/app/core/enum.pybackend/app/core/validators/README.mdbackend/app/core/validators/config/llm_critic_safety_validator_config.pybackend/app/core/validators/config/topic_relevance_safety_validator_config.pybackend/app/core/validators/prompts/topic_relevance/v1.mdbackend/app/core/validators/topic_relevance.pybackend/app/core/validators/validators.jsonbackend/app/crud/topic_relevance.pybackend/app/models/config/topic_relevance.pybackend/app/schemas/guardrail_config.pybackend/app/schemas/topic_relevance.pybackend/app/tests/test_topic_relevance_configs_api.pybackend/app/tests/test_topic_relevance_configs_api_integration.pybackend/app/tests/test_validate_with_guard.py
|
|
||
| DOCKER_IMAGE_BACKEND=kaapi-guardrails-backend | ||
|
|
||
| OPENAI_API_KEY="<ADD-KEY>" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if tests mock OpenAI or require real API key
echo "=== Checking for OpenAI mocking in tests ==="
rg -n -C3 'mock.*openai|patch.*openai|monkeypatch.*openai' --type=py --glob='**/test*.py'
echo -e "\n=== Checking if OPENAI_API_KEY is referenced in test files ==="
rg -n -C3 'OPENAI_API_KEY' --type=py --glob='**/test*.py'
echo -e "\n=== Checking if new validators use OpenAI client ==="
rg -n -C3 'OpenAI\(|openai\.' --type=py backend/app/core/validators/
echo -e "\n=== Checking CI workflow for OPENAI_API_KEY replacement ==="
rg -n 'OPENAI_API_KEY' .github/workflows/Repository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 301
🏁 Script executed:
# Check CI workflow directly
cat .github/workflows/continuous_integration.yml
# Search for the validators mentioned
fd -type f -name "*.py" | xargs grep -l "Topic.*Relevance\|LLM.*Critic" 2>/dev/null
# Check validators directory structure
find . -type d -name "validators" 2>/dev/null | head -20
# Search for OpenAI imports more broadly
rg -n "from openai\|import openai" --type=py
# Check environment variable usage in test setup
rg -n "OPENAI_API_KEY\|os\.environ\|getenv" --type=py backend/app/core/validators/ 2>/dev/null || echo "Path not found, checking alternatives"
# List what's in core directory
find backend -type d -name "core" 2>/dev/nullRepository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 2664
🏁 Script executed:
# List validators in core directory
ls -la backend/app/core/validators/
# List test validators
ls -la backend/app/tests/validators/
# Search for OpenAI usage more carefully (fix the type flag)
rg -n "from openai\|import openai\|OpenAI" --glob='**/*.py' backend/app/
# Check conftest and test setup files
find backend -name "conftest.py" -o -name "pytest.ini" -o -name "pyproject.toml" | head -10
# Check if there's test configuration
cat backend/pyproject.toml 2>/dev/null | grep -A 20 "\[tool.pytest"Repository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 1240
🏁 Script executed:
# Check topic_relevance.py implementation
cat backend/app/core/validators/topic_relevance.py
# Check conftest for mocking setup
cat backend/app/tests/conftest.py
# Search for LLM Critic validator
find backend -name "*llm*" -o -name "*critic*" 2>/dev/null
# Check if tests are mocking or patching openai
rg "mock\|patch\|monkeypatch" backend/app/tests/ --glob='*.py' -A 3 -B 1
# Check environment handling in tests
cat backend/app/tests/pytest.iniRepository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 9039
🏁 Script executed:
# Search for LLM Critic validator
find backend -name "*critic*" -type f
# Check if any test files import or use TopicRelevance
rg "TopicRelevance\|topic-relevance\|topic_relevance" backend/app/tests/ --glob='*.py'
# Check validators.json to see what's registered
cat backend/app/core/validators/validators.json
# Check if topic_relevance is imported in validators __init__
cat backend/app/core/validators/__init__.py 2>/dev/null || echo "No __init__.py"
# Check what happens when validator is missing OpenAI key - look for error handling
rg "OPENAI_API_KEY" backend/ --glob='*.py' -B 2 -A 2
# Check if there are integration tests or if validators are imported anywhere
rg "from.*validators import\|import.*validators" backend/app/ --glob='*.py' | head -20Repository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 943
🏁 Script executed:
# Check the llm_critic_safety_validator_config.py file
cat backend/app/core/validators/config/llm_critic_safety_validator_config.py
# Check for any usage of topic_relevance or llm_critic validators in actual code
rg "topic_relevance|llm_critic" backend/app/ --glob='*.py' --glob='*.json' -B 2 -A 2 | head -100
# Check if validators are instantiated/used during tests
rg "ValidatorConfig\|ValidatorPayload\|validate\(" backend/app/tests/ --glob='*.py' -B 1 -A 1 | head -80
# Check the actual seed_data to see what validators are being tested
find backend -name "seed_data.py" -exec cat {} \;Repository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 10613
🏁 Script executed:
# Check what _validate_with_guard actually does - does it instantiate validators?
cat backend/app/tests/test_validate_with_guard.py | head -200
# Check the actual validation route implementation
cat backend/app/api/routes/guardrails.py | grep -A 50 "def _validate_with_guard"
# Check seed_data.json to see what validators are seeded
cat backend/app/tests/seed_data.jsonRepository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 10410
🏁 Script executed:
# Check if TopicRelevance or LLMCritic validators are ever instantiated without mocking
rg "TopicRelevance\(|LLMCritic\(" backend/app/tests/ --glob='*.py'
# Check the build_guard function to understand how validators are instantiated
find backend -name "*.py" -exec grep -l "def build_guard" {} \;
# Check if the validators are tested directly
find backend -path "*/tests/*" -name "*topic*" -o -name "*critic*" 2>/dev/null
# Check how Guard is built and if validators are instantiated
rg "Guard\(" backend/app/ --glob='*.py' -B 3 -A 3 | head -150Repository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 4487
🏁 Script executed:
# Check the test_build_guard.py to see if TopicRelevance is tested
cat backend/app/tests/test_build_guard.py
# Check test_topic_relevance_configs_api_integration.py
cat backend/app/tests/test_topic_relevance_configs_api_integration.py
# Check if any test actually calls build_guard without mocking it
rg "build_guard" backend/app/tests/ --glob='*.py' | grep -v "patch\|Mock"
# Check guardrail_controller.py for more context
cat backend/app/core/guardrail_controller.pyRepository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 13104
🏁 Script executed:
# Check if there's an llm_critic.py validator file (as opposed to the config class)
find backend -name "*llm_critic*" -type f
# Check what's in the config file more carefully
cat backend/app/core/validators/config/llm_critic_safety_validator_config.py
# Verify which validators are actually defined locally vs imported from hub
ls -la backend/app/core/validators/*.py
# Check if any non-mocked test actually instantiates validators
rg "\.build\(\)" backend/app/tests/ --glob='*.py' -B 2 -A 2
# Look for any test configuration that uses TopicRelevance without mocking
rg "TopicRelevance|llm_critic" backend/app/tests/ --glob='*.py' -B 3 -A 3 | head -200Repository: ProjectTech4DevAI/kaapi-guardrails
Length of output: 8117
Update the CI workflow to populate OPENAI_API_KEY from secrets or mock OpenAI calls in tests.
The TopicRelevance and LLMCritic validators require a real OpenAI API key to function (via llm_callable). The CI workflow copies .env.test.example to .env.test and replaces AUTH_TOKEN and GUARDRAILS_HUB_API_KEY via sed, but does not replace OPENAI_API_KEY. Tests currently don't fail because build_guard is mocked everywhere; however, this is fragile. If these validators are instantiated without mocking—or if test coverage expands—CI will fail with the placeholder value "<ADD-KEY>".
Either:
- Add a
sedcommand to populateOPENAI_API_KEYfrom asecrets.OPENAI_API_KEYsecret, or - Implement OpenAI mocking in test fixtures (e.g., in conftest.py) to intercept client initialization.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.test.example at line 24, The CI is leaving OPENAI_API_KEY set to the
placeholder "<ADD-KEY>" which can break validators like TopicRelevance and
LLMCritic that call llm_callable; either update the CI workflow step that copies
.env.test.example to actually replace OPENAI_API_KEY from the CI secret (add a
sed/replace using secrets.OPENAI_API_KEY) or add a test fixture in conftest.py
that mocks OpenAI client initialization and/or intercepts llm_callable so
TopicRelevance and LLMCritic never attempt real network calls; modify whichever
you choose consistently so tests never run with the "<ADD-KEY>" placeholder.
nishika26
approved these changes
Mar 19, 2026
nishika26
reviewed
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Target issue is #72.
Explain the motivation for making this change. What existing problem does the pull request solve?
Through this PR, we are adding a new set of validators.
backend/app/models/config/topic_relevance.py.backend/app/core/validators/prompts/topic_relevance/v1.mdfile. However, with more languages and more NGOs, we will have more prompts. These prompts can be configured via theprompt_version field.configurationfield which is basically a text blob of each topic and their definition. Example -Currently, the only customization we are offering in the LLM prompt is the configuration field. However, we can also make it more customization by making it a JSON. Example -
Scope will be only a small part of the prompt and we can append the remaining keys to the end of the prompt to give more customization to the non-profits. Whether this is needed or not remains to be seen. We are counting on Tech4Dev to assist us with this decision.
The prompt version combined with the configuration will be the final system prompt for each NGO.
backend/app/api/routes/guardrails.py, I have updated logic to resolve if the validator is of Topic Relevance type. The prompt version and configuration will get updated and can be used after this.backend/app/api/routes/topic_relevance_configs.pycontains 5 set of new APIs related to adding / deleting / updating / listing topic relevance configs. Similar to the Ban Lists APIs, we are following the multitenant authentication where we make a call to kaapi-backend for validation.backend/app/api/docs/guardrails/run_guardrails.mdandbackend/app/api/API_USAGE.md.backend/app/tests/test_topic_relevance_configs_api.pyand integration tests inbackend/app/tests/test_topic_relevance_configs_api_integration.py.Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit