feat(studio): Anonymizer entity picker categories and custom labels [ASTD-335] - #912
Merged
Merged
Conversation
Base automatically changed from
astd-334-anonymizer-rewrite-params/mschwab
to
main
July 27, 2026 21:24
marcusds
force-pushed
the
astd-335-anonymizer-entity-picker/mschwab
branch
from
July 27, 2026 22:05
3f7ee39 to
23fd3e8
Compare
Contributor
📝 WalkthroughWalkthroughThe anonymizer builder now categorizes entity labels, supports custom labels and removable selections, loads workspace defaults, validates custom-mode selections, and builds detection configuration from default and user-selected labels. ChangesAnonymizer entity labels
Sequence Diagram(s)sequenceDiagram
participant AnonymizerBuilderForm
participant useAnonymizerListEntityLabels
participant buildAnonymizerJobRequest
participant useAnonymizerCreateRunJob
AnonymizerBuilderForm->>useAnonymizerListEntityLabels: load workspace default labels
useAnonymizerListEntityLabels-->>AnonymizerBuilderForm: return defaultEntityLabels
AnonymizerBuilderForm->>buildAnonymizerJobRequest: build request from form and defaults
buildAnonymizerJobRequest-->>AnonymizerBuilderForm: return run-job request
AnonymizerBuilderForm->>useAnonymizerCreateRunJob: create run job
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts (1)
189-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for the excluded-defaults + no-custom-labels case.
Once the
buildDetectConfiggap noted inschema.tsis addressed, add a test assertingentityMode: 'custom', includeDefaultEntities: false, entityLabels: []does not silently fall back to server defaults.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts` around lines 189 - 231, The tests around buildAnonymizerJobRequest need coverage for custom mode with defaults excluded and no selected labels. Add a case using includeDefaultEntities: false and entityLabels: [] that asserts the detect configuration explicitly represents an empty label selection rather than being omitted or falling back to server defaults, alongside the existing custom-label tests.web/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsx (1)
34-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate hook subscription with
AnonymizerBuilderForm.tsx.This component and
AnonymizerBuilderForm.tsxboth calluseAnonymizerListEntityLabels(workspace, { query: {} })independently. Likely deduped via react-query's cache since the query key matches, but consider lifting the fetch to the parent and passingdata/isLoadingdown to avoid the duplicate subscription and keep a single source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsx` around lines 34 - 35, Lift the useAnonymizerListEntityLabels subscription out of EntitiesSection and AnonymizerBuilderForm into their shared parent, then pass the fetched data and loading state to both components through props. Update EntitiesSection’s available derivation to use the provided data, preserving the existing empty-array fallback and single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx`:
- Line 53: Gate form submission on the entity-label query completing so default
labels cannot be replaced by the interim empty array. Update the state derived
from useAnonymizerListEntityLabels and the Full Run button’s disabled condition,
and ensure the submit/buildDetectConfig path remains blocked while labels are
loading, alongside the existing createJob and model-loading checks.
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsx`:
- Around line 94-105: Update the X icon in the selected-label rendering within
EntitiesSection to use the lucide-react `size` prop instead of `fontSize`,
preserving the intended small icon appearance and the existing Tag behavior.
In `@web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts`:
- Around line 182-187: Update the conditional in the label-building logic after
`labels` is deduplicated so it checks whether the merged labels contain any
custom `form.entityLabels`, rather than comparing `labels.length` with
`defaultEntityLabels.length`. Preserve returning `undefined` when only default
entities are selected, including when `defaultEntityLabels` contains duplicates,
while retaining custom labels in the request.
- Around line 176-190: Update buildDetectConfig so custom mode with
includeDefaultEntities=false and an empty form.entityLabels is rejected rather
than returning undefined and triggering backend defaults. Require at least one
custom label or preserve defaults enabled, while keeping existing behavior for
non-empty selections and default-inclusive configurations.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsx`:
- Around line 34-35: Lift the useAnonymizerListEntityLabels subscription out of
EntitiesSection and AnonymizerBuilderForm into their shared parent, then pass
the fetched data and loading state to both components through props. Update
EntitiesSection’s available derivation to use the provided data, preserving the
existing empty-array fallback and single source of truth.
In `@web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts`:
- Around line 189-231: The tests around buildAnonymizerJobRequest need coverage
for custom mode with defaults excluded and no selected labels. Add a case using
includeDefaultEntities: false and entityLabels: [] that asserts the detect
configuration explicitly represents an empty label selection rather than being
omitted or falling back to server defaults, alongside the existing custom-label
tests.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b58e6d3e-3fff-421d-a63d-e9dbfc7c7f89
📒 Files selected for processing (7)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/entityItems.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/entityItems.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
Contributor
|
steramae-nvidia
approved these changes
Jul 27, 2026
marcusds
force-pushed
the
astd-335-anonymizer-entity-picker/mschwab
branch
from
July 27, 2026 23:48
b02c6d8 to
3497604
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
web/packages/common/src/components/form/ControlledCombobox/index.tsx (1)
47-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit return type to the exported component.
ControlledComboboxis a public, complex API. As per coding guidelines, “Use explicit return types for public APIs and complex functions in TypeScript.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/form/ControlledCombobox/index.tsx` around lines 47 - 62, Add an explicit JSX return type to the exported ControlledCombobox component declaration, preserving its existing generic parameter and implementation behavior.Source: Coding guidelines
web/packages/common/src/components/form/ControlledCombobox/index.test.tsx (2)
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrder imports by group.
Move the
@nemo/...import below external-library imports. As per coding guidelines, “Group imports: external libraries, internal modules, relative imports in TypeScript.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/form/ControlledCombobox/index.test.tsx` around lines 6 - 10, Reorder the imports in the ControlledCombobox test so external-library imports come first, followed by the internal `@nemo/common` import, preserving all existing imports and behavior.Source: Coding guidelines
79-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest typing with an existing selection.
Starting with
[]misses regressions that clear existing labels. Select['email'].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/common/src/components/form/ControlledCombobox/index.test.tsx` around lines 79 - 88, Update the test case “never writes raw input into a multi-select field” to initialize the multi-select with the existing `email` selection, then type custom text into the combobox and assert the value remains exactly `['email']`. Preserve the array-type assertion while covering that free-form input does not clear existing selections.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/packages/common/src/components/form/ControlledCombobox/index.test.tsx`:
- Around line 6-10: Reorder the imports in the ControlledCombobox test so
external-library imports come first, followed by the internal `@nemo/common`
import, preserving all existing imports and behavior.
- Around line 79-88: Update the test case “never writes raw input into a
multi-select field” to initialize the multi-select with the existing `email`
selection, then type custom text into the combobox and assert the value remains
exactly `['email']`. Preserve the array-type assertion while covering that
free-form input does not clear existing selections.
In `@web/packages/common/src/components/form/ControlledCombobox/index.tsx`:
- Around line 47-62: Add an explicit JSX return type to the exported
ControlledCombobox component declaration, preserving its existing generic
parameter and implementation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7876af8a-e057-4700-8d02-6bab09dd35bb
📒 Files selected for processing (9)
web/packages/common/src/components/form/ControlledCombobox/index.test.tsxweb/packages/common/src/components/form/ControlledCombobox/index.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/entityItems.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/entityItems.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/entityItems.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/EntitiesSection.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/constants.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/entityItems.test.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
- web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts
…ASTD-335] Group the entity-label picker under the 13 curated categories, colour the selections as chips, and allow labels outside the default set. The entity-labels endpoint returns a flat list of 65 strings, so the category grouping is a curated map in Studio, keyed to the same label ids the API uses. Anything the API adds later that isn't mapped falls into an Other bucket rather than disappearing from the menu. Custom labels are offered as a synthetic "Custom label" item built from whatever is typed, because the underlying combobox has no create affordance and the shared ControlledCombobox forces an empty inputValue in multi-select mode. Include-defaults is no longer mutually exclusive with a custom selection: the picker stays visible, the checkbox carries the live default count, and the request merges the defaults with the picks since entity_labels replaces the default set server-side. Signed-off-by: mschwab <mschwab@nvidia.com>
Gate Full Run on the entity-labels query so a submit that races it can't merge custom picks against an empty default set, which would silently narrow detection to just those picks. Reject custom mode with no labels and defaults excluded: that combination sent no detect config at all, so the server fell back to its own defaults instead of the restricted set Custom mode promises. Compare the merged label count against the deduplicated default count, so a duplicate in the defaults can't mask a genuinely new custom label. Derive the chip colour type from Tag instead of restating its palette, and size the chip icon with `size` since lucide ignores `fontSize`. Signed-off-by: mschwab <mschwab@nvidia.com>
…335] Now that the shared component accepts a caller-controlled inputValue (ASTD-344), the picker no longer needs its own Combobox plus useController. Chips read the selection through useWatch and remove via setValue, so the field has a single registration. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
astd-335-anonymizer-entity-picker/mschwab
branch
from
July 28, 2026 20:04
3497604 to
d738273
Compare
marcusds
enabled auto-merge
July 28, 2026 20:06
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.
Closes ASTD-335.
Upgrades the lean entity picker from ASTD-327 to the full design.
What's here
Notable decisions
The category map is curated in Studio.
/entity-labelsreturns a flat array of 65 bare strings with no category field (verified against a live service), so grouping has to live client-side. The map is keyed to the exact label ids the API returns and covers all 65. Anything the API adds later that isn't mapped lands in anOthersection rather than vanishing from the menu —buildEntitySectionsis driven by what the API returns, not by the map.Custom labels are a synthetic menu item. KUI's
Comboboxhas no creatable/free-solo mode, and the sharedControlledComboboxisn't usable here: it pinsinputValueto''in multi-select, and itsfreeFormpath writes the raw input string into what is astring[]field (ControlledCombobox/index.tsx:75-82— a latent bug for any multi+freeForm caller, left alone here to keep this PR off shared components). So this builds directly on KUICombobox+useController, and surfaces whatever is typed as a one-item Custom label section.Trigger uses
multipleMode="count". Rendering chips below the field while the combobox also renders its own tags in the trigger duplicated every selection.renderValuedoesn't suppress them — it only applies when closed, and returningnullreads as "no custom renderer".countgives a compact "2 selected" summary and leaves the coloured chips as the single source of truth.Include-defaults merge.
entity_labelsreplaces the default set server-side, so "include defaults" has to transmit them explicitly.buildDetectConfigunions defaults with the picks, and omitsdetectentirely when the selection adds nothing (empty, or a subset of the defaults) so the server stays on its own defaults instead of receiving a redundant 65-item list.No Strict/Permissive tabs. The ticket flagged these as "consider". Pulling the Figma Entities frame shows
Strict/Permissiveare the variant names of the design component, whose two states render the existing Custom / Auto-detect segmented control — already shipped in ASTD-327. Nothing to build; the descriptive copy was updated to match the design's wording.Testing
Otherfallback, custom-candidate rules, and the four detect-merge casesice_cream_flavoris offered and selectable, chips appear coloured with working removal, and the checkbox shows the live count of 65Screenshots
Summary by CodeRabbit
New Features
Bug Fixes