Skip to content

feat(core): persist DPS resource groups and power profiles - #4734

Draft
pbreton wants to merge 1 commit into
NVIDIA:mainfrom
pbreton:codex/issue-2092-core
Draft

feat(core): persist DPS resource groups and power profiles#4734
pbreton wants to merge 1 commit into
NVIDIA:mainfrom
pbreton:codex/issue-2092-core

Conversation

@pbreton

@pbreton pbreton commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add an optional power profile to instance Core models, gRPC configuration, snapshots, and database persistence
  • add an optional DPS power resource group to VPC create, update, read, and database flows
  • update downstream protobuf consumers and add focused round-trip/persistence coverage

Why

This provides the Core persistence and API foundation for #2092. REST exposure and Max-Q/DPS orchestration can build on these persisted associations in follow-up work.

Impact

The protobuf changes are additive, and the new database columns are nullable. Existing callers continue to omit both fields.

Testing

  • cargo check -p carbide-api-model -p carbide-api-db -p carbide-rpc -p carbide-api-core --no-default-features
  • cargo check -p nico-admin-cli -p carbide-api-web -p carbide-machine-a-tron -p carbide-rvs
  • cargo test -p carbide-rpc --features model power_profile_round_trips
  • cargo test -p carbide-api-model instance::snapshot::tests::test_from_pg_json_and_os_uses_provided_os
  • git diff --check

Related issues

Signed-off-by: Patrice Breton <pbreton@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary by CodeRabbit

  • New Features

    • Added optional power provisioning settings for instances and VPCs.
    • Power profiles and resource groups are now preserved across configuration, API, and snapshot operations.
    • Added database persistence for power provisioning metadata.
  • Bug Fixes

    • Improved handling of unset power provisioning values during instance and VPC creation or updates.
  • Tests

    • Expanded coverage for power profile and resource group propagation and persistence.

Walkthrough

The change adds optional power_profile and power_resource_group fields across RPC contracts, internal models, database persistence, conversions, request builders, and test fixtures.

Changes

Power metadata contracts and conversions

Layer / File(s) Summary
RPC and model fields
crates/rpc/proto/forge.proto, crates/api-model/..., crates/rpc/src/model/...
Adds optional power fields and preserves them through RPC and internal model conversions.
Snapshot handling
crates/api-model/src/instance/snapshot.rs
Loads and preserves power_profile in instance snapshots.

Persistence and integration

Layer / File(s) Summary
Database persistence
crates/api-db/migrations/..., crates/api-db/src/...
Adds nullable columns and persists power metadata during inserts, updates, and batch operations.
Request and fixture updates
crates/admin-cli/..., crates/machine-a-tron/..., crates/rvs/..., crates/api-core/src/tests/...
Initializes unset power fields and adds coverage for VPC persistence and instance configuration updates.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes persistence of DPS resource groups and power profiles, which are the primary changes.
Description check ✅ Passed The description accurately covers the new optional fields, persistence flows, compatibility impact, testing, and deferred follow-up work.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@pbreton

pbreton commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@pbreton I will perform a complete review of pull request #4734.

✅ Action performed

Full review finished.

@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

Caution

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

⚠️ Outside diff range comments (1)
crates/api-db/src/instance.rs (1)

569-588: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve power_profile when omitted

An absent RPC field becomes None and is written directly to power_profile. An older client can therefore erase an existing profile during an unrelated update. Preserve instance.config.power_profile when omitted, or add an explicit clear operation and a mixed-version regression test.

🤖 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 `@crates/api-db/src/instance.rs` around lines 569 - 588, Update the instance
update flow around the power_profile binding to preserve the existing
instance.config.power_profile when the RPC field is omitted instead of writing
None. If clearing is required, distinguish an explicit clear from omission and
add the corresponding mixed-version regression coverage; keep explicit profile
updates unchanged.

Source: Path instructions

🧹 Nitpick comments (1)
crates/api-model/src/instance/snapshot.rs (1)

303-303: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Cover the persisted-row conversion path with power_profile.

The test at Lines 430-447 verifies the assignment at Line 187. The separate InstanceSnapshot::try_from assignment at Line 303 is not set or projected by test_try_from_derives_os_from_instance_columns. A regression in the persisted-row path can therefore pass all tests.

Add populated and unset power_profile cases to that scenario projection.

As per path instructions, optional-field round-trip tests should cover both unset and populated values.

Also applies to: 430-447

🤖 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 `@crates/api-model/src/instance/snapshot.rs` at line 303, Extend
test_try_from_derives_os_from_instance_columns to project both populated and
unset power_profile values through InstanceSnapshot::try_from, covering the
persisted-row conversion assignment at power_profile. Assert that populated data
round-trips correctly and the unset case remains None.

Source: Path instructions

🤖 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 `@crates/api-db/src/vpc.rs`:
- Line 338: Document the VPC update contract for power_resource_group in the
proto and update path: an omitted value preserves the stored value, while a
present value replaces it, including an empty string which must be stored as ''.
Update or add tests covering omit, set, and empty-string behavior without
introducing a clear state unless explicitly supported.

In `@crates/api-model/src/instance/config.rs`:
- Around line 69-71: Update InstanceConfigUpdateRequest and update_config so an
omitted power_profile preserves the existing config.power_profile, while an
explicit clear operation can still remove it; provide a compatibility path for
older clients as needed. Add a regression test covering an unrelated update with
a populated power profile and verify it remains unchanged.

In `@crates/rpc/proto/forge.proto`:
- Around line 1876-1877: Update the documentation and handling for
power_resource_group around the protobuf field power_resource_group to define
explicit update semantics: omission must preserve the existing value, while
empty strings must either represent a supported clear operation with tests or be
rejected and documented as unsupported for clearing. Ensure the chosen behavior
is enforced consistently by the update path.

---

Outside diff comments:
In `@crates/api-db/src/instance.rs`:
- Around line 569-588: Update the instance update flow around the power_profile
binding to preserve the existing instance.config.power_profile when the RPC
field is omitted instead of writing None. If clearing is required, distinguish
an explicit clear from omission and add the corresponding mixed-version
regression coverage; keep explicit profile updates unchanged.

---

Nitpick comments:
In `@crates/api-model/src/instance/snapshot.rs`:
- Line 303: Extend test_try_from_derives_os_from_instance_columns to project
both populated and unset power_profile values through
InstanceSnapshot::try_from, covering the persisted-row conversion assignment at
power_profile. Assert that populated data round-trips correctly and the unset
case remains None.
🪄 Autofix

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: 40080bc8-b700-4cc9-acda-697423810054

📥 Commits

Reviewing files that changed from the base of the PR and between e05a814 and 6c9b5ee.

📒 Files selected for processing (37)
  • crates/admin-cli/src/rpc.rs
  • crates/admin-cli/src/vpc/create/args.rs
  • crates/admin-cli/src/vpc/show/cmd.rs
  • crates/agent/src/tests/full.rs
  • crates/api-core/src/cfg/file.rs
  • crates/api-core/src/db_init.rs
  • crates/api-core/src/tests/client_resolution.rs
  • crates/api-core/src/tests/common/api_fixtures/instance.rs
  • crates/api-core/src/tests/instance.rs
  • crates/api-core/src/tests/instance_allocate.rs
  • crates/api-core/src/tests/instance_batch_allocate.rs
  • crates/api-core/src/tests/instance_config_update.rs
  • crates/api-core/src/tests/instance_find.rs
  • crates/api-core/src/tests/instance_ipxe_behaviors.rs
  • crates/api-core/src/tests/instance_os.rs
  • crates/api-core/src/tests/instance_type.rs
  • crates/api-core/src/tests/machine_discovery.rs
  • crates/api-core/src/tests/machine_states.rs
  • crates/api-core/src/tests/maintenance.rs
  • crates/api-core/src/tests/network_security_group.rs
  • crates/api-core/src/tests/network_segment.rs
  • crates/api-core/src/tests/vpc.rs
  • crates/api-core/src/tests/vpc_prefix.rs
  • crates/api-core/tests/integration/compute_allocation.rs
  • crates/api-db/migrations/20260806120000_power_provisioning_policy.sql
  • crates/api-db/src/instance.rs
  • crates/api-db/src/vpc.rs
  • crates/api-model/src/instance/config.rs
  • crates/api-model/src/instance/snapshot.rs
  • crates/api-model/src/vpc/mod.rs
  • crates/api-web/src/vpc.rs
  • crates/machine-a-tron/src/api_client.rs
  • crates/rpc/proto/forge.proto
  • crates/rpc/src/model/instance/config.rs
  • crates/rpc/src/model/instance/status.rs
  • crates/rpc/src/model/vpc.rs
  • crates/rvs/src/client/io.rs

Comment thread crates/api-db/src/vpc.rs
SET name=$1, version=$2, description=$3, network_security_group_id=$4,
labels=$5::json,
routing_profile_overrides=COALESCE($6::jsonb, routing_profile_overrides),
power_resource_group=COALESCE($7, power_resource_group),

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.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 5 \
  'power_resource_group|VpcUpdateRequest|UpdateVpc|COALESCE' \
  crates/rpc/proto/forge.proto \
  crates/api-model/src \
  crates/rpc/src \
  crates/api-db/src \
  crates/api-core/src \
  crates/admin-cli/src \
  crates/api-web/src \
  crates/machine-a-tron/src || true

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '%s\n' '--- VPC model and RPC conversion ---'
sed -n '90,115p' crates/api-model/src/vpc/mod.rs
sed -n '270,320p' crates/rpc/src/model/vpc.rs

printf '%s\n' '--- VPC update handler and database binding ---'
sed -n '150,240p' crates/api-core/src/handlers/vpc.rs
sed -n '313,365p' crates/api-db/src/vpc.rs

printf '%s\n' '--- Proto contract ---'
sed -n '1838,1882p' crates/rpc/proto/forge.proto

printf '%s\n' '--- UpdateVpc construction and power-resource-group references ---'
rg -n -C 3 'UpdateVpc\s*\{|power_resource_group|VpcUpdateRequest' \
  crates --glob '*.rs' --glob '*.yaml' --glob '*.yml' --glob '*.toml' --glob '*.md' \
  | rg -v 'target|power_resource_group: None'

printf '%s\n' '--- Candidate tests ---'
rg -n -C 4 'update.*vpc|Vpc.*update|power_resource_group|resource group' \
  crates/api-core crates/api-db crates/rpc --glob '*test*' --glob '*.rs' \
  | head -n 300

Repository: NVIDIA/infra-controller

Length of output: 50379


🏁 Script executed:

node - <<'JS'
function coalesce(value, current) {
  return value === null || value === undefined ? current : value;
}
const cases = [
  ['omitted', undefined, 'existing-group'],
  ['set', 'new-group', 'existing-group'],
  ['explicit empty string', '', 'existing-group'],
  ['explicit null', null, 'existing-group'],
];
for (const [name, value, current] of cases) {
  console.log(`${name}: ${JSON.stringify(coalesce(value, current))}`);
}
JS

Repository: NVIDIA/infra-controller

Length of output: 262


Document VPC update semantics.

power_resource_group omission preserves the stored value. A present value replaces it, but an empty string does not clear the column; it stores ''. Document this contract in the proto and update path, or add an explicit clear state with set, omit, and clear tests.

🤖 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 `@crates/api-db/src/vpc.rs` at line 338, Document the VPC update contract for
power_resource_group in the proto and update path: an omitted value preserves
the stored value, while a present value replaces it, including an empty string
which must be stored as ''. Update or add tests covering omit, set, and
empty-string behavior without introducing a clear state unless explicitly
supported.

Source: Path instructions

Comment on lines +69 to +71

/// Power profile managed by the external power provisioning service.
pub power_profile: Option<String>,

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.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 \
  '\bpower_profile\b|update_instance_config' \
  crates/api-model/src/instance/config.rs \
  crates/rpc/src/model/instance/config.rs \
  crates/api-core/src \
  crates/api-db/src \
  crates/api-web/src \
  crates/admin-cli/src \
  crates/machine-a-tron/src

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== definitions and references =='
rg -n --glob '*.rs' \
  'fn update_instance_config|update_instance_config\(|power_profile|InstanceConfigUpdateRequest|InstanceConfig\s*\{' \
  crates/api-core/src crates/api-db/src crates/api-model/src crates/rpc/src \
  | head -n 500

printf '%s\n' '== candidate files =='
git ls-files \
  crates/api-core/src \
  crates/api-db/src \
  crates/api-model/src/instance \
  crates/rpc/src/model/instance \
  | rg 'instance_config|instance/config|instance.*(update|model)|config'

Repository: NVIDIA/infra-controller

Length of output: 33144


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== update handler =='
sed -n '1160,1345p' crates/api-core/src/handlers/instance.rs

printf '%s\n' '== database write path =='
sed -n '520,610p' crates/api-db/src/instance.rs
sed -n '790,850p' crates/api-db/src/instance.rs
sed -n '895,945p' crates/api-db/src/instance.rs

printf '%s\n' '== focused update test =='
sed -n '90,345p' crates/api-core/src/tests/instance_config_update.rs

printf '%s\n' '== model and RPC conversions =='
sed -n '35,165p' crates/rpc/src/model/instance/config.rs

Repository: NVIDIA/infra-controller

Length of output: 29249


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== protocol and documentation =='
rg -n -C 6 --glob '*.{proto,rs,md,yaml,yml,toml}' \
  'power_profile|InstanceConfigUpdateRequest|complete replacement|full.*config|updateable configurations|external power' \
  .

printf '%s\n' '== non-test update callers =='
rg -n -C 5 --glob '*.rs' \
  'update_instance_config\(|InstanceConfigUpdateRequest\s*\{' \
  crates \
  | rg -v 'crates/api-core/src/tests|power_profile: None|power_profile: Some'

printf '%s\n' '== update handler continuation =='
sed -n '1340,1435p' crates/api-core/src/handlers/instance.rs

printf '%s\n' '== power profile references outside the initial files =='
rg -n -C 4 --glob '*.{rs,proto,md,yaml,yml,toml,json}' \
  '\bpower_profile\b|power profile' \
  . \
  | head -n 500

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== files containing power_profile =='
rg -l --glob '*.{rs,proto,md,yaml,yml,toml}' '\bpower_profile\b' . | sort

printf '%s\n' '== protocol declarations =='
for f in $(rg -l --glob '*.proto' '\bpower_profile\b|InstanceConfigUpdateRequest'); do
  printf '\n--- %s ---\n' "$f"
  rg -n -C 8 '\bpower_profile\b|InstanceConfigUpdateRequest' "$f"
done

printf '%s\n' '== production Rust references excluding tests =='
rg -n -C 4 --glob '*.rs' '\bpower_profile\b' crates \
  | rg -v '/tests/|power_profile: None|power_profile: Some' \
  | head -n 300

printf '%s\n' '== read-only behavioral verifier =='
python3 - <<'PY'
from pathlib import Path

handler = Path("crates/api-core/src/handlers/instance.rs").read_text()
db = Path("crates/api-db/src/instance.rs").read_text()
test = Path("crates/api-core/src/tests/instance_config_update.rs").read_text()

assert "let mut config: InstanceConfig = match request.config" in handler
assert "db::instance::update_config" in handler
assert "power_profile=$15" in db
assert ".bind(config.power_profile)" in db
assert "updated_config_2.power_profile = None;" in test
assert "assert_config_equals(instance.config.as_ref().unwrap(), &updated_config_2);" in test

print("update_instance_config accepts a complete config")
print("update_config binds config.power_profile directly to power_profile")
print("the focused integration test explicitly clears power_profile with None")
print("no retention assertion is present in the focused test")
PY

Repository: NVIDIA/infra-controller

Length of output: 1485


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== protocol declaration =='
rg -n -C 12 'power_profile|InstanceConfigUpdateRequest' crates/rpc/proto/forge.proto

printf '%s\n' '== production-facing references =='
for f in crates/admin-cli/src/rpc.rs crates/machine-a-tron/src/api_client.rs crates/rvs/src/client/io.rs; do
  printf '\n--- %s ---\n' "$f"
  rg -n -C 8 'power_profile|update_instance_config|InstanceConfigUpdateRequest' "$f" || true
done

printf '%s\n' '== source-level behavioral verifier =='
python3 - <<'PY'
from pathlib import Path

handler = Path("crates/api-core/src/handlers/instance.rs").read_text()
db = Path("crates/api-db/src/instance.rs").read_text()
test = Path("crates/api-core/src/tests/instance_config_update.rs").read_text()

assert "let mut config: InstanceConfig = match request.config" in handler
assert "db::instance::update_config" in handler
assert "power_profile=$15" in db
assert ".bind(config.power_profile)" in db
assert "updated_config_2.power_profile = None;" in test
assert "assert_config_equals(instance.config.as_ref().unwrap(), &updated_config_2);" in test

print("update_instance_config accepts a complete config")
print("update_config binds config.power_profile directly to power_profile")
print("the focused integration test explicitly clears power_profile with None")
print("the focused test does not assert retention during an unrelated update")
PY

Repository: NVIDIA/infra-controller

Length of output: 9660


Preserve power_profile during unrelated configuration updates.

InstanceConfigUpdateRequest replaces the complete configuration, and update_config writes config.power_profile directly. Clients that omit this newly added field therefore clear the externally managed profile. Treat omission as preservation with an explicit clear operation, or provide a compatibility path for older clients. Add a regression test for an unrelated update that retains a populated profile.

🤖 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 `@crates/api-model/src/instance/config.rs` around lines 69 - 71, Update
InstanceConfigUpdateRequest and update_config so an omitted power_profile
preserves the existing config.power_profile, while an explicit clear operation
can still remove it; provide a compatibility path for older clients as needed.
Add a regression test covering an unrelated update with a populated power
profile and verify it remains unchanged.

Source: Path instructions

Comment on lines +1876 to +1877
// Replaces the external power provisioning resource group when present.
optional string power_resource_group = 20;

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.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check VPC update callers and tests for clear semantics.
rg -n -C 8 'power_resource_group|VpcUpdateRequest|update_vpc|COALESCE' \
  crates/api-core crates/api-db crates/rpc crates/api-web crates/admin-cli

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- exact power_resource_group references (excluding migrations) ---'
rg -n -C 5 'power_resource_group' crates \
  -g '!**/migrations/**' \
  -g '!**/migrations.pre-squash/**' \
  | head -n 400

printf '%s\n' '--- VPC database update functions and SQL ---'
rg -n -C 12 'UPDATE .*vpc|power_resource_group|update_vpc|VpcUpdateRequest' \
  crates/api-db/src crates/api-core/src crates/api-web/src crates/admin-cli/src \
  -g '*.rs' | head -n 500

printf '%s\n' '--- VPC-related tests ---'
rg -l 'power_resource_group|VpcUpdateRequest|UpdateVpc' \
  crates -g '*test*.rs' -g '*.rs' \
  | sort -u

Repository: NVIDIA/infra-controller

Length of output: 50379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- files and focused symbols ---'
git ls-files | rg '(^|/)(vpc|forge\.proto|.*vpc.*test.*|.*vpc.*)\.(rs|proto)$' | head -n 200

printf '%s\n' '--- all non-generated VPC field references ---'
rg -n 'power_resource_group' \
  crates/api-db/src crates/api-core/src crates/api-web/src crates/admin-cli/src crates/rpc/proto/forge.proto \
  -g '*.rs' -g '*.proto' \
  --glob '!**/migrations/**' --glob '!**/migrations.pre-squash/**' \
  | head -n 300

Repository: NVIDIA/infra-controller

Length of output: 5188


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- RPC conversion ---'
sed -n '230,325p' crates/rpc/src/model/vpc.rs

printf '%s\n' '--- API update handler ---'
sed -n '150,245p' crates/api-core/src/handlers/vpc.rs

printf '%s\n' '--- VPC update tests ---'
sed -n '340,545p' crates/api-core/src/tests/vpc.rs

printf '%s\n' '--- VPC model update type ---'
sed -n '85,115p' crates/api-model/src/vpc/mod.rs

printf '%s\n' '--- database schema column ---'
rg -n -C 5 'power_resource_group' crates/api-db/migrations crates/api-db -g '*.sql' -g '*.rs' \
  | head -n 120

Repository: NVIDIA/infra-controller

Length of output: 20135


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

rpc = Path("crates/rpc/src/model/vpc.rs").read_text()
db = Path("crates/api-db/src/vpc.rs").read_text()
proto = Path("crates/rpc/proto/forge.proto").read_text()

checks = {
    "request_field_is_proto3_optional": "optional string power_resource_group = 20;" in proto,
    "request_conversion_preserves_only_option": "power_resource_group: value.power_resource_group" in rpc,
    "database_update_uses_coalesce": "power_resource_group=COALESCE($7, power_resource_group)" in db,
    "database_binds_option": ".bind(&value.power_resource_group)" in db,
}
for name, passed in checks.items():
    print(f"{name}: {'PASS' if passed else 'FAIL'}")

# Enumerate the only representable update values visible at the RPC/model boundary.
print("representations:")
for value in ("omitted", 'present("")', 'present("group")'):
    sql_argument = {
        "omitted": "NULL",
        'present("")': "''",
        'present("group")': "'group'",
    }[value]
    result = "existing value" if sql_argument == "NULL" else sql_argument
    print(f"  {value} -> SQL argument {sql_argument} -> COALESCE result {result}")
PY

Repository: NVIDIA/infra-controller

Length of output: 518


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

rpc = Path("crates/rpc/src/model/vpc.rs").read_text()
db = Path("crates/api-db/src/vpc.rs").read_text()
proto = Path("crates/rpc/proto/forge.proto").read_text()

checks = {
    "request_field_is_proto3_optional": "optional string power_resource_group = 20;" in proto,
    "request_conversion_preserves_only_option": "power_resource_group: value.power_resource_group" in rpc,
    "database_update_uses_coalesce": "power_resource_group=COALESCE($7, power_resource_group)" in db,
    "database_binds_option": ".bind(&value.power_resource_group)" in db,
}
for name, passed in checks.items():
    print(f"{name}: {'PASS' if passed else 'FAIL'}")

print("representations:")
for value in ("omitted", 'present("")', 'present("group")'):
    sql_argument = {
        "omitted": "NULL",
        'present("")': "''",
        'present("group")': "'group'",
    }[value]
    result = "existing value" if sql_argument == "NULL" else sql_argument
    print(f"  {value} -> SQL argument {sql_argument} -> COALESCE result {result}")
PY

Repository: NVIDIA/infra-controller

Length of output: 518


Document power_resource_group update semantics.

Omission preserves the current value, while an empty string stores '' and does not clear the column. Define an explicit clear representation with tests, or reject empty values and document that clearing is unsupported.

🤖 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 `@crates/rpc/proto/forge.proto` around lines 1876 - 1877, Update the
documentation and handling for power_resource_group around the protobuf field
power_resource_group to define explicit update semantics: omission must preserve
the existing value, while empty strings must either represent a supported clear
operation with tests or be rejected and documented as unsupported for clearing.
Ensure the chosen behavior is enforced consistently by the update path.

Source: Path instructions

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