Skip to content

feat(models): support boolean fileset filter on ModelEntityFilter - #1879

Merged
marcusds merged 2 commits into
mainfrom
astd-567-base-model-dropdown-server-side-filter-for-fileset-eligible/mschwab
Sep 8, 2026
Merged

feat(models): support boolean fileset filter on ModelEntityFilter#1879
marcusds merged 2 commits into
mainfrom
astd-567-base-model-dropdown-server-side-filter-for-fileset-eligible/mschwab

Conversation

@marcusds

@marcusds marcusds commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

ModelEntityFilter.fileset only supported exact-match to one fileset reference, unlike base_model which already supports true/false for "has a value" / "has no value" via the generic bool-coercible-field filter machinery. This widens fileset the same way, so callers can filter for models that do (or don't) have a fileset without pulling every page and filtering client-side.

Related Issue

Related to ASTD-567 — the Base Model dropdown filters for fine-tunable models (fileset set) entirely client-side today (web/packages/common/src/api/models/useModelSearch.ts), which forces it to page past runs of ineligible models one round trip at a time on a slow connection. This PR unblocks moving that filter server-side; the dropdown-side change to actually use filter[fileset]=true is not included here — draft until that follow-up work is settled.

Changes

  • services/core/models/src/nmp/core/models/schemas.py: widen ModelEntityFilter.fileset from Optional[str] to Optional[Union[bool, str]], mirroring base_model's existing type.
  • services/core/models/tests/unit/api/test_models_api.py: add fileset filter tests mirroring the existing base_model filter tests (exact-ref match, true, false).
  • openapi/openapi.yaml, openapi/ga/openapi.yaml, openapi/ga/individual/platform.openapi.yaml: regenerated via make refresh-openapi to reflect the widened fileset schema.
  • sdk/python/nemo-platform/src/nemo_platform/types/models/model_entity_filter_param.py, sdk/python/nemo-platform/.nmpcontext/openapi.yaml: hand-patched, not Stainless-generated — see note below.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with documentation updates
  • Documentation only
  • Contributor tooling or automation
  • CI, build, or test infrastructure

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Documentation updated for user-visible behavior
  • Documentation not applicable — justification: filter description string is the only user-facing doc surface, and it's updated inline in the schema change.

Verification

  • Pull request title follows the repository's Conventional Commit format
  • Every commit includes an appropriate Signed-off-by: trailer
  • uv run pre-commit run -a passes, or any blocked checks are identified below
  • Targeted tests pass, or tests are marked not applicable above
  • No secrets, API keys, or credentials are included

Targeted validation:

  • uv run pre-commit run -a — passed
  • pytest services/core/models/tests/unit/api/test_models_api.py -v — 34 passed (3 new fileset filter tests + all existing, including base_model's equivalent tests unaffected)
  • ruff check / ruff format --check on changed files — passed
  • ty check on schemas.py — passed
  • make refresh-openapi — regenerated cleanly, diff scoped to the fileset schema change only

make stainless (vendored Python SDK regen) was attempted repeatedly but Stainless's cloud build queue is stuck — every recent build (ours and others across the project) sits at not_started indefinitely; the queue's oldest unprocessed build predates this PR by a week. Given that, sdk/python/nemo-platform/src/nemo_platform/types/models/model_entity_filter_param.py and its .nmpcontext/openapi.yaml snapshot were hand-patched to mirror base_model's existing pattern, purely so nemo-platform-sdk-tools is-up-to-date (CI's lint-python-sdk) passes — confirmed locally. This is a stopgap, not a real Stainless generation, and is called out as such in its commit message; once Stainless recovers, someone should run make stainless for real, which may reformat these files slightly differently than this manual edit did.

Summary by CodeRabbit

  • New Features

    • Model filtering now supports checking whether entities have a fileset (true) or do not have one (false).
    • Filtering by a specific {workspace}/{fileset_name} reference remains supported.
    • The enhanced fileset filter is available across the platform’s model listing APIs.
  • Bug Fixes

    • Improved fileset filter handling for presence and absence checks.

fileset was Optional[str], the only filter on the field limited to an
exact fileset reference. base_model already supports Optional[Union[...,
bool, str]], letting callers ask true/false for presence via the generic
bool-coercible-field machinery in Filter._get_bool_coercible_fields().
Widen fileset to Optional[Union[bool, str]] the same way so a caller can
filter for models that have (or don't have) a fileset without walking
every page client-side to find one.

Follow-up to ASTD-562/ASTD-567: the Base Model dropdown filters for
fine-tunable models (fileset must be set) entirely client-side today,
which forces it to page past runs of ineligible models one round trip
at a time. This unblocks moving that filter server-side.

Signed-off-by: mschwab <mschwab@nvidia.com>
@github-actions github-actions Bot added the feat label Sep 8, 2026
@marcusds
marcusds marked this pull request as ready for review September 8, 2026 20:20
@marcusds
marcusds requested review from a team as code owners September 8, 2026 20:20
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 39284/50257 78.2% 62.3%
Integration Tests 23876/47485 50.3% 23.0%

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b0b32f93-429f-4162-9f00-dc1a716152f4

📥 Commits

Reviewing files that changed from the base of the PR and between 80bf1d0 and 60ba9e9.

📒 Files selected for processing (5)
  • openapi/ga/individual/platform.openapi.yaml
  • openapi/ga/openapi.yaml
  • openapi/openapi.yaml
  • services/core/models/src/nmp/core/models/schemas.py
  • services/core/models/tests/unit/api/test_models_api.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

ModelEntityFilter.fileset now accepts boolean presence filters and string fileset references. OpenAPI schemas, the Python model schema, and API unit tests reflect the new behavior.

Changes

Fileset filter support

Layer / File(s) Summary
Fileset filter contract
openapi/ga/individual/platform.openapi.yaml, openapi/ga/openapi.yaml, openapi/openapi.yaml, services/core/models/src/nmp/core/models/schemas.py
The fileset filter accepts true, false, or a {workspace}/{fileset_name} reference string.
Fileset filter translation
services/core/models/tests/unit/api/test_models_api.py
Tests verify reference equality, non-null filtering for fileset=true, and null filtering for fileset=false.

Suggested reviewers: ironcommit

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 60ba9

Model-list queries can now filter by fileset presence or a specific fileset reference, with matching API contracts and coverage for each supported filter form.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (3 skipped: 3 …
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 and concisely describes the main change: adding boolean fileset filtering to ModelEntityFilter.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch astd-567-base-model-dropdown-server-side-filter-for-fileset-eligible/mschwab

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

@marcusds
marcusds enabled auto-merge September 8, 2026 21:05
Stainless's cloud build queue has been stuck (builds stalled at
not_started for hours, going back over a week of prior build errors
across the project) — 'make stainless' cannot complete right now. This
hand-patches the vendored SDK to match the fileset: Optional[Union[bool,
str]] widening from 60ba9e9, mirroring the existing base_model
pattern, purely so 'nemo-platform-sdk-tools is-up-to-date' (and CI's
lint-python-sdk) passes.

This is a stopgap, not a real generation. Once Stainless recovers, run
'make stainless' for real — it may reformat these files differently
(docstring wrapping, etc.) than this manual edit did.

Signed-off-by: mschwab <mschwab@nvidia.com>
@marcusds
marcusds force-pushed the astd-567-base-model-dropdown-server-side-filter-for-fileset-eligible/mschwab branch from 8659808 to 807680c Compare September 8, 2026 22:22
@marcusds
marcusds added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 79a6731 Sep 8, 2026
60 checks passed
@marcusds
marcusds deleted the astd-567-base-model-dropdown-server-side-filter-for-fileset-eligible/mschwab branch September 8, 2026 22:52
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