feat(customizer): generate OpenAPI spec for customization plugin - #683
Conversation
The nemo-customizer plugin never declared a [tool.nemo.openapi] table, so discover_plugins() skipped it and no OpenAPI spec was ever generated for the customization router. Add the opt-in table and generate the spec. The customization router assembles its routes from installed nemo.customization.contributors (automodel, rl, unsloth), all workspace members, so the generated surface is deterministic in a fully-synced workspace. Also refresh openapi/README.md, which still described the removed per-service merge model: document the current aggregate-platform + per-opted-in-plugin layout and the plugin opt-in mechanism. Signed-off-by: Albert Cui <albcui@nvidia.com>
|
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:
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 selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR centralizes Customizer health reporting at ChangesCustomizer API surface
Customizer OpenAPI publication
Sequence Diagram(s)sequenceDiagram
participant Contributor
participant CustomizationRouterService
participant CustomizationSDK
Contributor->>CustomizationRouterService: provide jobs router only
CustomizationRouterService->>CustomizationSDK: expose /apis/customization/v2/healthz
CustomizationSDK->>CustomizationRouterService: GET v2/healthz
CustomizationRouterService-->>CustomizationSDK: JSON health payload
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openapi/README.md (1)
15-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the "Conflicts" section to match the new generation model.
This describes the aggregate platform spec as built from one platform-runner app rather than merged per-microservice specs, and plugin specs are never merged. The unchanged "Conflicts" section below (further down in this file) still frames conflicts as arising from merging schemas "coming from different microservices" — that premise no longer holds under this model. Update it to reflect that remaining conflicts, if any, are limited to GA/EA variant reconciliation within the single platform spec.
🤖 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/README.md` around lines 15 - 17, The “Conflicts” section in the OpenAPI README still describes merging schemas from different microservices. Update it to state that any remaining conflicts are limited to reconciling GA/EA schema variants within the single aggregate platform spec, and remove references to per-microservice spec merging.
🤖 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-customizer/openapi/openapi.yaml`:
- Around line 2161-2235: Split the shared jobsJob and jobsJobRequest schemas
into backend-specific RL and Unsloth variants, replacing AutomodelJobOutput and
AutomodelJobInput references with the corresponding RlJob and UnslothJob
schemas. Update the /rl/jobs and /unsloth/jobs operation references so each
endpoint uses its own request and response contract, preserving shared fields
while allowing backend-specific fields.
---
Nitpick comments:
In `@openapi/README.md`:
- Around line 15-17: The “Conflicts” section in the OpenAPI README still
describes merging schemas from different microservices. Update it to state that
any remaining conflicts are limited to reconciling GA/EA schema variants within
the single aggregate platform spec, and remove references to per-microservice
spec merging.
🪄 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: d66e4b34-82c9-47cd-8151-049f52fcce21
📒 Files selected for processing (3)
openapi/README.mdplugins/nemo-customizer/openapi/openapi.yamlplugins/nemo-customizer/pyproject.toml
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/nmp_customization_common/tests/test_sdk_client.py (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
from __future__ import annotationsstringifies all hints.Guideline prefers concrete type hints over string-based ones. Low impact here given simple hints, but worth reconsidering for new files.
🤖 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/nmp_customization_common/tests/test_sdk_client.py` at line 4, Remove the from __future__ import annotations directive from test_sdk_client.py so type hints remain concrete rather than being stringified, while preserving the existing annotations and test behavior.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 `@plugins/nemo-customizer/openapi/openapi.yaml`:
- Line 21: Restore the removed /apis/customization/v2/automodel/healthz route in
the OpenAPI specification as a deprecated compatibility endpoint, preserving its
health-check behavior or redirect for existing SDK clients and monitors until
migration is complete.
---
Nitpick comments:
In `@packages/nmp_customization_common/tests/test_sdk_client.py`:
- Line 4: Remove the from __future__ import annotations directive from
test_sdk_client.py so type hints remain concrete rather than being stringified,
while preserving the existing annotations and test 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: 288f022e-6a0f-4ec5-9d8b-bd1fd92fe2de
📒 Files selected for processing (7)
packages/nmp_customization_common/src/nmp/customization_common/contributor/base.pypackages/nmp_customization_common/src/nmp/customization_common/sdk/client.pypackages/nmp_customization_common/tests/test_sdk_client.pyplugins/nemo-automodel/tests/test_api.pyplugins/nemo-automodel/tests/test_contributor.pyplugins/nemo-customizer/openapi/openapi.yamlplugins/nemo-unsloth/tests/test_contributor.py
…oint
The per-contributor healthz routes (`/apis/customization/v2/<backend>/healthz`)
were static (`{"backend", "status": "ok"}`), fully subsumed by the router's own
health endpoint (which already enumerates registered contributors), and their
only SDK wrapper — `plugin_status()` — had no callers. They also produced an
invalid OpenAPI spec: the route was mounted under a `{workspace}` prefix its
handler never declared, breaking downstream TypeScript type generation.
Drop them wholesale and keep a single health endpoint, moved under `/v2` for
consistency with the backend routes and with other plugins' versioned health
(evaluator/auditor expose `/v1/healthz`):
- base.py: contributors return only their jobs router.
- router.py: the hub health endpoint moves from `/apis/customization/healthz`
to `/apis/customization/v2/healthz`.
- client.py: remove the orphaned sync/async `plugin_status()` and
`_healthz_url()`.
- Regenerate the customizer spec and update route/prefix tests.
Safe to remove without a compatibility alias: the customization spec had never
been generated before this branch, so no client was ever built from it, and the
API is pre-external-release.
Signed-off-by: Albert Cui <albcui@nvidia.com>
Expose customization router health via `client.customization.plugin_status()` (sync + async), replacing the per-backend `plugin_status()` removed alongside the per-contributor healthz routes. It calls the single hub endpoint `/apis/customization/v2/healthz`, which reports plugin status plus the list of registered contributors — mirroring the `plugin_status()` accessor that evaluator and auditor already expose. Signed-off-by: Albert Cui <albcui@nvidia.com>
5778e5e to
1175b47
Compare
There was a problem hiding this comment.
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 `@packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/routes.py`:
- Around line 214-221: Update the docstring near the schema-name normalization
logic to describe the current behavior: `.` is replaced with `-` before
splitting, so dots do not remain in `{job_type}JobRequest` or produce Pydantic
`__` separators. Remove the outdated explanation about dots surviving and
collapsing backend schema names, while preserving the rationale for distinct
per-backend names.
🪄 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: 0b1604f9-59a0-4061-8432-f88f0061e0e2
📒 Files selected for processing (3)
packages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/routes.pypackages/nemo_platform_plugin/tests/test_jobs_routes.pyplugins/nemo-customizer/openapi/openapi.yaml
Each customization backend (automodel, rl, unsloth) mounts its own
`POST .../{backend}/jobs`, and each declares a NemoJob whose `name` is dotted
(`automodel.jobs`, `rl.jobs`, `unsloth.jobs`). `_derive_job_type` PascalCased
that name for the generated schema class (`{job_type}JobRequest`) but only split
on `-`/`_`, so the dot survived: `automodel.jobs` -> `Automodel.jobs`. Pydantic
renders the dot as a `__` separator in the OpenAPI ref (`abc__Automodel__jobsJobRequest`),
and schema-name normalization strips everything before the last `__` to drop
module namespaces — collapsing all three to a single `jobsJobRequest`. The
duplicates were then silently dropped (first-wins, no warning), so every
backend's POST body aliased `AutomodelJobInput`.
Split `_derive_job_type` on `.` as well, folding the discriminator into the
final segment. Each backend now gets distinct `AutomodelJobsJobRequest` /
`RlJobsJobRequest` / `UnslothJobsJobRequest` (and matching Job/filter/sort)
schemas referencing its own input. Only the three dotted-name backends are
affected; no other plugin uses a dotted job name, so no other generated spec
changes.
Add a unit test for the dotted case and an integration test that runs two
dotted-name jobs through `add_job_routes` + `tweak_spec` and asserts the request
schemas stay distinct.
Signed-off-by: Albert Cui <albcui@nvidia.com>
637a58c to
6a8c214
Compare
The nemo-customizer plugin never declared a [tool.nemo.openapi] table, so discover_plugins() skipped it and no OpenAPI spec was ever generated for the customization router.
Add the opt-in table and generate the spec. The customization router assembles its routes from installed nemo.customization.contributors (automodel, rl, unsloth), all workspace members, so the generated surface is deterministic in a fully-synced workspace.
Also refresh openapi/README.md, which still described the removed per-service merge model: document the current aggregate-platform + per-opted-in-plugin layout and the plugin opt-in mechanism.
Summary by CodeRabbit