feat(studio): run guardrail checks against an unsaved draft config - #1267
Open
aray12 wants to merge 1 commit into
Open
feat(studio): run guardrail checks against an unsaved draft config#1267aray12 wants to merge 1 commit into
aray12 wants to merge 1 commit into
Conversation
The Test and Validate tab could only exercise the saved config, so verifying an edit meant saving it first — publishing a half-tested change and writing a config version the user did not want to keep. Add a Draft/Saved run-target control. A Draft run sends the merged form state inline via the /checks endpoint's `config` field, which the service already supports, and resolves the request model from the draft. No backend change is needed. Runs now record what produced them: `is_draft` in place of a config version, and a snapshot of the activated guardrails, so a result keeps describing the config that ran even after the config changes. Both fields are optional, so records written earlier render unchanged. Extract useDraftRailsConfig so the Configuration and checks tabs share one definition of the draft rather than deriving it twice. Signed-off-by: Alex Ray <alray@nvidia.com>
Contributor
📝 WalkthroughWalkthroughGuardrail checks now support saved and unsaved draft configurations. Draft runs submit inline configuration data, record draft metadata and guardrail snapshots, and display the correct run origin and guardrail coverage. ChangesDraft guardrail execution
Suggested reviewers: Mergeability Score: ⚪ Minimal · up to The PR enables testing unsaved guardrail configurations and preserves the configuration context used for each result. No actionable merge-blocking risk remains; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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/api/guardrail-checks/types.ts (1)
30-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark persisted snapshot fields as readonly.
ActivatedGuardrailandactivated_guardrailsdescribe execution-time snapshots. Mutable types permit consumers to alter recorded history in memory.Proposed change
export interface ActivatedGuardrail { - id: string; - label: string; - active: boolean; + readonly id: string; + readonly label: string; + readonly active: boolean; } - is_draft?: boolean; - activated_guardrails?: ActivatedGuardrail[]; + readonly is_draft?: boolean; + readonly activated_guardrails?: readonly ActivatedGuardrail[];As per coding guidelines, “Use
readonlyfor immutable properties.”Also applies to: 53-55
🤖 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/api/guardrail-checks/types.ts` around lines 30 - 35, Mark the persisted snapshot properties in ActivatedGuardrail as readonly, including id, label, and active. Apply the same readonly treatment to the activated_guardrails fields around the referenced section, preserving their existing types and structure.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/api/guardrail-checks/types.ts`:
- Around line 30-35: Mark the persisted snapshot properties in
ActivatedGuardrail as readonly, including id, label, and active. Apply the same
readonly treatment to the activated_guardrails fields around the referenced
section, preserving their existing types and structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 71336349-d2d8-477e-954e-d9e51cb6ef26
📒 Files selected for processing (15)
web/packages/studio/src/api/guardrail-checks/guardrailChecks.test.tsweb/packages/studio/src/api/guardrail-checks/guardrailChecks.tsweb/packages/studio/src/api/guardrail-checks/hooks.tsweb/packages/studio/src/api/guardrail-checks/types.tsweb/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.test.tsxweb/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RailStatusTab.tsxweb/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.test.tsxweb/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/RunHistoryTab.tsxweb/packages/studio/src/components/sidePanels/GuardrailCheckDetailSidePanel/railLabels.tsweb/packages/studio/src/mocks/handlers/guardrails.tsweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/GuardrailTestCasesEditor.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.test.tsxweb/packages/studio/src/routes/guardrails/GuardrailChecksTab/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailConfigTab/index.tsxweb/packages/studio/src/routes/guardrails/GuardrailForm/useDraftRailsConfig.ts
Contributor
|
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
1 participant
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 Test and Validate tab could only exercise the saved guardrail config, so checking whether an edit worked meant saving it first — publishing a half-tested change to whatever already pointed at that config, and writing a config version the user did not want to keep. This adds a Draft/Saved run-target control: a Draft run sends the current form state inline to
/checksinstead of referencing the saved config by id. No backend change is needed — the guardrails service already accepts a wholeRailsConfiginguardrails.config, and Studio simply never populated it.Runs now also record what produced them, so a result keeps describing the config that ran even after that config changes.
Changes
GuardrailTestCasesEditor) — aSegmentedControlbeside "Run N Tests". Draft is selected when the form is dirty and disabled when it is pristine; picking Saved on a dirty form still exercises the saved config. The choice resets when navigating to a different config.guardrailChecks.ts) —runGuardrailCheck/runGuardrailCheckstake an optionaldraftConfig. When present it becomes the effective config for both model resolution and the request, sent asguardrails: { config: <RailsConfig> }. Never alongsideconfig_ids: the service's validator nulls those out for an objectconfig, which would make the request's meaning non-obvious from the wire. A draft also takes precedence over a check's storedguardrails, so an explicitly chosen target is never silently discarded.types.ts) —RunRecordgainsis_draft(in place of aconfig_version, which is meaningless for a draft) andactivated_guardrails, a snapshot of the coverage the config declared at run time. Both optional, so records written before this change deserialize and render unchanged.v{n}; Rail Status prefers the run's own snapshot over deriving coverage from the currently-loaded config, falling back to today's behaviour when a record has no snapshot.useDraftRailsConfig— extracts the server-data-plus-form-values derivation the Configuration tab already performed inline, so both tabs share one definition of "the draft" rather than deriving it twice./checkshandler — now honours an inline config so a draft-run test asserts the code's behaviour rather than a constant in the mock.ActivatedGuardrailmoves from the side-panel module toapi/guardrail-checks/types.ts, since it is now persisted data; re-exported fromrailLabels.tsfor existing importers.Note for reviewers:
guardrailChecks.tsimportsgetActivatedGuardrailsfrom the side-panel module, which is an api → components direction. It is deliberate — the alternative duplicates the detector/flow catalog and lets check-result labels drift from the config editor's. The clean fix is hoistingdetectors.ts/flowRegistry.tsout ofGuardrailConfigTab/into a neutral module, which is larger than this change warrants.Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
pnpm --filter nemo-studio-ui test— 310 files, 2847 tests passed (full suite, pre-rebase).pnpm --filter nemo-studio-ui test src/api/guardrail-checks src/routes/guardrails src/components/sidePanels/GuardrailCheckDetailSidePanel— 13 files, 101 tests passed (re-run after rebasing onto currentmain).pnpm --filter nemo-studio-ui typecheck— clean.pnpm --filter @nemo/common typecheck— clean.pnpm lint:fixthenpnpm lint— clean, no reformatting required.uv run pre-commit run -a— ruff, ruff format,ty, config reference, copyright headers, UI lint-staged, and merge-conflict hooks all passed. Two hooks did not run for environmental reasons, neither related to this change:Helm Docs Container(no local Docker daemon; no Helm files are touched) andRun uv lock with platform uv(local uv is 0.9.28, the hook requires 0.9.14).Check for uv.lock driftpassed and nopyproject.tomlis modified, so the lockfile is unaffected. CI runs both.Not run: manual end-to-end against a live platform, which needs a running instance. The new tests cover the wire format for both run targets, the recorded run fields, and the two rendering branches.
Existing coverage was deliberately left unmodified — the pre-existing
guardrailChecks.test.tscases still pass as written, which is the evidence that a Saved run sends a byte-identical request.Summary by CodeRabbit
New Features
Bug Fixes