feat(experiments): Create Pareto chart view#882
Conversation
|
|
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:
📝 WalkthroughWalkthroughChangesExperiment Group Pareto
RL Jobs API
Deployment volume configuration
Evaluator streaming aggregation
Sequence Diagram(s)sequenceDiagram
participant User
participant ExperimentGroupDetailRoute
participant ExperimentGroupDataView
participant ExperimentGroupParetoChart
participant useListEvaluations
User->>ExperimentGroupDetailRoute: toggle Pareto view
ExperimentGroupDetailRoute->>ExperimentGroupDataView: pass paretoVisible
ExperimentGroupDataView->>ExperimentGroupParetoChart: render chart
ExperimentGroupParetoChart->>useListEvaluations: load group evaluations
useListEvaluations-->>ExperimentGroupParetoChart: evaluation rows
ExperimentGroupParetoChart-->>User: scatter plot and frontier
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@plugins/nemo-deployments/openapi/openapi.yaml`:
- Around line 973-983: Update the OpenAPI schema relationship between initChmod
and initImage so configurations that set initImage must also provide initChmod,
while preserving their existing types and descriptions. Add the same dependency
validation to DockerVolumeConfig, ensuring image-only configurations are
rejected before deployment.
- Around line 973-979: Restrict the initChmod property in the OpenAPI schema to
valid chmod mode strings, such as exactly three or four octal digits (for
example, 0777), using the schema’s pattern validation. Keep the existing string
type and description while ensuring malformed values are rejected before volume
creation.
In
`@web/packages/studio/src/components/charts/ExperimentGroupParetoChart/index.tsx`:
- Around line 140-154: Update ExperimentGroupParetoChart so xMetricId and
yMetricId reset when group.id changes, using the incoming group.pareto values
with the existing defaults. Add the synchronization in the component state flow
without changing the optimistic updates or persistence behavior.
In
`@web/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsx`:
- Around line 494-515: Add an accessible name to the Pareto toggle Button in the
paretoVisible control, using an aria-label that clearly identifies the
action/state when its hide-mobile text is not rendered. Preserve the existing
aria-pressed behavior and visible label switching.
🪄 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: 8b7cc2bb-d0e4-40cd-a0c9-cddd4567e004
⛔ Files ignored due to path filters (16)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiment_groups/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiment_groups/experiment_groups.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/experiment_group_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/experiment_group_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/experiment_group_update_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_config_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_data_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_metric_point.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/inference/container_executor_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/inference/container_executor_config_param.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_experiment_groups.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (15)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlplugins/nemo-customizer/openapi/openapi.yamlplugins/nemo-deployments/openapi/openapi.yamlplugins/nemo-evaluator/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/intake/src/nmp/intake/api/v2/experiments/endpoints.pyservices/intake/src/nmp/intake/api/v2/experiments/schemas.pyservices/intake/src/nmp/intake/entities/experiments.pyservices/intake/tests/integration/test_experiments_crud.pyweb/packages/studio/src/components/charts/ExperimentGroupParetoChart/index.tsxweb/packages/studio/src/components/charts/ExperimentGroupParetoChart/paretoMetrics.test.tsweb/packages/studio/src/components/charts/ExperimentGroupParetoChart/paretoMetrics.tsweb/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsx
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
…hange - Remove the now-unused ChartScatter import from ExperimentGroupDataView (the toggle moved to ExperimentGroupDetailRoute), fixing the Web lint + typecheck CI failures. - Key ExperimentGroupParetoChart by group.id so the axis selection re-seeds from the new group's saved config when navigating between groups without a route remount (CodeRabbit: axes could otherwise persist and be saved onto the wrong group). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Signed-off-by: shanaiabuggy <[email protected]>
2178bea to
ad249a1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@services/intake/src/nmp/intake/api/v2/experiments/schemas.py`:
- Around line 51-54: Update ParetoConfig validation for x_metric and y_metric so
values are restricted to cost_usd, latency_ms, or the evaluators.<name> pattern
while preserving dynamic evaluator names. Reject unsupported identifiers before
the configuration is persisted, and keep the existing default ParetoConfig
behavior unchanged.
- Around line 51-54: Update the ExperimentGroupRequest/Pareto configuration
handling so omitted pareto values remain distinguishable from explicitly
supplied defaults during updates. Ensure the update endpoint preserves
existing.pareto when the request omits this field, while still applying an
explicitly provided ParetoConfig; use a separate update schema or an equivalent
optional-field approach.
🪄 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: 280fa26c-bbc6-49ff-b524-43789f0b2cc5
⛔ Files ignored due to path filters (16)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/.nmpcontext/stainless.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiment_groups/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiment_groups/experiment_groups.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/experiment_group_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/experiment_group_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/experiment_group_update_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_config_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_data_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiment_groups/pareto_metric_point.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/inference/container_executor_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/inference/container_executor_config_param.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_experiment_groups.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (16)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlplugins/nemo-customizer/openapi/openapi.yamlplugins/nemo-deployments/openapi/openapi.yamlplugins/nemo-evaluator/openapi/openapi.yamlservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/intake/src/nmp/intake/api/v2/experiments/endpoints.pyservices/intake/src/nmp/intake/api/v2/experiments/schemas.pyservices/intake/src/nmp/intake/entities/experiments.pyservices/intake/tests/integration/test_experiments_crud.pyweb/packages/studio/src/components/charts/ExperimentGroupParetoChart/index.tsxweb/packages/studio/src/components/charts/ExperimentGroupParetoChart/paretoMetrics.test.tsweb/packages/studio/src/components/charts/ExperimentGroupParetoChart/paretoMetrics.tsweb/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsxweb/packages/studio/src/routes/ExperimentGroupDetailRoute/index.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- services/intake/src/nmp/intake/entities/experiments.py
- services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
- plugins/nemo-deployments/openapi/openapi.yaml
- plugins/nemo-evaluator/openapi/openapi.yaml
- web/packages/studio/src/routes/ExperimentGroupDetailRoute/index.tsx
- web/packages/studio/src/components/charts/ExperimentGroupParetoChart/paretoMetrics.ts
- web/packages/studio/src/components/charts/ExperimentGroupParetoChart/index.tsx
- openapi/openapi.yaml
- openapi/ga/individual/platform.openapi.yaml
- web/packages/studio/src/components/charts/ExperimentGroupParetoChart/paretoMetrics.test.ts
- services/intake/tests/integration/test_experiments_crud.py
- services/intake/src/nmp/intake/api/v2/experiments/endpoints.py
- openapi/ga/openapi.yaml
- plugins/nemo-customizer/openapi/openapi.yaml
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
…pdates Address CodeRabbit review on PR #882: - Restrict ParetoConfig x/y metrics to cost_usd, latency_ms, or evaluators.<name> so invalid identifiers can't be persisted as unusable chart defaults. - Make ExperimentGroupRequest.pareto optional so an update that omits it keeps the saved axes instead of resetting them to the cost/latency default. - Regenerate the OpenAPI spec; add tests covering rejection and preservation. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Signed-off-by: shanaiabuggy <[email protected]>
BrianNewsom
left a comment
There was a problem hiding this comment.
Backend changes look good once the build is successful!
Signed-off-by: shanaiabuggy <[email protected]>
https://www.loom.com/share/43716ee8bde241a1aeb186be5a5d4384
Summary by CodeRabbit