feat(jobs): add Source sorting and surface all sources in Jobs filter - #720
Merged
marcusds merged 1 commit intoJul 16, 2026
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughChangesJob source sorting and filtering
Sequence Diagram(s)sequenceDiagram
participant JobsDataView
participant JobsAPI
participant JobDispatcher
JobsDataView->>JobsAPI: Request jobs with sort=source
JobsAPI->>JobDispatcher: Pass PlatformJobListSortField
JobDispatcher-->>JobsAPI: Return jobs ordered by source
JobsAPI-->>JobsDataView: Return sorted job list
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/packages/studio/src/components/dataViews/JobsDataView/index.tsx (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a type-only import for
ComponentProps.Split the React imports so
ComponentPropsis imported withimport type. As per coding guidelines, TypeScript type-only imports must useimport type.Proposed fix
-import { ComponentProps, type ReactNode, useRef } from 'react'; +import { useRef } from 'react'; +import type { ComponentProps, ReactNode } from 'react';🤖 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/JobsDataView/index.tsx` at line 35, Update the React imports in JobsDataView so ComponentProps is imported via a type-only import, while ReactNode and useRef remain in the runtime import.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.
Inline comments:
In `@web/packages/studio/src/components/dataViews/JobsDataView/index.tsx`:
- Around line 116-129: Reset the source accumulator when the workspace changes
so JobsDataView does not retain sources from the previous workspace. Update the
logic around seenSourcesRef to use a workspace-keyed effect or memoized
collection, and add a test covering workspace switching to ensure dynamic source
options are isolated per workspace.
---
Nitpick comments:
In `@web/packages/studio/src/components/dataViews/JobsDataView/index.tsx`:
- Line 35: Update the React imports in JobsDataView so ComponentProps is
imported via a type-only import, while ReactNode and useRef remain in the
runtime import.
🪄 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: 71e135e9-ce6d-4bc0-8ef8-a8cbaed82aa4
📒 Files selected for processing (7)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.pyservices/core/jobs/tests/test_dispatcher.pyweb/packages/studio/src/components/dataViews/JobsDataView/index.test.tsxweb/packages/studio/src/components/dataViews/JobsDataView/index.tsx
Contributor
|
marcusds
force-pushed
the
astd-45-add-source-sorting-to-jobs-filters/mschwab
branch
from
July 15, 2026 22:59
0682586 to
3fe6f59
Compare
Contributor
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 `@openapi/ga/openapi.yaml`:
- Around line 15232-15242: Update the PlatformJobSourcesResponse schema to
declare sources as required by adding a required list containing sources
alongside its properties, while preserving the existing array definition and
descriptions.
In `@web/packages/studio/src/components/dataViews/JobsDataView/index.tsx`:
- Line 34: Update the React import in the JobsDataView module to make
ComponentProps and ReactNode type-only imports, preserving their existing usage
and avoiding a runtime React import.
🪄 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: 0f9a2d13-0f04-4a7f-896a-5247a24a0333
📒 Files selected for processing (13)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/endpoints.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/schemas.pyservices/core/jobs/src/nmp/core/jobs/app/dispatcher.pyservices/core/jobs/tests/conftest.pyservices/core/jobs/tests/test_dispatcher.pyservices/core/jobs/tests/test_jobs_api.pyweb/packages/studio/src/components/dataViews/JobsDataView/constants.tsweb/packages/studio/src/components/dataViews/JobsDataView/index.test.tsxweb/packages/studio/src/components/dataViews/JobsDataView/index.tsx
💤 Files with no reviewable changes (1)
- web/packages/studio/src/components/dataViews/JobsDataView/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/packages/studio/src/components/dataViews/JobsDataView/index.test.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@openapi/ga/openapi.yaml`:
- Around line 15232-15242: Update the PlatformJobSourcesResponse schema to
declare sources as required by adding a required list containing sources
alongside its properties, while preserving the existing array definition and
descriptions.
In `@web/packages/studio/src/components/dataViews/JobsDataView/index.tsx`:
- Line 34: Update the React import in the JobsDataView module to make
ComponentProps and ReactNode type-only imports, preserving their existing usage
and avoiding a runtime React import.
🪄 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: 0f9a2d13-0f04-4a7f-896a-5247a24a0333
📒 Files selected for processing (13)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/endpoints.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/schemas.pyservices/core/jobs/src/nmp/core/jobs/app/dispatcher.pyservices/core/jobs/tests/conftest.pyservices/core/jobs/tests/test_dispatcher.pyservices/core/jobs/tests/test_jobs_api.pyweb/packages/studio/src/components/dataViews/JobsDataView/constants.tsweb/packages/studio/src/components/dataViews/JobsDataView/index.test.tsxweb/packages/studio/src/components/dataViews/JobsDataView/index.tsx
💤 Files with no reviewable changes (1)
- web/packages/studio/src/components/dataViews/JobsDataView/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/packages/studio/src/components/dataViews/JobsDataView/index.test.tsx
🛑 Comments failed to post (2)
openapi/ga/openapi.yaml (1)
15232-15242: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Mark
sourcesas required.At Line 15234,
sourcesis declared only underproperties, so OpenAPI treats it as optional. The handler always returnsPlatformJobSourcesResponse(sources=sources); addrequired: [sources]to keep generated clients and validators aligned with the endpoint contract.Proposed fix
PlatformJobSourcesResponse: properties: sources: items: type: string type: array title: Sources description: Distinct job source values, sorted ascending. + required: + - sources type: object📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.PlatformJobSourcesResponse: properties: sources: items: type: string type: array title: Sources description: Distinct job source values, sorted ascending. required: - sources type: object title: PlatformJobSourcesResponse description: Distinct ``source`` values present across a workspace's jobs.🤖 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/openapi.yaml` around lines 15232 - 15242, Update the PlatformJobSourcesResponse schema to declare sources as required by adding a required list containing sources alongside its properties, while preserving the existing array definition and descriptions.web/packages/studio/src/components/dataViews/JobsDataView/index.tsx (1)
34-34: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' web/packages/studio/src/components/dataViews/JobsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 7850
🏁 Script executed:
rg -n "ComponentProps|ReactNode" web/packages/studio/src/components/dataViews/JobsDataView/index.tsxRepository: NVIDIA-NeMo/nemo-platform
Length of output: 406
Use a type-only React import.
ComponentPropsandReactNodeare type-only here, so switch to:Proposed fix
-import { ComponentProps, type ReactNode } from 'react'; +import type { ComponentProps, ReactNode } from 'react';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import type { ComponentProps, ReactNode } from 'react';🤖 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/JobsDataView/index.tsx` at line 34, Update the React import in the JobsDataView module to make ComponentProps and ReactNode type-only imports, preserving their existing usage and avoiding a runtime React import.Source: Coding guidelines
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.py (1)
90-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate mixin logic across two sort-field enums.
get_field_name/get_sort_directionare copy-pasted verbatim fromPlatformJobSortField. Extract a shared mixin/base class to avoid drift between the two enums.♻️ Suggested mixin
class _SortFieldMixin: def get_field_name(self) -> str: return self.value.lstrip("-") def get_sort_direction(self) -> str: return "desc" if self.value.startswith("-") else "asc" class PlatformJobSortField(_SortFieldMixin, str, Enum): ... class PlatformJobListSortField(_SortFieldMixin, str, Enum): ...🤖 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 `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.py` around lines 90 - 108, Extract the shared get_field_name and get_sort_direction implementations into a private _SortFieldMixin, then make both PlatformJobSortField and PlatformJobListSortField inherit from it alongside str and Enum. Remove the duplicated methods from each enum while preserving their existing sort values and 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.
Nitpick comments:
In `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.py`:
- Around line 90-108: Extract the shared get_field_name and get_sort_direction
implementations into a private _SortFieldMixin, then make both
PlatformJobSortField and PlatformJobListSortField inherit from it alongside str
and Enum. Remove the duplicated methods from each enum while preserving their
existing sort values and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 60f77bad-17ca-4126-8122-ce49ef07d587
📒 Files selected for processing (12)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/client.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/endpoints.pypackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.pyservices/core/auth/src/nmp/core/auth/assets/static-authz.yamlservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/endpoints.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/schemas.pyservices/core/jobs/src/nmp/core/jobs/app/dispatcher.pyservices/core/jobs/tests/test_dispatcher.pyweb/packages/studio/src/components/dataViews/JobsDataView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- services/core/jobs/src/nmp/core/jobs/api/v2/jobs/schemas.py
- services/core/jobs/src/nmp/core/jobs/app/dispatcher.py
- services/core/jobs/src/nmp/core/jobs/api/v2/jobs/endpoints.py
- web/packages/studio/src/components/dataViews/JobsDataView/index.tsx
marcusds
force-pushed
the
astd-45-add-source-sorting-to-jobs-filters/mschwab
branch
from
July 16, 2026 17:45
ef72fcb to
1646be8
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/packages/studio/src/components/dataViews/JobsDataView/index.tsx (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark
ComponentPropsas a type-only import.
ComponentPropsis used only as a type. As per coding guidelines, TypeScript type-only imports must be explicit.Proposed fix
-import { ComponentProps, type ReactNode, useRef } from 'react'; +import { type ComponentProps, type ReactNode, useRef } from 'react';🤖 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/JobsDataView/index.tsx` at line 35, Update the import in JobsDataView to mark ComponentProps as a type-only import, while leaving ReactNode and useRef as value imports.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/components/dataViews/JobsDataView/index.tsx`:
- Line 35: Update the import in JobsDataView to mark ComponentProps as a
type-only import, while leaving ReactNode and useRef as value imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 94208b2c-8886-4774-b2e0-9e5644ca981e
📒 Files selected for processing (10)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/types.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/endpoints.pyservices/core/jobs/src/nmp/core/jobs/api/v2/jobs/schemas.pyservices/core/jobs/src/nmp/core/jobs/app/dispatcher.pyservices/core/jobs/tests/test_dispatcher.pyweb/packages/studio/src/components/dataViews/JobsDataView/index.test.tsxweb/packages/studio/src/components/dataViews/JobsDataView/index.tsx
Sorting: - Add source/-source to PlatformJobSortField so the jobs API accepts sorting by source. list_jobs already sorts generically, so no dispatcher logic change is required. - Regenerate the OpenAPI spec with the new enum values. - Enable sorting on the Studio Jobs 'Source' column. Filtering: - Accumulate source values seen in returned job data into the Source filter options so plugin-supplied sources (e.g. nemo-agents-plugin) appear alongside the static set instead of a hardcoded list. Add dispatcher and Studio tests covering both. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
astd-45-add-source-sorting-to-jobs-filters/mschwab
branch
from
July 16, 2026 18:27
1646be8 to
8903c30
Compare
Contributor
steramae-nvidia
approved these changes
Jul 16, 2026
soluwalana
pushed a commit
that referenced
this pull request
Jul 17, 2026
…#720) Sorting: - Add source/-source to PlatformJobSortField so the jobs API accepts sorting by source. list_jobs already sorts generically, so no dispatcher logic change is required. - Regenerate the OpenAPI spec with the new enum values. - Enable sorting on the Studio Jobs 'Source' column. Filtering: - Accumulate source values seen in returned job data into the Source filter options so plugin-supplied sources (e.g. nemo-agents-plugin) appear alongside the static set instead of a hardcoded list. Add dispatcher and Studio tests covering both. Signed-off-by: mschwab <mschwab@nvidia.com>
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
2 participants
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
Implements ASTD-45 — add Source as a sort option in the Studio Jobs view, and fix the Source filter so it shows every source that actually appears (previously a hardcoded list that omitted plugin sources like
nemo-agents-plugin).Changes
Sorting
source/-sourcetoPlatformJobSortFieldso the jobs API accepts sorting by source.list_jobsalready sorts generically (getattr(job, field)+ store-sidesort), so no dispatcher logic change was required.Filtering
sourceis an open-ended string derived from the creating plugin's module name (or explicitservice_name), so plugin sources (nemo-agents-plugin,insights,rl, …) never appeared.Tests
test_list_jobs_sort_by_source(asc + desc) in the jobs dispatcher suite.JobsDataViewsuite passes (13/13).Notes / follow-ups
make update-sdk.Summary by CodeRabbit
New Features
Documentation