feat(studio): Anonymizer Rewrite strategy params [ASTD-334] - #904
Merged
Conversation
Unlock Rewrite in the anonymization strategy select and add its parameter
UI: privacy goal (default/custom with protect + preserve), LLM
instructions, risk tolerance, max repair rounds, and strict entity
protection, mapped onto config.rewrite.
Rewrite reuses the replacement generator internally, so the backend
validates replace.replacement_generator for rewrite jobs too. Without it
the request kept the library default alias and preflight failed with
"Selected model aliases not found in model pool". Map the replace role
for rewrite as well.
Also fixes two pre-existing issues surfaced while testing this form:
select items carry `children`, not `label`, so triggers were rendering
raw values ("rewrite", "dataset"); and the panel's scroll lived on the
root, which swept the footer into the scrolling box and floated the
Cancel/Full Run buttons over the form.
Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
astd-334-anonymizer-rewrite-params/mschwab
branch
from
July 27, 2026 16:17
f418d15 to
ecf3d1f
Compare
Contributor
|
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:
📝 WalkthroughWalkthroughThe anonymizer builder now supports rewrite-specific configuration, request serialization, model initialization, tabbed form rendering, and nested rewrite API error mapping. ChangesRewrite builder support
Sequence Diagram(s)sequenceDiagram
participant AnonymizerBuilderForm
participant buildAnonymizerJobRequest
participant useAnonymizerCreateRunJob
participant AnonymizerBuilderRoute
AnonymizerBuilderForm->>buildAnonymizerJobRequest: submit rewrite form data
AnonymizerBuilderForm->>useAnonymizerCreateRunJob: create run job
useAnonymizerCreateRunJob->>AnonymizerBuilderRoute: return job result
AnonymizerBuilderRoute->>AnonymizerBuilderRoute: navigate to job or workspace
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.
🧹 Nitpick comments (1)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/RewriteParamsSection.tsx (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a type-only React import.
Change to
import type { FC } from 'react';. As per coding guidelines, “Useimport typefor type-only 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/studio/src/routes/AnonymizerBuilderRoute/components/RewriteParamsSection.tsx` at line 17, Update the React import in RewriteParamsSection.tsx to use a type-only import for FC, preserving the existing type usage without importing a runtime value.Source: Coding guidelines
🤖 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/studio/src/routes/AnonymizerBuilderRoute/components/RewriteParamsSection.tsx`:
- Line 17: Update the React import in RewriteParamsSection.tsx to use a
type-only import for FC, preserving the existing type usage without importing a
runtime value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0254d7a9-e9cc-41bf-8fca-3ab024419824
📒 Files selected for processing (9)
web/packages/studio/src/routes/AnonymizerBuilderRoute/apiErrors.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/apiErrors.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/GenerationSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/RewriteParamsSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/StrategyParamsSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
Move the risk-tolerance step formatter and the panel attributes object to module scope so they keep referential identity across renders instead of being rebuilt each time. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
|
Extract the workspace model list and the per-role defaulting effect into useAnonymizerModels/useDefaultRoleModels, and run the defaulting from a RoleModelDefaults child of the form provider. Previously the defaults were seeded by an effect inside ModelSettingsSection, so they only applied because that tab stays mounted; ModelSettingsSection is now pure render. Groundwork for wrapping the tab panels in <Activity>, which is not safe here yet: react-hook-form's useWatch subscribes in an effect, and Activity tears down effects while hidden, so the inactive tab renders stale values. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
FCas a type.
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsx#L14-L14: splitFCintoimport type { FC } from 'react'.web/packages/studio/src/routes/AnonymizerBuilderRoute/components/RoleModelDefaults.tsx#L5-L5: change toimport type { FC } from 'react'.As per coding guidelines, “Use
import typefor type-only 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/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsx` at line 14, Update the React imports in ModelSettingsSection.tsx (line 14) and RoleModelDefaults.tsx (line 5) to import FC using import type, keeping any runtime React imports separate.Source: Coding guidelines
🤖 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/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsx`:
- Line 14: Update the React imports in ModelSettingsSection.tsx (line 14) and
RoleModelDefaults.tsx (line 5) to import FC using import type, keeping any
runtime React imports separate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e07b8bc4-c8ce-4531-938d-90f6fcbbd290
📒 Files selected for processing (5)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/RoleModelDefaults.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/useAnonymizerModels.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/useDefaultRoleModels.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsx
marcusds
changed the title
nakolean
reviewed
Jul 27, 2026
Move the form UI, tab state, and submit handling out of the route into AnonymizerBuilderForm, rendered under FormProvider. The component can now call useDefaultRoleModels directly, so the RoleModelDefaults null-render shim that only existed to reach form context is gone. The route's separate useModelsListProviders call was identical to the one inside useAnonymizerModels, so the submit button's loading state now comes from useDefaultRoleModels instead. Also import FC with the inline type modifier in the touched files. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx (1)
16-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGroup external imports before internal aliases.
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx#L16-L32: move React, react-hook-form, and react-router-dom imports before@studio/*.web/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsx#L8-L14: keep React and react-hook-form imports in the external import group before@studio/*.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/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx` around lines 16 - 32, Group external imports before internal aliases: in web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx lines 16-32, move React, react-hook-form, and react-router-dom imports above the `@studio` imports; apply the same ordering in web/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsx lines 8-14 by placing React and react-hook-form imports before `@studio` imports.Source: Coding guidelines
🤖 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/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx`:
- Around line 16-32: Group external imports before internal aliases: in
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsx
lines 16-32, move React, react-hook-form, and react-router-dom imports above the
`@studio` imports; apply the same ordering in
web/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsx lines 8-14 by
placing React and react-hook-form imports before `@studio` imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7d813951-1fa8-43fa-a396-4885eae76860
📒 Files selected for processing (4)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/AnonymizerBuilderForm.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/RewriteParamsSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/RewriteParamsSection.tsx
- web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsx
nakolean
approved these changes
Jul 27, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jul 27, 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.
Closes ASTD-334.
Unlocks Rewrite in the anonymization strategy select and adds its parameter UI, the last of the five strategies. Follows #898 (ASTD-333).
What's here
New
RewriteParamsSection, rendered below the strategy dropdown when Rewrite is selected:rewrite.privacy_goalrewrite.instructionsrewrite.risk_tolerancerewrite.max_repair_iterationsrewrite.strict_entity_protectionDefault mode omits
privacy_goalentirely so the server auto-populates it, rather than sending empty strings that would 422. Per the ASTD-333 precedent there are no client-side length rules on protect/preserve — the backend's 422s map to inline field errors, andapiErrors.tsnow covers the rewrite locs.Notable decisions
Risk Tolerance uses the raw KUI
Slider, notControlledSlider.ControlledSliderwrites a number into the form field unconditionally, which would clobber the enum. RawSlider+useControllerkeepsriskToleranceas'minimal' | 'low' | 'moderate' | 'high'in form state with the index mapping local to the component;stepFormatFnrenders the tick labels.RISK_TOLERANCE_LABELSisRecord<RiskTolerance, string>— exhaustive against the SDK enum, so a new preset fails the build rather than silently rendering without a label.Fix: rewrite jobs must map the replacement generator
Rewrite jobs were failing preflight with:
Rewrite reuses the replacement generator internally, so the library validates that role for rewrite too:
We were sending
selected_modelswith onlydetection+rewrite, so the server kept its own default (default_model_configs/replace.yaml→gpt-oss-120b), which isn't in the pool we send.activeRolesForStrategy('rewrite')now includesREPLACE_ROLEand the builder emitsselected_models.replace. Verified by callingvalidate_model_alias_referencesdirectly with both payload shapes — old fails with the exact error above, new passes.Drive-by fixes
Both pre-existing from ASTD-327, surfaced while testing this form:
rewrite,dataset,sha256). KUI'sSelectEntryis{ children, value }—labelisn't a prop, so it was dropped and the trigger fell back to the value. Fixed across the three option lists in this route. Typecheck didn't catch it because the extra key was structurally compatible.overflow-autowas on the Panel root, but.nv-panel-rootis the flex column holding the footer, so the footer scrolled with the content. Moved the scroll to the content slot (flex-1 min-h-0 overflow-auto).Screenshots
Testing
Out of scope
ASTD-336 filed separately: the Entity Detector role falls back to a general LLM when the workspace has no GLiNER model, and the library injects GLiNER-only params into whatever that role points at, so jobs die at detection with a 400. Pre-existing and affects all five strategies.
Summary by CodeRabbit
New Features
Bug Fixes