Nettacker icon indicating copy to clipboard operation
Nettacker copied to clipboard

📝 Add docstrings to `fix-windows-path-handling`

Open coderabbitai[bot] opened this issue 3 weeks ago • 6 comments

Docstrings generation was requested by @subhash-0000.

  • https://github.com/OWASP/Nettacker/pull/1173#issuecomment-3602915683

The following files were modified:

  • nettacker/api/engine.py
  • nettacker/core/arg_parser.py
  • nettacker/core/messages.py
  • nettacker/core/template.py
  • nettacker/core/utils/common.py
ℹ️ Note

CodeRabbit cannot perform edits on its own pull requests yet.

coderabbitai[bot] avatar Dec 02 '25 16:12 coderabbitai[bot]

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling for scan result retrieval with proper HTTP status codes when parameters are missing
    • Enhanced filename handling for downloaded files to ensure correct attachment names
  • Improvements

    • More robust language and module discovery mechanisms
    • Added proper character encoding support for file operations

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

This pull request refactors path handling across multiple modules to use path components and file stems instead of string splitting, adds explicit UTF-8 encoding to file operations, improves error handling in the API layer, and enhances documentation.

Changes

Cohort / File(s) Summary
Path handling refactoring
nettacker/core/arg_parser.py, nettacker/core/messages.py
Updated graph, language, and module loading functions to derive identifiers from path components and file stems rather than string splitting. load_modules return type changes from list to dict with <library>_<category> keys mapping to info dicts or None; adds "..." placeholder when limit is reached.
API improvements
nettacker/api/engine.py
Added parameter validation for "id" field in get_result_content, returning HTTP 400 on missing ID; improved error handling for database retrieval failures (HTTP 500). Updated docstring and response filename handling to use os.path.basename.
File encoding and documentation
nettacker/core/template.py
Introduced explicit UTF-8 encoding when opening YAML module files in TemplateLoader.open; added detailed docstrings for open and format methods.
Documentation improvements
nettacker/core/utils/common.py
Added descriptive docstrings clarifying that "/" is a key delimiter in re_address_repeaters_key_name and documenting the function's concatenation behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • load_modules return type change from list to dict with structured keys and limit-based truncation logic requires verification of all callsites
  • Module discovery and severity/description caching behavior should be validated
  • API parameter validation paths and error response codes need verification against specifications

Suggested reviewers

  • arkid15r
  • securestep9

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title references 'fix-windows-path-handling' branch but the actual changes are docstring additions across multiple core and API modules, not windows path handling fixes. Update title to accurately reflect that docstrings were added to multiple core modules, e.g., '📝 Add docstrings to core modules and API engine'
Docstring Coverage ⚠️ Warning Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains that docstrings were requested and lists all modified files, directly relating to the changeset.

[!TIP]

✨ Issue Enrichment is now available for GitHub issues!

CodeRabbit can now help you manage issues more effectively:

  • Duplicate Detection — Identify similar or duplicate issues
  • Related Issues & PRs — Find relevant issues and PR's from your repository
  • Suggested Assignees — Find the best person to work on the issue
  • Implementation Planning — Generate detailed coding plans for engineers and agents
Disable automatic issue enrichment

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

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

coderabbitai[bot] avatar Dec 02 '25 16:12 coderabbitai[bot]

Hi, um so I checked the PR locally and the docstring changes look good. but the CI is failing on pre-commit check though, it might just need a re run or quick formatting fix. Let me know if you want me to take a look.

deekshithaby avatar Dec 09 '25 20:12 deekshithaby

@coderabbitai review

subhash-0000 avatar Dec 10 '25 02:12 subhash-0000

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar Dec 10 '25 02:12 coderabbitai[bot]

@coderabbitai review

subhash-0000 avatar Dec 10 '25 02:12 subhash-0000

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar Dec 10 '25 02:12 coderabbitai[bot]

Thanks, this makes sense. These look like straightforward formatting fixes (exceeding line length + indentation). If you’d like, I can open a small follow-up PR with these changes to unblock CI.

deekshithaby avatar Dec 10 '25 17:12 deekshithaby

@deekshithaby Thanks for offering! I've already pushed the formatting fixes in my latest commit (4ba29e75). The docstrings are properly wrapped and indented now. Could you try re-running the CI checks to see if they pass?

subhash-0000 avatar Dec 11 '25 04:12 subhash-0000

Thanks for the quick update! I don't have permissions to re-run the CI checks on this repository, so I can't trigger the jobs myself. Once they're re-run from your side, I'm happy to verify the results!

deekshithaby avatar Dec 11 '25 18:12 deekshithaby

@deekshithaby Thanks! Just to clarify - this PR #1174 was created by CodeRabbit. My actual Windows path handling fixes are in PR #1173.

That PR is currently awaiting workflow approval from a maintainer to run CI checks. If you'd like to review the changes, please check out #1173 instead

subhash-0000 avatar Dec 12 '25 11:12 subhash-0000