docs(contributor): document exposing scheduling knobs without new flags by mchmarny · Pull Request #1379 · NVIDIA/aicr

mchmarny · 2026-06-16T10:34:41Z

Summary

Adds a contributor-docs section explaining how to expose node-placement (and other chart) knobs through existing value-path primitives instead of adding a new CLI flag per knob.

Motivation / Context

A contributor writing a new recipe asked whether to add per-role node-selector/toleration flags (e.g. slurm-controlplane-node-selector, slurm-gpu-worker-node-selector). AICR already covers this via the system/accelerated node classes plus --set/--dynamic, so the answer is "no new flags" — but that pattern was not written down. This generalizes the guidance for any component author.

Fixes: N/A
Related: N/A

Type of Change

Documentation update

Component(s) Affected

Docs/examples (docs/, examples/)

Implementation Notes

Extends docs/contributor/component.md, right after valueOverrideKeys, since the pattern builds on both the nodeScheduling node-class paths and --set routing. Covers the three override layers (registry paths → component value defaults → deploy-time --set/--dynamic), a decision table for where a knob belongs, and the "chart must actually render the path" caveat (with slinky-slurm-operator as the worked example already in the registry).

Testing

./tools/check-docs-mdx docs/contributor/component.md   # OK: all doc files are MDX-safe

Doc-only change; no Go sources touched.

Risk Assessment

Low — Isolated docs addition, easy to revert

Rollout notes: N/A

Checklist

Tests pass locally (make test with -race)
Linter passes (make lint)
I did not skip/disable tests to make CI green
I added/updated tests for new functionality
I updated docs if user-facing behavior changed
Changes follow existing patterns in the codebase
Commits are cryptographically signed (git commit -S)

github-actions · 2026-06-16T10:35:44Z

🌿 Preview your docs: https://nvidia-preview-docs-component-scheduling-knobs.docs.buildwithfern.com/aicr

coderabbitai · 2026-06-16T10:38:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 036668b6-235b-4495-bd7b-53ac779fcd4b

📥 Commits

Reviewing files that changed from the base of the PR and between 4333df8 and f8e7255.

📒 Files selected for processing (1)

docs/contributor/component.md

📝 Walkthrough

Walkthrough

A new section, "Exposing Scheduling Knobs Without New Flags," is added to docs/contributor/component.md. It documents how AICR models node placement through chart value paths rather than new CLI flags, describes the merge order among registry nodeScheduling paths, component values.yaml defaults, and deploy-time --set/--dynamic overrides, provides decision rules for choosing nodeScheduling.system versus nodeScheduling.accelerated, and notes that the upstream Helm chart must actually render the targeted nodeSelector path for the configuration to take effect.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding documentation about exposing scheduling configuration without new CLI flags, which matches the changeset.
Description check	✅ Passed	The description clearly explains the motivation, implementation approach, and scope of the documentation addition, and is directly related to the changeset.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch docs/component-scheduling-knobs

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

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 `@docs/contributor/component.md`:
- Around line 193-221: The section conflates path routing with value precedence
by including nodeScheduling paths as the first item in a "three layers"
precedence list. Restructure the documentation to separate these concerns:
first, explain nodeScheduling paths as a routing mechanism that maps chart value
paths to node classes (system/accelerated), making clear it is metadata for path
selection, not a value layer. Then, separately present the actual three layers
of value precedence: component value defaults (in values.yaml/overlays),
deploy-time overrides (--set/--dynamic), and describe how merge order works
across these layers. This clarifies that nodeScheduling operates orthogonally to
the merge precedence, routing which values get applied rather than participating
in the precedence ordering itself.

🪄 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: ASSERTIVE

Plan: Enterprise

Run ID: f9d9e4cd-c00d-4668-923c-1fb1bf070a7b

📥 Commits

Reviewing files that changed from the base of the PR and between 12b86df and 4333df8.

📒 Files selected for processing (1)

docs/contributor/component.md

Generalize node-placement guidance for recipe/component authors: model placement as value paths across registry nodeScheduling, component value defaults, and deploy-time --set/--dynamic overrides rather than adding a new CLI flag per knob. Separates path routing (nodeScheduling) from value precedence, includes a decision table and the chart-must-render caveat (slinky-slurm-operator as worked example).

github-actions · 2026-06-16T10:43:50Z

Coverage Report ✅

Metric	Value
Coverage	77.1%
Threshold	75%
Status	Pass

Coverage Badge

![Coverage](https://img.shields.io/badge/coverage-77.1%25-green)

No Go source files changed in this PR.

mchmarny requested a review from a team as a code owner June 16, 2026 10:34

mchmarny added the theme/recipes Recipe expansion, overlays, mixins, and component registry label Jun 16, 2026

mchmarny self-assigned this Jun 16, 2026

github-actions Bot added area/docs size/M labels Jun 16, 2026

coderabbitai Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread docs/contributor/component.md Outdated

mchmarny force-pushed the docs/component-scheduling-knobs branch from 4333df8 to f8e7255 Compare June 16, 2026 10:42

mchmarny added 2 commits June 16, 2026 03:55

Merge branch 'main' into docs/component-scheduling-knobs

94ce2f0

Merge branch 'main' into docs/component-scheduling-knobs

8355ef8

mchmarny enabled auto-merge (squash) June 16, 2026 11:27

lalitadithya approved these changes Jun 16, 2026

View reviewed changes

mchmarny disabled auto-merge June 16, 2026 11:28

mchmarny merged commit 0c2ebbe into main Jun 16, 2026
29 checks passed

mchmarny deleted the docs/component-scheduling-knobs branch June 16, 2026 11:29

mchmarny mentioned this pull request Jun 16, 2026

fix(ci): correct docs-only detection in merge gate #1381

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

docs(contributor): document exposing scheduling knobs without new flags#1379

docs(contributor): document exposing scheduling knobs without new flags#1379
mchmarny merged 3 commits into
mainfrom
docs/component-scheduling-knobs

mchmarny commented Jun 16, 2026

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mchmarny commented Jun 16, 2026

Summary

Motivation / Context

Type of Change

Component(s) Affected

Implementation Notes

Testing

Risk Assessment

Checklist

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report ✅

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

github-actions Bot commented Jun 16, 2026 •

edited

Loading