ENG-3160: BE - Access policy agent chat (types + prompt explorer)#7992
ENG-3160: BE - Access policy agent chat (types + prompt explorer)#7992thabofletcher wants to merge 9 commits intoENG-2959-FE-Astralis-agent-chat-for-policy-buildingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
thabofletcher
force-pushed
the
ENG-3160-BE-access-policy-chat
branch
from
c02560e to
698134e
Compare
…Config Adds the `access_policies.agent_enabled` field to the TypeScript config type so the frontend can read the feature flag from GET /api/v1/config. Co-Authored-By: Thabo Fletcher <thabo@ethyca.com>
Add ACCESS_POLICY_CHAT type and ACCESS_POLICIES category to the prompt explorer so the user prompt and current YAML are editable fields before clicking Render Prompt — matching the pattern used by questionnaire prompts.
@ant-design/x bundles refractor (and react-syntax-highlighter) which ship as ESM-only packages. Jest's CJS transform config can't handle them without a full transform exception chain. Instead, mock @ant-design/x entirely in unit tests — Bubble/Sender are not exercised there anyway.
This reverts commit a414408.
This reverts commit d92cd67.
thabofletcher
force-pushed
the
ENG-3160-BE-access-policy-chat
branch
from
69ba60d to
2a9794e
Compare
Change `*/api/v1/plus/llm/access-policy-chat` to `*/api/v1/plus/access-policy/agent` to match the endpoint defined in the route.
|
/code-review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #7992 — BE - Access policy agent chat (types + prompt explorer)
Overall this is a clean, focused change — the URL fix is correct and the new prompt-explorer wiring works logically. Two themes run through most of the findings:
1. Enum members vs. raw strings (all files)
The PR defines PromptCategory.ACCESS_POLICIES and PromptType.ACCESS_POLICY_CHAT in types.ts, then doesn't use them for any of the comparisons it adds. Every new conditional (=== "access_policy_chat", === "access_policies", === "questionnaire") should use the corresponding enum member. See inline comments on utils.ts:68, QuestionnaireControls.tsx:75, prompt-explorer.tsx:175, and prompt-explorer.tsx:321.
2. Generated enum divergence
types/api/models/PromptCategory.ts and PromptType.ts (auto-generated) do not include the new ACCESS_POLICIES / ACCESS_POLICY_CHAT values. The comment in types.ts notes these are "hardcoded until openapi:generate properly generates them," which is an acceptable short-term workaround — but it's worth tracking. If any code path uses the generated enums for comparison (e.g., via PromptInfo.category from the API response), those checks will silently miss the new values. The fix is to re-run npm run openapi:generate once the backend OpenAPI spec includes the new enum members.
3. QuestionnaireControls scope creep
The component now handles two unrelated prompt categories under one name and interface. The 4 new props are irrelevant to all non-access-policy paths. See inline comment on QuestionnaireControls.tsx:265 for a grouping option that keeps the interface manageable without a full refactor.
Minor
- Default
agentPromptstate vs. placeholder text duplication (see inline onprompt-explorer.tsx:82) — nit for a dev-tool page.
🔬 Codegraph: connected (47303 nodes)
💡 Write /code-review in a comment to re-run this review.
| @@ -60,7 +66,16 @@ export const buildQuestionnaireVariables = ({ | |||
| isFinalQuestion = false, | |||
| questionToRephrase = "", | |||
| previousPhrasings = "", | |||
There was a problem hiding this comment.
clients/admin-ui/src/features/prompt-explorer/utils.ts:68
Raw string comparison against enum values. PromptType.ACCESS_POLICY_CHAT exists in types.ts for exactly this purpose:
// current
if (promptType === "access_policy_chat") {
// better
if (promptType === PromptType.ACCESS_POLICY_CHAT) {Same issue applies to all string comparisons against prompt type/category values in this file (lines 79, 107, 119, 128). Using the enum member means a rename in types.ts is caught at compile time rather than silently breaking at runtime.
| setAgentPrompt, | ||
| currentPolicyYaml, | ||
| setCurrentPolicyYaml, | ||
| }: QuestionnaireControlsProps) => { |
There was a problem hiding this comment.
clients/admin-ui/src/features/prompt-explorer/QuestionnaireControls.tsx:75
Raw string literals in the guard — prefer the enum members that are already defined in types.ts:
// current
!["questionnaire", "access_policies"].includes(selectedPrompt.category)
// better
![PromptCategory.QUESTIONNAIRE, PromptCategory.ACCESS_POLICIES].includes(
selectedPrompt.category as PromptCategory,
)This is also a good opportunity to revisit the component name and JSDoc. QuestionnaireControls now handles access-policy prompts too, so the name is misleading. The long-term fix would be a dedicated AccessPolicyChatControls component, but at minimum the JSDoc on line ~65 should be updated to reflect the expanded scope.
| </div> | ||
| </Card> | ||
| )} | ||
|
|
There was a problem hiding this comment.
clients/admin-ui/src/features/prompt-explorer/QuestionnaireControls.tsx:265
Same raw string issue as the guard: promptType === "access_policy_chat" should be promptType === PromptType.ACCESS_POLICY_CHAT.
Also, the component interface has grown to ~20 props. The 4 new ones (agentPrompt, setAgentPrompt, currentPolicyYaml, setCurrentPolicyYaml) are entirely irrelevant to all non-access-policy paths. Consider either grouping them into an object prop:
accessPolicyChatState?: {
agentPrompt: string;
setAgentPrompt: (v: string) => void;
currentPolicyYaml: string;
setCurrentPolicyYaml: (v: string) => void;
};…or extracting an AccessPolicyChatControls component. Either keeps the prop surface manageable as more prompt types are added.
| @@ -168,7 +174,8 @@ const PromptExplorer: NextPage = () => { | |||
|
|
|||
| try { | |||
There was a problem hiding this comment.
clients/admin-ui/src/pages/poc/prompt-explorer.tsx:175
Raw string comparisons — both "questionnaire" and "access_policies" should use the enum:
selectedPrompt.category === PromptCategory.QUESTIONNAIRE ||
selectedPrompt.category === PromptCategory.ACCESS_POLICIESPromptCategory is already imported in this file, so this is a one-line fix.
| value={selectedCategory} | ||
| onChange={(value) => setSelectedCategory(value)} | ||
| options={[ | ||
| { label: "Access Policies", value: "access_policies" }, |
There was a problem hiding this comment.
clients/admin-ui/src/pages/poc/prompt-explorer.tsx:321
The new filter dropdown option uses a raw string value. Since selectedCategory is typed as PromptCategory | undefined, prefer the enum member to keep it consistent:
{ label: "Access Policies", value: PromptCategory.ACCESS_POLICIES },Same applies to the existing "assessment" and "questionnaire" entries while you're here.
| @@ -80,6 +80,12 @@ const PromptExplorer: NextPage = () => { | |||
| "How long do you keep this data?", | |||
| ); | |||
|
|
|||
There was a problem hiding this comment.
clients/admin-ui/src/pages/poc/prompt-explorer.tsx:82
The default state value hard-codes a test prompt:
const [agentPrompt, setAgentPrompt] = useState<string>(
"Create a policy that denies marketing use for EEA users",
);The TextArea below already has placeholder="e.g., Create a policy that denies marketing use", so the seeded default and the placeholder are redundant. Either initialize to "" and rely on the placeholder, or keep the default and remove the placeholder. Nit for a dev-tool page, but the current state is slightly confusing ("why is there already text?").
The backend uses control (singular string) but the frontend had controls (plural array) throughout. This caused 422s on create/update because extra="forbid" on the request schema rejected the unknown field. - Rename AccessPolicy.controls to control throughout - Map SidebarFormValues.controls[0] to control in save handlers - Fix list filtering, grouping, and table column to use control - Fix MSW mock data to match Note: AccessPolicyYaml.controls (the YAML content field) remains a list — that is a separate concept from the API metadata field.
1 participant
Ticket ENG-3160
Description Of Changes
Backend support changes for the access policy agent chat feature, targeting Lucano's UI branch. This PR adds the TypeScript types needed by the UI and extends the prompt explorer developer tool to support the new chat prompt.
Code Changes
AccessPoliciesApplicationConfigTypeScript type withagent_enabledfieldAccessPoliciesApplicationConfigintoPlusApplicationConfigtypes/api/index.tsACCESS_POLICY_CHATprompt type andACCESS_POLICIEScategory to prompt explorer typesagentPrompt/currentPolicyYamleditable inputs toQuestionnaireControlsfor the chat prompt typebuildQuestionnaireVariablesto handleaccess_policy_chatAccess Policiescategory to the prompt explorer filterSteps to Confirm
FIDESPLUS__ACCESS_POLICIES__AGENT_ENABLED=true/poc/prompt-explorer), select "Access Policy Agent Chat" — editable User Prompt and Current Policy YAML fields should appear before clicking Render PromptPre-Merge Checklist
CHANGELOG.mdupdated