Conversation
Contributor
📝 WalkthroughWalkthroughThe PR adds a ChangesVisualizer dry-run mode with demo data fallback
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
documentation
Add static visualizer demo page and set dim theme as default. Add a self-contained static HTML demo of the visualizer with embedded sample data (141 servers, 9 racks) that can be served via VitePress without a running backend. The demo starts in walk mode for an immersive first impression. Also switch the default theme from dark to dim in the live visualizer and fix init() to respect the currentTheme variable instead of hardcoding THEMES[0]. Signed-off-by: Andreas Fritzler <andreas.fritzler@sap.com>
![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 (1)
internal/cmd/visualizer/visualizer.go (1)
44-50: ⚡ Quick winConsider adding defensive validation for the Client/DryRun invariant.
The conditional registration logic correctly gates the
/api/serversendpoint based onDryRun. However, there's no validation to enforce that whenDryRunis false,Clientmust be non-nil. If a future caller misconfigures the visualizer (e.g., forgets to setDryRun = trueafter passing a nil client), the server will panic when the API endpoint is accessed at line 89.🛡️ Proposed defensive check
func (v *Visualizer) StartAndServe() error { + if !v.DryRun && v.Client == nil { + return fmt.Errorf("client must not be nil when dry-run is disabled") + } + url := fmt.Sprintf("http://%s", v.Address) http.HandleFunc("/", serveFrontend)🤖 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 `@internal/cmd/visualizer/visualizer.go` around lines 44 - 50, In StartAndServe, add a defensive validation enforcing the invariant between DryRun and Client: if v.DryRun is false then ensure v.Client is non-nil and return a descriptive error (instead of registering the handler that would later panic); perform this check before calling http.HandleFunc("/api/servers", v.handleGetServers()) so the function returns early if v.Client == nil. Reference: Visualizer.StartAndServe, Visualizer.Client, Visualizer.DryRun, and handleGetServers().
🤖 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 `@REUSE.toml`:
- Line 31: REUSE.toml contains a nonexistent file reference
"internal/cmd/visualizer/demo_data.json" which breaks REUSE checks; open
REUSE.toml and remove that exact entry (the string
"internal/cmd/visualizer/demo_data.json") from the listed files so only existing
assets (e.g., index.html and visualizer.go) remain referenced, then run the
REUSE compliance check to confirm the failure is resolved.
---
Nitpick comments:
In `@internal/cmd/visualizer/visualizer.go`:
- Around line 44-50: In StartAndServe, add a defensive validation enforcing the
invariant between DryRun and Client: if v.DryRun is false then ensure v.Client
is non-nil and return a descriptive error (instead of registering the handler
that would later panic); perform this check before calling
http.HandleFunc("/api/servers", v.handleGetServers()) so the function returns
early if v.Client == nil. Reference: Visualizer.StartAndServe,
Visualizer.Client, Visualizer.DryRun, and handleGetServers().
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0d20da0-172f-4ad5-9d8f-942ae19a16b8
📒 Files selected for processing (8)
REUSE.tomlcmd/metalctl/app/visualizer.goconfig/samples/visualizer/servers.yamldocs/public/visualizer-demo.htmldocs/usage/metalctl.mdinternal/cmd/client/client.gointernal/cmd/visualizer/index.htmlinternal/cmd/visualizer/visualizer.go
| "test/data/*.yaml", | ||
| "api/v1alpha1/zz_generated.deepcopy.go" | ||
| "api/v1alpha1/zz_generated.deepcopy.go", | ||
| "internal/cmd/visualizer/demo_data.json" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify demo_data.json existence
# Check if demo_data.json exists
if [ -f "internal/cmd/visualizer/demo_data.json" ]; then
echo "✓ demo_data.json exists"
ls -lh internal/cmd/visualizer/demo_data.json
else
echo "✗ demo_data.json not found"
echo "Demo data appears to be embedded in index.html instead"
fi
# Show what's actually in the visualizer directory
echo -e "\nFiles in internal/cmd/visualizer/:"
ls -lh internal/cmd/visualizer/Repository: ironcore-dev/metal-operator
Length of output: 322
Remove the non-existent demo_data.json reference from REUSE.toml line 31.
The annotation references internal/cmd/visualizer/demo_data.json, but this file does not exist. The visualizer directory only contains index.html (which embeds demo data inline) and visualizer.go. This incorrect annotation will cause REUSE compliance check failures.
🤖 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 `@REUSE.toml` at line 31, REUSE.toml contains a nonexistent file reference
"internal/cmd/visualizer/demo_data.json" which breaks REUSE checks; open
REUSE.toml and remove that exact entry (the string
"internal/cmd/visualizer/demo_data.json") from the listed files so only existing
assets (e.g., index.html and visualizer.go) remain referenced, then run the
REUSE compliance check to confirm the failure is resolved.
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.
Proposed Changes
Add static visualizer demo page and set dim theme as default. Add a self-contained static HTML demo of the visualizer with embedded sample data (141 servers, 9 racks) that can be served via VitePress without a running backend. The demo starts in walk mode for an immersive first impression.
Also switch the default theme from dark to dim in the live visualizer.
Summary by CodeRabbit
New Features
--dry-runflag to visualizer command for offline preview without cluster connectionDocumentation
go install