Fix bokeh charts generation

[amaslenn]

Summary

Ensure package version is aligned with CDN js scripts.

Test Plan

1. CI (extended)
2. Manual runs

Additional Notes

—

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Updates the report template's Bokeh CDN version, adds a unit test to assert the template's Bokeh version matches pyproject.toml, and changes comparison report chart logic to skip empty DataFrames and base x/y axis calculations on the longest non-empty dataset.

Changes

Cohort / File(s)	Summary
Bokeh CDN Version Update src/cloudai/util/nixl_report_template.jinja2	Updated Bokeh CDN script URLs from 3.4.0 to 3.8.0.
Chart Axis & Empty-Data Handling src/cloudai/report_generator/comparison_report.py	Skip whole-group rendering when all DataFrames are empty; skip individual empty DataFrames during chart construction; recompute y-axis bounds excluding empty datasets; choose x-axis bounds and tick positions based on the longest non-empty DataFrame.
Version Consistency Test tests/report_generation_strategy/test_comparison_report.py	Added test_bokeh_cdn_version_matches_pyproject which parses Bokeh dependency from pyproject.toml and asserts the resolved major.minor version appears in the template CDN URL. Added imports: Path, toml, Requirement, Version.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Inspect logic when all DataFrames are empty to ensure early return doesn't skip necessary side-effects.
• Verify y-axis and x-axis calculations using the longest DataFrame handle edge cases (single series, varying lengths, all empty).
• Confirm the new test robustly parses version specifiers (e.g., ranges, caret, tilde) and matches template lines containing the CDN URL.

Poem

🐰 Hopped in the code where the charts align,

Empty frames skipped, the x-axis now fine,
CDN version checked with a careful sniff,
Templates and pyproject now sing in a jiff,
I nibble on bugs and leave only a whiff.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix bokeh charts generation' is overly broad and does not clearly capture the main objective of aligning package version with CDN scripts; it also doesn't reflect the specific implementation changes to handle empty dataframes.	Consider a more specific title such as 'Update Bokeh CDN version and handle empty dataframes in chart generation' to better reflect the actual changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is related to the changeset, mentioning version alignment with CDN scripts which matches the template update changes in the PR.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch am/bokeh-ver

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7930169 and daf6be3.

📒 Files selected for processing (1)

• src/cloudai/report_generator/comparison_report.py (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-12-16T19:47:41.994Z

Learnt from: amaslenn
Repo: NVIDIA/cloudai PR: 754
File: src/cloudai/_core/registry.py:226-234
Timestamp: 2025-12-16T19:47:41.994Z
Learning: In this repository, prefer expressing behavioral documentation through tests rather than docstrings. Tests act as living, verified documentation. Reserve docstrings for interfaces or high-level descriptions, and avoid duplicating behavior that is already covered by tests.

Applied to files:

• src/cloudai/report_generator/comparison_report.py

🧬 Code graph analysis (1)

src/cloudai/report_generator/comparison_report.py (1)

src/cloudai/util/lazy_imports.py (1)

• bokeh_models (99-106)

🔇 Additional comments (2)

src/cloudai/report_generator/comparison_report.py (2)

182-184: Good defensive guard for all-empty DataFrames.

This early return addresses the critical issue raised in past review comments where max() and min() would fail on empty sequences if all DataFrames were empty. The debug logging is also helpful for troubleshooting.

---

187-188: LGTM - appropriate skip for empty DataFrames.

Skipping empty DataFrames during rendering prevents unnecessary processing and potential rendering issues.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7a2ab8 and 7930169.

📒 Files selected for processing (3)

• src/cloudai/report_generator/comparison_report.py (2 hunks)
• src/cloudai/util/nixl_report_template.jinja2 (1 hunks)
• tests/report_generation_strategy/test_comparison_report.py (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-12-16T19:47:41.994Z

Learnt from: amaslenn
Repo: NVIDIA/cloudai PR: 754
File: src/cloudai/_core/registry.py:226-234
Timestamp: 2025-12-16T19:47:41.994Z
Learning: In this repository, prefer expressing behavioral documentation through tests rather than docstrings. Tests act as living, verified documentation. Reserve docstrings for interfaces or high-level descriptions, and avoid duplicating behavior that is already covered by tests.

Applied to files:

• src/cloudai/report_generator/comparison_report.py
• tests/report_generation_strategy/test_comparison_report.py

🧬 Code graph analysis (1)

src/cloudai/report_generator/comparison_report.py (1)

src/cloudai/util/lazy_imports.py (1)

• bokeh_models (99-106)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Run pytest (3.12)

🔇 Additional comments (2)

src/cloudai/report_generator/comparison_report.py (1)

183-185: LGTM! Good defensive check.

Skipping empty DataFrames prevents rendering issues when no data is available for a particular test run.

src/cloudai/util/nixl_report_template.jinja2 (1)

7-9: LGTM! CDN version aligned with package version.

The Bokeh CDN URLs have been correctly updated to version 3.8.0, which should resolve compatibility issues between the Python package and the JavaScript libraries loaded from the CDN.

[greptile-apps]

Greptile Overview

Greptile Summary

Updated Bokeh CDN version from 3.4.0 to 3.8.0 to align with pyproject.toml dependency, and improved empty dataframe handling in chart generation.

Key Changes:

• Updated Bokeh CDN scripts in nixl_report_template.jinja2 from 3.4.0 to 3.8.0 to match the bokeh~=3.8 dependency
• Added early return when all dataframes are empty to prevent chart generation errors
• Added logic to skip individual empty dataframes during chart rendering loop
• Filtered empty dataframes from y-axis range calculations to prevent ValueError on empty sequences
• Changed x-axis range calculation to use longest dataframe instead of first dataframe
• Added automated test to ensure CDN version stays in sync with package dependency

Issues Found:

• Line 210 in comparison_report.py: The selection of longest dataframe doesn't filter empty dataframes, which could cause errors when accessing column values for x-axis range calculation

Confidence Score: 4/5

• This PR is mostly safe to merge with one edge case bug that needs fixing
• The PR correctly addresses the version mismatch between Bokeh package and CDN scripts, and adds proper empty dataframe handling. However, there's a logical error on line 210 where selecting the longest dataframe could return an empty one, potentially causing runtime errors when computing x-axis range. The fix significantly improves robustness with early return, skip logic, and filtered calculations, but this one remaining edge case prevents a higher score. The new test ensures future version alignment.
• Pay close attention to src/cloudai/report_generator/comparison_report.py line 210 - needs filtering for empty dataframes

Important Files Changed

File Analysis

Filename	Score	Overview
src/cloudai/report_generator/comparison_report.py	4/5	Added empty dataframe handling with early return and skip logic; fixed x-axis calculation to use longest dataframe instead of first; filtered empty dataframes in y-axis range calculations
src/cloudai/util/nixl_report_template.jinja2	5/5	Updated Bokeh CDN script versions from 3.4.0 to 3.8.0 to align with pyproject.toml dependency
tests/report_generation_strategy/test_comparison_report.py	5/5	Added automated test to ensure Bokeh CDN version in template matches pyproject.toml dependency version

Sequence Diagram

sequenceDiagram
    participant User
    participant ComparisonReport
    participant Jinja2
    participant BokehCDN
    participant Browser

    User->>ComparisonReport: generate()
    ComparisonReport->>ComparisonReport: load_test_runs()
    
    alt No test runs found
        ComparisonReport-->>User: Skip report generation
    else Test runs available
        ComparisonReport->>ComparisonReport: create_charts(cmp_groups)
        
        loop For each group
            ComparisonReport->>ComparisonReport: create_chart(group, dfs, ...)
            
            alt All dataframes empty
                ComparisonReport->>ComparisonReport: return empty figure (early exit)
            else Has non-empty dataframes
                ComparisonReport->>ComparisonReport: Skip empty dataframes in loop
                ComparisonReport->>ComparisonReport: Calculate y_range (filter empty dfs)
                ComparisonReport->>ComparisonReport: Select longest df for x_range
                ComparisonReport->>ComparisonReport: Render chart with data
            end
        end
        
        ComparisonReport->>Jinja2: render template with bokeh_script/div
        Note over Jinja2: Uses nixl_report_template.jinja2<br/>with Bokeh 3.8.0 CDN links
        Jinja2-->>ComparisonReport: HTML content
        ComparisonReport->>ComparisonReport: Write HTML to file
        ComparisonReport-->>User: Report created
        
        User->>Browser: Open HTML report
        Browser->>BokehCDN: Load bokeh-3.8.0.min.js
        Browser->>BokehCDN: Load bokeh-widgets-3.8.0.min.js
        Browser->>BokehCDN: Load bokeh-tables-3.8.0.min.js
        BokehCDN-->>Browser: JavaScript libraries
        Browser->>Browser: Render interactive charts
    end

Loading

[greptile-apps]

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}