Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis update introduces a client-side search modal with Meilisearch integration, new JSON output utilities, and extensive UI enhancements including dark mode and filterable search. The build process is improved with minification and progress bars, and logging is added throughout. Configuration and template files are updated to support the new features and improve maintainability. Changes
Sequence Diagram(s)Search Modal Interaction with MeilisearchsequenceDiagram
participant User
participant Browser
participant SearchModal (JS)
participant Meilisearch API
User->>Browser: Clicks search button / types shortcut
Browser->>SearchModal (JS): Opens modal, focuses input
User->>SearchModal (JS): Types query, selects filters
SearchModal (JS)->>Meilisearch API: Sends search request with query and filters
Meilisearch API-->>SearchModal (JS): Returns search results
SearchModal (JS)->>Browser: Renders results in modal
User->>SearchModal (JS): Closes modal
SearchModal (JS)->>Browser: Restores focus, hides modal
Build Process with MinificationsequenceDiagram
participant Developer
participant Makefile
participant Python Scripts
participant File System
Developer->>Makefile: Run build
Makefile->>Python Scripts: Generate HTML, JSON, assets
Python Scripts->>File System: Write output files
Makefile->>Python Scripts: Run minify_static
Python Scripts->>File System: Minify CSS/JS in output directory
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
.idea/misc.xml (1)
4-5: Reevaluate committing IDE-specific settings
Sharing.ideaconfiguration can lead to merge conflicts and inconsistent setups. For project-wide Black formatting, prefer usingpyproject.tomlor.editorconfig. Otherwise, consider adding.idea/to.gitignoreand distributing shared settings in a more portable format..idea/stigaview-static.iml (1)
6-8: Validate IDE module exclusions
Excludingoutand.venv/folders from IDE indexing correctly mirrors the Makefile’s$(OUT)variable and virtual environment updates. Ensure these paths are also reflected in.gitignore/.editorconfig, or move exclusions to a shared configuration rather than committing IDE metadata.stigaview_static/import_stig.py (1)
85-91: Avoid confusion in capture groups vs return order
The regex uses named groupsversionthenrelease, but the function returns(release, version)to preserve legacy behavior. This mismatch can be confusing. Consider one of the following:
- Swap capture group names to
?P<release>first, then?P<version>.- Update the function signature or add a docstring/type hint to clarify the return order.
Also, usingre.fullmatchinstead ofre.matchwould be more explicit given the^...$anchors..editorconfig (1)
18-21: Standardize file pattern globbing
The header[{**.py, **.css, **.js, **.html}]may not be recognized by all EditorConfig implementations. Consider using a standard glob pattern:[*.{py,css,js,html}] indent_size = 4 indent_style = spaceor separate sections
[*.py],[*.css], etc., to ensure consistent styling.utils/minify.py (1)
26-26: Fix typo in variable name.The variable name
minifedshould beminifiedfor clarity and correctness.- minifed = minify_html.minify(css_file.read_text(), minify_css=True) - css_file.write_text(minifed) + minified = minify_html.minify(css_file.read_text(), minify_css=True) + css_file.write_text(minified)- minifed = minify_html.minify( + minified = minify_html.minify( js_file.read_text(), minify_js=True, keep_comments=False ) - js_file.write_text(minifed) + js_file.write_text(minified)Also applies to: 32-32
utils/add_to_search.py (1)
60-60: Consider using list comprehension for better performance.Following the Pylint suggestion, you can improve performance and readability.
- docs = list() - for file in json_files: - docs.append(json.loads(file.read_text())) + docs = [json.loads(file.read_text()) for file in json_files]stigaview_static/json_output.py (1)
10-22: Clean implementation of product mapping generation.The function correctly generates JSON mappings for products and STIGs with appropriate data structures.
Consider applying this minor optimization suggested by pylint:
- products_list: Dict[str, str] = dict() + products_list: Dict[str, str] = {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.editorconfig(1 hunks).gitignore(1 hunks).idea/misc.xml(1 hunks).idea/stigaview-static.iml(1 hunks)Makefile(1 hunks)public_html/static/css/style.css(2 hunks)public_html/static/js/search.js(1 hunks)requirements.txt(1 hunks)setup.cfg(1 hunks)stigaview.toml(1 hunks)stigaview_static/html_output.py(6 hunks)stigaview_static/import_stig.py(1 hunks)stigaview_static/json_output.py(1 hunks)stigaview_static/main.py(3 hunks)stigaview_static/models.py(4 hunks)stigaview_static/utils.py(2 hunks)templates/base.html(1 hunks)templates/search.html(1 hunks)templates/stig.html(1 hunks)utils/add_to_search.py(1 hunks)utils/minify.py(1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 16-16: Missing required phony target "all"
(minphony)
[warning] 16-16: Missing required phony target "test"
(minphony)
[warning] 18-18: Target "all" should be declared PHONY.
(phonydeclared)
🪛 HTMLHint (1.5.0)
templates/search.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 Pylint (3.3.7)
stigaview_static/models.py
[refactor] 141-141: Too few public methods (0/2)
(R0903)
[refactor] 147-147: Too few public methods (0/2)
(R0903)
utils/add_to_search.py
[refactor] 60-60: Consider using [] instead of list()
(R1734)
stigaview_static/main.py
[refactor] 58-58: Consider using 'sys.exit' instead
(R1722)
[refactor] 98-98: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 129-129: Consider using [] instead of list()
(R1734)
[refactor] 130-130: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 139-139: Consider using 'sys.exit' instead
(R1722)
stigaview_static/json_output.py
[refactor] 12-12: Consider using '{}' instead of a call to 'dict'.
(R1735)
🪛 Gitleaks (8.26.0)
utils/add_to_search.py
11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Ruff (0.11.9)
stigaview_static/main.py
28-28: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
142-142: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
🔇 Additional comments (39)
requirements.txt (1)
7-8: Verify package versions for currency and security.The addition of
minify_htmlandtqdmdependencies supports the new HTML minification and progress bar features. However, ensure these specific versions are current and free from known vulnerabilities.#!/bin/bash # Description: Check for latest versions and security advisories for the new dependencies echo "Checking minify_html version..." curl -s https://pypi.org/pypi/minify-html/json | jq '.info.version' echo "Checking tqdm version..." curl -s https://pypi.org/pypi/tqdm/json | jq '.info.version' echo "Checking for security advisories..." gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: PIP, package: "minify-html") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: PIP, package: "tqdm") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'.gitignore (1)
195-196: LGTM! Proper exclusion of build artifacts.Adding
*.tar.gzto the gitignore under the "Output" section properly excludes generated archive files from version control, which is a best practice for build artifacts.stigaview_static/utils.py (1)
1-4: Verify Python version compatibility for tomllib.The
tomllibmodule is only available in Python 3.11+. Ensure your project's minimum Python version supports this, or consider usingtomlifor backward compatibility.#!/bin/bash # Description: Check if project specifies Python version requirements echo "Checking for Python version specifications..." fd -t f "pyproject.toml|setup.py|setup.cfg|.python-version|requirements.txt" --exec cat {}templates/stig.html (1)
11-11: LGTM! Good semantic HTML improvement.Adding the
id="stig_id_column"attribute enables targeted styling and scripting for the STIG ID column, which improves maintainability and supports the UI enhancements mentioned in the broader changes.stigaview.toml (1)
4-4: LGTM! Good feature flag implementation.The
use_search = falseconfiguration flag provides a clean way to control the new search functionality. Setting it tofalseby default is a conservative approach that prevents accidental activation during deployment.setup.cfg (1)
15-15: Ensure CLI entry point update is reflected across the project
The new console scriptstigaview = stigaview_static:main:mainaligns the CLI name with the application. Verify that documentation, CI/CD scripts, and any usage instructions have been updated accordingly.templates/base.html (2)
21-30: Well-implemented accessible search button.The conditional search button includes proper ARIA attributes (
aria-haspopup="dialog",aria-controls="search-modal") and semantic HTML structure. The SVG icon is appropriately sized and usescurrentColorfor theme compatibility.
44-47: Good conditional loading pattern.The search template and JavaScript are loaded only when the
use_searchflag is enabled, which prevents unnecessary resource loading when search is disabled. The async loading of the script is also a performance optimization.Makefile (2)
16-16: Static analysis warnings are acceptable in this context.The checkmake warnings about missing "all" and "test" phony targets are false positives for this specific build setup. The current .PHONY declaration appropriately covers the actual phony targets in use.
28-30: Good integration of minification step.The new
minify_statictarget properly integrates with the build process and uses the consistent$(OUT)variable pattern.templates/search.html (1)
1-30: Excellent accessibility implementation for the search modal.The modal structure includes proper ARIA roles, labels, and live regions for screen reader compatibility. The form elements are correctly labeled and the modal behavior attributes are properly set. The HTMLHint doctype warning is a false positive since this is a template fragment, not a standalone HTML document.
stigaview_static/html_output.py (7)
1-2: LGTM! Well-organized imports for new functionality.The new imports properly support the enhanced features: logging for monitoring, multiprocessing for performance, minify_html for optimization, tqdm for progress tracking, and integration with the new JSON output module.
Also applies to: 6-6, 8-8, 11-12
19-25: Excellent enhancements to template rendering.The configuration merging and HTML minification are well-implemented improvements that enhance both functionality and performance.
34-34: Good integration with JSON output generation.The addition of
render_json_controlproperly extends the rendering pipeline to generate search-compatible JSON data alongside HTML.
71-74: Clean multiprocessing helper function.The
process_product_argsfunction properly adapts arguments for the multiprocessing pool.
82-93: Excellent multiprocessing implementation with progress tracking.The combination of multiprocessing with tqdm provides both performance improvement and user feedback. Using
cpu_count()cores is appropriate for this CPU-bound rendering task.
77-77: Good addition of logging for process visibility.The logging statements provide helpful monitoring of the rendering pipeline stages.
Also applies to: 97-97, 110-110, 124-124
133-133: Consistent progress tracking for SRG details.The tqdm progress bar aligns with the overall improvement in user feedback during rendering operations.
public_html/static/css/style.css (4)
64-94: Well-designed button styles with proper accessibility.The button implementation includes appropriate hover states, focus indicators, and transitions. The focus ring using box-shadow provides good accessibility.
95-222: Comprehensive and accessible modal implementation.The modal styles include:
- Proper z-index layering
- Overlay for backdrop interaction
- Flexible layout with scrollable content
- Accessible close button
- Well-styled search input and results
The implementation follows modern CSS practices and accessibility guidelines.
223-323: Excellent responsive design and filter styling.The tag system, filter components, and responsive breakpoints are well-implemented. The media queries provide appropriate adaptations for different screen sizes.
325-471: Thorough dark mode support.The dark mode implementation comprehensively covers all UI components with appropriate color choices that maintain readability and accessibility.
stigaview_static/models.py (3)
4-4: Good improvement in error handling.Replacing
sys.stderr.writewithlogging.errorprovides better structured logging and integration with the application's logging system.Also applies to: 106-108
47-66: Well-designed search integration methods.The
search_primary_keyproperty andto_search_jsonmethod provide a clean interface for search functionality. The JSON structure includes all necessary fields for comprehensive search indexing.
141-151: Appropriate configuration models.The Pydantic models for configuration provide type safety and validation. The pylint warnings about too few public methods can be safely ignored as these are data models, not behavior-focused classes.
stigaview_static/json_output.py (1)
24-29: Efficient JSON control output generation.The function properly handles directory creation and file writing for individual control JSON files. The use of
pathlibfor path operations is a good practice.public_html/static/js/search.js (6)
51-80: Excellent accessibility implementation with focus trapping.The focus trap implementation properly handles keyboard navigation within the modal, ensuring good accessibility for screen reader and keyboard users.
82-93: Proper debounce implementation for search optimization.The debounce function effectively reduces API calls during rapid user input, improving both performance and user experience.
112-149: Robust search implementation with good error handling.The search function properly handles loading states, errors, and API communication. The filter integration and result limiting are well-implemented.
152-192: Well-structured result rendering with accessibility.The result rendering creates properly structured HTML with focusable links and semantic markup. The metadata tags provide useful context for users.
194-215: Good modal state management with focus restoration.The modal open/close logic properly manages focus, prevents body scrolling, and restores the previous focus state for accessibility.
284-322: Comprehensive dropdown initialization with proper error handling.The async initialization properly loads and populates filter dropdowns with appropriate error handling and sorting.
stigaview_static/main.py (7)
3-3: LGTM: Good import additions for new functionality.The new imports for logging, tqdm progress bars, and json_output align well with the enhanced functionality being added.
Also applies to: 9-9, 11-11
14-18: LGTM: Good modularization of model preparation.The extraction of model preparation into a dedicated function with logging is a good improvement. The switch from
update_forward_refs()tomodel_rebuild()suggests a Pydantic v2 migration, which is appropriate.
48-50: LGTM: Good addition of configurable log level.The log level argument with custom validation enhances the CLI experience and allows users to control verbosity.
66-84: LGTM: Excellent improvements to main function.The structured logging setup, runtime tracking, and addition of JSON output generation are all excellent improvements that enhance the CLI experience and functionality.
86-90: LGTM: Good extraction of product config loading.Extracting the product configuration loading into a dedicated function improves code organization and reusability.
92-112: LGTM: Good improvements to process_product function.The addition of progress bar integration and consistent use of pathlib.Path are excellent improvements that enhance user experience and code consistency.
115-123: LGTM: Good helper function for progress bar initialization.The file counting logic is correct and necessary for proper progress bar initialization.
adjust minify
So that if your timezone changes during the run you don't get odd results.
# Conflicts: # requirements.txt
Can seem to cope with comments correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
stigaview_static/main.py (3)
20-28: Exception chaining has been properly addressed.The exception handling in this function correctly uses
from Noneto suppress the original exception context, which follows Python best practices for exception chaining.
133-140: Good improvements to path handling and error logging.The use of
pathlib.Pathand structured logging for errors are good improvements. However, theexit(4)call should besys.exit(4)as noted in previous reviews.
141-147: SRG merging logic improvements noted in previous reviews.The progress bar integration here is good. However, the data structure initialization and dictionary key checking improvements were already identified in previous reviews.
🧹 Nitpick comments (5)
Makefile (1)
16-16: Add missing targets to .PHONY declaration.The
.PHONYdeclaration should include thealltarget to be complete and follow best practices.-.PHONY: clean build copy_assets minify_static sitemap +.PHONY: all clean build copy_assets minify_static sitemappublic_html/static/css/style.css (1)
83-93: Remove duplicate .btn:focus rule.The
.btn:focusrule is defined twice with identical properties. Remove the duplicate to avoid confusion and maintain clean code.-.btn:focus { - outline: 2px solid transparent; - outline-offset: 2px; -} - .btn:focus { outline: 2px solid transparent; outline-offset: 2px; /* Add a focus ring using box-shadow for better visibility */ box-shadow: 0 0 0 3px rgba(59, 130, 246, 0.5); }utils/add_to_search.py (1)
60-60: Consider using list literal instead of list() constructor.For better readability and slight performance improvement, use the list literal syntax.
-docs = list() +docs = []stigaview_static/json_output.py (1)
12-12: Use dict literal for better readability.Consider using the dict literal syntax for improved readability.
-products_list: Dict[str, str] = dict() +products_list: Dict[str, str] = {}stigaview_static/main.py (1)
115-122: Consider handling missing product paths consistently.The function only counts files for existing product paths, but
process_productswill error out when it encounters missing paths. This inconsistency could make the progress bar inaccurate.Consider either counting all expected files (and handling missing paths gracefully) or failing early if required paths don't exist.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.editorconfig(1 hunks).gitignore(1 hunks).idea/misc.xml(1 hunks).idea/stigaview-static.iml(1 hunks)Makefile(1 hunks)public_html/static/css/style.css(2 hunks)requirements.txt(1 hunks)setup.cfg(1 hunks)stigaview.toml(1 hunks)stigaview_static/html_output.py(6 hunks)stigaview_static/import_stig.py(1 hunks)stigaview_static/json_output.py(1 hunks)stigaview_static/main.py(3 hunks)stigaview_static/models.py(4 hunks)stigaview_static/utils.py(2 hunks)templates/base.html(1 hunks)templates/stig.html(1 hunks)utils/add_to_search.py(1 hunks)utils/minify.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- templates/stig.html
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (11)
- requirements.txt
- stigaview.toml
- stigaview_static/import_stig.py
- .idea/stigaview-static.iml
- .idea/misc.xml
- stigaview_static/utils.py
- templates/base.html
- setup.cfg
- stigaview_static/html_output.py
- .editorconfig
- utils/minify.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
stigaview_static/json_output.py (1)
stigaview_static/models.py (5)
Product(94-138)short_version(77-78)Control(22-66)search_primary_key(48-49)to_search_json(51-66)
🪛 checkmake (0.2.2)
Makefile
[warning] 16-16: Missing required phony target "all"
(minphony)
[warning] 16-16: Missing required phony target "test"
(minphony)
[warning] 18-18: Target "all" should be declared PHONY.
(phonydeclared)
🪛 Pylint (3.3.7)
stigaview_static/json_output.py
[refactor] 12-12: Consider using '{}' instead of a call to 'dict'.
(R1735)
stigaview_static/main.py
[refactor] 98-98: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 129-129: Consider using [] instead of list()
(R1734)
[refactor] 130-130: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 139-139: Consider using 'sys.exit' instead
(R1722)
stigaview_static/models.py
[refactor] 141-141: Too few public methods (0/2)
(R0903)
[refactor] 147-147: Too few public methods (0/2)
(R0903)
utils/add_to_search.py
[refactor] 60-60: Consider using [] instead of list()
(R1734)
🪛 Ruff (0.11.9)
stigaview_static/main.py
142-142: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (19)
Makefile (2)
28-30: LGTM! Good addition of the minify_static target.The new
minify_statictarget properly integrates with the build pipeline and uses the consistent$(OUT)variable.
36-37: LGTM! Clean target improves build hygiene.The
cleantarget is a useful addition that follows the standard pattern and uses the$(OUT)variable consistently.public_html/static/css/style.css (2)
95-175: LGTM! Well-structured modal implementation.The modal CSS implementation follows good practices with proper z-index layering, flexbox centering, and responsive design considerations.
325-471: Excellent dark mode implementation.The dark mode styles are comprehensive and provide good contrast ratios for accessibility. The color choices maintain readability while providing a cohesive dark theme experience.
utils/add_to_search.py (1)
54-66: LGTM! Proper Meilisearch integration with error handling.The implementation correctly:
- Creates the index if it doesn't exist
- Uses batch processing for efficient document addition
- Waits for indexing tasks to complete
- Provides appropriate error handling
stigaview_static/models.py (4)
47-49: LGTM! Well-designed search primary key.The
search_primary_keyproperty creates a unique, descriptive identifier that combines product, STIG version, and control ID. This will work well for search indexing.
51-66: Excellent search JSON serialization method.The
to_search_jsonmethod provides comprehensive metadata for search functionality including all the key fields needed for effective searching and filtering. The structure is well-organized and includes proper date formatting.
141-151: LGTM! Appropriate configuration models.The new
ProductConfigandStigAViewConfigclasses provide proper typing for configuration data and will improve type safety throughout the application.
106-107: Good improvement using structured logging.Replacing direct stderr writes with
logging.erroris a best practice that provides better logging control and consistency.stigaview_static/json_output.py (2)
10-21: LGTM! Well-structured product mapping generation.The function correctly generates the JSON mappings needed for the search frontend:
- Product to STIG version mapping for filters
- Product short name to full name mapping for display
- Proper sorting of STIGs for consistent output
24-29: LGTM! Efficient control JSON generation.The function properly:
- Creates the output directory structure as needed
- Uses the control's search primary key for consistent file naming
- Leverages the model's
to_search_json()method for data serializationstigaview_static/main.py (8)
3-3: LGTM! Imports support new functionality well.The addition of
logging,tqdm, andjson_outputimports properly support the structured logging, progress reporting, and JSON output functionality introduced in this refactor.Also applies to: 9-9, 11-11
14-17: Excellent refactoring with proper logging.The extraction of model preparation into a dedicated function with structured logging is a good improvement. Using
model_rebuild()instead ofupdate_forward_refs()suggests adoption of a newer API.
48-50: Good addition of configurable logging.The log level argument with proper validation enhances usability by allowing users to control logging verbosity at runtime.
55-55: Excellent improvements with proper logging and error handling.The structured logging, environment variable setting, and use of
sys.exit()are all good improvements. The environment variable will help other parts of the system locate the config file.Also applies to: 57-59, 63-63
66-73: Excellent modernization of the main function.The structured logging setup, JSON output generation, and proper timing with logging instead of print statements are all significant improvements that enhance both functionality and maintainability.
Also applies to: 81-83
86-89: Good refactoring to extract configuration loading.The extraction of product configuration loading into a dedicated function improves code organization and reusability.
93-93: Good improvements with progress tracking and cleaner code.The progress bar integration, use of the extracted config function, and cleaner SRG merging with
srgs.update()are all excellent improvements.Also applies to: 96-96, 108-108, 110-111
131-132: Excellent progress bar integration.The addition of total file counting and progress bar initialization significantly improves user experience by providing clear feedback during processing.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
utils/add_to_search.py (1)
31-33: Fix incorrect error message for SEARCH_HOST validation.The error message mentions the wrong environment variable name. It should reference
SEARCH_HOSTsince that's what's being checked.if not SEARCH_HOST: - print("Please set the environment variable SEARCH_MASTER_KEY", file=sys.stderr) + print("Please set the environment variable SEARCH_HOST", file=sys.stderr) sys.exit(1)public_html/static/js/search.js (1)
1-6: Critical security issue: Hardcoded API key exposure.The Meilisearch API key is still hardcoded in client-side JavaScript, making it publicly accessible and potentially exposing your search infrastructure to abuse.
🧹 Nitpick comments (10)
Makefile (1)
16-16: Add missing phony declarations.The static analysis correctly identifies that
allshould be declared as a phony target for consistency with Make best practices.-.PHONY: clean build copy_assets minify_static sitemap +.PHONY: all clean build copy_assets minify_static sitemaputils/add_to_search.py (1)
60-62: Consider using list comprehension for better performance.The static analysis correctly suggests using
[]instead oflist()for better performance and readability.- docs = list() + docs = []stigaview_static/json_output.py (2)
12-12: Use dictionary literal instead of dict() constructor.Following Python best practices, use
{}instead ofdict()for better readability and slight performance improvement.- products_list: Dict[str, str] = dict() + products_list: Dict[str, str] = {}
19-21: Consider using proper JSON indentation for readability.Using
indent=0produces compact but potentially hard-to-read JSON. Consider usingindent=Nonefor maximum compactness orindent=2for better readability in development.- f.write(json.dumps(product_stig_map, indent=0)) + f.write(json.dumps(product_stig_map))- f.write(json.dumps(products_list, indent=0)) + f.write(json.dumps(products_list))public_html/static/js/search.js (1)
287-292: Simplify Promise.all usage.The variable assignment can be streamlined for better readability.
- const productStigPromise = loadProductStigMap(); - const productPromise = loadProductMap(); - - const [productMap, productStigMap] = await Promise.all( - [productPromise, productStigPromise] - ) + const [productMap, productStigMap] = await Promise.all([ + loadProductMap(), + loadProductStigMap() + ]);stigaview_static/main.py (5)
98-98: Use dictionary literal instead of dict() constructor.Following Python best practices for consistency with the codebase.
- srgs: dict[str, list[models.Control]] = dict() + srgs: dict[str, list[models.Control]] = {}
129-130: Use modern literal syntax for better readability.Replace constructor calls with literal syntax for improved readability and slight performance benefits.
- result = list() - srgs_dict = dict() + result = [] + srgs_dict = {}
139-139: Use sys.exit() instead of bare exit().Following Python best practices for explicit imports and clearer intent.
- exit(4) + sys.exit(4)
142-142: Simplify dictionary key existence check.Remove unnecessary
.keys()call for cleaner and more efficient code.- if srg not in srgs_dict.keys(): + if srg not in srgs_dict:
131-147: Consider using collections.defaultdict for cleaner code.The current manual key checking and list appending can be simplified using
defaultdict(list).+import collections ... - srgs_dict = {} + srgs_dict = collections.defaultdict(list) total_files = _get_total_files(input_path, products) with tqdm(total=total_files, desc="Processing all STIG files", unit="file") as pbar: for product in products: product_path = pathlib.Path(input_path) / product.short_name if not product_path.exists(): logging.error( f"Unable to find path for {product.short_name} at {product_path}" ) sys.exit(4) product, srgs = process_product(product, product_path, pbar) for srg, controls in srgs.items(): - if srg not in srgs_dict: - srgs_dict[srg] = controls - else: - for control in controls: - srgs_dict[srg].append(control) + srgs_dict[srg].extend(controls) result.append(product)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.editorconfig(1 hunks).gitignore(1 hunks).idea/misc.xml(1 hunks).idea/stigaview-static.iml(1 hunks)Makefile(1 hunks)public_html/static/css/style.css(2 hunks)public_html/static/js/search.js(1 hunks)requirements.txt(1 hunks)setup.cfg(1 hunks)stigaview.toml(1 hunks)stigaview_static/html_output.py(6 hunks)stigaview_static/import_stig.py(1 hunks)stigaview_static/json_output.py(1 hunks)stigaview_static/main.py(3 hunks)stigaview_static/models.py(4 hunks)stigaview_static/utils.py(2 hunks)templates/base.html(1 hunks)templates/search.html(1 hunks)templates/stig.html(1 hunks)utils/add_to_search.py(1 hunks)utils/minify.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- stigaview.toml
- templates/stig.html
- .gitignore
- .idea/misc.xml
- setup.cfg
- .idea/stigaview-static.iml
- stigaview_static/import_stig.py
- stigaview_static/utils.py
- .editorconfig
- requirements.txt
- templates/base.html
- utils/minify.py
- public_html/static/css/style.css
🧰 Additional context used
🧬 Code Graph Analysis (1)
stigaview_static/json_output.py (1)
stigaview_static/models.py (5)
Product(94-138)short_version(77-78)Control(22-66)search_primary_key(48-49)to_search_json(51-66)
🪛 Gitleaks (8.26.0)
public_html/static/js/search.js
3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Pylint (3.3.7)
stigaview_static/main.py
[refactor] 98-98: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 129-129: Consider using [] instead of list()
(R1734)
[refactor] 130-130: Consider using '{}' instead of a call to 'dict'.
(R1735)
[refactor] 139-139: Consider using 'sys.exit' instead
(R1722)
stigaview_static/json_output.py
[refactor] 12-12: Consider using '{}' instead of a call to 'dict'.
(R1735)
stigaview_static/models.py
[refactor] 141-141: Too few public methods (0/2)
(R0903)
[refactor] 147-147: Too few public methods (0/2)
(R0903)
utils/add_to_search.py
[refactor] 60-60: Consider using [] instead of list()
(R1734)
🪛 Ruff (0.11.9)
stigaview_static/main.py
142-142: Use key not in dict instead of key not in dict.keys()
Remove .keys()
(SIM118)
🪛 checkmake (0.2.2)
Makefile
[warning] 16-16: Missing required phony target "all"
(minphony)
[warning] 16-16: Missing required phony target "test"
(minphony)
[warning] 18-18: Target "all" should be declared PHONY.
(phonydeclared)
🪛 HTMLHint (1.5.0)
templates/search.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (21)
Makefile (2)
28-30: Good addition of minification target.The new
minify_statictarget integrates well with the build process and uses consistent variable naming.
36-37: Useful clean target addition.The clean target provides a standard way to remove build artifacts and follows Make conventions.
templates/search.html (3)
1-9: Excellent accessibility implementation.The modal structure includes proper ARIA attributes (
role="dialog",aria-modal="true",aria-labelledby,aria-controls) and semantic HTML elements that ensure screen reader compatibility.
10-25: Well-structured filter interface.The filter controls with progressive disclosure (STIG select disabled until product is chosen) provide a logical user experience. The "Clear Filters" button being initially hidden is a good UX pattern.
26-28: Proper live region for dynamic content.Using
aria-live="polite"on the search results container ensures that screen readers announce search results without being disruptive.stigaview_static/html_output.py (6)
1-12: Good addition of imports for enhanced functionality.The new imports support logging, multiprocessing, minification, progress tracking, and JSON output generation - all key improvements for the build process.
19-24: Excellent template context enhancement.Merging global configuration into template context enables conditional features like search, and HTML minification reduces output size. This is a solid improvement to the rendering pipeline.
34-34: Good integration of JSON output generation.Adding
render_json_controlcall during product rendering ensures search index data is generated alongside HTML content.
71-73: Clean multiprocessing adapter function.The
process_product_argsfunction properly unpacks arguments for the multiprocessing pool, following the standard pattern for pool.imap usage.
82-93: Excellent multiprocessing implementation.The combination of multiprocessing pool with tqdm progress bar provides both performance improvement and user feedback. Using
cpu_count()cores andimapwith progress tracking is a best practice implementation.
133-133: Good addition of progress tracking.Adding tqdm progress bar to SRG rendering provides consistent user feedback across all rendering operations.
utils/add_to_search.py (3)
10-11: Good security improvement using environment variables.The implementation correctly retrieves sensitive credentials from environment variables rather than hardcoding them, addressing the security concerns from previous reviews.
25-27: Clean chunking implementation.The generator function for chunking lists is well-implemented and will handle large document batches efficiently.
63-65: Robust batch processing with task waiting.The implementation properly handles batch uploads and waits for all indexing tasks to complete, ensuring data consistency.
stigaview_static/models.py (4)
47-49: Well-designed search primary key generation.The
search_primary_keyproperty creates a unique identifier combining product, STIG version, and control ID, ensuring proper indexing and retrieval.
51-66: Comprehensive search JSON representation.The
to_search_jsonmethod provides all necessary fields for search indexing including metadata, content, and relationships. The date formatting and structured output are well-implemented.
106-108: Good logging improvement.Replacing direct stderr writing with structured logging is consistent with the broader codebase improvements and provides better error handling.
141-151: Well-structured configuration models.The new Pydantic models
ProductConfigandStigAViewConfigprovide proper type safety and validation for configuration data. The static analysis warnings about too few public methods are false positives for Pydantic data models.stigaview_static/json_output.py (1)
24-29: LGTM! Clean implementation of JSON control rendering.The function correctly creates the directory structure and writes control JSON data using pathlib. The logic is sound and the error handling is implicit through pathlib operations.
public_html/static/js/search.js (2)
51-80: LGTM! Well-implemented focus trapping for accessibility.The focus trap implementation correctly handles Tab and Shift+Tab navigation within the modal, ensuring good accessibility compliance. The logic for wrapping focus between first and last focusable elements is sound.
83-93: Clean debounce implementation.The debounce function is correctly implemented with proper timeout handling and argument passing. This will help reduce API calls during rapid user input.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
utils/add_to_search.py (1)
31-36: Fix swapped error messages in environment variable validation.The error messages are incorrect for their respective validation checks.
Apply this diff to correct the error messages:
if not SEARCH_HOST: - print("Please set the environment variable SEARCH_MASTER_KEY", file=sys.stderr) + print("Please set the environment variable SEARCH_HOST", file=sys.stderr) sys.exit(1) if not MASTER_KEY: - print("Please set the environment variable SEARCH_HOST", file=sys.stderr) + print("Please set the environment variable SEARCH_MASTER_KEY", file=sys.stderr) sys.exit(1)public_html/static/js/search.js (1)
1-6: Critical security issue: Hardcoded API key exposure.The Meilisearch API key is hardcoded in client-side JavaScript, making it visible to all users and potentially exposing your search infrastructure to abuse.
Move the API key to server-side configuration and implement a secure authentication mechanism or use read-only tenant tokens.
🧹 Nitpick comments (4)
Makefile (2)
28-30:minify_statictarget lacks explicit dependency oncopy_assetsIf somebody runs
make minify_staticdirectly (or addsminify_staticelsewhere), the output directory may be empty becausecopy_assetshasn’t executed yet. To make the target self-contained:-minify_static: +minify_static: copy_assetsAlternatively, document in the README that
minify_staticassumes a populated$(OUT)directory.
16-16: Consider adding atestphony target
checkmakewarns that the conventionaltestphony target is missing. Even if the project has no automated tests yet, stubbing the target avoids the warning and sets expectations for future contributors:.PHONY: test test: @echo "No tests defined yet"utils/add_to_search.py (1)
60-60: Use list literal instead of constructor.Apply this diff for more pythonic code:
- docs = list() + docs = []stigaview_static/json_output.py (1)
12-12: Use dict literal instead of constructor.Apply this diff for more pythonic code:
- products_list: Dict[str, str] = dict() + products_list: Dict[str, str] = {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefile(1 hunks)public_html/static/js/search.js(1 hunks)stigaview_static/json_output.py(1 hunks)utils/add_to_search.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
stigaview_static/json_output.py (1)
stigaview_static/models.py (5)
Product(94-138)short_version(77-78)Control(22-66)search_primary_key(48-49)to_search_json(51-66)
🪛 checkmake (0.2.2)
Makefile
[warning] 16-16: Missing required phony target "test"
(minphony)
🪛 Gitleaks (8.26.0)
public_html/static/js/search.js
3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Pylint (3.3.7)
stigaview_static/json_output.py
[refactor] 12-12: Consider using '{}' instead of a call to 'dict'.
(R1735)
utils/add_to_search.py
[refactor] 60-60: Consider using [] instead of list()
(R1734)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (3)
utils/add_to_search.py (1)
14-70: LGTM! Well-structured script for search indexing.The overall approach is solid - proper argument parsing, path validation, batch processing, and task waiting. The script correctly integrates with the JSON output from the main application.
stigaview_static/json_output.py (1)
10-29: LGTM! Clean JSON serialization implementation.The functions properly serialize the model data to JSON format. The integration with the
ProductandControlmodels is correct, using appropriate properties likeshort_name,full_name,short_version, andto_search_json().public_html/static/js/search.js (1)
44-356: LGTM! Comprehensive search modal implementation.The search functionality is well-implemented with good UX practices:
- Proper focus management and accessibility
- Debounced search for performance
- Clean modal state management
- Filterable search with dynamic dropdowns
| client = meilisearch.Client(SEARCH_HOST, MASTER_KEY) | ||
| client.create_index("controls") | ||
| index = client.get_index("controls") | ||
| if not index: | ||
| print("No index found", file=sys.stderr) | ||
| sys.exit(1) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve index creation and retrieval error handling.
The current implementation has potential issues:
create_indexwill fail if the index already exists- The subsequent index existence check is redundant since
get_indexwould raise an exception
Apply this diff to handle existing indexes gracefully:
- client = meilisearch.Client(SEARCH_HOST, MASTER_KEY)
- client.create_index("controls")
- index = client.get_index("controls")
- if not index:
- print("No index found", file=sys.stderr)
- sys.exit(1)
+ client = meilisearch.Client(SEARCH_HOST, MASTER_KEY)
+ try:
+ index = client.get_index("controls")
+ except Exception:
+ # Index doesn't exist, create it
+ client.create_index("controls")
+ index = client.get_index("controls")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client = meilisearch.Client(SEARCH_HOST, MASTER_KEY) | |
| client.create_index("controls") | |
| index = client.get_index("controls") | |
| if not index: | |
| print("No index found", file=sys.stderr) | |
| sys.exit(1) | |
| client = meilisearch.Client(SEARCH_HOST, MASTER_KEY) | |
| try: | |
| index = client.get_index("controls") | |
| except Exception: | |
| # Index doesn't exist, create it | |
| client.create_index("controls") | |
| index = client.get_index("controls") |
🤖 Prompt for AI Agents
In utils/add_to_search.py around lines 54 to 59, the code calls create_index
without handling the case where the index already exists, which causes an error,
and then redundantly checks for the index existence after get_index, which
raises an exception if the index does not exist. To fix this, wrap the
create_index call in a try-except block to catch the exception for an existing
index and ignore it, then directly call get_index without checking its return
value, handling any exceptions if the index retrieval fails.
| if (selectedProduct) { | ||
| filterParts.push(`product = "${selectedProduct}"`); | ||
| } |
There was a problem hiding this comment.
Fix missing product field in search implementation.
The code references a product field that doesn't exist in the JSON structure. Looking at Control.to_search_json() in models.py, there's no product field in the returned data.
You need to either:
- Add a
productfield toControl.to_search_json()method, or - Extract the product name from the existing
idfield which contains{product}-{stig}-{control_id}
For option 1, update models.py:
def to_search_json(self) -> Dict[str, str | List[str]]:
return {
"id": self.search_primary_key,
+ "product": self.stig.product.short_name,
"title": self.title,
# ... rest of fields
}For option 2, extract from the ID in JavaScript:
- const product = hit.product;
+ const product = hit.id.split('-')[0]; // Extract product from idAlso applies to: 167-169
🤖 Prompt for AI Agents
In public_html/static/js/search.js at lines 99-101 and also lines 167-169, the
code filters by a `product` field that does not exist in the JSON data returned
by Control.to_search_json(). To fix this, either update the
Control.to_search_json() method in models.py to include a `product` field, or
modify the JavaScript to extract the product name from the existing `id` field
by parsing the string before the first hyphen. Choose one approach and implement
it consistently in both places.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
utils/add_to_search.py (1)
54-59: Fix index creation and retrieval error handling.The current implementation still has the issues identified in previous reviews:
create_indexwill fail if the index already exists- The subsequent index existence check is redundant since
get_indexwould raise an exceptionApply this diff to handle existing indexes gracefully:
- client = meilisearch.Client(SEARCH_HOST, MASTER_KEY) - client.create_index("controls") - index = client.get_index("controls") - if not index: - print("No index found", file=sys.stderr) - sys.exit(1) + client = meilisearch.Client(SEARCH_HOST, MASTER_KEY) + try: + index = client.get_index("controls") + except Exception: + # Index doesn't exist, create it + client.create_index("controls") + index = client.get_index("controls")
🧹 Nitpick comments (1)
utils/add_to_search.py (1)
60-60: Minor style improvement: use list literal.Consider using the more Pythonic list literal syntax.
- docs = list() + docs = []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Makefile(1 hunks)stigaview_static/models.py(4 hunks)utils/add_to_search.py(1 hunks)
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 16-16: Missing required phony target "test"
(minphony)
🪛 Pylint (3.3.7)
stigaview_static/models.py
[refactor] 142-142: Too few public methods (0/2)
(R0903)
[refactor] 148-148: Too few public methods (0/2)
(R0903)
utils/add_to_search.py
[refactor] 60-60: Consider using [] instead of list()
(R1734)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy
🔇 Additional comments (7)
Makefile (2)
16-18: LGTM! Previous issue has been resolved.The
minify_statictarget is now properly included in thealldependency chain and correctly declared in.PHONY. This addresses the previous concern about skipping minification in the default build process.
36-37: Good addition of clean target.The new
cleantarget provides a convenient way to remove the output directory, following standard Makefile conventions.stigaview_static/models.py (4)
47-49: Well-designed search primary key.The composite key format using product, STIG version, and DISA STIG ID ensures uniqueness across the search index while maintaining readability.
51-67: Comprehensive search JSON serialization.The
to_search_jsonmethod includes all relevant fields for search functionality with proper formatting, including date serialization and URL path generation.
107-107: Improved error handling with structured logging.Replacing direct stderr output with proper logging is a good improvement that aligns with the application's logging strategy.
142-152: Appropriate Pydantic configuration models.These models provide proper type validation for configuration data. The static analysis warnings about too few public methods are false positives - configuration models typically don't need many public methods.
utils/add_to_search.py (1)
30-36: Good environment variable validation.The script properly validates required environment variables and provides clear error messages when they're missing.
1 participant
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores