feat(experiments): Add new fields to support Optimization team#540
Conversation
Signed-off-by: shanaiabuggy <[email protected]>
|
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 ignored due to path filters (9)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds optional lineage, status, and metadata fields to experiment and experiment-group models, propagates them through intake request/response schemas and endpoints, mirrors them in OpenAPI, and adds tests for round-trip, validation, and default-null behavior. ChangesExperiment evaluator fields
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/intake/tests/test_experiment_evaluator_fields.py (1)
19-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing group-update round-trip test.
Experiment fields get an update-path test (line 60-78) but group fields (
insight_id/summary/metadata) only cover create. Add a PUT test for parity.🤖 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 `@services/intake/tests/test_experiment_evaluator_fields.py` around lines 19 - 28, Add a missing update-path round-trip test for group evaluator fields in test_group_evaluator_fields_round_trip’s area: after the create assertion, exercise the groups update endpoint with a PUT on the created group and verify insight_id, summary, and metadata are preserved/returned just like the experiment update test does. Use the existing GROUPS client flow and the same field names so the test covers both create and update parity.
🤖 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 `@services/intake/tests/test_experiment_evaluator_fields.py`:
- Around line 19-28: Add a missing update-path round-trip test for group
evaluator fields in test_group_evaluator_fields_round_trip’s area: after the
create assertion, exercise the groups update endpoint with a PUT on the created
group and verify insight_id, summary, and metadata are preserved/returned just
like the experiment update test does. Use the existing GROUPS client flow and
the same field names so the test covers both create and update parity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 47f4425a-c99f-45e8-b6a0-24fe2d92d860
⛔ 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/experiment_groups.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiments/api.mdis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiments/experiments.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/experiments/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiments/experiment_create_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiments/experiment_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiments/experiment_status.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiments/experiment_update_params.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_experiment_groups.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_experiments.pyis excluded by!sdk/**sdk/stainless.yamlis excluded by!sdk/**
📒 Files selected for processing (7)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.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/test_experiment_evaluator_fields.py
|
BrianNewsom
left a comment
There was a problem hiding this comment.
Approved after changes discussed live!
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
* feat(experiments): Add new fields to support NeMo Evaluator Signed-off-by: shanaiabuggy <[email protected]> * pr comments Signed-off-by: shanaiabuggy <[email protected]> * lint Signed-off-by: shanaiabuggy <[email protected]> --------- Signed-off-by: shanaiabuggy <[email protected]>
Summary by CodeRabbit