refactor(api-web): adopt style guide rules for pub/module visibility - #4630
Merged
Conversation
This adopts the new style guide rules around `pub` and module visibility introduced in NVIDIA#4522, applying the correct visibility throughout API Web. Primary callouts are: - Replace unrestricted `pub` throughout `crates/api-web/src/**` with private, `pub(super)`, or test-only `pub(crate)` visibility based on real call sites. - Keep `carbide_api_web::routes` public as the crate's HTTP integration point while scoping handlers and helpers to their actual parents and siblings. - Keep managed-host row-display coverage without widening production APIs by colocating that test with its private implementation. - Remove the unused managed-host `time_in_state` display field exposed during dead-code cleanup. - Leave HTTP routes, templates, serialized responses, RPC calls, and runtime behavior unchanged. Tests updated! This supports NVIDIA#4548. This supports the updated Rust visibility scoping guidelines in `STYLE_GUIDE.md`, established in NVIDIA#4522. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
Contributor
Author
|
@coderabbitai full_review, thanks! |
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 (55)
Summary by CodeRabbit
WalkthroughThe PR narrows visibility across ChangesAPI web visibility narrowing
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.45.0)crates/api-web/src/lib.rsast-grep timed out on this file Comment |
Contributor
|
🐇🔍 ✅ Action performedFull review finished. |
ianderson-nvidia
approved these changes
Aug 5, 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.
This adopts the new style guide rules around
puband module visibility introduced in #4522, applying the correct visibility throughout API Web.Primary callouts are:
pubthroughoutcrates/api-web/src/**with private,pub(super), or test-onlypub(crate)visibility based on real call sites.carbide_api_web::routespublic as the crate's HTTP integration point while scoping handlers and helpers to their actual parents and siblings.time_in_statedisplay field exposed during dead-code cleanup.Tests updated!
Related issues
This supports #4548.
This supports the updated Rust visibility scoping guidelines in
STYLE_GUIDE.md, established in #4522.Type of Change
Breaking Changes
Testing
All 59 API Web tests passed during implementation. The focused managed-host row-display test also passed against an isolated PostgreSQL instance after the clean rebase that brought in #4600. I also ran:
make core/tests TEST_ARGS="-p carbide-api-web test_managed_host_row_display" cargo check --locked -p carbide-api-web --all-targets --all-features cargo make format-nightly cargo make clippy cargo make carbide-lints git diff --checkAdditional Notes
The initial visibility lint identified 280 overbroad declarations across 51 files. The final 55-file diff also includes the root and test-module boundaries exposed by that cleanup, and the only unrestricted source
publeft is the intentionalcarbide_api_web::routescrate API. Most hunks are visibility tokens plus the resulting rustfmt reflow.Local CodeRabbit and read-only Claude reviews completed over the full diff. All applicable findings are incorporated.
Closes #4548