Skip to content

fix(rest-api): serialize empty response labels as objects - #6151

Open
pbreton wants to merge 1 commit into
dsx-ai-factory:mainfrom
pbreton:codex/fix-6753874-expected-machine-empty-collections
Open

pbreton wants to merge 1 commit into
dsx-ai-factory:mainfrom
pbreton:codex/fix-6753874-expected-machine-empty-collections

Conversation

@pbreton

@pbreton pbreton commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

REST responses can return null for empty labels even though their published contract is an object. Use response-specific APILabels across all 11 label-bearing API models so empty labels return {}, and APIList for Expected Machine fallback DPU serial numbers so empty values return []. Request and database models retain their existing nil semantics.

Expected Machine batch create and update now convert database results through the API response model, matching single-resource endpoints' JSON field names and empty collection behavior while preserving result order.

Related issues

Fixes NVBug 6753874.

Type of Change

  • Fix - Bug fixes

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Collection serialization tests cover empty and populated values, escaping, ordering, and encoding errors. Constructor coverage checks all 11 response label models. Handler coverage checks Expected Machine single and batch responses.

Validation: make test-api passed. Formatting and git diff --check passed. make lint-go completed; its golangci-lint invocation permits existing findings via --issues-exit-code 0. A separate check against the final commit's parent found no new lint findings in the changed model and handler packages. The full API suite also passed after preserving the remote branch's newer base.

Additional Notes

The OpenAPI object and array contracts are unchanged. Batch Expected Machine responses now use the documented API field names instead of serializing database structs directly.

@pbreton
pbreton requested a review from a team as a code owner September 14, 2026 21:10
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-14T21:13:01.380950Z 4834cb5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 14, 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: 8beefe1a-8559-4b1f-9748-f652286b4b5e

📥 Commits

Reviewing files that changed from the base of the PR and between 29f5ce4 and 6954aab.

📒 Files selected for processing (29)
  • rest-api/api/pkg/api/handler/expectedmachine.go
  • rest-api/api/pkg/api/handler/expectedmachine_test.go
  • rest-api/api/pkg/api/handler/expectedpowershelf_test.go
  • rest-api/api/pkg/api/handler/expectedrack_test.go
  • rest-api/api/pkg/api/handler/expectedswitch_test.go
  • rest-api/api/pkg/api/handler/infinibandpartition_test.go
  • rest-api/api/pkg/api/handler/instancetype_test.go
  • rest-api/api/pkg/api/handler/machine_test.go
  • rest-api/api/pkg/api/handler/networksecuritygroup_test.go
  • rest-api/api/pkg/api/model/collections.go
  • rest-api/api/pkg/api/model/collections_test.go
  • rest-api/api/pkg/api/model/dpumachine.go
  • rest-api/api/pkg/api/model/dpumachine_test.go
  • rest-api/api/pkg/api/model/expectedmachine.go
  • rest-api/api/pkg/api/model/expectedmachine_test.go
  • rest-api/api/pkg/api/model/expectedpowershelf.go
  • rest-api/api/pkg/api/model/expectedpowershelf_test.go
  • rest-api/api/pkg/api/model/expectedrack.go
  • rest-api/api/pkg/api/model/expectedrack_test.go
  • rest-api/api/pkg/api/model/expectedswitch.go
  • rest-api/api/pkg/api/model/expectedswitch_test.go
  • rest-api/api/pkg/api/model/infinibandpartition.go
  • rest-api/api/pkg/api/model/instance.go
  • rest-api/api/pkg/api/model/instance_test.go
  • rest-api/api/pkg/api/model/instancetype.go
  • rest-api/api/pkg/api/model/machine.go
  • rest-api/api/pkg/api/model/machine_test.go
  • rest-api/api/pkg/api/model/networksecuritygroup.go
  • rest-api/api/pkg/api/model/vpc.go

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


Summary by CodeRabbit

  • Bug Fixes
    • API responses now return empty collections ({} and []) instead of null when labels or fallback DPU serial numbers are omitted.
    • Creating an expected machine without collection fields now succeeds with 201 Created.
    • Response handling consistently preserves populated collection values and ordering.

Walkthrough

The change adds typed API collection responses, serializes nil labels as {} and nil lists as [], updates API model mappings, and routes expected-machine batch responses through API constructors. Tests cover serialization and handler response shapes.

Changes

API collection contracts and model mappings

Layer / File(s) Summary
Collection types and model mappings
rest-api/api/pkg/api/model/collections.go, rest-api/api/pkg/api/model/*.go
Adds APILabels and APIList[T]. Nil labels serialize as {}. Nil lists serialize as []. API constructors convert database collections to these types.
Collection serialization validation
rest-api/api/pkg/api/model/*_test.go
Tests nil, empty, populated, ordered, and unsupported collection serialization. Model tests also verify empty label-object output.
Expected-machine response mapping
rest-api/api/pkg/api/handler/expectedmachine.go, rest-api/api/pkg/api/handler/*_test.go
Batch create and update handlers now serialize API response models. Tests cover omitted collections, response order, IDs, lowercase JSON fields, and empty label collections.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6954a

The API response normalization is covered by the changed model and handler tests, with no current merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 29 files. 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.
Title check ✅ Passed The title clearly and concisely identifies the main change: serializing empty REST response labels as objects.
Description check ✅ Passed The description directly explains the label and list serialization fixes, Expected Machine response changes, testing, and contract impact.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-09-14 21:14:48 UTC | Commit: 4834cb5

@pbreton
pbreton force-pushed the codex/fix-6753874-expected-machine-empty-collections branch from 4834cb5 to 29f5ce4 Compare September 15, 2026 07:05
@github-actions

Copy link
Copy Markdown

Use response-specific APILabels across all label-bearing API models so
nil labels serialize as {}. Use APIList for Expected Machine fallback
DPU serial numbers so nil serials serialize as []. Preserve request and
database nil semantics.

Convert Expected Machine batch results through the API response model
for consistent JSON field names and empty collections. Cover collection
serialization, all response label models, and batch endpoint behavior.

Fixes NVBug https://nvbugspro.nvidia.com/bug/6753874

Signed-off-by: Patrice Breton <pbreton@nvidia.com>
@pbreton
pbreton force-pushed the codex/fix-6753874-expected-machine-empty-collections branch from 29f5ce4 to 6954aab Compare September 15, 2026 15:14
@pbreton pbreton changed the title fix(rest-api): return empty Expected Machine collections fix(rest-api): serialize empty response labels as objects Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant