Module: Update Holoscan Sensor Bridge pin and accompanying website rendering#1610
Merged
Merged
Conversation
Update pin: - Bumps to Holoscan Sensor Bridge "2.6.0" release - Additional commit adds metadata.json for Holoscan Module description and website integration Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
The holoscan-sensor-bridge module is now pinned to a specific commit SHA in module-sites.json, replacing the mutable "main" branch reference. GitHub rejects bare-SHA shallow fetches from empty repos, causing the website builder to skip the HSB module card entirely; rendering all 22 operators in the card also caused it to overflow vertically. - Fixed clone_module.py to use a blobless partial clone (--filter=blob:none --no-checkout) for SHA refs, which downloads the full commit graph without file content and lazily fetches only the blobs needed for the target commit, resolving any reachable SHA on GitHub. - Updated generate_module_pages.py to limit module cards to 2 operator preview badges plus a "+N more" overflow pill, preventing vertical card overflow. - Added the full operator list to the module detail page metadata header via a new :octicons-tools-24: Operators entry in build_metadata_header(). Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
"5" == highest quality Signed-off-by: Tom Birdsong <tbirdsong@nvidia.com>
Contributor
Greptile SummaryThis PR updates the holoscan-sensor-bridge module pin from the named tag
Confidence Score: 4/5Safe to merge for the JSON and rendering changes; the clone script has one unguarded network call that can stall a website build indefinitely on a slow connection. The doc/website/scripts/clone_module.py — the Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant WB as Website Build
participant CM as clone_module.py
participant GH as GitHub
WB->>CM: clone_external_module(url, sha_ref)
CM->>GH: "git clone --filter=blob:none --no-checkout (timeout 300s)"
GH-->>CM: commit graph, no blobs
CM->>GH: git checkout sha (no timeout)
GH-->>CM: lazily fetch required blobs
CM-->>WB: clone_path, tmp_dir
Note over WB,GH: Named-tag or branch path
WB->>CM: clone_external_module(url, tag_ref)
CM->>GH: "git clone --depth=1 --branch tag (timeout 120s)"
GH-->>CM: shallow clone with blobs
CM-->>WB: clone_path, tmp_dir
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant WB as Website Build
participant CM as clone_module.py
participant GH as GitHub
WB->>CM: clone_external_module(url, sha_ref)
CM->>GH: "git clone --filter=blob:none --no-checkout (timeout 300s)"
GH-->>CM: commit graph, no blobs
CM->>GH: git checkout sha (no timeout)
GH-->>CM: lazily fetch required blobs
CM-->>WB: clone_path, tmp_dir
Note over WB,GH: Named-tag or branch path
WB->>CM: clone_external_module(url, tag_ref)
CM->>GH: "git clone --depth=1 --branch tag (timeout 120s)"
GH-->>CM: shallow clone with blobs
CM-->>WB: clone_path, tmp_dir
|
Contributor
WalkthroughThe PR improves module tooling and display infrastructure by addressing shallow fetch issues in the module cloning function, adding operator metadata to module pages with smart badge truncation in index cards, and updating the holoscan-sensor-bridge module configuration with a new commit ref and quality score. ChangesModule documentation and tooling enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
doc/website/scripts/generate_module_pages.py (1)
363-363:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle potential
Nonevalue foroperator_names.If the module metadata contains
"operator_names": null(explicitly null rather than missing), this list comprehension will raise aTypeErrorwhen attempting to iterate overNone. Lines 272-273 handle this case safely by checkingif operator_names, but this code does not.🛡️ Defensive fix
- operators = [html.escape(str(op)) for op in metadata_module.get("operator_names", [])] + operators = [html.escape(str(op)) for op in (metadata_module.get("operator_names") or [])]🤖 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 `@doc/website/scripts/generate_module_pages.py` at line 363, metadata_module.get("operator_names") can be None which causes the list comprehension building operators to fail; change the code to safely handle a None by coercing operator_names to an empty list before iterating (e.g., operator_names = metadata_module.get("operator_names") or []), then compute operators = [html.escape(str(op)) for op in operator_names]; this keeps the existing html.escape/str usage and the operators variable name intact.doc/website/scripts/clone_module.py (1)
44-45: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUpdate docstring to reflect the new blobless clone strategy.
The docstring still describes the old
init+fetch+checkoutapproach, but the implementation now uses a blobless partial clone (--filter=blob:none --no-checkout) followed by an explicit checkout.📝 Proposed docstring fix
Accepts branch names, annotated tags, or full 40-character commit SHAs. - Commit SHAs are fetched via init+fetch+checkout rather than --branch, which - only accepts named refs. + Commit SHAs are fetched via blobless partial clone (--filter=blob:none) to avoid + GitHub's shallow-fetch restrictions, then checked out explicitly. Named refs use + shallow clone (--depth=1 --branch).🤖 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 `@doc/website/scripts/clone_module.py` around lines 44 - 45, Update the docstring to describe the current blobless partial clone strategy: replace the old "init+fetch+checkout" wording with a clear description that the implementation performs a partial clone using git clone --filter=blob:none --no-checkout (to avoid blobs), then explicitly fetch and checkout the desired commit SHA(s) or refs; mention the use of commit SHAs and that checkout is performed after the partial clone. Locate and edit the top-level module or the clone function docstring (e.g., the clone_repo/clone_module function) to include the exact flags (--filter=blob:none --no-checkout) and the sequence: partial clone → fetch specific commits/refs → explicit checkout.
🧹 Nitpick comments (3)
doc/website/scripts/generate_module_pages.py (1)
380-393: 💤 Low valueConsider adding a comment to explain the 2-operator limit.
The badge limiting logic is correct but would benefit from a brief inline comment explaining why the first two operators are shown and overflow is calculated.
📝 Suggested comment
+ # Limit operator badges to first 2 operators; show "+N more" for overflow preview_ops = operators[:2] overflow = len(operators) - 2🤖 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 `@doc/website/scripts/generate_module_pages.py` around lines 380 - 393, The code builds operator badges showing only the first two operators using preview_ops = operators[:2] and computes overflow = len(operators) - 2; add a brief inline comment above that block explaining the reason for the 2-operator limit (e.g., to keep the UI compact and show a "+N more" indicator), referencing preview_ops, overflow, and operator_badges so future readers understand why only two operators are displayed and why overflow is calculated.doc/website/scripts/clone_module.py (2)
68-74: 💤 Low valueConsider aligning the timeout for named-ref clones.
The SHA-based clone uses a 300s timeout (line 59), while the named-ref clone uses 120s. If the timeout increase was motivated by larger repository sizes, named refs might benefit from the same extension. If the difference is intentional (blobless clones are expected to take longer), consider adding a comment explaining the rationale.
🤖 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 `@doc/website/scripts/clone_module.py` around lines 68 - 74, The named-ref git clone call using subprocess.run([... "--branch", ref, ...], timeout=120) should use the same 300s timeout as the SHA-based clone (or document why they differ); update the timeout argument from 120 to 300 in the subprocess.run invocation (the call that references _GIT, "clone", "--branch", ref, url, str(clone_path)) or add a brief comment explaining the intentional difference.
51-53: 💤 Low valueMinor: Consider refining the comment about GitHub's shallow fetch behavior.
The comment mentions "empty repos," but the underlying issue is that GitHub's shallow fetch protocol requires local ref tips to prove commit reachability. This fails when fetching a bare SHA with no existing local refs, regardless of whether the remote repo is empty.
🔍 Suggested refinement
- # GitHub rejects bare-SHA shallow fetches from empty repos (no local tips to prove - # reachability). Use a blobless partial clone instead: downloads the full commit graph + # GitHub rejects bare-SHA shallow fetches when there are no local refs to prove + # reachability. Use a blobless partial clone instead: downloads the full commit graph # without file content, then lazily fetches only the blobs needed for the target commit.🤖 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 `@doc/website/scripts/clone_module.py` around lines 51 - 53, Update the inline comment that currently reads "GitHub rejects bare-SHA shallow fetches from empty repos..." to say that GitHub's shallow-fetch protocol requires local ref tips to prove commit reachability, so attempting to fetch a bare SHA fails when there are no local refs (not only when the remote is empty); locate the comment starting "GitHub rejects bare-SHA shallow fetches..." in the blobless partial clone explanatory block and replace it with this refined phrasing to accurately describe the reachability requirement.
🤖 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 `@modules/module-sites.json`:
- Around line 15-18: Update the holoscan-sensor-bridge entry to document why
nvidia_quality_score was raised from 1 to 5: in the modules/module-sites.json
entry for "holoscan-sensor-bridge" (ref
c34f9fe876462e2fa83877527f86fe0427346cc0) add a brief rationale comment or
metadata field explaining the change (e.g., tests passed, security review, API
stability, or upstream verification that the pinned SHA matches main/HEAD and
fixes prior issues) and reference any verification artifacts (CI run IDs, test
results, or review ticket) used to justify the higher score so reviewers can
validate the rationale.
---
Outside diff comments:
In `@doc/website/scripts/clone_module.py`:
- Around line 44-45: Update the docstring to describe the current blobless
partial clone strategy: replace the old "init+fetch+checkout" wording with a
clear description that the implementation performs a partial clone using git
clone --filter=blob:none --no-checkout (to avoid blobs), then explicitly fetch
and checkout the desired commit SHA(s) or refs; mention the use of commit SHAs
and that checkout is performed after the partial clone. Locate and edit the
top-level module or the clone function docstring (e.g., the
clone_repo/clone_module function) to include the exact flags (--filter=blob:none
--no-checkout) and the sequence: partial clone → fetch specific commits/refs →
explicit checkout.
In `@doc/website/scripts/generate_module_pages.py`:
- Line 363: metadata_module.get("operator_names") can be None which causes the
list comprehension building operators to fail; change the code to safely handle
a None by coercing operator_names to an empty list before iterating (e.g.,
operator_names = metadata_module.get("operator_names") or []), then compute
operators = [html.escape(str(op)) for op in operator_names]; this keeps the
existing html.escape/str usage and the operators variable name intact.
---
Nitpick comments:
In `@doc/website/scripts/clone_module.py`:
- Around line 68-74: The named-ref git clone call using subprocess.run([...
"--branch", ref, ...], timeout=120) should use the same 300s timeout as the
SHA-based clone (or document why they differ); update the timeout argument from
120 to 300 in the subprocess.run invocation (the call that references _GIT,
"clone", "--branch", ref, url, str(clone_path)) or add a brief comment
explaining the intentional difference.
- Around line 51-53: Update the inline comment that currently reads "GitHub
rejects bare-SHA shallow fetches from empty repos..." to say that GitHub's
shallow-fetch protocol requires local ref tips to prove commit reachability, so
attempting to fetch a bare SHA fails when there are no local refs (not only when
the remote is empty); locate the comment starting "GitHub rejects bare-SHA
shallow fetches..." in the blobless partial clone explanatory block and replace
it with this refined phrasing to accurately describe the reachability
requirement.
In `@doc/website/scripts/generate_module_pages.py`:
- Around line 380-393: The code builds operator badges showing only the first
two operators using preview_ops = operators[:2] and computes overflow =
len(operators) - 2; add a brief inline comment above that block explaining the
reason for the 2-operator limit (e.g., to keep the UI compact and show a "+N
more" indicator), referencing preview_ops, overflow, and operator_badges so
future readers understand why only two operators are displayed and why overflow
is calculated.
🪄 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: 58b252ee-a9a4-45f6-aa24-f303ca79fdd6
📒 Files selected for processing (3)
doc/website/scripts/clone_module.pydoc/website/scripts/generate_module_pages.pymodules/module-sites.json
wyli
approved these changes
Jun 12, 2026
wyli
reviewed
Jun 12, 2026
bhashemian
approved these changes
Jun 22, 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
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.
Background
Holoscan Sensor Bridge latest top-of-tree development adds descriptive
module.jsonat the project root.Changes
Summary by CodeRabbit
New Features
Updates