Conversation
📝 WalkthroughWalkthroughThe dependency constraint for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
soimkim
changed the title
|
Ignore
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 36: The dependency line "fosslight_binary>=5.1.22" is unsatisfiable
because 5.1.22 does not exist; change the pyproject dependency to either (a)
gate the requirement with an environment marker like 'fosslight_binary>=5.1.21;
python_version < "3.13"' so only Python <3.13 installs fosslight_binary, (b) pin
to the latest available release 'fosslight_binary==5.1.21' combined with an
environment marker to avoid breaking advertised Python 3.13/3.14 support, or (c)
update the project's python_requires to <3.13 to match the dependency—apply the
chosen fix by editing the dependency string in the pyproject.toml where
"fosslight_binary>=5.1.22" appears.
In `@src/fosslight_source/run_scancode.py`:
- Around line 159-170: The os.walk over path_to_scan still traverses directories
that were previously marked excluded, causing unnecessary check_binary() calls;
modify the loop in run_scancode.py where for root, _, files in
os.walk(path_to_scan): by skipping any root that falls under the existing
excluded paths (path_to_exclude) before iterating files — e.g., normalize root
and compare against abs_path_to_scan + each path_to_exclude entry (or maintain a
set of excluded absolute prefixes) and continue when
root.startswith(excluded_prefix); keep the rest of the logic (check_binary,
building rel_path/rel_norm, appending to excluded_files, logger.debug) unchanged
so binary checks are only done for non-excluded directories.
- Around line 159-169: The parameter excluded_files currently uses a mutable
default (excluded_files: list = [])—change the function signature to use
excluded_files: Optional[list] = None (or excluded_files=None) and inside the
function initialize it with excluded_files = [] if excluded_files is None before
the os.walk loop; ensure all existing append usages (e.g., where rel_norm is
appended to excluded_files in the loop) operate on this freshly allocated list
so callers get a new list each invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c9d1424-f927-4027-a622-ce473f8d3f8f
📒 Files selected for processing (2)
pyproject.tomlsrc/fosslight_source/run_scancode.py
| for root, _, files in os.walk(path_to_scan): | ||
| for name in files: | ||
| full_path = os.path.join(root, name) | ||
| try: | ||
| if not check_binary(full_path, True): | ||
| continue | ||
| except Exception: | ||
| continue | ||
| rel_path = os.path.relpath(full_path, abs_path_to_scan) | ||
| rel_norm = os.path.normpath(rel_path).replace("\\", "/") | ||
| excluded_files.append(rel_norm) | ||
| logger.debug(f"Excluded binary from scancode: {rel_norm}") |
There was a problem hiding this comment.
Skip path_to_exclude during the binary pre-pass.
The new os.walk(path_to_scan) still descends into trees that were already excluded above, so large ignored directories still pay the full check_binary() cost. On repos that exclude build/vendor output, this can easily become the dominant runtime of the scan.
🧰 Tools
🪛 Ruff (0.15.7)
[error] 165-166: try-except-continue detected, consider logging the exception
(S112)
[warning] 165-165: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fosslight_source/run_scancode.py` around lines 159 - 170, The os.walk
over path_to_scan still traverses directories that were previously marked
excluded, causing unnecessary check_binary() calls; modify the loop in
run_scancode.py where for root, _, files in os.walk(path_to_scan): by skipping
any root that falls under the existing excluded paths (path_to_exclude) before
iterating files — e.g., normalize root and compare against abs_path_to_scan +
each path_to_exclude entry (or maintain a set of excluded absolute prefixes) and
continue when root.startswith(excluded_prefix); keep the rest of the logic
(check_binary, building rel_path/rel_norm, appending to excluded_files,
logger.debug) unchanged so binary checks are only done for non-excluded
directories.
| for root, _, files in os.walk(path_to_scan): | ||
| for name in files: | ||
| full_path = os.path.join(root, name) | ||
| try: | ||
| if not check_binary(full_path, True): | ||
| continue | ||
| except Exception: | ||
| continue | ||
| rel_path = os.path.relpath(full_path, abs_path_to_scan) | ||
| rel_norm = os.path.normpath(rel_path).replace("\\", "/") | ||
| excluded_files.append(rel_norm) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -70 src/fosslight_source/run_scancode.py | tail -20Repository: fosslight/fosslight_source_scanner
Length of output: 832
🏁 Script executed:
sed -n '50,85p' src/fosslight_source/run_scancode.pyRepository: fosslight/fosslight_source_scanner
Length of output: 1436
🏁 Script executed:
sed -n '159,175p' src/fosslight_source/run_scancode.pyRepository: fosslight/fosslight_source_scanner
Length of output: 977
🏁 Script executed:
sed -n '1,30p' src/fosslight_source/run_scancode.pyRepository: fosslight/fosslight_source_scanner
Length of output: 1022
🏁 Script executed:
rg "run_scan\(" -A 2 -B 2 --type pyRepository: fosslight/fosslight_source_scanner
Length of output: 4170
🏁 Script executed:
fd -e py -type f | xargs grep -l "run_scan" | head -10Repository: fosslight/fosslight_source_scanner
Length of output: 248
🏁 Script executed:
rg "run_scan\(" src/ tests/ -A 3 --type py | grep -A 3 "run_scan("Repository: fosslight/fosslight_source_scanner
Length of output: 3763
🏁 Script executed:
sed -n '1,300p' src/fosslight_source/cli.py | grep -A 10 "run_scan"Repository: fosslight/fosslight_source_scanner
Length of output: 1101
🏁 Script executed:
grep -n "excluded_files" src/fosslight_source/run_scancode.pyRepository: fosslight/fosslight_source_scanner
Length of output: 344
🏁 Script executed:
sed -n '100,160p' src/fosslight_source/cli.py | grep -A 15 "run_scan"Repository: fosslight/fosslight_source_scanner
Length of output: 769
🏁 Script executed:
sed -n '135,155p' src/fosslight_source/cli.pyRepository: fosslight/fosslight_source_scanner
Length of output: 940
🏁 Script executed:
grep -n "def run_scanners" src/fosslight_source/*.pyRepository: fosslight/fosslight_source_scanner
Length of output: 127
🏁 Script executed:
sed -n '90,150p' src/fosslight_source/cli.pyRepository: fosslight/fosslight_source_scanner
Length of output: 2356
🏁 Script executed:
sed -n '389,500p' src/fosslight_source/cli.pyRepository: fosslight/fosslight_source_scanner
Length of output: 5896
🏁 Script executed:
sed -n '389,600p' src/fosslight_source/cli.py | head -100Repository: fosslight/fosslight_source_scanner
Length of output: 5506
Fix mutable default argument pattern for excluded_files.
The function signature defines excluded_files: list = [] as a default parameter. While all current callers explicitly provide this argument, the mutable default pattern is a Python anti-pattern that could cause issues if calling conventions change. Replace the default with None and allocate a fresh list per invocation before appending (line 169).
🧰 Tools
🪛 Ruff (0.15.7)
[error] 165-166: try-except-continue detected, consider logging the exception
(S112)
[warning] 165-165: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fosslight_source/run_scancode.py` around lines 159 - 169, The parameter
excluded_files currently uses a mutable default (excluded_files: list =
[])—change the function signature to use excluded_files: Optional[list] = None
(or excluded_files=None) and inside the function initialize it with
excluded_files = [] if excluded_files is None before the os.walk loop; ensure
all existing append usages (e.g., where rel_norm is appended to excluded_files
in the loop) operate on this freshly allocated list so callers get a new list
each invocation.
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThe dependency constraint for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Description
Enhanced binary file detection and exclusion during source code scans for improved accuracy