fix(studio): scope anonymizer sampling params to LLM roles [ASTD-417] - #1246
Merged
Conversation
The builder exposed temperature, max tokens, and top P for every Anonymizer role and sent max_tokens to all of them, including the GLiNER entity detector. GLiNER is a token-classification NIM driven by labels and a threshold, and the library's default models.yaml gives it only max_parallel_requests and timeout. Hide the params dropdown for the detector role and strip sampling keys from its request params. For the LLM roles, seed the defaults the library uses for gpt-oss-120b (temperature 0.3, top_p 0.95, max_tokens 16384) and raise the max-tokens slider ceiling so the dropdown reflects what is actually sent. Signed-off-by: mschwab <mschwab@nvidia.com>
Contributor
|
The absent dropdown reads clearly enough on its own. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
marked this pull request as ready for review
August 11, 2026 23:05
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds typed inference metadata overrides, role-specific sampling controls, and role-specific request parameter construction. GLiNER excludes sampling parameters. Tests cover control rendering and separate model configurations. ChangesInference parameter handling
Sequence Diagram(s)sequenceDiagram
participant ModelSettingsSection
participant ParamsDropdown
participant InferenceParameters
participant buildRoleParams
ModelSettingsSection->>ParamsDropdown: pass ANONYMIZER_PARAM_METADATA
ParamsDropdown->>InferenceParameters: forward fieldMetadata
InferenceParameters->>InferenceParameters: merge field metadata
ModelSettingsSection->>buildRoleParams: build role parameters
buildRoleParams->>buildRoleParams: filter unsupported sampling fields
buildRoleParams->>buildRoleParams: apply defaults and overrides
🚥 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: 1
🧹 Nitpick comments (1)
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsx (1)
10-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse type-only imports.
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsx#L10-L15: ImportAnonymizerFormData,FC, andReactNodewithimport type.web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts#L12-L17: ImportAnonymizerFormDatawithimport type.As per coding guidelines: “Use
import typefor type-only imports.”🤖 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.test.tsx` around lines 10 - 15, Use type-only imports for AnonymizerFormData, FC, and ReactNode in web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsx lines 10-15, and for AnonymizerFormData in web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts lines 12-17; leave the runtime imports unchanged.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.
Inline comments:
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsx`:
- Around line 39-40: Strengthen the test around the dropdown triggers by
explicitly asserting that the Entity Detector section has no
params-dropdown-trigger, while a sampling role still has one. Update the test
near activeRolesForStrategy(STRATEGY_SUBSTITUTE) so it verifies both the missing
entity_detector control and the retained sampling-role control, rather than
relying only on the aggregate count.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsx`:
- Around line 10-15: Use type-only imports for AnonymizerFormData, FC, and
ReactNode in
web/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsx
lines 10-15, and for AnonymizerFormData in
web/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.ts lines
12-17; leave the runtime imports unchanged.
🪄 Autofix
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: 21e44cf7-dd91-4d8d-94e7-2bb7c52efda6
📒 Files selected for processing (7)
web/packages/common/src/components/ModelSelectV2/InferenceParameters.tsxweb/packages/common/src/components/ModelSelectV2/ParamsDropdown.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.test.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/components/ModelSettingsSection.tsxweb/packages/studio/src/routes/AnonymizerBuilderRoute/constants.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.test.tsweb/packages/studio/src/routes/AnonymizerBuilderRoute/schema.ts
A bare trigger count passes even if the wrong role drops its controls. Signed-off-by: mschwab <mschwab@nvidia.com>
steramae-nvidia
approved these changes
Aug 12, 2026
SAMPLING_PARAM_KEYS in the anonymizer constants duplicated the private FIELDS const in InferenceParameters.tsx, which already backs InferenceParameterField. Export it as INFERENCE_PARAMETER_FIELDS and drop the anonymizer copy, so the set of sampling params GLiNER cannot take lives next to the controls that render them. Signed-off-by: mschwab <mschwab@nvidia.com>
Swap the anonymizer test mocks from openai/gpt-oss-120b to nvidia/nemotron-3-nano-30b-a3b, and import AnonymizerFormData, FC and ReactNode with import type. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
enabled auto-merge
August 12, 2026 18:46
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.
Summary
The Anonymizer builder showed the same three inference params (temperature, max tokens, top P) for every role and sent
max_tokensto all of them — including theentity_detector, which must be a GLiNER model. GLiNER is a token-classification NIM driven bylabels/thresholdinextra_body; the library's defaultmodels.yamlgives its config onlymax_parallel_requestsandtimeout, while the LLM roles gettemperature,top_p, andmax_tokens. This change makes Studio match that split.Answering the questions in the ticket:
entity_validator,entity_augmenter,latent_detector, andreplacement_generator— those roles are chat-completion models inanonymizer/config/default_model_configs/models.yaml.entity_detector— the upstream default config never sets them.gpt-oss-120bprofile:temperature 0.3,top_p 0.95,max_tokens 16384. (latent_detectordefaults to the nemotron profile upstream —temperature 0.4,top_p 1.0— but Studio applies one default across LLM roles and the user can adjust per role.)Related Issue
ASTD-417
Changes
ModelSettingsSection: no Params dropdown for the GLiNER detector role. Other roles keep the dropdown.schema.ts:buildRoleParamsbuilds request params per role —timeoutonly for the detector (sampling keys stripped even if form state holds them),timeout+max_tokens+temperature+top_pfor LLM roles.constants.ts: addsDEFAULT_MODEL_TEMPERATURE(0.3),DEFAULT_MODEL_TOP_P(0.95),MAX_MODEL_MAX_TOKENS(32768),SAMPLING_PARAM_KEYS,supportsSamplingParams, and the Anonymizer slider metadata overrides.InferenceParameters/ParamsDropdown(@nemo/common): new optionalfieldMetadataprop to override the shared slider bounds and defaults per caller. Existing callers are unaffected. Studio uses it so the sliders show the values actually sent — previously the dropdown showed the generic defaults (temperature 1.0, max tokens 1024, ceiling 4096) while the request carriedmax_tokens: 16384.ModelSettingsSection.test.tsx, plusschema.test.tscoverage that the detector receives no sampling params.Behavior note: because the detector's params now differ from the LLM roles', a run emits one
model_configfor the detector and one per distinct LLM param set instead of collapsing them into a single alias. Role-to-alias mapping is unchanged.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified below — not run repo-wide; the commit-stage hooks ran and passed on the changed files.Targeted validation:
pnpm --filter nemo-studio-ui test src/routes/AnonymizerBuilderRoute— 7 files, 54 tests passedpnpm --filter @nemo/common test src/components/ModelSelectV2— 3 files, 30 tests passedpnpm --filter nemo-studio-ui typecheckandpnpm --filter @nemo/common typecheck— cleanpnpm lint:fix(fromweb/) — clean, no changesSummary by CodeRabbit
New Features
Bug Fixes
Tests