feat: CLI with config file and env var support#9
Conversation
📝 WalkthroughWalkthroughThis PR transforms ChangesCore Package Refactoring and Modularization
🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Replace hardcoded Python scripts with an installable package that supports YAML config files, SLSKD_* environment variables, and CLI flags — users no longer need to edit source code to configure the tool. Key changes: - Click CLI with search/rename subcommands - Config priority: CLI > env vars > config.yml > defaults - Recursive directory scanning (--recursive flag) - Proper Python package (pyproject.toml, src layout) - Docker support (multi-stage Dockerfile) - Backwards-compat shims for main.py / rename-files.py Closes #8
GeiserX
force-pushed
the
feat/cli-config-refactor
branch
from
60e7877 to
80acb1a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9 +/- ##
===========================================
- Coverage 98.41% 86.24% -12.17%
===========================================
Files 2 6 +4
Lines 126 269 +143
===========================================
+ Hits 124 232 +108
- Misses 2 37 +35
🚀 New features to boost your workflow:
|
- Fix mutagen.File() returning None crash for non-audio files - Fix ZeroDivisionError when num_threads > song count - Eliminate thread-safety race on shared unfound_songs list (per-thread lists merged after join) - Remove dead format_filter parameter - Accept Path in list_files_with_duration signature - Remove unused imports (sys, Optional) - Add config.yml to .gitignore (prevents committing API keys) - Pin requests>=2.31 (CVE-2023-32681) - Remove duplicate dataclass defaults (DEFAULTS dict is single source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
.coderabbit.yaml (1)
44-45: ⚡ Quick winRemove duplicate
filePatternsentry.
"CLAUDE.md"is listed twice. Keep a single entry to avoid redundant matching and config noise.Proposed fix
code_guidelines: enabled: true filePatterns: - "CLAUDE.md" - - "CLAUDE.md"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit.yaml around lines 44 - 45, The filePatterns array in .coderabbit.yaml contains a duplicate entry "CLAUDE.md"; remove the redundant duplicate so only one "CLAUDE.md" remains in the filePatterns list to avoid duplicate matching and clutter the config. Locate the filePatterns block and delete one of the two "CLAUDE.md" entries (keep a single occurrence) then save the YAML ensuring valid syntax is preserved.src/slskd_transform/config.py (1)
96-101: ⚡ Quick winConsider adding validation for type conversion failures.
If a user provides non-numeric values for fields like
num_threadsorduration_tolerance(via config file or env vars), theint()conversion on line 99 will raise aValueErrorwith a generic message. Adding try/except with a clear error message would improve UX.♻️ Proposed validation improvement
# Convert types for field in _BOOL_FIELDS: merged[field] = _parse_bool(merged[field]) for field in _INT_FIELDS: - merged[field] = int(merged[field]) # type: ignore[arg-type] + try: + merged[field] = int(merged[field]) # type: ignore[arg-type] + except (ValueError, TypeError) as e: + raise ValueError(f"Invalid value for {field}: {merged[field]!r} (must be an integer)") from e for field in _PATH_FIELDS: merged[field] = Path(str(merged[field]))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slskd_transform/config.py` around lines 96 - 101, The loop converting fields in _INT_FIELDS currently calls int(merged[field]) directly and will raise a raw ValueError on invalid input; wrap that conversion in a try/except inside the same loop (where merged is built) to catch ValueError/TypeError, and re-raise or log a clearer error that includes the field name (from _INT_FIELDS) and the offending value (merged[field]) and guidance (e.g., expected integer), so callers can see which config key failed (reference the merged dict and the _INT_FIELDS iteration that performs the int() conversion).tests/test_rename.py (1)
66-67: 💤 Low valueOptional: Inconsistent Path usage.
Line 67 uses
os.path.joinwhile the rest of the test suite usesPath. Minor style inconsistency.♻️ Proposed consistency fix
- expected = os.path.join(dest, "My Artist - My Song.flac") + expected = str(Path(dest) / "My Artist - My Song.flac") mock_shutil.move.assert_called_once_with(str(Path("/source/file.flac")), expected)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_rename.py` around lines 66 - 67, The test builds an expected path using os.path.join which is inconsistent with the project's use of pathlib.Path; change the construction of expected to use Path (e.g., Path(dest) / "My Artist - My Song.flac") so the assertion mock_shutil.move.assert_called_once_with(str(Path("/source/file.flac")), expected) matches the rest of the suite — update the variable `expected` in tests/test_rename.py accordingly and ensure types passed to mock_shutil.move remain strings if other tests expect that.README.md (2)
160-160: ⚡ Quick winConsider using version variables or
:latesttag in Docker examples.The hardcoded
2.0.0version will require manual updates with each release. Consider using:latestor a substitution pattern.🐳 Suggested approach
Option 1: Use
:latestfor examples:- ghcr.io/geiserx/slskd-transform:2.0.0 search --recursive + ghcr.io/geiserx/slskd-transform:latest search --recursiveOption 2: Add a note directing users to check releases:
# Use the latest version from GitHub releases docker run ... ghcr.io/geiserx/slskd-transform:latestAlso applies to: 175-175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 160, Replace the hardcoded image tag "ghcr.io/geiserx/slskd-transform:2.0.0" in the README examples with a maintainable pattern (for example use ":latest" or a placeholder variable like "${VERSION}"/`<version>`), and update all occurrences (e.g., the "ghcr.io/geiserx/slskd-transform:2.0.0 search --recursive" example and the other instance at the same section) so examples either point to :latest or instruct users to substitute/check the current release; keep the example invocation text unchanged except for replacing the version token and add a short note directing users to check releases if you choose the placeholder approach.
74-150: ⚡ Quick winAdd language specifiers to fenced code blocks.
Lines 74, 121, 138, and 147 have fenced code blocks without language identifiers, which reduces rendering quality in some Markdown processors.
📝 Suggested language specifiers
Line 74 (priority order):
-``` +```text CLI flags > Environment variables > Config file > DefaultsLines 121, 138, 147 (CLI help output): ```diff -``` +```text slskd-transform [OPTIONS] COMMAND [ARGS]...</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@README.mdaround lines 74 - 150, Add explicit language specifiers (use
"text") to the unlabelled fenced code blocks in README.md: the header block
containing "CLI flags > Environment variables > Config file > Defaults"
and each CLI help/output block starting with "slskd-transform [OPTIONS] COMMAND
[ARGS]..." so Markdown renderers correctly treat them as plain text; update
those three/ four fenced blocks to usetext instead ofto improve
rendering.</details> </blockquote></details> <details> <summary>src/slskd_transform/cli.py (1)</summary><blockquote> `1-1`: **Update Click to 8.3.3.** Click 8.1.0 is outdated. The latest stable version is 8.3.3. No security advisories found for Click, but updating is recommended to benefit from recent improvements and fixes. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slskd_transform/cli.py` at line 1, The project imports Click (seen as "import click") but the dependency is pinned to an older release; update the Click dependency to 8.3.3 in your project's dependency manifests (requirements.txt, pyproject.toml, setup.cfg, or Pipfile) and run a fresh install; then run the CLI entrypoints and tests that reference click (any functions that call click.command, click.group, click.option, etc.) to verify there are no breaking API changes and update any call signatures if tests fail. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@Dockerfile:
- Around line 14-17: The mkdir step fails because WORKDIR /app is root-owned and
the Dockerfile switches to USER appuser before creating directories; either
create the dirs while still root or change ownership of /app to appuser. Move
the RUN mkdir -p /app/music /app/downloads /app/organized to before USER appuser
(i.e., run as root after WORKDIR /app), or add a root RUN chown -R
appuser:appuser /app before switching to USER appuser so appuser can
create/write the directories.In
@pyproject.toml:
- Around line 15-21: Replace the vulnerable requests spec in the dependencies
list with a safe minimum version: update the "requests>=2.28" entry in the
pyproject.toml dependencies array to "requests>=2.33.0" so the project installs
a release that contains fixes for the cited CVEs; ensure the exact string
"requests>=2.28" is changed to "requests>=2.33.0" in the dependencies block.In
@src/slskd_transform/rename.py:
- Around line 31-37: The loop in move_and_rename_flac_files currently calls
shutil.move without handling failures; wrap the move (the call to shutil.move
inside move_and_rename_flac_files) in a try-except that catches Exception, logs
or prints a clear error including the source path, intended destination (use the
sanitized new_filename), and the exception message, and then continues
processing the remaining files; ensure you still call extract_metadata and
sanitize_filename to build the target name but guard the actual move so a single
failure doesn’t abort the whole operation.- Around line 31-37: move_and_rename_flac_files currently uses shutil.move to
write destination / sanitize_filename(f"{artist} - {title}.flac") which silently
overwrites an existing file when metadata collide; update
move_and_rename_flac_files to check for existing target paths before calling
shutil.move and generate a unique filename (e.g., append a numeric suffix "
(1)", " (2)" or timestamp) using sanitize_filename and the original extension
until the path is free, then perform the move; use extract_metadata and
sanitize_filename as before and ensure the uniqueness logic runs atomically per
file so no overwrite occurs.- Around line 23-28: The extract_metadata function currently calls FLAC(...)
without handling exceptions, so corrupted/non-FLAC/I/O errors can crash the
rename operation; wrap the FLAC(...) call and subsequent tag access in a
try-except that catches mutagen.MutagenError and OSError (or a broad Exception
if mutagen types are not imported), log or quietly handle the error, and return
("Unknown", "Unknown") as a safe default from extract_metadata when an error
occurs so the rest of the rename flow can continue.In
@src/slskd_transform/search.py:
- Around line 130-136: The current split computes chunk_size =
len(songs_with_duration) // num_threads which yields 0 when
len(songs_with_duration) < num_threads so all work lands on the last iteration;
change to cap threads using actual_threads = min(num_threads, max(1,
len(songs_with_duration))) (or similar) and use actual_threads in the for-loop
and when computing chunk_size (and distribute remainder if desired) so chunks
are balanced; update references to chunk_size, songs_with_duration, and the for
i in range(...) loop to use actual_threads.- Around line 27-46: The loop that calls mutagen.File (inside the for loops over
music_dir) doesn't handle when mutagen returns None or raises on corrupted
files; update the logic around mutagen.File(file_path, easy=True) to: wrap the
call in a try/except catching mutagen.MutagenError and OSError, and after the
call check that audio_info is not None and has audio_info.info and
audio_info.info.length before using it to compute duration; on error or missing
info, skip that file (and optionally log a warning) and continue appending only
valid (file_without_ext, duration) tuples to filenames.- Around line 75-117: search_and_enqueue currently appends to the shared list
unfound_songs from multiple threads, causing a race; fix by adding a
threading.Lock parameter (e.g., search_lock) to search_and_enqueue and
acquiring/releasing it around every unfound_songs.append call (the three places
where unfound_songs.append is used inside search_and_enqueue), and update
threaded_search_and_enqueue to create the lock, pass it to each worker
invocation, and ensure callers use the same lock instance so all concurrent
appends are protected.
Nitpick comments:
In @.coderabbit.yaml:
- Around line 44-45: The filePatterns array in .coderabbit.yaml contains a
duplicate entry "CLAUDE.md"; remove the redundant duplicate so only one
"CLAUDE.md" remains in the filePatterns list to avoid duplicate matching and
clutter the config. Locate the filePatterns block and delete one of the two
"CLAUDE.md" entries (keep a single occurrence) then save the YAML ensuring valid
syntax is preserved.In
@README.md:
- Line 160: Replace the hardcoded image tag
"ghcr.io/geiserx/slskd-transform:2.0.0" in the README examples with a
maintainable pattern (for example use ":latest" or a placeholder variable like
"${VERSION}"/<version>), and update all occurrences (e.g., the
"ghcr.io/geiserx/slskd-transform:2.0.0 search --recursive" example and the other
instance at the same section) so examples either point to :latest or instruct
users to substitute/check the current release; keep the example invocation text
unchanged except for replacing the version token and add a short note directing
users to check releases if you choose the placeholder approach.- Around line 74-150: Add explicit language specifiers (use "text") to the
unlabelled fenced code blocks in README.md: the header block containing "CLI
flags > Environment variables > Config file > Defaults" and each CLI
help/output block starting with "slskd-transform [OPTIONS] COMMAND [ARGS]..." so
Markdown renderers correctly treat them as plain text; update those three/ four
fenced blocks to usetext instead ofto improve rendering.In
@src/slskd_transform/cli.py:
- Line 1: The project imports Click (seen as "import click") but the dependency
is pinned to an older release; update the Click dependency to 8.3.3 in your
project's dependency manifests (requirements.txt, pyproject.toml, setup.cfg, or
Pipfile) and run a fresh install; then run the CLI entrypoints and tests that
reference click (any functions that call click.command, click.group,
click.option, etc.) to verify there are no breaking API changes and update any
call signatures if tests fail.In
@src/slskd_transform/config.py:
- Around line 96-101: The loop converting fields in _INT_FIELDS currently calls
int(merged[field]) directly and will raise a raw ValueError on invalid input;
wrap that conversion in a try/except inside the same loop (where merged is
built) to catch ValueError/TypeError, and re-raise or log a clearer error that
includes the field name (from _INT_FIELDS) and the offending value
(merged[field]) and guidance (e.g., expected integer), so callers can see which
config key failed (reference the merged dict and the _INT_FIELDS iteration that
performs the int() conversion).In
@tests/test_rename.py:
- Around line 66-67: The test builds an expected path using os.path.join which
is inconsistent with the project's use of pathlib.Path; change the construction
of expected to use Path (e.g., Path(dest) / "My Artist - My Song.flac") so the
assertion
mock_shutil.move.assert_called_once_with(str(Path("/source/file.flac")),
expected) matches the rest of the suite — update the variableexpectedin
tests/test_rename.py accordingly and ensure types passed to mock_shutil.move
remain strings if other tests expect that.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `f451b203-2ca3-4122-b406-e3bfe0d2a870` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 29db6e1c98c72f4b299cc7e722cb0784f044580e and 60e78779896b8cabfbba2bc638ae3a66b761cec5. </details> <details> <summary>📒 Files selected for processing (23)</summary> * `.coderabbit.yaml` * `.github/workflows/tests.yml` * `CLAUDE.md` * `Dockerfile` * `README.md` * `config.example.yml` * `main.py` * `pyproject.toml` * `rename-files.py` * `requirements-test.txt` * `requirements.txt` * `src/slskd_transform/__init__.py` * `src/slskd_transform/cli.py` * `src/slskd_transform/config.py` * `src/slskd_transform/rename.py` * `src/slskd_transform/search.py` * `src/slskd_transform/utils.py` * `tests/test_cli.py` * `tests/test_config.py` * `tests/test_rename.py` * `tests/test_search.py` * `tests/test_slskd_transform.py` * `tests/test_utils.py` </details> <details> <summary>💤 Files with no reviewable changes (3)</summary> * requirements.txt * requirements-test.txt * tests/test_slskd_transform.py </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| RUN useradd --create-home appuser | ||
| USER appuser | ||
|
|
||
| RUN mkdir -p /app/music /app/downloads /app/organized |
There was a problem hiding this comment.
Critical: Permission denied—appuser cannot create directories under root-owned /app.
WORKDIR /app (line 11) creates /app owned by root. After switching to appuser (line 15), the RUN mkdir (line 17) fails because appuser lacks write permissions to /app.
🐛 Proposed fix: create directories before switching user
WORKDIR /app
COPY --from=builder /install /usr/local
+RUN mkdir -p /app/music /app/downloads /app/organized
RUN useradd --create-home appuser
+RUN chown -R appuser:appuser /app
USER appuser
-RUN mkdir -p /app/music /app/downloads /app/organized
ENTRYPOINT ["slskd-transform"]
CMD ["search"]📝 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.
| RUN useradd --create-home appuser | |
| USER appuser | |
| RUN mkdir -p /app/music /app/downloads /app/organized | |
| WORKDIR /app | |
| COPY --from=builder /install /usr/local | |
| RUN mkdir -p /app/music /app/downloads /app/organized | |
| RUN useradd --create-home appuser | |
| RUN chown -R appuser:appuser /app | |
| USER appuser | |
| ENTRYPOINT ["slskd-transform"] | |
| CMD ["search"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 14 - 17, The mkdir step fails because WORKDIR /app
is root-owned and the Dockerfile switches to USER appuser before creating
directories; either create the dirs while still root or change ownership of /app
to appuser. Move the RUN mkdir -p /app/music /app/downloads /app/organized to
before USER appuser (i.e., run as root after WORKDIR /app), or add a root RUN
chown -R appuser:appuser /app before switching to USER appuser so appuser can
create/write the directories.
| dependencies = [ | ||
| "slskd-api>=0.1.5", | ||
| "mutagen>=1.47", | ||
| "requests>=2.28", | ||
| "click>=8.1", | ||
| "pyyaml>=6.0", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check PyPI for specified minimum versions and security advisories
echo "=== Checking slskd-api >=0.1.5 ==="
curl -s https://pypi.org/pypi/slskd-api/json | jq -r '.releases | keys[]' | grep -E '^0\.1\.[5-9]|^0\.[2-9]\.|^[1-9]\.' | head -5
echo -e "\n=== Checking pyyaml >=6.0 ==="
curl -s https://pypi.org/pypi/pyyaml/json | jq -r '.releases | keys[]' | grep -E '^6\.|^[7-9]\.' | head -5
echo -e "\n=== Checking mutagen >=1.47 ==="
curl -s https://pypi.org/pypi/mutagen/json | jq -r '.releases | keys[]' | grep -E '^1\.(4[7-9]|[5-9][0-9])|^[2-9]\.' | head -5
echo -e "\n=== Security advisories ==="
for pkg in slskd-api pyyaml mutagen requests click; do
echo "Checking $pkg..."
gh api graphql -f query="
{
securityVulnerabilities(first: 3, ecosystem: PIP, package: \"$pkg\") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}" --jq ".data.securityVulnerabilities.nodes[] | \"[\(.advisory.severity)] \(.advisory.summary) | Vulnerable: \(.vulnerableVersionRange) | Patched: \(.firstPatchedVersion.identifier // \"N/A\")\""
doneRepository: GeiserX/slskd-transform
Length of output: 153
🌐 Web query:
slskd-api PyPI version 0.1.5 release history
💡 Result:
The project “slskd-api” (PyPI: slskd-api) lists version 0.1.5 in its version history as released on 2024-02-11.[1] However, I did not find an authoritative “release history / changelog” entry specifically describing what changed in slskd-api 0.1.5 (e.g., release notes) in the sources surfaced here. The PyPI page snippet available shows the version and date, but not the per-version change log.[1] If you want the actual 0.1.5 release notes, the most likely primary source is the upstream repository’s tagged release (bigoulours/slskd-python-api) for v0.1.5, but the search results returned only general release pages without a 0.1.5 notes excerpt in the evidence captured.[2][3]
Citations:
- 1: https://pypi.org/project/slskd-api/
- 2: https://github.com/bigoulours/slskd-python-api/releases
- 3: https://github.com/bigoulours/slskd-python-api/tree/3d31cf7a992f057de1bf5faf8a767eee1fa0a16b
🌐 Web query:
pyyaml version 6.0 security vulnerabilities CVE
💡 Result:
PyYAML version 6.0 has no known security vulnerabilities assigned a CVE. All documented CVEs for PyYAML, such as CVE-2020-14343 (arbitrary code execution via FullLoader on untrusted input, fixed in 5.4), CVE-2017-18342 (unsafe yaml.load, deprecated in 5.1), and CVE-2019-20477 (incomplete fix allowing class deserialization in 5.1-5.1.2), affect versions before 6.0.[1][2][3][4] PyYAML 6.0 incorporates the fix for CVE-2020-14343 by moving arbitrary Python tags to UnsafeLoader.[4][5] Security databases confirm no issues for 6.0 or later (up to 6.0.3).[6][7] There is an open DoS issue with merge keys (#897), but it is not CVE-assigned and affects safe_load.[8] Always use yaml.safe_load or SafeLoader for untrusted input.[4]
Citations:
- 1: https://app.opencve.io/cve/?vendor=pyyaml
- 2: https://github.com/pypa/advisory-database/blob/main/vulns/pyyaml/PYSEC-2021-142.yaml
- 3: https://nvd.nist.gov/vuln/detail/cve-2017-18342
- 4: https://github.com/yaml/pyyaml/blob/main/CHANGES
- 5: https://github.com/yaml/pyyaml/blob/6.0.3/CHANGES
- 6: https://security.snyk.io/package/pip/pyyaml
- 7: https://security.snyk.io/package/pip/PyYAML/6.0
- 8: [Bug]: YAML Merge Keys Drive Exponential Expansion DoS yaml/pyyaml#897
🌐 Web query:
mutagen version 1.47 release history PyPI
💡 Result:
mutagen 1.47.0 release history (PyPI) - Release: 1.47.0 - PyPI version date: 2023-09-03 [1] - Release highlights / changelog items included in 1.47.0: - ID3: allow reading TYER tags in the form of “yyyy-mm-dd” [2] - ID3: handle negative extended header sizes [2] - ID3: fix bad cast due to str.isdigit in id3 frames [2] - MP3: auto-detect MP3 files without ID3 tags [2] - WAVE: extensible wave format support [2] - WAVE, AIFF: handle truncated IFF files [2] - MP4: add a check for wrong offsets in MP4 files [2] - FLAC: fix cuesheet and seektable saving [2] - TrueAudio: read sample rate as unsigned and handle zero sample rate [2] - Docs: fixed documentation for id3.ID3.save parameter v2_version; sphinx 6.0 extlink compatibility; various typos [2] - Type annotations and various minor/oss-fuzz-related fixes [2] Evidence from authoritative sources: - The official Mutagen changelog lists an entry “1.47.0 - 2023-09-03” with the items above [2]. - The PyPI project page’s version history includes “1.47.0 | 2023-09-03” [1].
Citations:
🌐 Web query:
requests version 2.28 release history security
💡 Result:
The Python requests library version 2.28.0 was released on 2022-06-09.[1][2][3] Subsequent patch releases are 2.28.1 (2022-06-29) and 2.28.2 (2023-01-12).[1][4][2][5] Key changes in 2.28.0: - Dropped support for Python 2.7 and 3.6.[1][2][3] - Improvements: Consistent JSONDecodeError wrapping, consistent header parsing with InvalidHeader error, provisional Python 3.11 support, UI updates.[1][2] - Bugfixes: Fixed CURL_CA_BUNDLE empty string disabling cert verification (affects all prior 2.x versions), wrapped urllib3 SSLError properly, fixed Windows proxy registry exception handling, limited JSONDecodeError payload exposure.[1][2][3] Regarding security: The 2.28.0 release includes a security-related bugfix for the CURL_CA_BUNDLE issue, which could disable certificate verification unexpectedly in prior versions.[1][2][3] No CVEs were issued specifically for issues fixed in 2.28.0; it addresses bugs proactively.[6] However, requests 2.28.0 (and 2.28.1/2.28.2) is vulnerable to later-discovered issues: - CVE-2023-32681 (Proxy-Authorization header leak on HTTPS redirects): Fixed in 2.31.0. Affects [2.3.0, 2.31.0).[7][8][9] - CVE-2024-35195 (Session cert verify=False persists): Fixed in 2.32.0.[10][7][9] - CVE-2024-47081 (.netrc credentials leak via malicious URLs): Fixed in 2.32.4. Affects versions before 2.32.4, including 2.28.x.[10][11][9] - CVE-2026-25645 (Insecure temp file in extract_zipped_paths): Fixed in 2.33.0.[6][7][8] Snyk reports no direct vulnerabilities unique to 2.28.0 beyond inherited ones, but flags later CVEs as affecting it.[12][10] Official HISTORY.md does not label 2.28 fixes as security issues beyond the cert one.[2][6][3] As of 2026-05-12, latest is 2.34.0; upgrade recommended for security.[6]
Citations:
- 1: https://chromium.googlesource.com/external/github.com/kennethreitz/requests/+/refs/heads/master/HISTORY.md
- 2: https://github.com/psf/requests/blob/main/HISTORY.md
- 3: https://github.com/psf/requests/releases/tag/v2.28.0
- 4: https://requests.readthedocs.io/en/latest/community/updates/
- 5: https://github.com/psf/requests/releases/tag/v2.28.2
- 6: https://github.com/psf/requests/releases
- 7: https://app.opencve.io/cve/?product=requests&vendor=python
- 8: https://repology.org/project/python%3Arequests/cves
- 9: https://stack.watch/product/python/requests/
- 10: https://security.snyk.io/package/pip/requests/2.28.0
- 11: https://www.versioneye.com/Python/requests/2.28.0?page=4
- 12: https://security.snyk.io/package/pip/requests
🌐 Web query:
click version 8.1 release history PyPI
💡 Result:
Click (PyPI project: click) version 8.1.x release history on PyPI corresponds to these releases: - 8.1.0 — 2022-03-28 (feature release) [1] - 8.1.1 — 2022-03-30 [2] - 8.1.2 — 2022-03-31 [3] - 8.1.3 — 2022-04-28 [2] - 8.1.4 — 2023-07-06 [2] - 8.1.5 — 2023-07-13 [2] - 8.1.6 — 2023-07-18 [2] - 8.1.7 — 2023-08-17 [2] - 8.1.8 — 2024-12-19 (fix release) [4][5] 8.1.8 release notes (highlights): - Fix an issue with type hints for click.open_file. [4] - Fix issue where error message for invalid click.Path displays on multiple lines. [4] - Fixed issue that prevented a default value of "" from being displayed in the help for an option. [4] Primary sources used are the Click project’s GitHub release tags plus the Click “Changes” documentation and PyPI release listings. [4][2][6][3]
Citations:
- 1: https://github.com/pallets/click/releases/tag/8.1.0
- 2: https://pypi.org/project/click/
- 3: https://pypi.org/project/click/8.1.2/
- 4: https://github.com/pallets/click/releases/tag/8.1.8
- 5: pallets/click@8.1.7...8.1.8
- 6: https://click.palletsprojects.com/en/stable/changes/
Update requests dependency to >=2.32.4 to avoid multiple CVEs.
The requests>=2.28 specification is affected by multiple security vulnerabilities discovered after release:
- CVE-2023-32681: Proxy-Authorization header leak on HTTPS redirects
- CVE-2024-35195: Session cert verify=False persists
- CVE-2024-47081: .netrc credentials leak via malicious URLs
- CVE-2026-25645: Insecure temp file in extract_zipped_paths
Upgrade to requests>=2.33.0 to address all known issues. All other dependencies (slskd-api 0.1.5, mutagen 1.47, click 8.1, pyyaml 6.0) exist and have no known security advisories.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` around lines 15 - 21, Replace the vulnerable requests spec in
the dependencies list with a safe minimum version: update the "requests>=2.28"
entry in the pyproject.toml dependencies array to "requests>=2.33.0" so the
project installs a release that contains fixes for the cited CVEs; ensure the
exact string "requests>=2.28" is changed to "requests>=2.33.0" in the
dependencies block.
| def extract_metadata(file_path: Path) -> tuple[str, str]: | ||
| """Read FLAC tags, return (title, artist). Defaults to 'Unknown' if missing.""" | ||
| audio = FLAC(str(file_path)) | ||
| title = audio.get("title", ["Unknown"])[0] | ||
| artist = audio.get("artist", ["Unknown"])[0] | ||
| return title, artist |
There was a problem hiding this comment.
Major: No error handling for corrupted or invalid FLAC files.
mutagen.FLAC raises exceptions for corrupted files, non-FLAC files, or I/O errors. Unhandled exceptions will crash the entire rename operation.
🛡️ Proposed fix: wrap in try-except
def extract_metadata(file_path: Path) -> tuple[str, str]:
"""Read FLAC tags, return (title, artist). Defaults to 'Unknown' if missing."""
- audio = FLAC(str(file_path))
- title = audio.get("title", ["Unknown"])[0]
- artist = audio.get("artist", ["Unknown"])[0]
- return title, artist
+ try:
+ audio = FLAC(str(file_path))
+ title = audio.get("title", ["Unknown"])[0]
+ artist = audio.get("artist", ["Unknown"])[0]
+ return title, artist
+ except Exception as e:
+ print(f"Warning: Could not read metadata from {file_path}: {e}")
+ return "Unknown", "Unknown"📝 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.
| def extract_metadata(file_path: Path) -> tuple[str, str]: | |
| """Read FLAC tags, return (title, artist). Defaults to 'Unknown' if missing.""" | |
| audio = FLAC(str(file_path)) | |
| title = audio.get("title", ["Unknown"])[0] | |
| artist = audio.get("artist", ["Unknown"])[0] | |
| return title, artist | |
| def extract_metadata(file_path: Path) -> tuple[str, str]: | |
| """Read FLAC tags, return (title, artist). Defaults to 'Unknown' if missing.""" | |
| try: | |
| audio = FLAC(str(file_path)) | |
| title = audio.get("title", ["Unknown"])[0] | |
| artist = audio.get("artist", ["Unknown"])[0] | |
| return title, artist | |
| except Exception as e: | |
| print(f"Warning: Could not read metadata from {file_path}: {e}") | |
| return "Unknown", "Unknown" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slskd_transform/rename.py` around lines 23 - 28, The extract_metadata
function currently calls FLAC(...) without handling exceptions, so
corrupted/non-FLAC/I/O errors can crash the rename operation; wrap the FLAC(...)
call and subsequent tag access in a try-except that catches mutagen.MutagenError
and OSError (or a broad Exception if mutagen types are not imported), log or
quietly handle the error, and return ("Unknown", "Unknown") as a safe default
from extract_metadata when an error occurs so the rest of the rename flow can
continue.
| def move_and_rename_flac_files(flac_files: list[Path], destination: Path) -> None: | ||
| """Move each file to destination as 'Artist - Title.flac' (sanitized).""" | ||
| for file in flac_files: | ||
| title, artist = extract_metadata(file) | ||
| new_filename = sanitize_filename(f"{artist} - {title}.flac") | ||
| shutil.move(str(file), str(destination / new_filename)) | ||
| print(f"Moved and renamed: {destination / new_filename}") |
There was a problem hiding this comment.
Major: No error handling for move failures.
shutil.move can fail due to permissions, disk space, cross-device moves, or destination access errors. Unhandled exceptions leave the operation incomplete.
🛡️ Proposed fix: wrap in try-except
def move_and_rename_flac_files(flac_files: list[Path], destination: Path) -> None:
"""Move each file to destination as 'Artist - Title.flac' (sanitized)."""
for file in flac_files:
- title, artist = extract_metadata(file)
- new_filename = sanitize_filename(f"{artist} - {title}.flac")
- shutil.move(str(file), str(destination / new_filename))
- print(f"Moved and renamed: {destination / new_filename}")
+ try:
+ title, artist = extract_metadata(file)
+ new_filename = sanitize_filename(f"{artist} - {title}.flac")
+ shutil.move(str(file), str(destination / new_filename))
+ print(f"Moved and renamed: {destination / new_filename}")
+ except Exception as e:
+ print(f"Error moving {file}: {e}")📝 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.
| def move_and_rename_flac_files(flac_files: list[Path], destination: Path) -> None: | |
| """Move each file to destination as 'Artist - Title.flac' (sanitized).""" | |
| for file in flac_files: | |
| title, artist = extract_metadata(file) | |
| new_filename = sanitize_filename(f"{artist} - {title}.flac") | |
| shutil.move(str(file), str(destination / new_filename)) | |
| print(f"Moved and renamed: {destination / new_filename}") | |
| def move_and_rename_flac_files(flac_files: list[Path], destination: Path) -> None: | |
| """Move each file to destination as 'Artist - Title.flac' (sanitized).""" | |
| for file in flac_files: | |
| try: | |
| title, artist = extract_metadata(file) | |
| new_filename = sanitize_filename(f"{artist} - {title}.flac") | |
| shutil.move(str(file), str(destination / new_filename)) | |
| print(f"Moved and renamed: {destination / new_filename}") | |
| except Exception as e: | |
| print(f"Error moving {file}: {e}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slskd_transform/rename.py` around lines 31 - 37, The loop in
move_and_rename_flac_files currently calls shutil.move without handling
failures; wrap the move (the call to shutil.move inside
move_and_rename_flac_files) in a try-except that catches Exception, logs or
prints a clear error including the source path, intended destination (use the
sanitized new_filename), and the exception message, and then continues
processing the remaining files; ensure you still call extract_metadata and
sanitize_filename to build the target name but guard the actual move so a single
failure doesn’t abort the whole operation.
Critical: File collision causes silent data loss.
When two FLAC files have identical artist and title metadata, shutil.move overwrites the first file with the second. No collision detection or conflict resolution.
🛡️ Proposed fix: detect and resolve collisions
def move_and_rename_flac_files(flac_files: list[Path], destination: Path) -> None:
"""Move each file to destination as 'Artist - Title.flac' (sanitized)."""
for file in flac_files:
title, artist = extract_metadata(file)
new_filename = sanitize_filename(f"{artist} - {title}.flac")
- shutil.move(str(file), str(destination / new_filename))
+ target = destination / new_filename
+ if target.exists():
+ # Append counter to avoid collision
+ stem = target.stem
+ suffix = target.suffix
+ counter = 1
+ while target.exists():
+ target = destination / f"{stem}_{counter}{suffix}"
+ counter += 1
+ shutil.move(str(file), str(target))
- print(f"Moved and renamed: {destination / new_filename}")
+ print(f"Moved and renamed: {target}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slskd_transform/rename.py` around lines 31 - 37,
move_and_rename_flac_files currently uses shutil.move to write destination /
sanitize_filename(f"{artist} - {title}.flac") which silently overwrites an
existing file when metadata collide; update move_and_rename_flac_files to check
for existing target paths before calling shutil.move and generate a unique
filename (e.g., append a numeric suffix " (1)", " (2)" or timestamp) using
sanitize_filename and the original extension until the path is free, then
perform the move; use extract_metadata and sanitize_filename as before and
ensure the uniqueness logic runs atomically per file so no overwrite occurs.
| for root, _dirs, files in os.walk(music_dir): | ||
| for file in files: | ||
| if file.startswith('.'): | ||
| continue | ||
| file_without_ext = os.path.splitext(file)[0] | ||
| file_path = os.path.join(root, file) | ||
| audio_info = mutagen.File(file_path, easy=True) | ||
| duration = int(audio_info.info.length) | ||
| filenames.append((file_without_ext, duration)) | ||
| else: | ||
| for file in os.listdir(music_dir): | ||
| if file.startswith('.'): | ||
| continue | ||
| file_path = os.path.join(music_dir, file) | ||
| if not os.path.isfile(file_path): | ||
| continue | ||
| file_without_ext = os.path.splitext(file)[0] | ||
| audio_info = mutagen.File(file_path, easy=True) | ||
| duration = int(audio_info.info.length) | ||
| filenames.append((file_without_ext, duration)) |
There was a problem hiding this comment.
Major: No error handling for mutagen parsing failures.
mutagen.File returns None for unrecognized formats and raises exceptions for corrupted files or I/O errors. Line 34/45 will crash with AttributeError if audio_info is None.
🛡️ Proposed fix: skip unparseable files
if recursive:
for root, _dirs, files in os.walk(music_dir):
for file in files:
if file.startswith('.'):
continue
file_without_ext = os.path.splitext(file)[0]
file_path = os.path.join(root, file)
- audio_info = mutagen.File(file_path, easy=True)
- duration = int(audio_info.info.length)
- filenames.append((file_without_ext, duration))
+ try:
+ audio_info = mutagen.File(file_path, easy=True)
+ if audio_info is None or not hasattr(audio_info, 'info'):
+ continue
+ duration = int(audio_info.info.length)
+ filenames.append((file_without_ext, duration))
+ except Exception:
+ # Skip files that can't be parsed
+ continue
else:
for file in os.listdir(music_dir):
# ... similar fix needed here ...
+ try:
+ audio_info = mutagen.File(file_path, easy=True)
+ if audio_info is None or not hasattr(audio_info, 'info'):
+ continue
+ duration = int(audio_info.info.length)
+ filenames.append((file_without_ext, duration))
+ except Exception:
+ continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slskd_transform/search.py` around lines 27 - 46, The loop that calls
mutagen.File (inside the for loops over music_dir) doesn't handle when mutagen
returns None or raises on corrupted files; update the logic around
mutagen.File(file_path, easy=True) to: wrap the call in a try/except catching
mutagen.MutagenError and OSError, and after the call check that audio_info is
not None and has audio_info.info and audio_info.info.length before using it to
compute duration; on error or missing info, skip that file (and optionally log a
warning) and continue appending only valid (file_without_ext, duration) tuples
to filenames.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/slskd_transform/search.py (1)
27-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle mutagen parse exceptions during directory scan.
At Line 32 and Line 45, parse/read failures can still raise and break the scan loop. Skip bad files with targeted exception handling around
mutagen.File(...).For mutagen 1.47.0, does `mutagen.File(path, easy=True)` raise `MutagenError`/`OSError` for corrupted or unreadable files, and can `audio.info`/`audio.info.length` be missing?Proposed fix
- audio_info = mutagen.File(file_path, easy=True) - if audio_info is None or audio_info.info is None: + try: + audio_info = mutagen.File(file_path, easy=True) + except (mutagen.MutagenError, OSError): + continue + if audio_info is None or audio_info.info is None or getattr(audio_info.info, "length", None) is None: continue duration = int(audio_info.info.length) @@ - audio_info = mutagen.File(file_path, easy=True) - if audio_info is None or audio_info.info is None: + try: + audio_info = mutagen.File(file_path, easy=True) + except (mutagen.MutagenError, OSError): + continue + if audio_info is None or audio_info.info is None or getattr(audio_info.info, "length", None) is None: continue duration = int(audio_info.info.length)Also applies to: 45-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slskd_transform/search.py` around lines 27 - 35, Wrap the call to mutagen.File(...) inside a try/except that catches mutagen.MutagenError and OSError (and any other I/O related exceptions) so corrupted/unreadable files don't break the os.walk loop; if an exception is raised, log or silently skip the file and continue. After the try, still guard access to audio_info and audio_info.info.length (e.g., check audio_info is not None and audio_info.info is not None before using duration) in the same block for both locations where mutagen.File is used (the call site with variable names audio_info and duration) so bad files are safely skipped.
🧹 Nitpick comments (1)
src/slskd_transform/search.py (1)
152-152: ⚡ Quick winRemove or make thread-start delay configurable.
Line 152 adds a fixed 10s stagger per thread, which can add large startup latency and reduce the expected throughput gains from multithreading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/slskd_transform/search.py` at line 152, The hardcoded time.sleep(10) call should not impose a fixed 10s stagger; remove it or make it configurable. Replace the literal sleep with a configurable start delay (e.g., parameter name start_delay or config key SEARCH_THREAD_START_DELAY) defaulting to 0, update the function that spawns/search threads (refer to the thread-starting routine where time.sleep(10) is called) to accept and pass this value, and use the config/env value when launching threads so callers can control or disable the stagger.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 74-76: The README contains unlabeled fenced code blocks (e.g., the
block showing "CLI flags > Environment variables > Config file > Defaults"
and other blocks at the referenced lines); add appropriate language identifiers
after the opening backticks (for example use ```text for plain text tables,
```bash for shell examples, or ```yaml for YAML snippets) for each fenced block
at lines 74, 121-123, 138-139, and 147-148 so markdownlint MD040 is satisfied
and the blocks are properly highlighted.
In `@rename-files.py`:
- Around line 4-5: The main block currently calls cli(["rename"]) which drops
any user-provided arguments; modify the shim so it forwards legacy script CLI
args by importing sys and passing sys.argv[1:] into the cli invocation (i.e.,
call cli with a list beginning with "rename" followed by sys.argv[1:]) so that
the existing function cli continues to receive the "rename" command plus any
options the user supplied.
In `@src/slskd_transform/search.py`:
- Around line 65-70: The current try/except around parsing file_info['length']
only catches KeyError but int(file_info['length']) can raise TypeError or
ValueError; update the exception handling in the block that computes
result_duration (parsing file_info['length'] and comparing to
local_duration/tolerance) to catch KeyError, TypeError, and ValueError (or use a
broader except (KeyError, TypeError, ValueError):) so malformed or non-numeric
length fields don't abort the search batch and simply skip that result.
- Around line 91-99: Wrap the calls to client.searches.search_text and
client.searches.search_responses in a try/except so exceptions don't escape the
worker loop; after calling client.searches.search_text(checking
searchText=(song_name_cleaned + " " + config.format)), verify search_response
contains an 'id' before using search_response['id'], and if missing or any call
raises, log the error and mark the song as unfound (e.g., via the existing
unfound recording mechanism or by appending to the unfound list) so the worker
continues processing other songs; keep the time.sleep(config.search_timeout)
behavior but only after a successful search_text call.
---
Duplicate comments:
In `@src/slskd_transform/search.py`:
- Around line 27-35: Wrap the call to mutagen.File(...) inside a try/except that
catches mutagen.MutagenError and OSError (and any other I/O related exceptions)
so corrupted/unreadable files don't break the os.walk loop; if an exception is
raised, log or silently skip the file and continue. After the try, still guard
access to audio_info and audio_info.info.length (e.g., check audio_info is not
None and audio_info.info is not None before using duration) in the same block
for both locations where mutagen.File is used (the call site with variable names
audio_info and duration) so bad files are safely skipped.
---
Nitpick comments:
In `@src/slskd_transform/search.py`:
- Line 152: The hardcoded time.sleep(10) call should not impose a fixed 10s
stagger; remove it or make it configurable. Replace the literal sleep with a
configurable start delay (e.g., parameter name start_delay or config key
SEARCH_THREAD_START_DELAY) defaulting to 0, update the function that
spawns/search threads (refer to the thread-starting routine where time.sleep(10)
is called) to accept and pass this value, and use the config/env value when
launching threads so callers can control or disable the stagger.
🪄 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: 3af23cc9-0813-4774-be9f-e02f4dae9f08
📒 Files selected for processing (22)
.github/workflows/tests.yml.gitignoreDockerfileREADME.mdconfig.example.ymlmain.pypyproject.tomlrename-files.pyrequirements-test.txtrequirements.txtsrc/slskd_transform/__init__.pysrc/slskd_transform/cli.pysrc/slskd_transform/config.pysrc/slskd_transform/rename.pysrc/slskd_transform/search.pysrc/slskd_transform/utils.pytests/test_cli.pytests/test_config.pytests/test_rename.pytests/test_search.pytests/test_slskd_transform.pytests/test_utils.py
💤 Files with no reviewable changes (3)
- requirements.txt
- requirements-test.txt
- tests/test_slskd_transform.py
✅ Files skipped from review due to trivial changes (3)
- config.example.yml
- .gitignore
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (11)
- src/slskd_transform/init.py
- main.py
- .github/workflows/tests.yml
- src/slskd_transform/utils.py
- tests/test_rename.py
- Dockerfile
- src/slskd_transform/cli.py
- src/slskd_transform/config.py
- tests/test_search.py
- tests/test_config.py
- src/slskd_transform/rename.py
| ``` | ||
| CLI flags > Environment variables > Config file > Defaults | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Line 74, Line 121, Line 138, and Line 147 use unlabeled fenced blocks (MD040). Add a language (for example text, bash, or yaml) to satisfy markdownlint.
Also applies to: 121-123, 138-139, 147-148
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 74 - 76, The README contains unlabeled fenced code
blocks (e.g., the block showing "CLI flags > Environment variables > Config
file > Defaults" and other blocks at the referenced lines); add appropriate
language identifiers after the opening backticks (for example use ```text for
plain text tables, ```bash for shell examples, or ```yaml for YAML snippets) for
each fenced block at lines 74, 121-123, 138-139, and 147-148 so markdownlint
MD040 is satisfied and the blocks are properly highlighted.
| if __name__ == '__main__': | ||
| main() | ||
| cli(["rename"]) |
There was a problem hiding this comment.
Forward legacy script arguments to the new CLI command.
Line 5 hardcodes ["rename"], so options passed to rename-files.py are ignored. Forward sys.argv[1:] to keep shim behavior practical.
Proposed fix
+import sys
from slskd_transform.cli import cli
if __name__ == '__main__':
- cli(["rename"])
+ cli(["rename", *sys.argv[1:]])📝 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.
| if __name__ == '__main__': | |
| main() | |
| cli(["rename"]) | |
| import sys | |
| from slskd_transform.cli import cli | |
| if __name__ == '__main__': | |
| cli(["rename", *sys.argv[1:]]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rename-files.py` around lines 4 - 5, The main block currently calls
cli(["rename"]) which drops any user-provided arguments; modify the shim so it
forwards legacy script CLI args by importing sys and passing sys.argv[1:] into
the cli invocation (i.e., call cli with a list beginning with "rename" followed
by sys.argv[1:]) so that the existing function cli continues to receive the
"rename" command plus any options the user supplied.
| try: | ||
| result_duration = int(file_info['length']) | ||
| if abs(local_duration - result_duration) <= tolerance: | ||
| return result | ||
| except KeyError: | ||
| pass |
There was a problem hiding this comment.
Widen duration parse exception handling.
At Line 66, int(file_info['length']) can also raise TypeError/ValueError. Catch those too to avoid aborting a search batch on malformed payloads.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slskd_transform/search.py` around lines 65 - 70, The current try/except
around parsing file_info['length'] only catches KeyError but
int(file_info['length']) can raise TypeError or ValueError; update the exception
handling in the block that computes result_duration (parsing file_info['length']
and comparing to local_duration/tolerance) to catch KeyError, TypeError, and
ValueError (or use a broader except (KeyError, TypeError, ValueError):) so
malformed or non-numeric length fields don't abort the search batch and simply
skip that result.
| search_response = client.searches.search_text( | ||
| searchText=(song_name_cleaned + " " + config.format) | ||
| ) | ||
|
|
||
| time.sleep(config.search_timeout) | ||
|
|
||
| search_id = search_response['id'] | ||
| search_results = client.searches.search_responses(id=search_id) | ||
|
|
There was a problem hiding this comment.
Guard search API calls and missing search IDs.
At Line 91–99, failures in search_text, search_responses, or missing 'id' currently escape the loop and can kill a worker thread without recording the song as unfound.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/slskd_transform/search.py` around lines 91 - 99, Wrap the calls to
client.searches.search_text and client.searches.search_responses in a try/except
so exceptions don't escape the worker loop; after calling
client.searches.search_text(checking searchText=(song_name_cleaned + " " +
config.format)), verify search_response contains an 'id' before using
search_response['id'], and if missing or any call raises, log the error and mark
the song as unfound (e.g., via the existing unfound recording mechanism or by
appending to the unfound list) so the worker continues processing other songs;
keep the time.sleep(config.search_timeout) behavior but only after a successful
search_text call.
1 participant
Summary
SLSKD_*env vars, or CLI flags — no more editing source code--recursiveflag to scan nested library structures (closes Question: Music Directory for slkskd-transform #8)Changes
src/slskd_transform/with cli, config, search, rename, utils modulesslskd-transform searchandslskd-transform renamesubcommandspyproject.tomlreplacesrequirements.txtmain.pyandrename-files.pykept as thin shimsTest plan
pip install -e ".[dev]"succeedspytest tests/ -v— 41 tests passslskd-transform --helpshows group with search/rename commandsslskd-transform search --helpshows all optionsCloses #8
Summary by CodeRabbit
Release Notes
New Features
searchandrenamecommandsDocumentation
Breaking Changes