docs: expand golden values update strategy in release process guide#3268
Open
docs: expand golden values update strategy in release process guide#3268
Conversation
Add explicit step-by-step instructions for when and how to update golden values: paired internal PRs for metric-affecting changes, exact CI inputs (container, MCore commit, MBridge commit) for both main and release branches, and which internal CI repo branch to target in each case. Signed-off-by: oliver könig <okoenig@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: oliver könig <okoenig@nvidia.com>
Specify that the last bulk golden values update happens in Week 5 of code-freeze; after that, engineers are individually responsible for keeping golden values current on the release branch. Signed-off-by: oliver könig <okoenig@nvidia.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: oliver könig <okoenig@nvidia.com>
Contributor
📝 WalkthroughWalkthroughDocumentation update to the release process guide that expands and clarifies guidance on golden values management, including new rules for PR updates, step-by-step procedures for updating golden values for main and release branches, and revised release-branch code-freeze timelines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 (2)
docs/releases/release-process.md (2)
33-36: Add explicit requirement to record perf deltas + run context.Line 35 mandates paired golden-values PRs, which is good. Please also require before/after performance numbers and the exact run configuration/context in the PR description so reviewers can validate why golden values changed.
Based on learnings, if a change could affect performance, the PR description should include before-and-after performance numbers, as well as the configuration and context in which they apply.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/releases/release-process.md` around lines 33 - 36, Update the "When to update golden values" section to require that any PR triggering a golden-values update must include before-and-after performance numbers and the exact run configuration/context in the PR description; specifically, augment the guideline under the "When to update golden values" header to explicitly state that PRs must record performance deltas (numeric before/after metrics), the benchmark commands, dataset/seed/environment details, and any relevant hardware/software configuration so reviewers can validate the change.
37-46: Minor wording cleanup on Line 45.“merged together” is a bit wordy; “merged in sync” (or similar) reads cleaner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/releases/release-process.md` around lines 37 - 46, Update the wording in the "Updating golden values for PRs targeting `main`" step 4: replace the phrase "The MBridge PR and the internal golden-values PR should be merged together (or the golden-values PR first)." with a cleaner alternative such as "The MBridge PR and the internal golden-values PR should be merged in sync (or the golden-values PR first)." Modify the text under that header/step to use "merged in sync" (or a similar concise phrase) instead of "merged together" to improve clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/releases/release-process.md`:
- Around line 47-57: The phrase "MBridge PR commit" defined as "head of the
MBridge release branch" is ambiguous; update the text so it explicitly instructs
to use the head commit of the PR branch (or the exact PR SHA) to avoid picking
the wrong commit when multiple PRs target the same release branch—replace the
current wording "MBridge PR commit (head of the MBridge release branch)" with
something like "MBridge PR commit — the head commit of the PR branch (use the
exact PR branch tip SHA or PR merge SHA)".
---
Nitpick comments:
In `@docs/releases/release-process.md`:
- Around line 33-36: Update the "When to update golden values" section to
require that any PR triggering a golden-values update must include
before-and-after performance numbers and the exact run configuration/context in
the PR description; specifically, augment the guideline under the "When to
update golden values" header to explicitly state that PRs must record
performance deltas (numeric before/after metrics), the benchmark commands,
dataset/seed/environment details, and any relevant hardware/software
configuration so reviewers can validate the change.
- Around line 37-46: Update the wording in the "Updating golden values for PRs
targeting `main`" step 4: replace the phrase "The MBridge PR and the internal
golden-values PR should be merged together (or the golden-values PR first)."
with a cleaner alternative such as "The MBridge PR and the internal
golden-values PR should be merged in sync (or the golden-values PR first)."
Modify the text under that header/step to use "merged in sync" (or a similar
concise phrase) instead of "merged together" to improve clarity.
🪄 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: Pro
Run ID: d0ed51b4-22f2-4771-8999-699c40b889af
📒 Files selected for processing (1)
docs/releases/release-process.md
Comment on lines
+47
to
57
| ### Updating golden values during a release | ||
|
|
||
| When golden values need to be refreshed on the release branch (e.g. at the start of code-freeze or after an accepted regression): | ||
|
|
||
| 1. **Rebase the MBridge PR against the MBridge release branch** so it is at the head of that branch. | ||
| 2. **Launch an internal CI run** using: | ||
| - The **latest internal RC container** for the release. | ||
| - The **MCore commit pinned on the release branch**. | ||
| - The **MBridge PR commit** (head of the MBridge release branch). | ||
| 3. Open a PR against the **internal CI repository's release branch** with the updated golden values. | ||
|
|
Contributor
There was a problem hiding this comment.
Clarify the commit source on Line 55 to avoid wrong SHA selection.
Line 55 says “MBridge PR commit” and then defines it as “head of the MBridge release branch.” That can conflict when multiple PRs target the same release branch. Recommend explicitly saying “the head commit of the PR branch (or exact PR SHA).”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/releases/release-process.md` around lines 47 - 57, The phrase "MBridge
PR commit" defined as "head of the MBridge release branch" is ambiguous; update
the text so it explicitly instructs to use the head commit of the PR branch (or
the exact PR SHA) to avoid picking the wrong commit when multiple PRs target the
same release branch—replace the current wording "MBridge PR commit (head of the
MBridge release branch)" with something like "MBridge PR commit — the head
commit of the PR branch (use the exact PR branch tip SHA or PR merge SHA)".
malay-nagda
reviewed
Apr 10, 2026
| ### When to update golden values | ||
|
|
||
| Any PR that can affect performance metrics (e.g. changes to model code, training loop, optimizer, or numerical kernels) **must be accompanied by a corresponding internal PR that updates the golden values** before merging. Do not wait until after the PR lands. | ||
|
|
Contributor
There was a problem hiding this comment.
Should there be a statement for exceptions- "Exceptions can be made on rare occasion of issues with GPU availability- cluster is offline, compute availability is low, etc."?
yaoyu-33
added
docs-only
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
3 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
main: rebase MBridge againstmain, launch CI with nightly container + latest MCore commit + MBridge PR commit, open internal PR againstinternal-ci/main.internal-ci/<release-branch>.Example (what a dev sees now)
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit