feat: add landing page to the report containing custom metadata defined with a YTE yaml template#3452
Conversation
📝 WalkthroughWalkthroughThe changes introduce a custom report metadata feature, including a new documentation section explaining how to specify custom metadata via a YAML template. The API and CLI now support an optional global report settings parameter, allowing users to pass a YAML file with metadata for report generation. New classes and functions are added to parse and render the metadata, and client-side components are updated to display it. Unit tests have been added to verify the metadata functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as CLI Parser
participant WF as Workflow
participant MR as MetadataRecord
participant RP as Report Plugin (HTML)
participant JS as Client-side JS
U->>CLI: Run snakemake with --report-metadata metadata.yaml
CLI->>WF: Parse args, create GlobalReportSettings with metadata_template
WF->>WF: Call create_report(..., global_report_settings)
WF->>RP: Invoke auto_report(..., global_report_settings)
RP->>MR: Instantiate MetadataRecord with metadata_template
MR->>MR: Parse YAML template to generate metadata dict
MR->>RP: Return metadata dict
RP->>JS: Include metadata in rendered report
JS->>U: Display metadata on landing page
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (55)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/snakemake/report/html_reporter/data/__init__.py (1)
8-8: Fix unused import warningThe
render_metadatafunction is imported but not used in this file. Consider adding it to the__all__list to make it part of the module's public API.from .results import render_results from .categories import render_categories from .rulegraph import render_rulegraph from .rules import render_rules from .runtimes import render_runtimes from .timeline import render_timeline from .packages import get_packages from .metadata import render_metadata +__all__ = [ + "render_results", + "render_categories", + "render_rulegraph", + "render_rules", + "render_runtimes", + "render_timeline", + "get_packages", + "render_metadata", +]🧰 Tools
🪛 Ruff (0.8.2)
8-8:
.metadata.render_metadataimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
src/snakemake/report/html_reporter/data/metadata.py (1)
1-4: Remove unused importThe
snakemakemodule is imported but not directly used in this file.import json -import snakemake🧰 Tools
🪛 Ruff (0.8.2)
3-3:
snakemakeimported but unusedRemove unused import:
snakemake(F401)
src/snakemake/report/html_reporter/template/components/metadata.js (3)
1-1: Remove redundant 'use strict' directiveThe 'use strict' directive is unnecessary in ES modules, which are automatically in strict mode.
-'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
3-6: Remove unnecessary constructorThis constructor only calls super(props) which React does automatically when no constructor is provided.
class MetaData extends React.Component { - constructor(props) { - super(props); - }🧰 Tools
🪛 Biome (1.9.4)
[error] 4-6: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
12-14: Remove console.log statementsRemove debugging console.log statements before merging to production.
console.log("value"); console.log(value); metadatalist.push(... this.innerRender(key, value)); } - console.log(metadatalist);Also applies to: 16-16
src/snakemake/report/__init__.py (1)
360-386: Add error handling for YAML parsingThe
parse_ytemethod lacks error handling for potential YAML parsing errors. If the YAML template is malformed, it could lead to confusing error messages or application crashes.def parse_yte(self): if self.path: - with open(self.path, "r") as template: - return process_yaml(template) + try: + with open(self.path, "r") as template: + return process_yaml(template) + except Exception as e: + logger.warning(f"Error parsing metadata template: {e}") + return None else: return None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/snakefiles/reporting.rst(1 hunks)src/snakemake/api.py(3 hunks)src/snakemake/cli.py(3 hunks)src/snakemake/report/__init__.py(5 hunks)src/snakemake/report/html_reporter/__init__.py(2 hunks)src/snakemake/report/html_reporter/data/__init__.py(1 hunks)src/snakemake/report/html_reporter/data/metadata.py(1 hunks)src/snakemake/report/html_reporter/template/components/app.js(1 hunks)src/snakemake/report/html_reporter/template/components/content.js(1 hunks)src/snakemake/report/html_reporter/template/components/metadata.js(1 hunks)src/snakemake/report/html_reporter/template/components/navbar.js(1 hunks)src/snakemake/report/html_reporter/template/index.html.jinja2(1 hunks)src/snakemake/settings/types.py(1 hunks)src/snakemake/workflow.py(4 hunks)tests/common.py(2 hunks)tests/tests.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
src/snakemake/report/html_reporter/data/__init__.pysrc/snakemake/report/html_reporter/__init__.pysrc/snakemake/settings/types.pytests/tests.pytests/common.pysrc/snakemake/report/__init__.pysrc/snakemake/report/html_reporter/data/metadata.pysrc/snakemake/cli.pysrc/snakemake/workflow.pysrc/snakemake/api.py
🪛 Ruff (0.8.2)
src/snakemake/report/html_reporter/data/__init__.py
8-8: .metadata.render_metadata imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/snakemake/report/html_reporter/data/metadata.py
3-3: snakemake imported but unused
Remove unused import: snakemake
(F401)
🪛 Biome (1.9.4)
src/snakemake/report/html_reporter/template/components/metadata.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 4-6: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: tests (10, windows-latest, py312)
- GitHub Check: tests (9, macos-latest, py312)
- GitHub Check: tests (8, macos-latest, py312)
- GitHub Check: tests (8, windows-latest, py312)
- GitHub Check: tests (7, macos-latest, py312)
- GitHub Check: tests (6, macos-latest, py312)
- GitHub Check: tests (6, windows-latest, py312)
- GitHub Check: tests (5, macos-latest, py312)
- GitHub Check: tests (5, windows-latest, py312)
- GitHub Check: tests (4, windows-latest, py312)
- GitHub Check: tests (3, windows-latest, py312)
- GitHub Check: tests (2, windows-latest, py312)
🔇 Additional comments (24)
src/snakemake/report/html_reporter/template/components/app.js (1)
10-10: LGTM: Updated default content to metadataThis change correctly updates the initial state of the App component to display the metadata (landing page) by default, which aligns with the PR objective of adding a landing page to the report.
tests/tests.py (1)
301-307: LGTM: Test for report metadata functionalityThe test function follows the established pattern in the codebase and properly tests the new metadata report functionality by providing a YAML template file.
src/snakemake/settings/types.py (1)
436-440: LGTM: Well-structured settings classThe
GlobalReportSettingsclass is appropriately designed as a dataclass with clear documentation and follows the established pattern of other settings classes in the file.src/snakemake/report/html_reporter/data/metadata.py (1)
6-16: LGTM! Function implementation is clean and well-documentedThe
render_metadatafunction is well-designed and follows a good pattern: it creates a base dictionary with the Snakemake version, conditionally adds more data from the metadata object, and then serializes to JSON.src/snakemake/report/html_reporter/template/index.html.jinja2 (2)
73-75: LGTM! Metadata script element looks goodThe script element for metadata follows the same pattern as other data elements in the template.
81-81: LGTM! Component import correctly positionedThe metadata.js component import is logically placed before other component imports.
src/snakemake/report/html_reporter/__init__.py (2)
112-112: LGTM! Metadata rendering is integrated correctlyThe metadata rendering is implemented consistently with other data rendering in the Reporter class.
125-125: LGTM! Metadata included in template renderingThe metadata variable is properly passed to the template rendering method.
src/snakemake/report/html_reporter/template/components/navbar.js (1)
156-170: LGTM! Home button now displays metadata contentThe modification to the
getMenuBreadcrumbmethod now sets the content to "metadata" when the home button is clicked, enabling the landing page functionality described in the PR.src/snakemake/report/html_reporter/template/components/content.js (1)
28-41: Looks good! The metadata content handling follows the existing pattern.The addition of the "metadata" case in the switch statement allows for rendering the new landing page. The implementation correctly follows the structure used by other cases and includes appropriate class names for styling.
docs/snakefiles/reporting.rst (1)
332-355: Excellent documentation addition for the new feature.The new "Custom report metadata" section provides clear and comprehensive information about how to use the new metadata feature, including:
- Default behavior description
- Command-line usage with
--report-metadata- A practical YAML template example showing different metadata types
This follows the documentation style of the rest of the file and gives users all the information they need to use this feature effectively.
tests/common.py (3)
177-177: Good addition of the new parameter with proper default value.The
report_metadataparameter is correctly added with a default value ofNone, maintaining backward compatibility with existing test cases.
398-398: Correctly passing the global_report_settings to create_report method.The implementation properly passes the global_report_settings parameter to the create_report method.
387-394:Details
✅ Verification successful
Proper integration of the metadata template into the report generation workflow.
The code correctly:
- Converts the report_metadata to a Path object when provided
- Creates a GlobalReportSettings instance with the metadata_template
- Passes the settings to the create_report method
This implementation ensures the test framework fully supports the new metadata feature.
Ensure that the
test_report_metadatafunction intests/tests.pyexists and properly exercises this functionality.
🏁 Script executed:
#!/bin/bash # Check if the test_report_metadata function exists in tests.py grep -A 20 "test_report_metadata" tests/tests.pyLength of output: 560
Metadata Template Integration Verified – Approved
The integration of the metadata template is solid. The code correctly converts
report_metadatainto aPathobject, initializesGlobalReportSettingswith it, and passes the settings to the report creation process. Additionally, thetest_report_metadatafunction intests/tests.pyis present and exercises this functionality as expected. No changes are required.src/snakemake/workflow.py (4)
37-37: Correctly importing GlobalReportSettings.The import of
GlobalReportSettingsfromsnakemake.settings.typesis properly added to the existing imports.
154-154: Good addition of the global_report_settings attribute with proper default.The
global_report_settingsattribute is correctly added to the Workflow class with a default value ofNone, making it an optional setting.
1028-1032: Appropriately updated method signature.The
create_reportmethod signature is properly updated to include the newglobal_report_settingsparameter. This change enables the method to accept metadata information for the report.
1043-1050: Correctly passing global_report_settings to auto_report.The call to
auto_reportis properly updated to include theglobal_report_settingsparameter, ensuring the metadata information flows through to the report generation process.src/snakemake/cli.py (2)
975-981: LGTM: Great enhancement for report customizationThis new parameter allows users to specify custom metadata for the landing page of reports through a YAML template, enhancing report configurability.
2121-2123: LGTM: Proper implementation of the GlobalReportSettingsThe implementation correctly passes the report metadata template to the GlobalReportSettings object, ensuring the metadata is available during report generation.
src/snakemake/report/html_reporter/template/components/metadata.js (1)
42-42: Security concern with dangerouslySetInnerHTMLUsing
dangerouslySetInnerHTMLwith user-provided content could pose a security risk if the content isn't properly sanitized. Consider using a library like DOMPurify to sanitize the HTML content before rendering it.Are you certain that the metadata content is safe to be rendered as HTML? If there's any possibility of user-supplied content, consider adding sanitization.
src/snakemake/api.py (1)
641-642: LGTM: Well-implemented API extensionThe addition of the
global_report_settingsparameter to thecreate_reportmethod is implemented correctly, maintaining backward compatibility while allowing for the new metadata functionality.Also applies to: 662-662
src/snakemake/report/__init__.py (2)
754-755: LGTM: Clean metadata integrationThe MetadataRecord is properly instantiated with the template from global_report_settings, and the implementation allows for cases where no template is provided.
763-763: LGTM: Proper metadata passing to reporterThe metadata object is correctly passed to the reporter, ensuring that the custom metadata will be available for rendering in the report.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/snakemake/report/html_reporter/data/metadata.py (1)
1-4: Remove unused import of snakemake module.The
snakemakemodule is imported but never directly used in the code. Instead, the Snakemake version is accessed through themetadataparameter.import json -import snakemake🧰 Tools
🪛 Ruff (0.8.2)
3-3:
snakemakeimported but unusedRemove unused import:
snakemake(F401)
src/snakemake/report/html_reporter/template/components/metadata.js (3)
1-1: Remove redundant 'use strict' directive.The 'use strict' directive is unnecessary in JavaScript modules as they are automatically in strict mode.
-'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
3-7: Simplify by removing unnecessary constructor.This constructor doesn't add any functionality beyond what the parent class provides.
class MetaData extends React.Component { - constructor(props) { - super(props); - }🧰 Tools
🪛 Biome (1.9.4)
[error] 4-6: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
8-15: Remove debugging console.log statement.The console.log statement on line 14 should be removed before production deployment.
render() { // Iterate over the metadata dictionary and add the entries to the landing page let metadatalist = []; for (const [key, value] of Object.entries(metadata)) { metadatalist.push(... this.innerRender(key, value)); } - console.log(metadatalist); return e(src/snakemake/report/__init__.py (1)
32-32: Verify snakemake import usage.The
snakemakemodule is imported but appears to be only used in the newMetadataRecordclass.Consider moving this import closer to where it's used if it's not needed by other parts of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/snakemake/cli.py(3 hunks)src/snakemake/report/__init__.py(5 hunks)src/snakemake/report/html_reporter/data/metadata.py(1 hunks)src/snakemake/report/html_reporter/template/components/metadata.js(1 hunks)src/snakemake/settings/types.py(1 hunks)src/snakemake/workflow.py(4 hunks)tests/common.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/snakemake/settings/types.py
- tests/common.py
- src/snakemake/workflow.py
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
src/snakemake/cli.pysrc/snakemake/report/__init__.pysrc/snakemake/report/html_reporter/data/metadata.py
🪛 Ruff (0.8.2)
src/snakemake/report/html_reporter/data/metadata.py
3-3: snakemake imported but unused
Remove unused import: snakemake
(F401)
🪛 Biome (1.9.4)
src/snakemake/report/html_reporter/template/components/metadata.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
[error] 4-6: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: tests (10, windows-latest, py312)
- GitHub Check: tests (9, macos-latest, py312)
- GitHub Check: tests (9, windows-latest, py312)
- GitHub Check: tests (8, macos-latest, py312)
- GitHub Check: tests (8, windows-latest, py312)
- GitHub Check: tests (7, macos-latest, py312)
- GitHub Check: tests (6, macos-latest, py312)
- GitHub Check: tests (5, macos-latest, py312)
- GitHub Check: tests (5, windows-latest, py312)
- GitHub Check: tests (4, windows-latest, py312)
- GitHub Check: tests (3, windows-latest, py312)
- GitHub Check: tests (2, windows-latest, py312)
- GitHub Check: tests (1, windows-latest, py312)
🔇 Additional comments (11)
src/snakemake/report/html_reporter/data/metadata.py (1)
6-16: Function looks good and handles metadata appropriately.The function correctly creates a dictionary with the Snakemake version and merges it with user-defined metadata if available. The JSON serialization ensures the data is properly formatted for the report frontend.
src/snakemake/report/html_reporter/template/components/metadata.js (1)
22-46: Consider security implications of dangerouslySetInnerHTML.Using
dangerouslySetInnerHTMLcan pose security risks if the content comes from untrusted sources. Ensure that the metadata values are properly sanitized before rendering.The implementation correctly handles recursive object rendering, which was addressed in a previous commit. However, make sure that the metadata values coming from user-defined YAML templates are validated or sanitized to prevent XSS attacks.
src/snakemake/cli.py (3)
72-72: Import looks good.The import of
GlobalReportSettingsis appropriately added to the existing imports.
975-981: Well-structured CLI argument for report metadata.The new CLI argument is well-defined with appropriate metavar, type, and helpful description that clearly explains its purpose.
2121-2123: Proper integration with GlobalReportSettings.The metadata template is correctly passed to the GlobalReportSettings constructor and integrated into the existing report creation flow.
src/snakemake/report/__init__.py (6)
24-24: Import for YAML processing looks good.The import of
process_yamlfrom theytemodule is correctly added for processing YAML templates.
56-64: Imports are properly updated.The necessary imports for
GlobalReportSettingsandMetadataRecordInterfaceare correctly added.
360-387: Well-implemented MetadataRecord class.The new class has good documentation and properly implements the interface. It handles the case where no metadata template is provided and extracts the Snakemake version correctly.
One minor note: The splitting of
snakemake.__version__on '+' suggests there might be version suffixes (like development versions) that should be removed for clarity in the report.
510-514: Function signature update is correctly implemented.The
auto_reportfunction has been properly updated to accept the newglobal_report_settingsparameter of typeGlobalReportSettings.
755-756: Appropriate initialization of MetadataRecord.The metadata record is correctly initialized using the template from global_report_settings.
761-765: Reporter is correctly updated to include metadata.The metadata parameter is properly passed to the reporter along with the other parameters.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_report_metadata/Snakefile (1)
1-12: Appropriate test workflow structure for metadata reporting feature.This Snakefile provides a minimal but effective workflow structure to test the new landing page and metadata reporting feature. The workflow correctly uses bash as the shell executor and defines a simple rule with proper output definition and shell command. The rule reuse pattern demonstrates a common Snakemake pattern that would be useful to verify in the metadata reporting context.
Consider adding a header comment that explains this file's purpose in testing the report metadata feature, which would make the test intent clearer for future contributors:
+# Test workflow for verifying the report metadata feature +# This simple workflow generates two output files that will be included in a report +# with custom metadata from a YTE yaml template. shell.executable("bash")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/test_report_metadata/Snakefile(1 hunks)tests/test_report_metadata/yte_template.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/test_report_metadata/yte_template.yaml
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: tests (9, windows-latest, py312)
- GitHub Check: tests (8, macos-latest, py312)
- GitHub Check: tests (8, windows-latest, py312)
- GitHub Check: tests (5, windows-latest, py312)
- GitHub Check: tests (3, windows-latest, py312)
- GitHub Check: tests (2, windows-latest, py312)
- GitHub Check: tests (1, windows-latest, py312)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/snakemake/report/html_reporter/template/components/metadata.js (3)
1-1: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, so this directive is unnecessary.
-'use strict'; class MetaData extends React.Component {🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
5-16: Consider passing metadata as a prop rather than using a global variable.The render method is using a global
metadatavariable which isn't defined in this file. This approach can make testing difficult and creates implicit dependencies.class MetaData extends React.Component { render() { // Iterate over the metadata dictionary and add the entries to the landing page let metadatalist = []; - for (const [key, value] of Object.entries(metadata)) { + for (const [key, value] of Object.entries(this.props.metadata || {})) { metadatalist.push(... this.innerRender(key, value)); } return e( "ul", { }, metadatalist ) }
36-38: Consider adding a unique identifier for list items.Using just the key as the React key might lead to conflicts if you have nested objects with the same key names at different levels. Consider using a compound key that includes the path.
{ - key: `${key}`, + key: `${key}-item`, className: "p-1", dangerouslySetInnerHTML: { __html: value }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/snakemake/report/html_reporter/data/metadata.py(1 hunks)src/snakemake/report/html_reporter/template/components/metadata.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/snakemake/report/html_reporter/data/metadata.py
🧰 Additional context used
🪛 Biome (1.9.4)
src/snakemake/report/html_reporter/template/components/metadata.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: tests (10, windows-latest, py312)
- GitHub Check: tests (9, macos-latest, py312)
- GitHub Check: tests (8, macos-latest, py312)
- GitHub Check: tests (8, windows-latest, py312)
- GitHub Check: tests (7, macos-latest, py312)
- GitHub Check: tests (7, windows-latest, py312)
- GitHub Check: tests (6, macos-latest, py312)
- GitHub Check: tests (6, windows-latest, py312)
- GitHub Check: tests (5, macos-latest, py312)
- GitHub Check: tests (5, windows-latest, py312)
- GitHub Check: tests (4, windows-latest, py312)
- GitHub Check: tests (2, windows-latest, py312)
- GitHub Check: tests (1, windows-latest, py312)
- GitHub Check: apidocs
🔇 Additional comments (2)
src/snakemake/report/html_reporter/template/components/metadata.js (2)
3-4: LGTM: React component properly defined.The MetaData component is correctly defined as a class extending React.Component.
21-27: LGTM: Recursive metadata handling is now working correctly.This implementation correctly accumulates results from recursive calls, addressing the previously identified issue where only the first key-value pair was being processed.
|
First though (will review later): I think it should also display the workflow description on the landing page. |
|
Seems like there are some import errors now? |
Yes I have added the MetaDataRecord to the report interface in this pull request: snakemake/snakemake-interface-report-plugins#6. This should resolve the error: |
johanneskoester
left a comment
There was a problem hiding this comment.
Thanks a lot, very nice work! A few remarks below.
johanneskoester
left a comment
There was a problem hiding this comment.
I think this is ready for being merged. But the PR in the report interface still has some todos: snakemake/snakemake-interface-report-plugins#6
The used snakemake version is not shown by default.
This is required, to allow custom metadata in the report (snakemake/snakemake#3452). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for including optional metadata when creating reports. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/snakemake/report/html_reporter/data/metadata.py (1)
4-11: Consider more descriptive function name to avoid confusion.There's another
render_metadatafunction insrc/snakemake/report/__init__.py(lines 796-809) that processes metadata with reStructuredText, while this function performs JSON serialization. The naming similarity could cause confusion.Consider renaming this function to be more specific about its purpose, such as
serialize_metadata_to_jsonorexport_metadata_json.-def render_metadata(metadata): +def serialize_metadata_to_json(metadata): """Render the metadata to be displayed on the report landing page. Arguments --------- - metadata: dict -- Metadata dictionary containing (user specified) metadata + metadata: dict -- Metadata dictionary containing (user specified) metadata """ return json.dumps(metadata)src/snakemake/report/html_reporter/template/components/metadata.js (3)
1-1: Remove redundant 'use strict' directive.The
'use strict'directive is redundant in JavaScript modules as they are automatically in strict mode.-'use strict';
26-26: Remove debugging console.log statement.The
console.log(workflow_desc)appears to be leftover debugging code that should be removed before production.- console.log(workflow_desc)
28-28: Remove commented code.The commented line
//workflow_desc,appears to be leftover from development and should be cleaned up.- //workflow_desc,src/snakemake/report/__init__.py (2)
15-15: Remove unused imports flagged by static analysis.Static analysis indicates
typing.Anyandtyping.Typeare imported but unused.-from typing import Any, Dict, List, Optional, Type, Union +from typing import Dict, List, Optional, Union
30-30: Remove unused snakemake import if not needed.Static analysis indicates the
snakemakeimport is unused. If it's not needed elsewhere in the file, consider removing it.-import snakemake
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (14)
docs/snakefiles/reporting.rst(1 hunks)src/snakemake/api.py(3 hunks)src/snakemake/cli.py(3 hunks)src/snakemake/report/__init__.py(5 hunks)src/snakemake/report/html_reporter/data/metadata.py(1 hunks)src/snakemake/report/html_reporter/template/components/app.js(1 hunks)src/snakemake/report/html_reporter/template/components/metadata.js(1 hunks)src/snakemake/settings/types.py(1 hunks)src/snakemake/workflow.py(4 hunks)tests/common.py(2 hunks)tests/test_report_metadata/Snakefile(1 hunks)tests/test_report_metadata/report.rst(1 hunks)tests/test_report_metadata/yte_template.yaml(1 hunks)tests/tests.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/test_report_metadata/report.rst
- tests/test_report_metadata/yte_template.yaml
- tests/test_report_metadata/Snakefile
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/snakefiles/reporting.rst
- src/snakemake/settings/types.py
- tests/common.py
- src/snakemake/report/html_reporter/template/components/app.js
- tests/tests.py
- src/snakemake/api.py
- src/snakemake/cli.py
- src/snakemake/workflow.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit Configuration File
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
Files:
src/snakemake/report/html_reporter/data/metadata.pysrc/snakemake/report/__init__.py
🧠 Learnings (1)
src/snakemake/report/__init__.py (7)
Learnt from: johanneskoester
PR: #3159
File: docs/tutorial/interaction_visualization_reporting/workdir/workflow/Snakefile:1-1
Timestamp: 2024-10-20T19:42:16.694Z
Learning: The report directive at the workflow level is still valid in Snakemake and is not deprecated.
Learnt from: johanneskoester
PR: #3117
File: snakemake/deployment/conda.py:0-0
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In snakemake/deployment/conda.py, the exception handling in the _get_version method is intentional. The method raises a WorkflowError when the version cannot be determined, and this behavior is desired. Do not suggest modifying this exception handling.
Learnt from: johanneskoester
PR: #3117
File: snakemake/deployment/conda.py:0-0
Timestamp: 2024-10-06T14:09:26.494Z
Learning: In snakemake/deployment/conda.py, the exception handling in the _get_version method is intentional. The method raises a WorkflowError when the version cannot be determined, and this behavior is desired. Do not suggest modifying this exception handling.
Learnt from: johanneskoester
PR: #3171
File: snakemake/cli.py:106-145
Timestamp: 2024-10-29T09:26:26.636Z
Learning: In the snakemake codebase, exception chaining (using raise ... from ...) is avoided due to backward compatibility reasons. Do not suggest using exception chaining in snakemake codebase.
Learnt from: cademirch
PR: #3107
File: snakemake/logging.py:20-20
Timestamp: 2025-01-13T18:50:40.791Z
Learning: Dependencies for Snakemake are managed through test-environment.yml file, which includes snakemake-interface-logger-plugins (>= 0.1.0) and other required packages.
Learnt from: johanneskoester
PR: #3114
File: snakemake/cli.py:1707-1707
Timestamp: 2024-10-05T09:55:00.509Z
Learning: ReportPluginRegistry is a singleton; multiple instantiations are acceptable and do not need optimization in cli.py.
Learnt from: johanneskoester
PR: #3114
File: snakemake/cli.py:1707-1707
Timestamp: 2024-10-08T17:41:54.542Z
Learning: ReportPluginRegistry is a singleton; multiple instantiations are acceptable and do not need optimization in cli.py.
🧬 Code Graph Analysis (2)
src/snakemake/report/html_reporter/data/metadata.py (2)
src/snakemake/report/__init__.py (1)
render_metadata(796-809)src/snakemake/persistence.py (1)
metadata(423-424)
src/snakemake/report/__init__.py (4)
src/snakemake/settings/types.py (1)
GlobalReportSettings(440-443)src/snakemake/io/__init__.py (1)
open(386-405)src/snakemake/report/html_reporter/data/metadata.py (1)
render_metadata(4-11)src/snakemake/report/html_reporter/__init__.py (1)
render(67-178)
🪛 Ruff (0.12.2)
src/snakemake/report/__init__.py
15-15: typing.Any imported but unused
(F401)
15-15: typing.Type imported but unused
(F401)
30-30: snakemake imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
⏰ 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). (51)
- GitHub Check: tests (9, windows-latest, py313)
- GitHub Check: tests (10, windows-latest, py313)
- GitHub Check: tests (7, ubuntu-latest, py311)
- GitHub Check: tests (10, ubuntu-latest, py313)
- GitHub Check: tests (9, ubuntu-latest, py312)
- GitHub Check: tests (9, ubuntu-latest, py313)
- GitHub Check: tests (8, ubuntu-latest, py312)
- GitHub Check: tests (6, windows-latest, py312)
- GitHub Check: tests (10, macos-latest, py313)
- GitHub Check: tests (6, windows-latest, py313)
- GitHub Check: tests (10, windows-latest, py312)
- GitHub Check: tests (8, windows-latest, py313)
- GitHub Check: tests (10, ubuntu-latest, py311)
- GitHub Check: tests (8, ubuntu-latest, py313)
- GitHub Check: tests (6, macos-latest, py312)
- GitHub Check: tests (9, windows-latest, py312)
- GitHub Check: tests (7, windows-latest, py313)
- GitHub Check: tests (9, ubuntu-latest, py311)
- GitHub Check: tests (5, ubuntu-latest, py312)
- GitHub Check: tests (7, ubuntu-latest, py313)
- GitHub Check: tests (10, ubuntu-latest, py312)
- GitHub Check: tests (6, ubuntu-latest, py311)
- GitHub Check: tests (8, windows-latest, py312)
- GitHub Check: tests (8, macos-latest, py312)
- GitHub Check: tests (7, ubuntu-latest, py312)
- GitHub Check: tests (1, windows-latest, py312)
- GitHub Check: tests (4, windows-latest, py313)
- GitHub Check: tests (8, ubuntu-latest, py311)
- GitHub Check: tests (6, ubuntu-latest, py312)
- GitHub Check: tests (5, ubuntu-latest, py313)
- GitHub Check: tests (5, windows-latest, py313)
- GitHub Check: tests (4, ubuntu-latest, py312)
- GitHub Check: tests (5, windows-latest, py312)
- GitHub Check: tests (5, ubuntu-latest, py311)
- GitHub Check: tests (4, macos-latest, py312)
- GitHub Check: tests (6, ubuntu-latest, py313)
- GitHub Check: tests (4, windows-latest, py312)
- GitHub Check: tests (3, ubuntu-latest, py312)
- GitHub Check: tests (2, windows-latest, py312)
- GitHub Check: tests (3, windows-latest, py313)
- GitHub Check: tests (3, ubuntu-latest, py313)
- GitHub Check: tests (3, ubuntu-latest, py311)
- GitHub Check: tests (4, ubuntu-latest, py311)
- GitHub Check: tests (3, macos-latest, py313)
- GitHub Check: tests (2, ubuntu-latest, py313)
- GitHub Check: tests (3, windows-latest, py312)
- GitHub Check: tests (4, ubuntu-latest, py313)
- GitHub Check: tests (2, ubuntu-latest, py312)
- GitHub Check: tests (2, windows-latest, py313)
- GitHub Check: tests (1, windows-latest, py313)
- GitHub Check: tests (2, ubuntu-latest, py311)
🔇 Additional comments (7)
src/snakemake/report/html_reporter/template/components/metadata.js (2)
55-56: LGTM: Security concern properly addressed.The comment correctly indicates that values are "already rendered by docutils" which addresses the security concern about using
dangerouslySetInnerHTML. The upstream RST processing ensures the content is safe.
38-44: LGTM: Recursive object handling is correct.The recursive handling of nested objects has been properly implemented based on previous review feedback. The logic correctly iterates through all key-value pairs and accumulates results.
src/snakemake/report/__init__.py (5)
732-734: LGTM: Correct fix for workflow description rendering.The change from
"body"to"html_body"ensures that the title is properly extracted from the rendered content, which aligns with the comment's intention.
776-783: LGTM: Well-structured validation helper function.The
_is_valid_flat_valuefunction correctly validates the allowed types for metadata values and handles both individual values and lists appropriately.
785-794: LGTM: Comprehensive dictionary validation.The
_validate_flat_dictfunction properly validates the metadata structure, ensuring keys are strings and values conform to the allowed types.
796-810: LGTM: Consistent RST rendering implementation.The
render_metadatafunction correctly processes string values usingpublish_parts, following the same pattern used elsewhere in the codebase. The in-place modification approach with key copying is appropriate to avoid iteration issues.
743-760: Consider adding validation for YAML processing errors.While the file I/O error handling suggestion above covers basic file access, consider also validating that the YAML processing succeeded and produced valid output before validation.
The error handling improvement suggested above should also include specific handling for YAML processing errors to provide clearer error messages to users.
⛔ Skipped due to learnings
Learnt from: johanneskoester PR: snakemake/snakemake#3171 File: snakemake/cli.py:106-145 Timestamp: 2024-10-29T09:26:26.636Z Learning: Avoid adding input validation or error handling that unnecessarily complicates the code in the `snakemake` codebase, especially when the cases handled don't make sense.
|
Time to merge! Thanks a lot @LKress! |
🤖 I have created a release *beep* *boop* --- ## [9.11.0](v9.10.1...v9.11.0) (2025-09-05) ### Features * add landing page to the report containing custom metadata defined with a YTE yaml template ([#3452](#3452)) ([c754d80](c754d80)) * Issue a warning when unsupported characters are used in wildcard names ([#1587](#1587)) ([fa57355](fa57355)) ### Bug Fixes * avoid checking output files in immediate-submit mode ([#3569](#3569)) ([58c42cf](58c42cf)) * clarify --keep-going flag help text to distinguish runtime vs DAG construction errors ([#3705](#3705)) ([a3a8ef4](a3a8ef4)) * enable docstring assignment in `use rule ... with:` block ([#3710](#3710)) ([2191962](2191962)) * fix missing attribute error in greedy scheduler settings when using touch, dryrun or immediate submit. ([6471004](6471004)) * only trigger script with CODE label ([#3707](#3707)) ([2d01f8c](2d01f8c)) * parser.py incorrectly parsing multiline f-strings ([#3701](#3701)) ([06ece76](06ece76)) * parsing ordinary yaml strings ([#3506](#3506)) ([ddf334e](ddf334e)) * remove temp files when using checkpoints ([#3716](#3716)) ([5ff3e20](5ff3e20)) * Restore python rules changes triggering reruns. (GH: [#3213](#3213)) ([#3485](#3485)) ([2f663be](2f663be)) * unit testing ([#3680](#3680)) ([06ba7e6](06ba7e6)) * use queue_input_wait_time when updating queue input jobs ([#3712](#3712)) ([a945a19](a945a19)) ### Documentation * output files within output directories is an error ([#2848](#2848)) ([#2913](#2913)) ([37272e5](37272e5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
…ed with a YTE yaml template (snakemake#3452) <!--Add a description of your PR here--> Adds a landing page for the report, that contains metadata. The landing page can be reached from every page in the report by pressing the home button in the navbar on the top left. The metadata can be user defined by passing a YTE yaml template with `--report-metadata`. If no metadata template is provided by the user, only the Snakemake version that was used to run the workflow is displayed on the landing page. Related to issue snakemake#3395 ### QC <!-- Make sure that you can tick the boxes below. --> * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced report customization by allowing users to supply a YAML template for additional metadata. - Improved the report interface to display enriched, user-defined contextual information. - Added a new command-line option to specify custom metadata for reports. - **Documentation** - Added a new section with clear examples and instructions for incorporating custom metadata into reports. - **Tests** - Introduced automated tests to ensure that the metadata integration functions as expected. - Added new test cases covering standard and nested metadata scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de> Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
🤖 I have created a release *beep* *boop* --- ## [9.11.0](snakemake/snakemake@v9.10.1...v9.11.0) (2025-09-05) ### Features * add landing page to the report containing custom metadata defined with a YTE yaml template ([snakemake#3452](snakemake#3452)) ([c754d80](snakemake@c754d80)) * Issue a warning when unsupported characters are used in wildcard names ([snakemake#1587](snakemake#1587)) ([fa57355](snakemake@fa57355)) ### Bug Fixes * avoid checking output files in immediate-submit mode ([snakemake#3569](snakemake#3569)) ([58c42cf](snakemake@58c42cf)) * clarify --keep-going flag help text to distinguish runtime vs DAG construction errors ([snakemake#3705](snakemake#3705)) ([a3a8ef4](snakemake@a3a8ef4)) * enable docstring assignment in `use rule ... with:` block ([snakemake#3710](snakemake#3710)) ([2191962](snakemake@2191962)) * fix missing attribute error in greedy scheduler settings when using touch, dryrun or immediate submit. ([6471004](snakemake@6471004)) * only trigger script with CODE label ([snakemake#3707](snakemake#3707)) ([2d01f8c](snakemake@2d01f8c)) * parser.py incorrectly parsing multiline f-strings ([snakemake#3701](snakemake#3701)) ([06ece76](snakemake@06ece76)) * parsing ordinary yaml strings ([snakemake#3506](snakemake#3506)) ([ddf334e](snakemake@ddf334e)) * remove temp files when using checkpoints ([snakemake#3716](snakemake#3716)) ([5ff3e20](snakemake@5ff3e20)) * Restore python rules changes triggering reruns. (GH: [snakemake#3213](snakemake#3213)) ([snakemake#3485](snakemake#3485)) ([2f663be](snakemake@2f663be)) * unit testing ([snakemake#3680](snakemake#3680)) ([06ba7e6](snakemake@06ba7e6)) * use queue_input_wait_time when updating queue input jobs ([snakemake#3712](snakemake#3712)) ([a945a19](snakemake@a945a19)) ### Documentation * output files within output directories is an error ([snakemake#2848](snakemake#2848)) ([snakemake#2913](snakemake#2913)) ([37272e5](snakemake@37272e5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Adds a landing page for the report, that contains metadata. The landing page can be reached from every page in the report by pressing the home button in the navbar on the top left.
The metadata can be user defined by passing a YTE yaml template with
--report-metadata. If no metadata template is provided by the user, only the Snakemake version that was used to run the workflow is displayed on the landing page.Related to issue #3395
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).Summary by CodeRabbit
New Features
Documentation
Tests