refactor: reach block-bodied-closure error messages, and clarify the fix hint#3613
Merged
Conversation
Contributor
|
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 (6)
🚧 Files skipped from review as they are similar to previous changes (6)
Summary by CodeRabbit
WalkthroughThe pull request standardizes capitalization in error contexts across agent and controller code. It also extends the error-message casing linter to process block-bodied closures, improves violation guidance, and adds end-to-end coverage. ChangesError message standardization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
🤖 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/machine-controller/src/scout_firmware_scripts.rs`:
- Around line 257-262: Update the expected error string in
errors_for_zero_timeout_metadata to use lowercase “scout,” matching the
normalized message emitted by read_metadata. Keep the rest of the assertion
unchanged.
🪄 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: 12dcb74b-ca56-4b43-95f6-888566f357eb
📒 Files selected for processing (6)
crates/agent/src/agent_platform.rscrates/agent/src/extension_services/k8s_pod_handler.rscrates/agent/src/hbn.rscrates/machine-controller/src/scout_firmware_scripts.rscrates/xtask/src/error_message_case.rscrates/xtask/src/workspace_deps.rs
…fix hint NVIDIA#3595 taught the `lint-error-messages` checker to peel a `format!` inside an expression-bodied closure (`.wrap_err_with(|| format!(...))`), but the common *block*-bodied form (`.wrap_err_with(|| { format!(...) })`) still slipped through -- the exact siblings CodeRabbit pointed at on NVIDIA#3595. This closes that gap, and while we're in the failure path, makes the "how do I fix this?" output impossible to miss for a contributor who trips the check. Primary callouts are: - `leading_str_lit` gains an `Expr::Block` arm that forwards to the block's tail expression, so a message behind `|| { format!(...) }` is reached just like the expression-bodied form. `--fix` then lowercased 19 messages across 5 files (agent file-ops, `k8s_pod_handler`, `hbn`, machine-controller scout scripts, and the xtask workspace-deps check). - The report-mode failure now prints the rule, the exact `cargo xtask lint-error-messages --fix` command on its own line, and the `// xtask:allow-error-case` opt-out for a deliberate capital -- instead of a single sentence tacked onto the violation count. Still out of reach, and marked in the checker `TODO` as the remaining follow-up: struct-literal error fields (`CarbideError::Internal { message: "..." }`, ~150 sites), manual `impl Display`/`write!` bodies, and macro-body-nested sites. Tests added! -- `end_to_end_rewrite_reaches_block_closure` drives the real span -> byte-range -> splice path through a `.wrap_err_with(|| { format!(...) })` and checks the `{host}` interpolation survives. This continues NVIDIA#3595 and supports NVIDIA#3468 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
bcavnvidia
approved these changes
Jul 16, 2026
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
2 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
A small follow-up to #3595. That PR reached
format!inside expression-bodied closures (.wrap_err_with(|| format!(...))); this closes the sibling gap for block-bodied closures (.wrap_err_with(|| { format!(...) })) — the exact form CodeRabbit flagged there — and makes the checker's failure output easy for a contributor to act on.leading_str_litgains anExpr::Blockarm (forward to the block's tail expression), so|| { format!(...) }is reached just like|| format!(...).--fixlowercased 19 messages across 5 files (agent file-ops,k8s_pod_handler,hbn, machine-controller scout scripts, and the xtask workspace-deps check).cargo xtask lint-error-messages --fixcommand on its own line, and the// xtask:allow-error-caseopt-out for a deliberate capital — instead of a one sentence tacked onto the violation count.Still deferred, and documented in the checker
TODO: struct-literal error fields (CarbideError::Internal { message: "..." }, ~150 sites), manualDisplay/write!bodies, and macro-body-nested sites.Continues #3595 · supports #3468