Guardrails UI: Configure and list validators#116
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 a Guardrails feature: client Guardrails UI and components, new Guardrails API proxy routes and guardrailsClient helper, prompt-editor integration for guardrails, validator types/catalog, new UI primitives/icons, nav entries, and a small Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Frontend as Guardrails UI (client)
participant VerifyAPI as /api/apikeys/verify
participant ProxyAPI as /api/guardrails/*
participant GuardrailsSvc as Guardrails Backend
User->>Frontend: Open Guardrails page
Frontend->>VerifyAPI: GET /api/apikeys/verify (X-API-KEY)
VerifyAPI->>GuardrailsSvc: Forward verify request
GuardrailsSvc-->>VerifyAPI: Return org/project
VerifyAPI-->>Frontend: Return org/project
Frontend->>ProxyAPI: GET /api/guardrails/validators/configs?org... (X-API-KEY or Authorization)
ProxyAPI->>GuardrailsSvc: Forward with auth
GuardrailsSvc-->>ProxyAPI: Return validator configs
ProxyAPI-->>Frontend: Return configs
User->>Frontend: Create/Update/Delete config
Frontend->>ProxyAPI: POST/PATCH/DELETE (includes org/project, X-API-KEY or Authorization)
ProxyAPI->>GuardrailsSvc: Forward change request
GuardrailsSvc-->>ProxyAPI: Return result
ProxyAPI-->>Frontend: Return saved/deleted response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (5)
app/api/guardrails/validators/configs/route.ts (1)
4-18: Consider extracting the shared guardrails proxy helper.
getAuthHeader, query forwarding, and the sametry/catch -> NextResponseshape now exist inapp/api/guardrails/route.ts, this file, andapp/api/guardrails/validators/configs/[config_id]/route.ts. Pulling that into one helper will keep auth/error behavior from drifting as more guardrails endpoints land.Also applies to: 20-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/guardrails/validators/configs/route.ts` around lines 4 - 18, Extract the repeated Guardrails proxy logic (auth header creation, query param forwarding, and the try/catch → NextResponse error/response shape) into a single helper function (e.g., proxyToGuardrails or forwardGuardrailsRequest) and replace duplicates in getAuthHeader, buildEndpoint and the route handlers in app/api/guardrails/route.ts, app/api/guardrails/validators/configs/route.ts, and app/api/guardrails/validators/configs/[config_id]/route.ts; the helper should accept the incoming NextRequest and a target path (or accept optional overrides), build the auth header (or accept the token), forward searchParams, perform fetch to `/api/v1/guardrails...`, and return a normalized NextResponse inside a try/catch so all endpoints reuse identical auth/error behavior.app/(main)/guardrails/page.tsx (2)
134-150: Add confirmation before deleting a configuration.The delete action immediately sends the DELETE request without asking the user to confirm. Consider adding a confirmation dialog to prevent accidental deletions.
♻️ Add confirmation dialog
const handleDeleteConfig = async (configId: string) => { if (!configsQueryString) return; + if (!window.confirm("Are you sure you want to delete this configuration?")) { + return; + } try {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(main)/guardrails/page.tsx around lines 134 - 150, Add a user confirmation step to handleDeleteConfig so deletion is not sent immediately: before performing the fetch call (and before checking configsQueryString), prompt the user (e.g., window.confirm or your app modal) to confirm deletion of the configId; if the user cancels, return early, otherwise proceed with the existing fetch, toast handling, handleClearForm() if selectedSavedConfig?.id matches configId, and fetchSavedConfigs(); keep all existing error handling intact.
77-77: Missing dependency in useEffect.The
toastobject is used inside the effect but not listed in the dependency array. Whiletoastmethods are likely stable references, ESLint's exhaustive-deps rule may flag this.♻️ Add toast to dependency array
- }, [isHydrated, activeKey?.key]); + }, [isHydrated, activeKey?.key, toast]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(main)/guardrails/page.tsx at line 77, The useEffect that depends on isHydrated and activeKey?.key also uses the toast object but doesn't include it in the dependency array; update the effect's dependency array to include toast (or, if you intentionally want to ignore it, explicitly silence exhaustive-deps with a well-documented // eslint-disable-next-line comment). Locate the useEffect that references isHydrated, activeKey?.key and toast in page.tsx and either add toast to the dependencies or add a concise eslint-disable comment and explanation to prevent false positives.app/components/guardrails/types.ts (1)
39-44: Minor:formatValidatorNamemay produce unexpected output for edge cases.The function works well for snake_case inputs but consider edge cases:
- Empty string returns empty string (acceptable)
- Single word like
"pii"becomes"Pii"(may want"PII")- Already formatted
"LLM Critic"becomes"Llm Critic"This is acceptable if input is always guaranteed to be snake_case validator types from the API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/guardrails/types.ts` around lines 39 - 44, The formatValidatorName function can mangle already-formatted or acronym words; update formatValidatorName to handle edge cases by: if the input contains spaces, return it unchanged (preserve "LLM Critic"); otherwise split on '_' and for each token preserve existing ALL CAPS tokens, convert short all-lowercase tokens (length <= 3, e.g., "pii") to UPPERCASE, and title-case other tokens as currently done; implement these checks inside formatValidatorName to avoid breaking snake_case handling while preserving acronyms and pre-formatted names.app/components/prompt-editor/ConfigEditorPane.tsx (1)
48-69: Consider adding error feedback for fetch failures.The
catchblock on line 67 silently sets validators to an empty array. While this prevents UI crashes, the user won't know if the fetch failed due to a network issue or misconfiguration. Consider accepting an optionalonErrorcallback or logging the error.♻️ Optional: Add error callback
function GuardrailsSection({ label, guardrails, onChange, apiKey, queryString, stage, + onError, }: { label: string; guardrails: GuardrailRef[]; onChange: (refs: GuardrailRef[]) => void; apiKey: string; queryString: string | null; stage: "input" | "output"; + onError?: (message: string) => void; }) { // ... .catch(() => { setValidators([]); + onError?.("Failed to load validators"); })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/prompt-editor/ConfigEditorPane.tsx` around lines 48 - 69, The fetch in the useEffect (which uses queryString and apiKey) currently swallows errors in the .catch(() => setValidators([])) block; update this to accept an optional onError callback prop (or at least log the error) and call it with the caught Error before falling back to setValidators([]) and setLoading(false); locate the fetch block in ConfigEditorPane.tsx (the useEffect where setValidators and setLoading are used) and modify the catch to .catch((err) => { if (onError) onError(err); else console.error('Failed to load guardrail validators', err); setValidators([]); }) so callers can react to failures and you still clear loading state.
🤖 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:
- Around line 3-5: Normalize the new environment entries by removing spaces
around the equals sign and trimming trailing whitespace so they conform to
dotenv syntax and pass dotenv-linter; specifically update the GUARDRAILS_URL and
GUARDRAILS_TOKEN lines (remove spaces before/after '=' and delete any trailing
spaces on GUARDRAILS_TOKEN) so the variables are defined as VAR=VALUE with no
extra whitespace.
In `@app/`(main)/configurations/prompt-editor/page.tsx:
- Around line 297-302: applyConfig currently rebuilds currentConfigBlob using
only the completion field, which drops existing input_guardrails and
output_guardrails when saving; update applyConfig to preserve those fields by
either spreading the existing currentConfigBlob or explicitly including
input_guardrails: currentConfigBlob.input_guardrails (when truthy) and
output_guardrails: currentConfigBlob.output_guardrails (when truthy) alongside
completion so loading an existing config doesn't remove guardrails on save.
In `@app/`(main)/guardrails/page.tsx:
- Around line 168-177: The PATCH payload currently strips validator-specific
fields by building body when isUpdate is true; update the body construction in
page.tsx (the isUpdate ? {...} branch) to include the dynamic validator fields
from configValues (e.g., ban_list_id, entity_types, and any other
validator-specific keys you support) so edits persist, or alternatively disable
those dynamic inputs when editing by gating their rendering when isUpdate is
true; adjust the code around the body variable and the form rendering to either
include the dynamic keys in the PATCH payload or hide/disable the dynamic fields
during updates.
In `@app/api/guardrails/ban_lists/`[ban_list_id]/route.ts:
- Line 10: The code interpolates ban_list_id directly into upstream paths (calls
to guardrailsClient) which breaks when IDs contain reserved URL characters;
update every place that builds the path (the guardrailsClient calls in route.ts)
to use an encoded ID by wrapping ban_list_id with encodeURIComponent before
interpolation so all three occurrences (the calls that construct
`/api/v1/guardrails/ban_lists/${ban_list_id}`) use the encoded value.
In `@app/api/guardrails/ban_lists/route.ts`:
- Around line 16-29: The POST handler currently treats JSON parse failures from
request.json() as 500; update the POST function to detect JSON parsing errors
(e.g., catch SyntaxError or check error.message from request.json()) and return
NextResponse.json({ error: "Malformed JSON" }, { status: 400 }) for those cases,
while preserving the existing 500 response for other unexpected errors;
specifically locate the POST function and modify the try/catch so parsing errors
from request.json() result in a 400 response and other errors still return 500.
In `@app/api/guardrails/validators/configs/`[config_id]/route.ts:
- Around line 57-62: The handlers (GET, PATCH, DELETE) call guardrailsClient and
always return NextResponse.json(data, { status }), which serializes null into a
message body for 204 No Content responses; change each handler (the
GET/PATCH/DELETE blocks that call guardrailsClient and buildEndpoint) to detect
a 204 response (e.g., status === 204 || (status && data === null)) and return an
empty response using NextResponse (or new Response) with no body and the same
status instead of NextResponse.json, otherwise continue to return
NextResponse.json(data, { status }).
In `@app/components/guardrails/BanListModal.tsx`:
- Around line 60-85: Wrap the modal container in proper dialog semantics and
programmatic labels: add role="dialog", aria-modal="true", and assign an id to
the title element (e.g., dialogTitle) then set aria-labelledby on the container
and aria-describedby pointing to the short description element; ensure the close
control has an accessible name (e.g., add aria-label="Close" to the close button
or use a visually hidden label) and keyboard focus handling if present; for all
form controls inside BanListModal, give each input a unique id and bind their
labels with htmlFor to those ids so labels are programmatically associated
(locate the title <h2>, the description <p>, the close button using onClose, and
all input/label pairs in BanListModal to apply these changes).
In `@app/components/guardrails/ValidatorConfigPanel.tsx`:
- Around line 104-112: The two useEffect hooks in ValidatorConfigPanel call
fetchBanLists and fetchBannedWords while relying on apiKey but don't list apiKey
in their dependency arrays; update the dependency arrays to include apiKey
(i.e., useEffect(() => { fetchBanLists(); }, [apiKey]) and useEffect(() => { ...
}, [value, apiKey])) and to satisfy exhaustive-deps, wrap fetchBanLists and
fetchBannedWords with useCallback keyed on apiKey (e.g., const fetchBanLists =
useCallback(..., [apiKey]) and const fetchBannedWords = useCallback(...,
[apiKey])) so the effects re-run when apiKey changes.
- Around line 371-391: The useEffect that initializes form state references the
local variable validator (and calls buildDefaultValues(validator.config) and
setFieldValues) but only lists selectedType, existingValues, and existingName in
its dependency array, which can lead to stale initialization when validators
change; update the dependency array to include validator (and if validator is
derived from an external validators array, include that array too) so the effect
reruns when the resolved validator changes, ensuring setFieldValues and other
setters use the correct config.
In `@app/components/InfoTooltip.tsx`:
- Around line 14-24: The InfoTooltip component currently only shows the tooltip
on mouse enter/leave; update the button to also handle keyboard and touch users
by wiring onFocus to setVisible(true) and onBlur to setVisible(false), add an
onClick handler to toggle visibility as a touch fallback, and add an onKeyDown
handler to close on Escape; ensure proper tooltip semantics by adding an id on
the tooltip element and connecting the button via aria-describedby (and
aria-expanded on the button) and marking the tooltip with role="tooltip" so
screen readers can discover it; keep using the existing setVisible state and the
same classnames/visual styling while only adding these handlers and ARIA
attributes in InfoTooltip.
In `@app/components/MultiSelect.tsx`:
- Around line 45-53: The div that toggles the dropdown (using setOpen and open
in MultiSelect) is not keyboard-accessible; replace or augment it with proper
combobox/button semantics: add tabindex={0} (or use a native <button>),
role="combobox" (or role="button") and aria-expanded={open} and
aria-haspopup="listbox", and implement an onKeyDown handler that toggles setOpen
when Enter or Space are pressed and handles Escape to close; ensure the same
changes are applied to the other trigger block (the second clickable region
around lines 104-139) so both mouse and keyboard users can open/close the
dropdown and focus moves appropriately.
In `@app/lib/apiClient.ts`:
- Around line 62-64: The code unconditionally calls JSON.parse on the response
body (variables: response, text, data), which will throw for non-JSON responses
and convert upstream errors into local exceptions; change this by wrapping
JSON.parse(text) in a try/catch and only set data to the parsed object on
success, otherwise set data to null (or keep the raw text) and
preserve/propagate the original response status and body so upstream errors
aren’t masked; update the logic around response.text(), JSON.parse, and the data
variable to safely handle non-JSON responses without throwing.
---
Nitpick comments:
In `@app/`(main)/guardrails/page.tsx:
- Around line 134-150: Add a user confirmation step to handleDeleteConfig so
deletion is not sent immediately: before performing the fetch call (and before
checking configsQueryString), prompt the user (e.g., window.confirm or your app
modal) to confirm deletion of the configId; if the user cancels, return early,
otherwise proceed with the existing fetch, toast handling, handleClearForm() if
selectedSavedConfig?.id matches configId, and fetchSavedConfigs(); keep all
existing error handling intact.
- Line 77: The useEffect that depends on isHydrated and activeKey?.key also uses
the toast object but doesn't include it in the dependency array; update the
effect's dependency array to include toast (or, if you intentionally want to
ignore it, explicitly silence exhaustive-deps with a well-documented //
eslint-disable-next-line comment). Locate the useEffect that references
isHydrated, activeKey?.key and toast in page.tsx and either add toast to the
dependencies or add a concise eslint-disable comment and explanation to prevent
false positives.
In `@app/api/guardrails/validators/configs/route.ts`:
- Around line 4-18: Extract the repeated Guardrails proxy logic (auth header
creation, query param forwarding, and the try/catch → NextResponse
error/response shape) into a single helper function (e.g., proxyToGuardrails or
forwardGuardrailsRequest) and replace duplicates in getAuthHeader, buildEndpoint
and the route handlers in app/api/guardrails/route.ts,
app/api/guardrails/validators/configs/route.ts, and
app/api/guardrails/validators/configs/[config_id]/route.ts; the helper should
accept the incoming NextRequest and a target path (or accept optional
overrides), build the auth header (or accept the token), forward searchParams,
perform fetch to `/api/v1/guardrails...`, and return a normalized NextResponse
inside a try/catch so all endpoints reuse identical auth/error behavior.
In `@app/components/guardrails/types.ts`:
- Around line 39-44: The formatValidatorName function can mangle
already-formatted or acronym words; update formatValidatorName to handle edge
cases by: if the input contains spaces, return it unchanged (preserve "LLM
Critic"); otherwise split on '_' and for each token preserve existing ALL CAPS
tokens, convert short all-lowercase tokens (length <= 3, e.g., "pii") to
UPPERCASE, and title-case other tokens as currently done; implement these checks
inside formatValidatorName to avoid breaking snake_case handling while
preserving acronyms and pre-formatted names.
In `@app/components/prompt-editor/ConfigEditorPane.tsx`:
- Around line 48-69: The fetch in the useEffect (which uses queryString and
apiKey) currently swallows errors in the .catch(() => setValidators([])) block;
update this to accept an optional onError callback prop (or at least log the
error) and call it with the caught Error before falling back to
setValidators([]) and setLoading(false); locate the fetch block in
ConfigEditorPane.tsx (the useEffect where setValidators and setLoading are used)
and modify the catch to .catch((err) => { if (onError) onError(err); else
console.error('Failed to load guardrail validators', err); setValidators([]); })
so callers can react to failures and you still clear loading state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b2f52ae-9558-4390-b8eb-cac05ae0291a
📒 Files selected for processing (22)
.env.exampleapp/(main)/coming-soon/guardrails/page.tsxapp/(main)/configurations/prompt-editor/page.tsxapp/(main)/guardrails/page.tsxapp/api/apikeys/verify/route.tsapp/api/guardrails/ban_lists/[ban_list_id]/route.tsapp/api/guardrails/ban_lists/route.tsapp/api/guardrails/route.tsapp/api/guardrails/validators/configs/[config_id]/route.tsapp/api/guardrails/validators/configs/route.tsapp/components/InfoTooltip.tsxapp/components/MultiSelect.tsxapp/components/Sidebar.tsxapp/components/guardrails/BanListModal.tsxapp/components/guardrails/ValidatorConfigPanel.tsxapp/components/guardrails/types.tsapp/components/guardrails/validators.jsonapp/components/icons/index.tsxapp/components/icons/sidebar/ShieldCheckIcon.tsxapp/components/prompt-editor/ConfigEditorPane.tsxapp/lib/apiClient.tsapp/lib/types/configs.ts
💤 Files with no reviewable changes (1)
- app/(main)/coming-soon/guardrails/page.tsx
Comment on lines
+3
to
+5
| #for guardrails | ||
| GUARDRAILS_URL = http://localhost:8001 | ||
| GUARDRAILS_TOKEN = |
There was a problem hiding this comment.
Normalize the new env entries before merging.
This hunk will still trip dotenv-linter because of the spaces around = and the trailing whitespace on GUARDRAILS_TOKEN.
🧹 Suggested cleanup
-#for guardrails
-GUARDRAILS_URL = http://localhost:8001
-GUARDRAILS_TOKEN =
+# for guardrails
+GUARDRAILS_TOKEN=
+GUARDRAILS_URL=http://localhost:8001📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #for guardrails | |
| GUARDRAILS_URL = http://localhost:8001 | |
| GUARDRAILS_TOKEN = | |
| # for guardrails | |
| GUARDRAILS_TOKEN= | |
| GUARDRAILS_URL=http://localhost:8001 |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 4-4: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 5-5: [SpaceCharacter] The line has spaces around equal sign
(SpaceCharacter)
[warning] 5-5: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 5-5: [UnorderedKey] The GUARDRAILS_TOKEN key should go before the GUARDRAILS_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 3 - 5, Normalize the new environment entries by
removing spaces around the equals sign and trimming trailing whitespace so they
conform to dotenv syntax and pass dotenv-linter; specifically update the
GUARDRAILS_URL and GUARDRAILS_TOKEN lines (remove spaces before/after '=' and
delete any trailing spaces on GUARDRAILS_TOKEN) so the variables are defined as
VAR=VALUE with no extra whitespace.
| ) { | ||
| try { | ||
| const { ban_list_id } = await params; | ||
| const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${ban_list_id}`); |
There was a problem hiding this comment.
ban_list_id should be URL-encoded before building upstream paths.
Direct interpolation at Line 10, Line 27, and Line 46 can break routing for IDs containing reserved URL characters.
🔐 Suggested fix
try {
const { ban_list_id } = await params;
- const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${ban_list_id}`);
+ const encodedId = encodeURIComponent(ban_list_id);
+ const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${encodedId}`); try {
const { ban_list_id } = await params;
+ const encodedId = encodeURIComponent(ban_list_id);
const body = await request.json();
- const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${ban_list_id}`, {
+ const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${encodedId}`, {
method: "PUT",
body: JSON.stringify(body),
}); try {
const { ban_list_id } = await params;
- const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${ban_list_id}`, {
+ const encodedId = encodeURIComponent(ban_list_id);
+ const { status, data } = await guardrailsClient(request, `/api/v1/guardrails/ban_lists/${encodedId}`, {
method: "DELETE",
});Also applies to: 27-27, 46-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/guardrails/ban_lists/`[ban_list_id]/route.ts at line 10, The code
interpolates ban_list_id directly into upstream paths (calls to
guardrailsClient) which breaks when IDs contain reserved URL characters; update
every place that builds the path (the guardrailsClient calls in route.ts) to use
an encoded ID by wrapping ban_list_id with encodeURIComponent before
interpolation so all three occurrences (the calls that construct
`/api/v1/guardrails/ban_lists/${ban_list_id}`) use the encoded value.
Comment on lines
+14
to
+24
| <button | ||
| type="button" | ||
| className="w-3.5 h-3.5 rounded-full text-[10px] font-bold flex items-center justify-center leading-none select-none" | ||
| style={{ | ||
| backgroundColor: colors.bg.secondary, | ||
| color: colors.text.secondary, | ||
| border: `1px solid ${colors.border}`, | ||
| }} | ||
| onMouseEnter={() => setVisible(true)} | ||
| onMouseLeave={() => setVisible(false)} | ||
| > |
There was a problem hiding this comment.
Tooltip is mouse-only; keyboard/touch users cannot access helper text.
At Line 22-Line 24, visibility is tied only to hover events. Add focus/blur (and preferably click fallback) plus tooltip semantics so non-mouse users can access the content.
♿ Suggested fix
export default function InfoTooltip({ text }: InfoTooltipProps) {
const [visible, setVisible] = useState(false);
return (
<span className="relative inline-flex items-center ml-1" style={{ verticalAlign: 'text-bottom' }}>
<button
type="button"
+ aria-label="More information"
+ aria-expanded={visible}
+ aria-describedby="info-tooltip"
className="w-3.5 h-3.5 rounded-full text-[10px] font-bold flex items-center justify-center leading-none select-none"
style={{
backgroundColor: colors.bg.secondary,
color: colors.text.secondary,
border: `1px solid ${colors.border}`,
}}
onMouseEnter={() => setVisible(true)}
onMouseLeave={() => setVisible(false)}
+ onFocus={() => setVisible(true)}
+ onBlur={() => setVisible(false)}
+ onClick={() => setVisible((v) => !v)}
>
i
</button>
{visible && (
<div
+ id="info-tooltip"
+ role="tooltip"
className="absolute z-50 left-5 top-0 w-64 text-xs rounded-lg p-2.5 shadow-lg"
style={{
backgroundColor: colors.bg.primary,
border: `1px solid ${colors.border}`,
color: colors.text.secondary,
lineHeight: "1.5",
}}
>
{text}
</div>
)}
</span>
);
}Also applies to: 27-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/InfoTooltip.tsx` around lines 14 - 24, The InfoTooltip
component currently only shows the tooltip on mouse enter/leave; update the
button to also handle keyboard and touch users by wiring onFocus to
setVisible(true) and onBlur to setVisible(false), add an onClick handler to
toggle visibility as a touch fallback, and add an onKeyDown handler to close on
Escape; ensure proper tooltip semantics by adding an id on the tooltip element
and connecting the button via aria-describedby (and aria-expanded on the button)
and marking the tooltip with role="tooltip" so screen readers can discover it;
keep using the existing setVisible state and the same classnames/visual styling
while only adding these handlers and ARIA attributes in InfoTooltip.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
.env.example (1)
3-4:⚠️ Potential issue | 🟡 MinorNormalize dotenv entries to pass linter.
Line 3 and Line 4 use invalid spacing around
=(and Line 4 includes trailing whitespace), and key order currently violates the linter rule.💡 Proposed fix
-GUARDRAILS_URL = http://localhost:8001 -GUARDRAILS_TOKEN = +GUARDRAILS_TOKEN= +GUARDRAILS_URL=http://localhost:8001🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 3 - 4, Normalize the .env entries by removing spaces around the equals sign and removing trailing whitespace for the keys shown: change "GUARDRAILS_URL = http://localhost:8001" to "GUARDRAILS_URL=http://localhost:8001" and "GUARDRAILS_TOKEN = " to "GUARDRAILS_TOKEN=" (no trailing space), and reorder the keys if required by the linter so the key order matches the project's dotenv ordering convention; update the lines containing GUARDRAILS_URL and GUARDRAILS_TOKEN accordingly.app/lib/apiClient.ts (1)
186-187:⚠️ Potential issue | 🟠 MajorGuard JSON parsing for non-JSON upstream responses.
Line 187 can throw on non-JSON bodies and mask the real upstream failure payload/status handling in this helper.
💡 Proposed fix
const text = response.status === 204 ? "" : await response.text(); - const data = text ? JSON.parse(text) : null; + let data: unknown = null; + if (text) { + try { + data = JSON.parse(text); + } catch { + data = { raw: text }; + } + } return { status: response.status, data };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/apiClient.ts` around lines 186 - 187, The JSON.parse call on the response body can throw for non-JSON upstream responses; update the parsing logic around response, text and data to first check the Content-Type (response.headers.get('content-type')) for a JSON media type and only then JSON.parse, or otherwise wrap JSON.parse in a try/catch and fall back to returning the raw text (or null) while preserving the original response.status; change the code that computes text and data to handle parse failures safely so callers of this helper (the variables response, text, data) won't throw on non-JSON bodies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/Sidebar.tsx`:
- Around line 80-118: There are two conflicting declarations of the navItems
constant in the Sidebar component; remove the duplicate inline const navItems
(the first declaration containing Guardrails and Settings) and instead add those
entries to the shared NAV_ITEMS configuration in app/lib/navConfig.ts, then
update the iconMap to include icons for "Guardrails" and "Settings" so the
Sidebar JSX uses the single NAV_ITEMS source consistently; ensure you only keep
one navItems reference (the NAV_ITEMS import) used by the Sidebar render logic.
In `@app/lib/apiClient.ts`:
- Around line 172-174: The code currently overwrites any caller-provided
Content-Type when the body is not a FormData; update the logic in apiClient.ts
to only set headers.set("Content-Type", "application/json") if there is a body
and the headers do not already include a Content-Type (i.e., use headers.has or
headers.get to check caller-provided options.headers first). Locate the block
that reads fetchOptions and headers (references: fetchOptions, headers,
options.headers) and add the guard so explicit media types passed by the caller
are preserved.
- Around line 4-6: There are two declarations of the same module-scoped constant
BACKEND_URL causing a TypeScript error; remove the duplicate declaration so only
one const BACKEND_URL exists (keep the preferred default value and environment
fallback), and ensure any code using BACKEND_URL still references that single
symbol; check nearby constants like GUARDRAILS_URL to confirm no other
duplicates were added.
---
Duplicate comments:
In @.env.example:
- Around line 3-4: Normalize the .env entries by removing spaces around the
equals sign and removing trailing whitespace for the keys shown: change
"GUARDRAILS_URL = http://localhost:8001" to
"GUARDRAILS_URL=http://localhost:8001" and "GUARDRAILS_TOKEN = " to
"GUARDRAILS_TOKEN=" (no trailing space), and reorder the keys if required by the
linter so the key order matches the project's dotenv ordering convention; update
the lines containing GUARDRAILS_URL and GUARDRAILS_TOKEN accordingly.
In `@app/lib/apiClient.ts`:
- Around line 186-187: The JSON.parse call on the response body can throw for
non-JSON upstream responses; update the parsing logic around response, text and
data to first check the Content-Type (response.headers.get('content-type')) for
a JSON media type and only then JSON.parse, or otherwise wrap JSON.parse in a
try/catch and fall back to returning the raw text (or null) while preserving the
original response.status; change the code that computes text and data to handle
parse failures safely so callers of this helper (the variables response, text,
data) won't throw on non-JSON bodies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c59fda0-5933-43e0-a588-0bbc2da2cd82
📒 Files selected for processing (6)
.env.exampleapp/(main)/configurations/prompt-editor/page.tsxapp/components/Sidebar.tsxapp/components/icons/index.tsxapp/components/prompt-editor/ConfigEditorPane.tsxapp/lib/apiClient.ts
✅ Files skipped from review due to trivial changes (1)
- app/components/icons/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/(main)/configurations/prompt-editor/page.tsx
- app/components/prompt-editor/ConfigEditorPane.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
app/components/Sidebar.tsx (1)
79-117:⚠️ Potential issue | 🔴 CriticalRemove the duplicate
navItemsdeclaration.Line 79 and Line 157 both declare
const navItemsin the same scope, so this component will not compile. The render path also consumes theNAV_ITEMS-derived array, so the inline Guardrails/Settings list is not a viable second source of truth anyway. Move those entries into the shared nav config and extendiconMapinstead of keeping two local declarations.Also applies to: 157-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/Sidebar.tsx` around lines 79 - 117, There are two conflicting nav arrays—remove the duplicate const navItems declaration and consolidate all menu entries into the single shared NAV_ITEMS (or the original navItems used by the render path) so the component compiles; add the missing "Guardrails" and "Settings" entries to that shared nav configuration and extend iconMap to include ShieldCheckIcon and SlidersIcon (or map the existing names to these icons) so the UI uses the single source of truth (navItems/NAV_ITEMS) and no inline duplicate list remains.app/components/guardrails/ValidatorConfigPanel.tsx (2)
441-472:⚠️ Potential issue | 🟠 MajorInclude
validatorin the initialization effect dependencies.Line 455 reads
validator.config, but the effect never reruns whenvalidatorsresolves to a newvalidator. If the catalog arrives afterselectedTypeorexistingValues, the dynamic fields stay blank/defaulted and a save can overwrite the existing config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/guardrails/ValidatorConfigPanel.tsx` around lines 441 - 472, The useEffect that initializes fields reads validator.config (and calls buildDefaultValues) but does not list validator in its dependency array, so the effect won't rerun when the validator variable becomes available; update the effect dependencies to include validator so that when validator changes (e.g., catalog resolves) the initialization branch that uses validator, buildDefaultValues, and setFieldValues runs again and correctly populates dynamic fields instead of leaving them blank/defaulted.
74-125:⚠️ Potential issue | 🟡 MinorRe-fetch ban-list data when
apiKeychanges.Line 77 and Line 100 read
apiKey, but the effects at Line 115 and Line 119 never depend on it. If credentials are updated, this field keeps using the stale key and never refreshes until the component remounts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/guardrails/ValidatorConfigPanel.tsx` around lines 74 - 125, The component reads apiKey inside fetchBanLists and fetchBannedWords but the useEffect hooks that call them (the one that calls fetchBanLists on mount and the one that calls fetchBannedWords when value changes) do not include apiKey in their dependency arrays, causing stale credentials to be used; update the two useEffect dependency arrays to include apiKey (i.e., change useEffect(() => { fetchBanLists(); }, [] ) to useEffect(() => { fetchBanLists(); }, [apiKey]) and change useEffect(() => { if (value) fetchBannedWords(value); else setBannedWords([]); }, [value]) to include apiKey as well so that when apiKey changes you re-run the effects and refetch ban lists and banned words (ensuring fetchBannedWords is called with the current value).app/api/guardrails/validators/configs/[config_id]/route.ts (1)
33-38:⚠️ Potential issue | 🟠 MajorReturn an empty response for upstream 204s.
These handlers always call
NextResponse.json(data, { status }). If the upstream proxy comes back with204 No Content, serializingnullhere adds a body to a no-content response and can break DELETE/PATCH flows.📦 Minimal 204 handling
- return NextResponse.json(data, { status }); + return status === 204 + ? new NextResponse(null, { status }) + : NextResponse.json(data, { status }); @@ - return NextResponse.json(data, { status }); + return status === 204 + ? new NextResponse(null, { status }) + : NextResponse.json(data, { status }); @@ - return NextResponse.json(data, { status }); + return status === 204 + ? new NextResponse(null, { status }) + : NextResponse.json(data, { status });Run this read-only check to confirm the helper can surface 204s and that these return sites always serialize the proxy result:
#!/bin/bash set -euo pipefail ROUTE_FILE="$(fd -a 'route.ts' app/api | rg 'guardrails/validators/configs/\[config_id\]/route\.ts$' | head -n1)" API_CLIENT_FILE="$(fd -a 'apiClient.ts' app/lib | head -n1)" sed -n '1,180p' "$ROUTE_FILE" sed -n '1,260p' "$API_CLIENT_FILE" rg -n -C2 'NextResponse\.json\(data, \{ status \}\)|\b204\b' "$ROUTE_FILE" "$API_CLIENT_FILE"Also applies to: 61-70, 92-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/guardrails/validators/configs/`[config_id]/route.ts around lines 33 - 38, The handler is always calling NextResponse.json(data, { status }) which will serialize a body even when the upstream returned 204; update the response logic around the guardrailsClient call (the const { status, data } = await guardrailsClient(request, buildEndpoint(request, config_id), { authHeader })) to special-case status === 204 and return an empty NextResponse with that status (e.g., return new NextResponse(null, { status })) and only call NextResponse.json(data, { status }) for non-204 statuses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/guardrails/ValidatorConfigPanel.tsx`:
- Around line 74-113: fetchBanLists and fetchBannedWords currently treat non-OK
HTTP responses as successful empty data; update both to check response.ok (e.g.,
after fetch, if (!r.ok) throw new Error(await r.text() || r.statusText)) so
errors flow to .catch, introduce two error state variables (e.g., banListsError
and bannedWordsError with setters setBanListsError and setBannedWordsError),
clear those errors at the start of each fetch, and in .catch set the appropriate
error state instead of calling setBanLists([]) or setBannedWords([]); ensure
setLoading/setWordsLoading are still cleared in .finally and wire the new error
states into the component UI to show an inline alert when non-OK responses occur
rather than rendering the empty-state text.
In `@app/components/prompt-editor/ConfigEditorPane.tsx`:
- Around line 48-71: The effect in ConfigEditorPane's useEffect that fetches
validator configs (using queryString, apiKey, fetch, setValidators, setLoading)
must cancel in-flight requests and clear stale state when the scope changes:
create an AbortController inside the effect, pass controller.signal to fetch,
call controller.abort in the cleanup, and in the case queryString is falsy
ensure you immediately setValidators([]) and setLoading(false) (or bail early)
so old validators aren’t left behind; also handle fetch aborts in the catch to
avoid overwriting state from aborted requests.
- Around line 251-265: When apiKey changes we must clear previous org/project
scope and ignore stale verify responses; in the useEffect that depends on
apiKey, call setGuardrailsQueryString("") immediately when apiKey is falsy or
before starting a new verification, and cancel/ignore prior fetches by using an
AbortController or a local request token so that late-resolving promises from
fetch("/api/apikeys/verify") do not overwrite the cleared state; update the
effect surrounding fetch("/api/apikeys/verify") and setGuardrailsQueryString to
implement this cancellation/ignore logic.
---
Duplicate comments:
In `@app/api/guardrails/validators/configs/`[config_id]/route.ts:
- Around line 33-38: The handler is always calling NextResponse.json(data, {
status }) which will serialize a body even when the upstream returned 204;
update the response logic around the guardrailsClient call (the const { status,
data } = await guardrailsClient(request, buildEndpoint(request, config_id), {
authHeader })) to special-case status === 204 and return an empty NextResponse
with that status (e.g., return new NextResponse(null, { status })) and only call
NextResponse.json(data, { status }) for non-204 statuses.
In `@app/components/guardrails/ValidatorConfigPanel.tsx`:
- Around line 441-472: The useEffect that initializes fields reads
validator.config (and calls buildDefaultValues) but does not list validator in
its dependency array, so the effect won't rerun when the validator variable
becomes available; update the effect dependencies to include validator so that
when validator changes (e.g., catalog resolves) the initialization branch that
uses validator, buildDefaultValues, and setFieldValues runs again and correctly
populates dynamic fields instead of leaving them blank/defaulted.
- Around line 74-125: The component reads apiKey inside fetchBanLists and
fetchBannedWords but the useEffect hooks that call them (the one that calls
fetchBanLists on mount and the one that calls fetchBannedWords when value
changes) do not include apiKey in their dependency arrays, causing stale
credentials to be used; update the two useEffect dependency arrays to include
apiKey (i.e., change useEffect(() => { fetchBanLists(); }, [] ) to useEffect(()
=> { fetchBanLists(); }, [apiKey]) and change useEffect(() => { if (value)
fetchBannedWords(value); else setBannedWords([]); }, [value]) to include apiKey
as well so that when apiKey changes you re-run the effects and refetch ban lists
and banned words (ensuring fetchBannedWords is called with the current value).
In `@app/components/Sidebar.tsx`:
- Around line 79-117: There are two conflicting nav arrays—remove the duplicate
const navItems declaration and consolidate all menu entries into the single
shared NAV_ITEMS (or the original navItems used by the render path) so the
component compiles; add the missing "Guardrails" and "Settings" entries to that
shared nav configuration and extend iconMap to include ShieldCheckIcon and
SlidersIcon (or map the existing names to these icons) so the UI uses the single
source of truth (navItems/NAV_ITEMS) and no inline duplicate list remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb67bafc-7483-4222-925c-bda2a120012f
📒 Files selected for processing (15)
app/(main)/guardrails/page.tsxapp/api/guardrails/ban_lists/[ban_list_id]/route.tsapp/api/guardrails/ban_lists/route.tsapp/api/guardrails/route.tsapp/api/guardrails/validators/configs/[config_id]/route.tsapp/api/guardrails/validators/configs/route.tsapp/components/InfoTooltip.tsxapp/components/MultiSelect.tsxapp/components/Sidebar.tsxapp/components/guardrails/BanListModal.tsxapp/components/guardrails/ValidatorConfigPanel.tsxapp/components/guardrails/types.tsapp/components/icons/index.tsxapp/components/icons/sidebar/ShieldCheckIcon.tsxapp/components/prompt-editor/ConfigEditorPane.tsx
✅ Files skipped from review due to trivial changes (5)
- app/components/icons/index.tsx
- app/components/guardrails/types.ts
- app/components/icons/sidebar/ShieldCheckIcon.tsx
- app/api/guardrails/ban_lists/route.ts
- app/(main)/guardrails/page.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- app/api/guardrails/route.ts
- app/components/InfoTooltip.tsx
- app/components/MultiSelect.tsx
- app/api/guardrails/ban_lists/[ban_list_id]/route.ts
- app/components/guardrails/BanListModal.tsx
Comment on lines
+251
to
+265
| useEffect(() => { | ||
| if (!apiKey) return; | ||
| fetch("/api/apikeys/verify", { headers: { "X-API-KEY": apiKey } }) | ||
| .then((r) => r.json()) | ||
| .then((data) => { | ||
| const org_id = data?.data?.organization_id; | ||
| const proj_id = data?.data?.project_id; | ||
| if (org_id != null && proj_id != null) { | ||
| setGuardrailsQueryString( | ||
| `?organization_id=${parseInt(String(org_id), 10)}&project_id=${parseInt(String(proj_id), 10)}`, | ||
| ); | ||
| } | ||
| }) | ||
| .catch(() => {}); | ||
| }, [apiKey]); |
There was a problem hiding this comment.
Clear the derived org/project scope before re-verifying the API key.
This effect only updates guardrailsQueryString on success. If apiKey becomes empty/invalid, or an older verify call resolves last, the previous org/project query remains active and later guardrails requests stay scoped to the wrong context.
🧭 Reset and cancel stale verification
useEffect(() => {
- if (!apiKey) return;
- fetch("/api/apikeys/verify", { headers: { "X-API-KEY": apiKey } })
- .then((r) => r.json())
+ const controller = new AbortController();
+ setGuardrailsQueryString(null);
+ if (!apiKey) return () => controller.abort();
+ fetch("/api/apikeys/verify", {
+ headers: { "X-API-KEY": apiKey },
+ signal: controller.signal,
+ })
+ .then(async (r) => {
+ if (!r.ok) throw new Error("Failed to verify API key");
+ return r.json();
+ })
.then((data) => {
const org_id = data?.data?.organization_id;
const proj_id = data?.data?.project_id;
if (org_id != null && proj_id != null) {
- setGuardrailsQueryString(
- `?organization_id=${parseInt(String(org_id), 10)}&project_id=${parseInt(String(proj_id), 10)}`,
- );
+ const params = new URLSearchParams({
+ organization_id: String(org_id),
+ project_id: String(proj_id),
+ });
+ setGuardrailsQueryString(`?${params.toString()}`);
+ } else {
+ setGuardrailsQueryString(null);
}
})
- .catch(() => {});
+ .catch((error) => {
+ if (error.name !== "AbortError") {
+ setGuardrailsQueryString(null);
+ }
+ });
+ return () => controller.abort();
}, [apiKey]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (!apiKey) return; | |
| fetch("/api/apikeys/verify", { headers: { "X-API-KEY": apiKey } }) | |
| .then((r) => r.json()) | |
| .then((data) => { | |
| const org_id = data?.data?.organization_id; | |
| const proj_id = data?.data?.project_id; | |
| if (org_id != null && proj_id != null) { | |
| setGuardrailsQueryString( | |
| `?organization_id=${parseInt(String(org_id), 10)}&project_id=${parseInt(String(proj_id), 10)}`, | |
| ); | |
| } | |
| }) | |
| .catch(() => {}); | |
| }, [apiKey]); | |
| useEffect(() => { | |
| const controller = new AbortController(); | |
| setGuardrailsQueryString(null); | |
| if (!apiKey) return () => controller.abort(); | |
| fetch("/api/apikeys/verify", { | |
| headers: { "X-API-KEY": apiKey }, | |
| signal: controller.signal, | |
| }) | |
| .then(async (r) => { | |
| if (!r.ok) throw new Error("Failed to verify API key"); | |
| return r.json(); | |
| }) | |
| .then((data) => { | |
| const org_id = data?.data?.organization_id; | |
| const proj_id = data?.data?.project_id; | |
| if (org_id != null && proj_id != null) { | |
| const params = new URLSearchParams({ | |
| organization_id: String(org_id), | |
| project_id: String(proj_id), | |
| }); | |
| setGuardrailsQueryString(`?${params.toString()}`); | |
| } else { | |
| setGuardrailsQueryString(null); | |
| } | |
| }) | |
| .catch((error) => { | |
| if (error.name !== "AbortError") { | |
| setGuardrailsQueryString(null); | |
| } | |
| }); | |
| return () => controller.abort(); | |
| }, [apiKey]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/prompt-editor/ConfigEditorPane.tsx` around lines 251 - 265,
When apiKey changes we must clear previous org/project scope and ignore stale
verify responses; in the useEffect that depends on apiKey, call
setGuardrailsQueryString("") immediately when apiKey is falsy or before starting
a new verification, and cancel/ignore prior fetches by using an AbortController
or a local request token so that late-resolving promises from
fetch("/api/apikeys/verify") do not overwrite the cleared state; update the
effect surrounding fetch("/api/apikeys/verify") and setGuardrailsQueryString to
implement this cancellation/ignore logic.
Collaborator
|
The linting checks fails due to the name 
|
Ayush8923
reviewed
Apr 11, 2026
Ayush8923
requested changes
Apr 12, 2026
Collaborator
|
@nishika26 I have a question regarding the authentication flow. Could you please clarify how this is handled here? Will the JWT token be verified on the backend, or is there a different mechanism in place for these flows? |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/lib/apiClient.ts (2)
185-186:⚠️ Potential issue | 🟠 MajorAvoid unconditional JSON parsing of Guardrails responses.
At Line 185-186,
JSON.parse(text)can throw for non-JSON/error payloads and turn upstream failures into local exceptions.Proposed fix
const text = response.status === 204 ? "" : await response.text(); - const data = text ? JSON.parse(text) : null; + let data: unknown = null; + if (text) { + try { + data = JSON.parse(text); + } catch { + data = { raw: text }; + } + } return { status: response.status, data };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/apiClient.ts` around lines 185 - 186, The code unconditionally parses response.text into JSON (variables response → text → data), which can throw for non-JSON payloads; update the handling in the API client to first check response.status and Content-Type (response.headers.get('content-type')) and then attempt JSON.parse within a try/catch around the existing JSON.parse(text) so parse failures do not throw—on failure set data to null or a safe fallback (e.g., { rawText: text }) and ensure callers of the data variable can handle the null/fallback case.
171-173:⚠️ Potential issue | 🟡 MinorPreserve caller-provided
Content-TypeinguardrailsClient.Line 171-173 currently overwrites explicit media types from
options.headers. This can break callers sending non-JSON payloads.Proposed fix
- if (!(fetchOptions.body instanceof FormData)) { + if ( + !(fetchOptions.body instanceof FormData) && + fetchOptions.body != null && + !headers.has("Content-Type") + ) { headers.set("Content-Type", "application/json"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/apiClient.ts` around lines 171 - 173, The code in guardrailsClient currently unconditionally sets Content-Type to application/json when the body isn't FormData, overwriting caller-provided media types; update the logic in app/lib/apiClient.ts around fetchOptions.body and headers so you only set Content-Type when the caller did not supply one (e.g., change the condition to check that headers does not already have a Content-Type: if (!(fetchOptions.body instanceof FormData) && !headers.has("Content-Type")) { headers.set("Content-Type", "application/json"); }), ensuring case-insensitive header detection via the Headers API used in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/lib/apiClient.ts`:
- Around line 185-186: The code unconditionally parses response.text into JSON
(variables response → text → data), which can throw for non-JSON payloads;
update the handling in the API client to first check response.status and
Content-Type (response.headers.get('content-type')) and then attempt JSON.parse
within a try/catch around the existing JSON.parse(text) so parse failures do not
throw—on failure set data to null or a safe fallback (e.g., { rawText: text })
and ensure callers of the data variable can handle the null/fallback case.
- Around line 171-173: The code in guardrailsClient currently unconditionally
sets Content-Type to application/json when the body isn't FormData, overwriting
caller-provided media types; update the logic in app/lib/apiClient.ts around
fetchOptions.body and headers so you only set Content-Type when the caller did
not supply one (e.g., change the condition to check that headers does not
already have a Content-Type: if (!(fetchOptions.body instanceof FormData) &&
!headers.has("Content-Type")) { headers.set("Content-Type", "application/json");
}), ensuring case-insensitive header detection via the Headers API used in the
file.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/lib/navConfig.ts (1)
52-56: Consider addinggateDescriptionfor Settings for UX consistency.At Line 53-56, this is the only new top-level item without gated copy; unauth flows may show inconsistent messaging compared with other entries.
Suggested tweak
{ name: "Settings", route: "/settings/credentials", icon: "sliders", + gateDescription: "Log in to manage your settings and credentials.", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/navConfig.ts` around lines 52 - 56, The Settings nav entry currently { name: "Settings", route: "/settings/credentials", icon: "sliders" } lacks a gateDescription causing inconsistent gated copy; update that object to include a gateDescription string (matching tone/format used by other top-level items) so unauth flows display consistent messaging — locate the Settings entry in navConfig.ts and add a gateDescription property to the same object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/lib/navConfig.ts`:
- Around line 52-56: The Settings nav entry currently { name: "Settings", route:
"/settings/credentials", icon: "sliders" } lacks a gateDescription causing
inconsistent gated copy; update that object to include a gateDescription string
(matching tone/format used by other top-level items) so unauth flows display
consistent messaging — locate the Settings entry in navConfig.ts and add a
gateDescription property to the same object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4efccfbc-ec2c-4e2a-baab-be71807cb7c1
📒 Files selected for processing (3)
app/(main)/configurations/prompt-editor/page.tsxapp/components/Sidebar.tsxapp/lib/navConfig.ts
✅ Files skipped from review due to trivial changes (1)
- app/components/Sidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/(main)/configurations/prompt-editor/page.tsx
Ayush8923
reviewed
Apr 15, 2026
Comment on lines
+11
to
+37
| const KNOWN_ARRAY_OPTIONS: Record<string, string[]> = { | ||
| entity_types: [ | ||
| "CREDIT_CARD", | ||
| "EMAIL_ADDRESS", | ||
| "IBAN_CODE", | ||
| "IP_ADDRESS", | ||
| "LOCATION", | ||
| "MEDICAL_LICENSE", | ||
| "NRP", | ||
| "PERSON", | ||
| "PHONE_NUMBER", | ||
| "URL", | ||
| "IN_AADHAAR", | ||
| "IN_PAN", | ||
| "IN_PASSPORT", | ||
| "IN_VEHICLE_REGISTRATION", | ||
| "IN_VOTER", | ||
| ], | ||
| languages: ["en", "hi"], | ||
| }; | ||
|
|
||
| const KNOWN_SINGLE_OPTIONS: Record<string, string[]> = { | ||
| categories: ["generic", "healthcare", "education", "all"], | ||
| }; | ||
|
|
||
| type SchemaProp = ValidatorConfigSchema["properties"][string]; | ||
| type AnyOfMember = { $ref?: string; enum?: string[]; type?: string }; |
Collaborator
There was a problem hiding this comment.
Move this inside the types folder also and then use it here.
Comment on lines
+176
to
+185
| <select | ||
| value={onFailAction} | ||
| onChange={(e) => setOnFailAction(e.target.value)} | ||
| className={inputClass} | ||
| > | ||
| <option value="fix">Fix</option> | ||
| <option value="exception">Exception</option> | ||
| <option value="rephrase">Rephrase</option> | ||
| </select> | ||
| </div> |
Collaborator
There was a problem hiding this comment.
I think for this just create the one simple select component and then in that component pass the array of the options so that the select component iterate the value from the array and render multiple options.
Please use that select component all the places instead of the manually creating the dropdown via select tag.
Comment on lines
+161
to
+168
| <select | ||
| value={stage} | ||
| onChange={(e) => setStage(e.target.value as "input" | "output")} | ||
| className={inputClass} | ||
| > | ||
| <option value="input">Input</option> | ||
| <option value="output">Output</option> | ||
| </select> |
Collaborator
There was a problem hiding this comment.
Update this also, as per this comment: https://github.com/ProjectTech4DevAI/kaapi-frontend/pull/116/changes#r3086023634
Comment on lines
+54
to
+55
| export const VALIDATOR_META_BY_TYPE: Record<string, ValidatorMeta> = | ||
| Object.fromEntries(VALIDATOR_META.map((v) => [v.validator_type, v])); |
Collaborator
There was a problem hiding this comment.
Move this inside the guardrails.ts utils file. this is not a good place for this function.
Comment on lines
+88
to
+99
| <select | ||
| value={(value as string) ?? ""} | ||
| onChange={(e) => onChange(name, e.target.value || null)} | ||
| className={inputClass} | ||
| > | ||
| <option value="">Select…</option> | ||
| {singleOptions.map((v) => ( | ||
| <option key={v} value={v}> | ||
| {v} | ||
| </option> | ||
| ))} | ||
| </select> |
Collaborator
There was a problem hiding this comment.
Update this select component as well.
Comment on lines
+32
to
+34
| const KNOWN_SINGLE_OPTIONS: Record<string, string[]> = { | ||
| categories: ["generic", "healthcare", "education", "all"], | ||
| }; |
Collaborator
There was a problem hiding this comment.
Move this inside the data/guardrails.
Comment on lines
+11
to
+30
| const KNOWN_ARRAY_OPTIONS: Record<string, string[]> = { | ||
| entity_types: [ | ||
| "CREDIT_CARD", | ||
| "EMAIL_ADDRESS", | ||
| "IBAN_CODE", | ||
| "IP_ADDRESS", | ||
| "LOCATION", | ||
| "MEDICAL_LICENSE", | ||
| "NRP", | ||
| "PERSON", | ||
| "PHONE_NUMBER", | ||
| "URL", | ||
| "IN_AADHAAR", | ||
| "IN_PAN", | ||
| "IN_PASSPORT", | ||
| "IN_VEHICLE_REGISTRATION", | ||
| "IN_VOTER", | ||
| ], | ||
| languages: ["en", "hi"], | ||
| }; |
Collaborator
There was a problem hiding this comment.
This one also, move this inside the data/guardrails.
Comment on lines
+104
to
+125
| <p className="text-xs px-3 py-2 rounded-md bg-red-50 border border-red-200 text-red-600"> | ||
| {fetchError} | ||
| </p> | ||
| ) : loading ? ( | ||
| <div className="h-8 rounded-md animate-pulse bg-bg-secondary" /> | ||
| ) : ( | ||
| <select | ||
| value={value ?? ""} | ||
| onChange={handleChange} | ||
| className={inputClass} | ||
| > | ||
| {banLists.length === 0 ? ( | ||
| <> | ||
| <option value="" disabled> | ||
| No ban lists yet | ||
| </option> | ||
| <option value="__create__">+ Create Ban List</option> | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <option value="">Select a ban list…</option> | ||
| <option value="__create__">+ Create Ban List</option> |
Collaborator
There was a problem hiding this comment.
Too many if else ternary condition here, create the own function for that in this same file and then return the corresponding html. So that from the code bit clean and easy for readable.
Comment on lines
+138
to
+159
| {wordsLoading ? ( | ||
| <div className="h-6 rounded animate-pulse bg-bg-secondary" /> | ||
| ) : wordsError ? ( | ||
| <p className="text-xs px-3 py-2 rounded-md bg-red-50 border border-red-200 text-red-600"> | ||
| {wordsError} | ||
| </p> | ||
| ) : bannedWords.length > 0 ? ( | ||
| <div className="flex flex-wrap gap-1"> | ||
| {bannedWords.map((word) => ( | ||
| <span | ||
| key={word} | ||
| className="inline-block text-xs px-2 py-0.5 rounded-full bg-bg-secondary text-text-secondary" | ||
| > | ||
| {word} | ||
| </span> | ||
| ))} | ||
| </div> | ||
| ) : ( | ||
| <p className="text-xs text-text-secondary"> | ||
| No words in this ban list. | ||
| </p> | ||
| )} |
Comment on lines
+205
to
+217
| <select | ||
| value={selectedType ?? ""} | ||
| onChange={(e) => onTypeChange(e.target.value || null)} | ||
| className={inputClass} | ||
| > | ||
| <option value="">Select a validator type…</option> | ||
| {validators.map((v) => ( | ||
| <option key={v.type} value={v.type}> | ||
| {VALIDATOR_META_BY_TYPE[v.type]?.validator_name ?? | ||
| formatValidatorName(v.type)} | ||
| </option> | ||
| ))} | ||
| </select> |
Collaborator
There was a problem hiding this comment.
Update with the Select component and other also, if any.
Ayush8923
approved these changes
Apr 15, 2026
This was referenced Apr 28, 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
2 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.
Target Issue is #48
Notes
New Features
Chores
Summary by CodeRabbit
New Features
Bug Fixes
Chores