Skip to content

feat(experiments): Allow an evaluation to belong to multiple experiment groups#759

Merged
shanaiabuggy merged 14 commits into
mainfrom
sbuggy/ase-559
Jul 22, 2026
Merged

feat(experiments): Allow an evaluation to belong to multiple experiment groups#759
shanaiabuggy merged 14 commits into
mainfrom
sbuggy/ase-559

Conversation

@shanaiabuggy

@shanaiabuggy shanaiabuggy commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
Screen.Recording.2026-07-20.at.12.20.35.PM.mov

Summary

Lets a single evaluation belong to multiple experiment groups instead of exactly one, and adds the UI to curate evaluations across groups. Membership becomes many-to-many while the old single-group field keeps working as a deprecated alias. (ASE-559)

Backend

Entities

  • Add a $contains array-membership filter operator (SQL + in-memory parity) so lists can match rows whose array field contains a value.
  • Fix eq(field, None) to also match an absent JSON key — on PostgreSQL a missing key extracts to SQL NULL, which the previous == "null" check missed (added an IS NULL branch).

Intake

  • EvaluationRequest/Response gain experiment_ids: list[str] (≥1) as the canonical membership field. experiment_group_id stays as a deprecated request alias (coalesced into experiment_ids) and response alias (first of experiment_ids).
  • The experiment_group_id list filter is rewritten to a membership match spanning experiment_ids (new rows) and the legacy scalar (old rows), so listing a group returns every member.
  • Deleting a group cascades: members drop that membership; a member whose only membership was that group is soft-deleted.
  • New PATCH /evaluations/{name} — a partial update (only fields present in the request change), mirroring the models service's PATCH. The add-to-group flow sends the merged experiment_ids; any new group must exist and the set must be non-empty (the ≥1-group invariant). Rejects empty experiment_ids (400) and unknown group ids (400).

Design note: membership is a field on the evaluation, so we curate it via the resource's PATCH — additive and non-breaking, matching the house "PATCH = partial" convention — rather than adding dedicated add/remove endpoints. The full-body PUT is unchanged.

Frontend

  • New bulk "Add to experiment group" action on the evaluations list: select rows, add them to another group (via PATCH with the merged experiment_ids).
  • Modal picks the target group from a dropdown, with an inline "Create new group" option (name / description / default sort) that creates the group and then adds the selected evaluations.
  • Groups every selected evaluation already belongs to are excluded; adds are best-effort and surface a warning on partial failure.

Generated artifacts / follow-ups

  • Regenerated OpenAPI specs, web SDK, and static authz mappings.
  • Python SDK (Stainless) regen is a separate key-gated step — run make stainless before merge.

Summary by CodeRabbit

Summary

  • New Features
    • Partial update API for evaluations (PATCH) with experiment_ids to replace group memberships.
    • Multi-group ownership for evaluations via experiment_ids.
    • Studio bulk “Add to experiment group” (add to existing or create new).
    • Array “contains” filtering support.
  • Improvements
    • experiment_group_id remains as a deprecated alias; APIs now primarily use experiment_ids.
    • Group deletion and listing behavior now respects multi-group memberships.
  • Bug Fixes
    • Reject empty experiment_ids and unknown group IDs.
    • Deleting a group won’t remove evaluations that belong to other groups.

Adds a CONTAINS ("$contains") comparison operator to the entity-store filter
engine so callers can query "value is an element of a JSON array field" — the
prerequisite for modeling an experiment's group membership as a list (ASE-559).

- filter_ops.py: new FilterOperator.CONTAINS, a default contains() on the
  FilterRepository ABC (additive — existing repos need not implement it), and
  dispatch in ComparisonOperation.apply. The JSON filter parser is generic over
  the enum, so $contains parses with no parser change.
- SQLAlchemyFilterRepository.contains: portable across SQLite (JSON) and
  PostgreSQL (JSONB) via a quote-delimited match on the serialized array text;
  wildcard-escaped and prefix-collision-safe ("g1" never matches ["g10"]).
- InMemoryFilterRepository.contains: native membership, matching SQL semantics.
- Parity harness extended with $contains cases incl. a prefix near-miss.

Validated: 32 parity cases pass (SQLite + in-memory agree); 113 entities +
167 plugin filter tests pass; ruff + ty clean. The Postgres path is portable by
design; Docker-backed integration tests confirm it in CI.

Signed-off-by: shanaiabuggy <[email protected]>
Relax the one-group-per-evaluation constraint to many-to-many. An evaluation's
single experiment_group_id becomes an experiment_ids list (>=1); everything that
renders a group's runs (list view, rollups, sort) now spans the membership set.

- Entity: Experiment.experiment_group_id -> experiment_ids: list[str] (min 1),
  with a model_validator(mode="before") coercing legacy single-id rows to
  [id] on read (no data migration). parent_experiment_id (lineage) untouched.
- Create/update API: EvaluationRequest accepts experiment_ids (>=1);
  experiment_group_id kept as a deprecated coalesced alias. EvaluationResponse
  exposes experiment_ids with experiment_group_id as a deprecated computed alias
  (= experiment_ids[0]). Each group id is validated to exist + be live.
- Membership queries: a shared _group_membership_filter() matches
  "$contains experiment_ids OR legacy scalar experiment_group_id", used by the
  list filter (rewriting the exposed experiment_group_id param), both count
  helpers, and the delete cascade — so un-migrated rows stay queryable.
- Delete cascade is now reference-counted: a member whose sole membership was the
  deleted group is soft-deleted; a member shared with another group just drops
  that membership and survives there.

Tests: multi-group create appears in both leaderboards; empty experiment_ids
rejected; reference-counted cascade (shared member survives, sole member gone);
existing single-group + deprecated-alias paths still pass. Full intake
experiment/evaluation suite green (SQLite).

Signed-off-by: shanaiabuggy <[email protected]>
@github-actions github-actions Bot added the feat label Jul 17, 2026
…ASE-559]

Address review: the create request should not force the new field on clients.

- EvaluationRequest.experiment_ids is now optional (default_factory=list) rather
  than required, so the request stays purely additive; experiment_group_id
  remains an accepted (deprecated) field. A model_validator requires at least one
  of the two and coalesces the deprecated single id into experiment_ids.
- experiment_group_id is unchanged in the response body (a deprecated computed
  alias = experiment_ids[0]), consistent with the existing parent_experiment_id
  deprecation pattern.

Signed-off-by: shanaiabuggy <[email protected]>
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 27113/34832 77.8% 62.1%
Integration Tests 15891/33443 47.5% 19.9%

Curate existing runs into comparison groups (e.g. an all-benchmarks board)
after creation:

- POST   /evaluations/{name}/groups/{group_id} — add a membership. Validates the
  group exists + is live; idempotent when already a member.
- DELETE /evaluations/{name}/groups/{group_id} — remove a membership. Rejects
  removing the evaluation's last group with 409 (>=1-group invariant; delete the
  evaluation instead); idempotent when not a member.

Modeled on the pin/unpin endpoints. Tests cover add (idempotent + appears on the
target board), add-to-unknown-group (400), remove (drops the membership), and
remove-last-group (409).

Signed-off-by: shanaiabuggy <[email protected]>
…p [ASE-559]

Regenerated via `make refresh-openapi` after the many-to-many API changes:
- EvaluationRequest/EvaluationResponse gain experiment_ids; experiment_group_id
  retained as a deprecated field/alias.
- New POST/DELETE /evaluations/{name}/groups/{group_id} membership endpoints.

Signed-off-by: shanaiabuggy <[email protected]>
…559]

A per-row "Add to group" action (FolderPlus, next to the pin toggle) in the
ExperimentGroupDataView opens a modal with a searchable list of the workspace's
experiment groups — excluding the ones the evaluation already belongs to — and
adds the run to the selected group so it appears on that board too. Mirrors the
pin action's mutation + scoped query invalidation, with success/error toasts.

Lets users assemble a cross-benchmark ("all-benchmarks") board from existing
runs. Uses the generated useAddEvaluationToGroup / useListExperimentGroups hooks.

- AddToGroupModal.tsx (+ .test.tsx, 4 cases: lists/excludes members, filters by
  search, calls the add hook, empty state)
- ExperimentGroupDataView: row-action button + modal wiring (pin logic unchanged)

typecheck + component test + eslint clean.

Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>
@shanaiabuggy
shanaiabuggy marked this pull request as ready for review July 20, 2026 19:59
@shanaiabuggy
shanaiabuggy requested review from a team as code owners July 20, 2026 19:59
Signed-off-by: shanaiabuggy <[email protected]>
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The API and intake service now support multi-group evaluation ownership, backward-compatible aliases, containment filtering, PATCH updates, authorization, lifecycle tests, and Studio bulk assignment through existing or newly created groups.

Changes

Evaluation membership

Layer / File(s) Summary
Membership contracts and compatibility
openapi/..., services/intake/src/nmp/intake/api/v2/experiments/schemas.py, services/intake/src/nmp/intake/entities/experiments.py
Requests and entities use experiment_ids; legacy experiment_group_id values are coalesced or exposed as aliases.
Array containment filtering
packages/nemo_platform_plugin/src/nemo_platform_plugin/*, services/core/entities/src/.../filter.py, services/core/entities/tests/*
Adds $contains support to in-memory and SQL repositories and verifies parity for JSON arrays.
Membership lifecycle and API integration
services/intake/src/nmp/intake/api/v2/experiments/endpoints.py, services/core/auth/src/.../static-authz.yaml, services/intake/tests/integration/*, services/intake/tests/test_experiment_*.py
Creation, updates, listings, deletion, counts, PATCH operations, authorization, and lifecycle tests use multi-group semantics.

Studio bulk assignment

Layer / File(s) Summary
Bulk assignment modal
web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.*
The modal filters eligible groups, supports group creation, patches selected evaluations, and tests success and edge cases.
Data-view wiring and pinned layout
web/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsx, ExperimentGroupDataView.css
The data view adds a bulk action, manages modal state, pins selection columns, and updates pinned-column styling.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant EvaluationEndpoint
  participant EntityStore
  participant EvaluationResponse
  Client->>EvaluationEndpoint: PATCH evaluation experiment_ids
  EvaluationEndpoint->>EntityStore: validate groups and replace membership
  EntityStore-->>EvaluationEndpoint: updated evaluation
  EvaluationEndpoint->>EvaluationResponse: hydrate rollups
  EvaluationResponse-->>Client: return updated evaluation
Loading
sequenceDiagram
  participant User
  participant ExperimentGroupDataView
  participant AddToGroupModal
  participant PlatformAPI
  User->>ExperimentGroupDataView: select evaluations
  ExperimentGroupDataView->>AddToGroupModal: open assignment modal
  AddToGroupModal->>PlatformAPI: create group or patch evaluations
  PlatformAPI-->>AddToGroupModal: return mutation results
  AddToGroupModal-->>ExperimentGroupDataView: clear selection on success
Loading

Possibly related PRs

Suggested reviewers: briannewsom, mckornfield, marcusds

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: evaluations can now belong to multiple experiment groups.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sbuggy/ase-559

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.test.tsx (1)

79-152: 🎯 Functional Correctness | 🔵 Trivial

Missing coverage for error paths.

No test exercises the duplicate-name creation failure (setError('name', ...) path) or a partial-failure Promise.allSettled scenario (some mockMutateAsync rejections). These are the exact error-handling branches most likely to regress silently.

Want me to draft these two additional test cases (duplicate-name rejection and partial-add-failure toast)?

🤖 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/components/dataViews/ExperimentGroupDataView/AddToGroupModal.test.tsx`
around lines 79 - 152, Extend the AddToGroupModal tests with coverage for both
error branches: make mockCreateMutateAsync reject with the duplicate-name error
and assert the name field receives the setError validation message, then make
some mockMutateAsync calls reject and assert the Promise.allSettled
partial-failure path displays the expected failure toast while preserving
successful additions and completion behavior.
web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx (1)

185-201: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Extract duplicate-name handling into a shared helper

This hardcodes a backend sentence in UI logic, and the same check is duplicated in ExperimentGroupCreateModal. Centralize the duplicate-name detection in one helper so a backend copy tweak only needs one update.

🤖 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/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx`
around lines 185 - 201, The duplicate-name detection in the AddToGroupModal
catch block is hardcoded and duplicated elsewhere. Extract it into a shared
helper, such as an isDuplicateExperimentGroupError utility, and update both
AddToGroupModal and ExperimentGroupCreateModal to use it while preserving the
existing inline name-field error behavior.
🤖 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 `@openapi/ga/openapi.yaml`:
- Around line 3555-3652: Add deprecated POST and DELETE aliases for the existing
groups/{group_id} evaluation routes alongside the experiment routes, preserving
their current request parameters, responses, and operation behavior while
marking both operations as deprecated. Keep the new experiments/{experiment_id}
endpoints unchanged.

In
`@web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx`:
- Line 1: Run Prettier with write enabled on AddToGroupModal.tsx (anchor),
AddToGroupModal.test.tsx, and ExperimentGroupDataView.css to apply the
repository’s formatting requirements.

In
`@web/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsx`:
- Line 175: Update the columnPinning.left configuration in
ExperimentGroupDataView to order the pinned columns as row-selection followed by
pin, ensuring the checkbox renders before the pin toggle.

---

Nitpick comments:
In
`@web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.test.tsx`:
- Around line 79-152: Extend the AddToGroupModal tests with coverage for both
error branches: make mockCreateMutateAsync reject with the duplicate-name error
and assert the name field receives the setError validation message, then make
some mockMutateAsync calls reject and assert the Promise.allSettled
partial-failure path displays the expected failure toast while preserving
successful additions and completion behavior.

In
`@web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx`:
- Around line 185-201: The duplicate-name detection in the AddToGroupModal catch
block is hardcoded and duplicated elsewhere. Extract it into a shared helper,
such as an isDuplicateExperimentGroupError utility, and update both
AddToGroupModal and ExperimentGroupCreateModal to use it while preserving the
existing inline name-field error behavior.
🪄 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: dd1584e0-592c-44c1-b46c-b608b824457c

📥 Commits

Reviewing files that changed from the base of the PR and between d18f684 and bf2fd45.

⛔ Files ignored due to path filters (13)
  • sdk/python/nemo-platform/.github/workflows/ci.yml is excluded by !sdk/**
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/.nmpcontext/stainless.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/evaluations/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/evaluations/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/evaluations/evaluations.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/evaluations/experiments.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/evaluations/evaluation_create_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/evaluations/evaluation_response.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/evaluations/evaluation_update_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/evaluations/test_experiments.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_evaluations.py is excluded by !sdk/**
  • sdk/stainless.yaml is excluded by !sdk/**
📒 Files selected for processing (19)
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/filter_ops.py
  • packages/nemo_platform_plugin/src/nemo_platform_plugin/in_memory_filter.py
  • services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py
  • services/core/entities/tests/test_filter_matches_sql_parity.py
  • services/intake/src/nmp/intake/api/v2/experiments/endpoints.py
  • services/intake/src/nmp/intake/api/v2/experiments/schemas.py
  • services/intake/src/nmp/intake/entities/experiments.py
  • services/intake/tests/integration/test_experiments_crud.py
  • services/intake/tests/test_experiment_default_sort.py
  • services/intake/tests/test_experiment_metric_filter.py
  • services/intake/tests/test_experiment_sort.py
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.test.tsx
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/ExperimentGroupDataView.css
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsx

Comment thread openapi/ga/openapi.yaml Outdated
Signed-off-by: shanaiabuggy <[email protected]>
tylersbray added a commit that referenced this pull request Jul 20, 2026
Align vLLM startup, GPU allocation, pull authentication, pod image secrets, and WaitForFirstConsumer handling so the documented deployment flow works on the deployments plugin backend.

Signed-off-by: Tyler Bray <[email protected]>
tylersbray added a commit that referenced this pull request Jul 20, 2026
Ensure the adapters sidecar reaches the platform API and its sibling vLLM server instead of silently defaulting to its own localhost listener.

Signed-off-by: Tyler Bray <[email protected]>
tylersbray added a commit that referenced this pull request Jul 21, 2026
Align vLLM startup, GPU allocation, pull authentication, pod image secrets, and WaitForFirstConsumer handling so the documented deployment flow works on the deployments plugin backend.

Signed-off-by: Tyler Bray <[email protected]>
tylersbray added a commit that referenced this pull request Jul 21, 2026
Ensure the adapters sidecar reaches the platform API and its sibling vLLM server instead of silently defaulting to its own localhost listener.

Signed-off-by: Tyler Bray <[email protected]>
tylersbray added a commit that referenced this pull request Jul 21, 2026
#759)

ModelsConfig only accepts deployments_plugin after the cutover; leftover
nim_operator/none keys crash API and core-controller on Kind install.

Signed-off-by: Tyler Bray <[email protected]>
Comment thread services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py Outdated
tylersbray added a commit that referenced this pull request Jul 21, 2026
…#759)

Update the static helm values assertion after removing the legacy
none/nim_operator models backends from the Authentik demo chart.

Signed-off-by: Tyler Bray <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py`:
- Line 135: Update the null comparison in the filter method containing
_cast_json_to_raw_text so it matches both JSON null values represented as the
string "null" and missing PostgreSQL keys represented as SQL NULL. Add an IS
NULL condition or coalesce while preserving the existing filter behavior for
present JSON values.
🪄 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: 1b919ae5-5e79-4293-9ebe-ef06ae1bc65d

📥 Commits

Reviewing files that changed from the base of the PR and between b0d5687 and 5845ce9.

📒 Files selected for processing (1)
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py

Comment thread services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py Outdated
Comment thread services/intake/src/nmp/intake/api/v2/experiments/endpoints.py Outdated
Comment thread services/intake/src/nmp/intake/api/v2/experiments/endpoints.py Outdated
Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: shanaiabuggy <[email protected]>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
openapi/ga/individual/platform.openapi.yaml (1)

3330-3604: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Keep the membership-add route as a deprecated alias or bump the API version. PATCH /apis/intake/v2/workspaces/{workspace}/evaluations/{name} only replaces experiment_ids, so removing POST /apis/intake/v2/workspaces/{workspace}/evaluations/{name}/experiments/{experiment_id} breaks existing clients.

🤖 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 `@openapi/ga/individual/platform.openapi.yaml` around lines 3330 - 3604,
Preserve backward compatibility by retaining the membership-add POST route as a
deprecated alias, or move the replacement behavior to a new API version. Ensure
existing clients can still add an experiment through the
/experiments/{experiment_id} endpoint while the patch_evaluation operation
continues to replace the full experiment_ids set.

Source: Linters/SAST tools

🤖 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/endpoints.py`:
- Around line 594-604: Deduplicate caller-supplied IDs in the experiment_ids
update path before assigning them to existing.experiment_ids. Preserve the
non-empty validation and group-existence validation, but use the deduplicated
collection for new-group detection and persistence so each group ID appears at
most once.

---

Outside diff comments:
In `@openapi/ga/individual/platform.openapi.yaml`:
- Around line 3330-3604: Preserve backward compatibility by retaining the
membership-add POST route as a deprecated alias, or move the replacement
behavior to a new API version. Ensure existing clients can still add an
experiment through the /experiments/{experiment_id} endpoint while the
patch_evaluation operation continues to replace the full experiment_ids set.
🪄 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: 621d5522-7784-42c9-8875-50ca20a7b9f6

📥 Commits

Reviewing files that changed from the base of the PR and between 5845ce9 and 5ac06d6.

⛔ Files ignored due to path filters (8)
  • sdk/python/nemo-platform/.nmpcontext/openapi.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/.nmpcontext/stainless.yaml is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/evaluations/api.md is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/resources/evaluations/evaluations.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/evaluations/__init__.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/types/evaluations/evaluation_patch_params.py is excluded by !sdk/**
  • sdk/python/nemo-platform/tests/api_resources/test_evaluations.py is excluded by !sdk/**
  • sdk/stainless.yaml is excluded by !sdk/**
📒 Files selected for processing (10)
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • services/core/auth/src/nmp/core/auth/assets/static-authz.yaml
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py
  • services/intake/src/nmp/intake/api/v2/experiments/endpoints.py
  • services/intake/src/nmp/intake/api/v2/experiments/schemas.py
  • services/intake/tests/integration/test_experiments_crud.py
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.test.tsx
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.test.tsx
  • services/intake/src/nmp/intake/api/v2/experiments/schemas.py
  • services/core/entities/src/nmp/core/entities/app/repository/sqlalchemy/filter.py
  • web/packages/studio/src/components/dataViews/ExperimentGroupDataView/AddToGroupModal.tsx

Comment thread services/intake/src/nmp/intake/api/v2/experiments/endpoints.py Outdated
@shanaiabuggy
shanaiabuggy enabled auto-merge July 22, 2026 00:36
@shanaiabuggy
shanaiabuggy added this pull request to the merge queue Jul 22, 2026
Merged via the queue into main with commit 45d0925 Jul 22, 2026
58 checks passed
@shanaiabuggy
shanaiabuggy deleted the sbuggy/ase-559 branch July 22, 2026 01:29
soluwalana pushed a commit that referenced this pull request Jul 22, 2026
…nt groups (#759)

* feat(entities): add $contains array-membership filter operator

Adds a CONTAINS ("$contains") comparison operator to the entity-store filter
engine so callers can query "value is an element of a JSON array field" — the
prerequisite for modeling an experiment's group membership as a list (ASE-559).

- filter_ops.py: new FilterOperator.CONTAINS, a default contains() on the
  FilterRepository ABC (additive — existing repos need not implement it), and
  dispatch in ComparisonOperation.apply. The JSON filter parser is generic over
  the enum, so $contains parses with no parser change.
- SQLAlchemyFilterRepository.contains: portable across SQLite (JSON) and
  PostgreSQL (JSONB) via a quote-delimited match on the serialized array text;
  wildcard-escaped and prefix-collision-safe ("g1" never matches ["g10"]).
- InMemoryFilterRepository.contains: native membership, matching SQL semantics.
- Parity harness extended with $contains cases incl. a prefix near-miss.

Validated: 32 parity cases pass (SQLite + in-memory agree); 113 entities +
167 plugin filter tests pass; ruff + ty clean. The Postgres path is portable by
design; Docker-backed integration tests confirm it in CI.

Signed-off-by: shanaiabuggy <[email protected]>

* feat(intake): allow an evaluation to belong to multiple groups [ASE-559]

Relax the one-group-per-evaluation constraint to many-to-many. An evaluation's
single experiment_group_id becomes an experiment_ids list (>=1); everything that
renders a group's runs (list view, rollups, sort) now spans the membership set.

- Entity: Experiment.experiment_group_id -> experiment_ids: list[str] (min 1),
  with a model_validator(mode="before") coercing legacy single-id rows to
  [id] on read (no data migration). parent_experiment_id (lineage) untouched.
- Create/update API: EvaluationRequest accepts experiment_ids (>=1);
  experiment_group_id kept as a deprecated coalesced alias. EvaluationResponse
  exposes experiment_ids with experiment_group_id as a deprecated computed alias
  (= experiment_ids[0]). Each group id is validated to exist + be live.
- Membership queries: a shared _group_membership_filter() matches
  "$contains experiment_ids OR legacy scalar experiment_group_id", used by the
  list filter (rewriting the exposed experiment_group_id param), both count
  helpers, and the delete cascade — so un-migrated rows stay queryable.
- Delete cascade is now reference-counted: a member whose sole membership was the
  deleted group is soft-deleted; a member shared with another group just drops
  that membership and survives there.

Tests: multi-group create appears in both leaderboards; empty experiment_ids
rejected; reference-counted cascade (shared member survives, sole member gone);
existing single-group + deprecated-alias paths still pass. Full intake
experiment/evaluation suite green (SQLite).

Signed-off-by: shanaiabuggy <[email protected]>

* fix(intake): make experiment_ids additive, keep experiment_group_id [ASE-559]

Address review: the create request should not force the new field on clients.

- EvaluationRequest.experiment_ids is now optional (default_factory=list) rather
  than required, so the request stays purely additive; experiment_group_id
  remains an accepted (deprecated) field. A model_validator requires at least one
  of the two and coalesces the deprecated single id into experiment_ids.
- experiment_group_id is unchanged in the response body (a deprecated computed
  alias = experiment_ids[0]), consistent with the existing parent_experiment_id
  deprecation pattern.

Signed-off-by: shanaiabuggy <[email protected]>

* feat(intake): add evaluation-to-group membership endpoints [ASE-559]

Curate existing runs into comparison groups (e.g. an all-benchmarks board)
after creation:

- POST   /evaluations/{name}/groups/{group_id} — add a membership. Validates the
  group exists + is live; idempotent when already a member.
- DELETE /evaluations/{name}/groups/{group_id} — remove a membership. Rejects
  removing the evaluation's last group with 409 (>=1-group invariant; delete the
  evaluation instead); idempotent when not a member.

Modeled on the pin/unpin endpoints. Tests cover add (idempotent + appears on the
target board), add-to-unknown-group (400), remove (drops the membership), and
remove-last-group (409).

Signed-off-by: shanaiabuggy <[email protected]>

* chore(intake): regenerate OpenAPI spec for experiment_ids + membership [ASE-559]

Regenerated via `make refresh-openapi` after the many-to-many API changes:
- EvaluationRequest/EvaluationResponse gain experiment_ids; experiment_group_id
  retained as a deprecated field/alias.
- New POST/DELETE /evaluations/{name}/groups/{group_id} membership endpoints.

Signed-off-by: shanaiabuggy <[email protected]>

* feat(studio): add "Add to group" action to the evaluations list [ASE-559]

A per-row "Add to group" action (FolderPlus, next to the pin toggle) in the
ExperimentGroupDataView opens a modal with a searchable list of the workspace's
experiment groups — excluding the ones the evaluation already belongs to — and
adds the run to the selected group so it appears on that board too. Mirrors the
pin action's mutation + scoped query invalidation, with success/error toasts.

Lets users assemble a cross-benchmark ("all-benchmarks") board from existing
runs. Uses the generated useAddEvaluationToGroup / useListExperimentGroups hooks.

- AddToGroupModal.tsx (+ .test.tsx, 4 cases: lists/excludes members, filters by
  search, calls the add hook, empty state)
- ExperimentGroupDataView: row-action button + modal wiring (pin logic unchanged)

typecheck + component test + eslint clean.

Signed-off-by: shanaiabuggy <[email protected]>

* FE & endpoint renames

Signed-off-by: shanaiabuggy <[email protected]>

* pretty

Signed-off-by: shanaiabuggy <[email protected]>

* lint

Signed-off-by: shanaiabuggy <[email protected]>

* helper

Signed-off-by: shanaiabuggy <[email protected]>

* review comments

Signed-off-by: shanaiabuggy <[email protected]>

* invalidate query

Signed-off-by: shanaiabuggy <[email protected]>

* bunny

Signed-off-by: shanaiabuggy <[email protected]>

---------

Signed-off-by: shanaiabuggy <[email protected]>
Signed-off-by: Sam Oluwalana <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants