Updates metadata flow for authenticated Audible#18
Merged
Conversation
- Updated test_error_recovery.py to utilize MetadataCoordinator for metadata fetching and improved error handling for network timeouts, rate limits, and service unavailability. - Refactored test_integration.py to mock MetadataCoordinator instead of direct metadata fetch calls, ensuring better integration testing. - Modified test_main_integration.py to replace direct metadata fetch mocks with MetadataCoordinator, enhancing test reliability. - Adjusted test_metadata_extended.py to incorporate MetadataCoordinator in tests, ensuring accurate metadata fetching and error handling. - Enhanced test_security.py to mock MetadataCoordinator for security tests, improving coverage against SQL injection and header injection. - Cleaned up test_utils_metadata.py by removing unused fetch_metadata test, streamlining the test suite. Co-authored-by: Copilot <copilot@github.com>
… external API - Created async.rst.txt to document support for asynchronous requests using httpx. - Added changelog.md.txt to include a reference to the main changelog file. - Introduced examples.rst.txt with sample code for using the Audible client. - Added external_api.rst.txt to document the Audible API endpoints and their parameters. - Created load_save.rst.txt to explain how to load and save authentication data, both encrypted and unencrypted. - Added logging.rst.txt to describe logging capabilities and configuration. - Updated modules documentation to include new modules and their members. - Enhanced audible_scraper.py to improve image resolution handling and API response groups. - Updated tests to reflect changes in the Audible scraper's API usage. Co-authored-by: Copilot <copilot@github.com>
…nt; update requirements and improve metadata retrieval tests Co-authored-by: Copilot <copilot@github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds authenticated Audible support (AudibleClientProvider + Audible-backed catalog searches), refactors metadata retrieval to use a MetadataCoordinator-first flow, updates installation/config/docs for Audible, and updates tests and CI to exercise the new coordinator and Audible integration. ChangesAudible integration & metadata coordinator
Sequence DiagramsequenceDiagram
participant Webhook as Webhook Handler
participant Coordinator as MetadataCoordinator
participant Scraper as AudibleScraper
participant Provider as AudibleClientProvider
participant Audible as mkb79/Audible
participant Audnex as Audnex API
Webhook->>Coordinator: get_metadata_from_webhook(payload)
activate Coordinator
Coordinator->>Scraper: search_from_webhook_name(name)
activate Scraper
Scraper->>Scraper: extract_title_author_from_name()
Scraper->>Scraper: search_by_title_author(title, author)
Scraper->>Provider: get_client(region)
activate Provider
Provider->>Audible: Authenticator.from_file(auth_file, password)
Provider->>Audible: AsyncClient(authenticator, country_code)
Audible-->>Provider: authenticated_client
deactivate Provider
Scraper->>Audible: client.get("1.0/catalog/products", params)
Audible-->>Scraper: catalog_results
Scraper-->>Coordinator: normalized_result
deactivate Scraper
alt Audible catalog found
Coordinator-->>Webhook: enhanced_metadata
else Audible not found
Coordinator->>Audnex: search fallback
Audnex-->>Coordinator: audnex_result
Coordinator-->>Webhook: normalized_result
end
deactivate Coordinator
Webhook->>Webhook: create_response(metadata)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Veteran mecha engineer pre-flight check (Issue → Impact → Fix → Test)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88fab9af22
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 39
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
tests/test_security.py (1)
203-211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast on server errors in the JSON-injection test
- Issue: Line 208 accepts
500, and Line 209 catches broadException.- Impact: The test can pass even when the endpoint crashes, which weakens security regression detection.
- Fix: Remove
500from accepted statuses and narrow/remove the catch block so unexpected failures fail the test.- Test: Inject a malformed payload that should be rejected and assert only expected client-side/API validation codes.
Suggested patch
- try: - resp = self.client.post( - "/webhook/audiobook-requests", json=malicious_json, headers={"X-Autobrr-Token": "test_token"} - ) - - # Should handle malicious JSON safely - assert resp.status_code in [200, 400, 422, 500] - except Exception as e: - # Should not cause unhandled exceptions - assert "json" in str(e).lower() or "decode" in str(e).lower() + resp = self.client.post( + "/webhook/audiobook-requests", json=malicious_json, headers={"X-Autobrr-Token": "test_token"} + ) + # Should handle malicious JSON safely without server errors + assert resp.status_code in [200, 400, 422] + assert resp.status_code != 500As per coding guidelines, "
**/tests/**: Verify pytest best practices, proper mocking of external services, and adequate coverage of edge cases."🤖 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_security.py` around lines 203 - 211, The test currently tolerates server errors and swallows unexpected exceptions: in the block that posts via self.client.post("/webhook/audiobook-requests", json=malicious_json, headers={"X-Autobrr-Token": "test_token"}) remove 500 from the allowed resp.status_code list so only expected client-side validation codes remain (e.g., 200, 400, 422), and tighten the exception handling by removing the broad try/except or replacing it with a specific exception assertion (e.g., assert raises JSONDecodeError or let the test fail on any uncaught Exception) so unexpected server crashes cause the test to fail. Ensure changes target the resp assertion and the surrounding try/except block in tests/test_security.py.tests/test_end_to_end.py (1)
294-307: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAsync mock side_effect implementation is fragile.
- Issue: Using a regular
patch()with an async function asside_effectworks, but the argument extraction logic (lines 299-303) guesses at the call signature rather than matching the actual coordinator API.- Impact: If the coordinator's
get_metadata_from_webhooksignature changes, this test may silently misbehave.- Fix: Use
AsyncMockexplicitly and match the actual method signature.Proposed improvement
- with ( - patch.dict("os.environ", {"AUTOBRR_TOKEN": "test_token"}), - patch("src.metadata_coordinator.MetadataCoordinator.get_metadata_from_webhook") as mock_fetch, - ): - # Mock needs to return different values for different payloads - # Use async side_effect to match the real async function signature - async def _mock_fetch_metadata(*args, **kwargs): - # Extract payload from the last positional argument or kwargs. - payload = kwargs.get("webhook_payload") or kwargs.get("payload", {}) - if not payload and args and isinstance(args[-1], dict): - payload = args[-1] - if isinstance(payload, dict): - return {"title": payload.get("name", "Unknown")} - return {"title": "Unknown"} - - mock_fetch.side_effect = _mock_fetch_metadata + with ( + patch.dict("os.environ", {"AUTOBRR_TOKEN": "test_token"}), + patch( + "src.metadata_coordinator.MetadataCoordinator.get_metadata_from_webhook", + new_callable=AsyncMock, + ) as mock_fetch, + ): + async def _mock_fetch_metadata(payload): + if isinstance(payload, dict): + return {"title": payload.get("name", "Unknown")} + return {"title": "Unknown"} + + mock_fetch.side_effect = _mock_fetch_metadata🤖 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_end_to_end.py` around lines 294 - 307, The async test mock is fragile because the side_effect _mock_fetch_metadata tries to guess args; replace the plain patch of MetadataCoordinator.get_metadata_from_webhook with an AsyncMock that matches the real method signature (e.g., async def signature(self, webhook_payload) or async def signature(webhook_payload) depending on the bound/unbound method) and implement its return logic against the explicit parameter (webhook_payload) instead of inspecting args/kwargs; update the mock assignment from mock_fetch.side_effect = _mock_fetch_metadata to using AsyncMock(side_effect=...) or AsyncMock(return_value=...) bound to MetadataCoordinator.get_metadata_from_webhook so the test behavior follows the actual API.tests/test_integration.py (1)
29-46:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
AsyncMockfor async method patches in test_integration.py.
- Issue:
get_metadata_from_webhookis an async method, but all four patches in this file (lines 31, 162, 298, 508) use regularpatch()withoutnew_callable=AsyncMock. Awaiting a regularMagicMockreturn value raisesTypeError.- Impact: Mocked return values aren't applied. Tests pass only because production code catches exceptions and defaults to empty metadata—masking the mock's ineffectiveness.
- Fix: Import
AsyncMockand apply it to all four occurrences, matching the pattern already used inconftest.py.Import addition:
from unittest.mock import patch, AsyncMockThen update each patch:
patch("src.metadata_coordinator.MetadataCoordinator.get_metadata_from_webhook", new_callable=AsyncMock) as mock_coord,🤖 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_integration.py` around lines 29 - 46, The async method MetadataCoordinator.get_metadata_from_webhook is being patched with plain patch() which yields a synchronous MagicMock causing TypeError when awaited; update tests/test_integration.py to import AsyncMock from unittest.mock and replace each patch("src.metadata_coordinator.MetadataCoordinator.get_metadata_from_webhook") usage with patch(..., new_callable=AsyncMock) (for all four occurrences) so the mock is awaitable and mock return values are applied correctly.tests/test_error_recovery.py (1)
125-149: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winThis "concurrent" test is still sequential.
- Issue: The loop sends five
self.client.post(...)calls one after another on the same thread.- Impact: It never exercises real overlap, so queue contention, shared-state races, and lifespan interactions under concurrent load remain untested.
- Fix: Send the requests in parallel with a thread pool or an async client/ASGI transport, and make the mocked coordinator block long enough for the requests to overlap.
- Test: Assert all requests complete while a synchronized probe confirms at least two requests were in-flight at the same time.
As per coding guidelines,
**/tests/**: Verify pytest best practices, proper mocking of external services, and adequate coverage of edge cases.🤖 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_error_recovery.py` around lines 125 - 149, The test_concurrent_error_handling test is sequential and doesn't exercise real concurrency; change it to send the five requests in parallel (e.g., use ThreadPoolExecutor or pytest-asyncio with an async test client) instead of looping calling self.client.post synchronously, ensure the mocked coordinator handler used by /webhook/audiobook-requests blocks long enough (use an Event or Barrier in the mock) so requests overlap, and add an in-flight counter or synchronized probe to assert at least two requests were concurrently active while still asserting all five complete and no unhandled exceptions occur.src/metadata.py (1)
352-360:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve scalar
authorwhenauthorsis absent.
- Issue:
clean_metadata()now setsresult["author"]only fromclean_author_list(item.get("authors"))and ignores the normalized/baseauthor.- Impact: Callers that pass already-normalized metadata such as
{"author": "Brandon Sanderson"}lose the author in templates/tests.- Fix: Fall back to
base.get("author")oritem.get("author")when the list-based path is empty.- Test: Add a case where the input has only
authorand ensure the compatibility wrapper preserves it.Suggested fix
authors_raw = item.get("authors") or [] filtered_authors = clean_author_list(authors_raw) result["authors_raw"] = authors_raw - result["author"] = filtered_authors[0] if filtered_authors else None + result["author"] = filtered_authors[0] if filtered_authors else base.get("author") or item.get("author")🤖 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/metadata.py` around lines 352 - 360, clean_metadata() currently sets result["author"] only from clean_author_list(item.get("authors")) and overwrites/omits any scalar author provided via normalize_book_result/base or the original item; change the assignment for result["author"] to prefer the first entry from filtered_authors but fall back to base.get("author") or item.get("author") when filtered_authors is empty (use normalize_book_result(item) referenced as base and the clean_author_list(...) path), and add a unit test that passes input with only "author": "Brandon Sanderson" to ensure the compatibility wrapper preserves that scalar author.
🤖 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 @.env.example:
- Around line 51-53: Update the .env.example comment about Audible to clarify
it's conditional: change the sentence "The auth file and decrypt password are
required for the Audible backend to return results." to something like "The auth
file and decrypt password are required only when the authenticated Audible
backend is enabled (e.g., when using an encrypted auth file)." Reference the
Audible-related comment block and any related env variables (e.g.,
AUDIBLE_AUTH_FILE, AUDIBLE_DECRYPT_PASSWORD or the Audible section header) so
the new text clearly indicates this is optional unless the authenticated Audible
backend is in use.
In `@docs/SYSTEM_COMPLETION_SUMMARY.md`:
- Line 12: Update the documentation entry that currently lists
"audible_scraper.py" to use the repository-relative path
"src/audible_scraper.py" so it matches the other entries; search for the string
"audible_scraper.py" in SYSTEM_COMPLETION_SUMMARY.md and replace it with
"src/audible_scraper.py", then scan the surrounding bullet list to ensure all
file entries follow the same src/ prefix convention.
In `@docs/user-guide/configuration.md`:
- Line 64: Update the documentation to resolve the inconsistency: change the
`auth_file` description to explicitly state it is optional and only required for
authenticated Audible lookups (e.g., "Optional — required only for authenticated
Audible lookups") and modify the checklist entry referencing `AUDIBLE_AUTH_FILE`
and `AUDIBLE_AUTH_FILE_PASSWORD` so it clarifies these environment variables
must be set only when using authenticated Audible lookups rather than implying
they are always required.
In `@docs/vendor/audible/.env.example`:
- Line 126: Update the checklist entry that currently reads "# [ ] File
permissions secured (600 for .env, 644 for logs)" to recommend stricter log
permissions (e.g., "640 for logs, or 600 where feasible") and mention using a
dedicated service user and log-group so group read is intentional; also add a
test step suggesting `stat` to verify deployed log files are not world-readable.
Target the checklist line "# [ ] File permissions secured (600 for .env, 644 for
logs)" when making the change.
In `@docs/vendor/audible/config/config.yaml.example`:
- Line 56: The comment for the configuration key autobrr_webhook_endpoint
contains a spelling mistake ("webook"); update the comment to read "webhook
token location" so the line reads: autobrr_webhook_endpoint:
"/webhook/audiobook-requests" # autobrr webhook token location .env
"AUTOBRR_TOKEN", ensuring the change targets the autobrr_webhook_endpoint entry.
In `@docs/vendor/audible/docs/README.md`:
- Line 5: Replace the broken replacement-character glyphs in the two markdown
headings: find the headings currently written as "� Quick Start" and "�
Documentation Structure" and replace them with valid characters (for example "🚀
Quick Start" and "📖 Documentation Structure" or plain "Quick Start" and
"Documentation Structure"); update the two header lines in README.md where those
glyphs appear and verify by rendering the markdown in a viewer to confirm the
headings display cleanly across renderers.
- Around line 77-78: Update the README metadata so there is a single canonical
"Last Updated" field with an exact calendar date; locate both occurrences of
"Last Updated" (the block around "Last Updated: June 2025" and the later block
showing "June 16, 2025") and replace them with one consistent entry (e.g., "Last
Updated: May 6, 2026"), removing the duplicate metadata block so only one
last-updated line remains and verify the date matches the actual edit date.
In `@docs/vendor/audible/docs/user-guide/configuration.md`:
- Around line 154-157: The example validation command currently imports
load_config but never calls it; update the one-liner to invoke load_config()
(e.g., call load_config() and only print "✅ Config valid" after it returns) and
ensure any exceptions from load_config propagate (do not swallow them) so the
process exits non‑zero on invalid config; locate the example using the symbol
load_config in the docs and change the command accordingly.
In `@docs/vendor/audible/docs/user-guide/troubleshooting.md`:
- Around line 283-291: Update the "Restart application" step to avoid killing
all Python processes: replace the unsafe `pkill -f python` with a targeted stop
such as `pkill -f "src/main.py"` or instruct users to `kill <PID>` for the
specific PID they found earlier (or a service/ container stop command if
applicable), keeping the restart command `python src/main.py`; ensure the docs
text under "Restart application" explains using the specific PID or process
pattern so only the audiobook service is stopped and other Python jobs remain
running.
- Around line 237-243: Replace the insecure "grep DISCORD_WEBHOOK_URL .env" and
the direct test call "python -c 'from src.notify.discord import DiscordNotifier;
DiscordNotifier().test()'" with non-leaking presence checks: run a shell check
that returns only whether DISCORD_WEBHOOK_URL is set (e.g. use grep -q or a
shell conditional to echo "DISCORD_WEBHOOK_URL set" / "unset" rather than
printing the value) and update the Python test to verify existence of
os.environ.get('DISCORD_WEBHOOK_URL') or call a non-disclosing method on
DiscordNotifier that reports status without printing the webhook secret,
ensuring the troubleshooting snippet confirms set/unset without exposing the
value.
In `@docs/vendor/audible/examples.md`:
- Around line 12-13: The example calls Authenticator.from_file(filename) and
then Client(auth) but never defines filename; update the snippet so filename is
defined before use (e.g., set a variable like filename =
"path/to/credentials.json" or replace with an inline string) so
Authenticator.from_file and audible.Client(auth) run without NameError; after
editing, verify by pasting into a REPL/script and confirming execution proceeds
past auth initialization.
In `@docs/vendor/audible/raw/auth/authorization.rst.txt`:
- Line 149: Update the documentation strings that contain spelling/casing
errors: replace "programatically" with "programmatically" and "webbrowser" with
"web browser" in the sentence "Paste this url into a web browser or use it
programatically to authorize yourself." (and the other occurrences noted at
lines 154, 166, 173), and normalize other tokens such as "loggin" → "logging"
and "MacOS" → "macOS" wherever they appear in this file; run the language linter
after changes to confirm the tokens are resolved.
In `@docs/vendor/audible/raw/intro/getting_started.rst.txt`:
- Around line 18-19: Fix the incomplete sentence in the docs string "An overview
about all known Audible marketplaces and associated country codes be found at
:ref:`country_codes`." by inserting the missing word "can" so it reads "An
overview about all known Audible marketplaces and associated country codes can
be found at :ref:`country_codes`." Update that sentence in
docs/vendor/audible/raw/intro/getting_started.rst.txt and re-render the docs to
verify it reads naturally.
- Around line 81-83: Update the incorrect sentence "The response for the example
above are very minimized." to a grammatically correct, clearer phrasing such as
"The response for the example above is minimal." in the docs text that
references :http:get:`/1.0/library` and the `response_groups` parameter; ensure
the revised sentence uses singular verb agreement ("is") and plain wording so
the note about `response_groups` and library endpoint clarity is immediate when
read aloud.
In `@docs/vendor/audible/raw/intro/install.rst.txt`:
- Around line 174-176: The install instructions use a mismatched directory name:
the clone creates "audible" but the next step uses "cd Audible"; update the
second step in the snippet (the "cd Audible" entry) to "cd audible" so the
sequence matches the actual git clone target and works on case-sensitive
filesystems.
In `@docs/vendor/audible/raw/marketplaces/marketplaces.rst.txt`:
- Around line 8-15: Replace the two incorrect phrases in the marketplaces
overview: change "offers his service" to "offers its service" and change "which
marketplace are used" to "which marketplace is used" in the paragraph that
begins "Audible offers his service on 11 different marketplaces." Also rebuild
the docs and review that paragraph for plain-language readability to ensure the
grammar reads correctly.
- Around line 79-81: Edit the confusing guidance sentence about
locale/country-code: replace the fragment starting with "The locale argument
have..." (and the later "didn't renamed" / "'locale' than provide") with a
corrected sentence such as: "The locale argument has the same meaning as the
country code argument. For backward compatibility we haven't renamed the locale
argument yet, so if you are asked for a `locale` then provide a country code
from above." Update the sentence containing the phrases "The locale argument
have", "didn't renamed", and "'locale' than provide" accordingly and verify the
rendered docs read unambiguously.
In `@docs/vendor/audible/raw/misc/advanced.rst.txt`:
- Line 207: Update the misspellings in the docs: replace "converson" with
"conversion" and "ENPOINT" with "ENDPOINT" wherever they appear in this document
(notably the occurrences around the shown chunk and also the other hits at the
commented lines ~243 and ~249), then run your docs spellcheck/static linter to
confirm there are no remaining instances of those incorrect terms; ensure the
edited strings are exact matches so example identifiers remain valid.
- Around line 46-47: The request example uses invalid Python dict syntax: keys
"response_groups" and "num_results" are paired with values using "="; change
those assignments to ":" so the example becomes a valid dict literal (i.e.,
replace `"response_groups"="..."` with `"response_groups": "..."` and
`"num_results"=20` with `"num_results": 20`) and verify by pasting the snippet
into a Python REPL.
In `@docs/vendor/audible/raw/misc/async.rst.txt`:
- Around line 1-6: Replace the incorrect phrases in the document: change the
title "Asynchron requests" to "Asynchronous requests", update the body phrase
"asynchronous request" to "asynchronous requests", and change "a async Client"
to "an async client" (ensure consistent capitalization if you prefer "Async
client" or "async client"); then render the page to verify the heading and body
read naturally.
In `@docs/vendor/audible/raw/misc/examples.rst.txt`:
- Line 23: Replace the misspelled forms of "aggregated/aggregate/aggregation" in
the examples heading and comments — e.g., change the line that reads "Get
listening statistics aggragated month-over-month from 2021-03 to 2021-06::" to
use "aggregated", and apply the same correction to the related sentences on
lines 32-33 so all occurrences of "aggragated/aggragate/aggragation" are fixed
to "aggregated/aggregate/aggregation".
- Around line 27-30: Remove the redundant unmanaged client creation: delete the
standalone "client = audible.Client(auth)" so only the context-managed "with
audible.Client(auth=auth) as client:" remains; ensure the example still calls
"stats = client.get(...)" inside that with-block (using
audible.Authenticator.from_file for auth) and run the snippet to verify it
returns stats.
In `@docs/vendor/audible/raw/misc/external_api.rst.txt`:
- Line 469: The documented HTTP endpoint is missing a leading slash; update the
directive string `.. http:get:: 1.0/content/FairPlay/certificate` to include the
leading slash so it reads `.. http:get:: /1.0/content/FairPlay/certificate`,
ensuring the endpoint formatting matches the other docs and renders as an
absolute-style path.
- Around line 187-195: The Sphinx HTTP-domain parameter directive for the
endpoint /1.0/catalog/categories/(category_id) is misspelled: change the
malformed key ":quers products_not_in_plan_timestamp:" to the correct ":query
products_not_in_plan_timestamp:" in the docs so Sphinx correctly parses and
renders that parameter for the GET /1.0/catalog/categories/(category_id)
endpoint; after updating the directive, rebuild the docs to confirm the
parameter appears.
In `@docs/vendor/audible/raw/misc/load_save.rst.txt`:
- Around line 116-121: Fix two typos in the encryption docs: change "argumeent"
to "argument" in the `kdf_iterations` description and change "header consist" to
"header consists" in the sentence that begins "The header and the salt are
written..."; then re-render the paragraph to verify the wording reads clearly
and unambiguously.
In `@docs/vendor/audible/raw/README.md`:
- Around line 5-11: In docs/vendor/audible/raw/README.md replace machine-local
absolute paths with repo-relative references: change the manifest link to the
repo-relative path referencing docs/vendor/audible/raw/manifest.json and change
the refresh command to run from the repo root (invoke the script by its
repo-relative path, e.g., python scripts/sync_audible_raw_docs.py) so the README
and the sync_audible_raw_docs.py invocation work across environments and render
correctly on GitHub.
In `@docs/vendor/audible/README.md`:
- Around line 61-63: The quick-start commands clone and cd into
kingpaging/audiobook-automation and audiobook-automation, but later docs
reference the project root audiobook_dev; update the quick-start block so the
git clone URL and the subsequent cd target match the actual repository used by
this project (e.g., clone the correct repo that contains audiobook_dev or change
later references to audiobook-automation), then verify by running the exact
commands in a clean shell to ensure the directory name and repo path are
consistent (search for the quick-start block and occurrences of
"audiobook-automation" and "audiobook_dev" to update all instances).
- Around line 5-8: The README contains a hardcoded security status header ("🛡️
Security Status: ✅ VERIFIED") and fixed "Last Audit" line which can become
stale; replace these static strings by removing the fixed status text and audit
date and instead embed a dynamic CI/badge link (e.g., a shields.io badge or a
link to the project's security/CI pipeline) that reflects current security
checks, or omit the status entirely; update the README section that includes the
"🛡️ Security Status" heading and the "Last Audit" line so it reads as a
badge/link or a pointer to the CI results and ensure your CI updates that badge
or the linked report automatically after security runs.
In `@Makefile`:
- Around line 34-35: The Audible dependency list is missing Pillow (>=9.4.0)
which will cause runtime ImportError; update requirements.txt to include
"Pillow>=9.4.0" alongside the other Audible packages referenced by the
install-audible Makefile target (AUDIBLE_PIP_SPEC), and add a short note in your
docs or requirements header about the supported Python range (>=3.10,<3.14) or
otherwise verify runtime compatibility since the Makefile uses
--ignore-requires-python.
In `@requirements.txt`:
- Around line 26-29: The requirements.txt currently uses open lower bounds for
pbkdf2, pyaes, and rsa which risks drift for the mkb79/Audible (--no-deps)
install; either pin these three packages to the exact versions verified against
commit 458131b (replace >= entries with exact versions for pbkdf2, pyaes, rsa)
or add a constraints file locking their versions and reference it from
requirements.txt, and add a CI job that runs the Audible install pathway (e.g.,
run the equivalent of make install-audible in a fresh virtualenv/container) to
validate authenticated Audible initialization; ensure the CI step fails on any
mismatch so future changes to pbkdf2/pyaes/rsa are caught.
In `@src/audible_client.py`:
- Around line 82-103: aclose() currently iterates over self._clients and awaits
client.close() without guarding against exceptions, which can leak remaining
clients if one close() raises; update aclose() to mirror __aexit__'s safe
cleanup by iterating over list(self._clients.values()), wrapping each await
client.close() in a try/except that catches Exception and calls
log.warning("audible.client.close_error", error=str(close_exc)), then clear
self._clients; alternatively extract the loop into a private helper (e.g.,
_close_all_clients()) and have both aclose() and __aexit__ call that helper to
ensure consistent per-client error isolation and logging.
In `@src/audible_scraper.py`:
- Around line 33-37: The __aexit__ currently always calls
self._audible_client_provider.aclose(), which closes injected/shared providers;
update __init__ to record whether the provider was created internally (e.g., set
self._owns_audible_provider = True when you create a provider, False when one is
injected) and change __aexit__ to only await
self._audible_client_provider.aclose() if self._owns_audible_provider is True;
reference the constructor (__init__), the async context exit method (__aexit__),
and the member self._audible_client_provider, and add/adjust a unit test that
injects one AudibleClientProvider into two AudibleScraper instances, exits the
first, then verifies the second can still call search without closed-client
errors.
In `@src/metadata.py`:
- Around line 45-59: clean_series_sequence currently assumes sequence is a
string and passes it to re.search, which fails for numeric inputs; change
clean_series_sequence(signature and body) to accept sequence: str | int | float
(or Any) and coerce sequence = "" if falsy else str(sequence) before running
re.search, update the docstring/type hints accordingly, and ensure callers that
pass series_entry.get("position", "") (the code around the
seriesPrimary/seriesSecondary handling) either pass through the normalized
function or explicitly cast to str; add unit tests covering integer and decimal
positions to validate normalization for inputs like 1 and 1.5.
In `@tests/conftest.py`:
- Around line 304-317: The coordinator fixture creates mocks for
MetadataCoordinator adapters but doesn't set default return values, so tests may
receive raw MagicMocks; update the fixture (in the coordinator function) to
assign sensible defaults on coord.mam_adapter, coord.audnex, and coord.audible
for the common methods used across tests (e.g., set
coord.mam_adapter.fetch_metadata.return_value = {} or [] and
coord.mam_adapter.search.return_value = [], set
coord.audnex.get_metadata.return_value = {}, and set
coord.audible.scrape.return_value = {} or similar neutral values) so tests get
predictable primitive/collection responses unless they explicitly override them.
In `@tests/test_audible_client.py`:
- Around line 33-34: The test currently asserts
mock_from_file.assert_called_once_with(auth_file, password="test-password")
which is brittle because the provider may pass str(auth_file); update the
assertion to normalize the captured args: get the actual call from
mock_from_file.call_args, compare Path(called_path_arg) == auth_file for the
first positional arg, and assert the password kwarg equals "test-password"; keep
the separate assertion mock_async_client.assert_called_once_with(auth=mock_auth,
country_code="us") unchanged.
In `@tests/test_integration.py`:
- Around line 97-100: The patch for
MetadataCoordinator.get_metadata_from_webhook is patching an async method with a
plain return_value, which won't behave correctly; update the patch to use
new_callable=AsyncMock and set its return_value to {"title": "Rejection Book"}
so the async call is awaited correctly (refer to
MetadataCoordinator.get_metadata_from_webhook in the test where patch(...) is
used).
In `@tests/test_metadata_extended.py`:
- Around line 115-133: The test test_coordinator_fallback_to_audible_search
currently only mocks coordinator.audible.search_from_webhook_name, but
unconfigured mocks like mam_adapter or audnex may be truthy and prevent the
fallback; update the test so before calling
coordinator.get_metadata_from_webhook you explicitly set
coordinator.mam_adapter.get_metadata_from_webhook = AsyncMock(return_value=None)
and coordinator.audnex.lookup = AsyncMock(return_value=None) (or equivalent
methods on those adapters) to ensure they return None/empty and force the
coordinator to exercise the audible fallback path when calling
get_metadata_from_webhook.
- Around line 135-148: The test
test_coordinator_returns_none_when_no_metadata_found currently only mocks
audible.search_from_webhook_name; also mock the primary adapters (MAM and
Audnex) on the coordinator to return None so the
coordinator.get_metadata_from_webhook path for no metadata is exercised — e.g.,
set the coordinator.mam.* and coordinator.audnex.* async methods that the
coordinator calls (the primary lookup methods used by get_metadata_from_webhook)
to AsyncMock returning None before calling
coordinator.get_metadata_from_webhook.
In `@tests/test_security.py`:
- Around line 351-355: Patch the AUTOBRR_TOKEN environment value inside the test
before calling the webhook endpoint so the X-Autobrr-Token header value is
deterministic (e.g., set os.environ["AUTOBRR_TOKEN"] or use monkeypatch) and
then change the assertions on the client.post to expect a single explicit status
for the scenario under test (e.g., when Origin is omitted but token matches
expect 200; when token mismatches expect 401/403). Update the calls around the
client.post to run both cases (with and without Origin) under the fixed env
token and assert the explicit expected status codes rather than allowing [200,
401, 403]; reference the X-Autobrr-Token header usage in the failing client.post
and the AUTOBRR_TOKEN env var as the locations to modify.
---
Outside diff comments:
In `@src/metadata.py`:
- Around line 352-360: clean_metadata() currently sets result["author"] only
from clean_author_list(item.get("authors")) and overwrites/omits any scalar
author provided via normalize_book_result/base or the original item; change the
assignment for result["author"] to prefer the first entry from filtered_authors
but fall back to base.get("author") or item.get("author") when filtered_authors
is empty (use normalize_book_result(item) referenced as base and the
clean_author_list(...) path), and add a unit test that passes input with only
"author": "Brandon Sanderson" to ensure the compatibility wrapper preserves that
scalar author.
In `@tests/test_end_to_end.py`:
- Around line 294-307: The async test mock is fragile because the side_effect
_mock_fetch_metadata tries to guess args; replace the plain patch of
MetadataCoordinator.get_metadata_from_webhook with an AsyncMock that matches the
real method signature (e.g., async def signature(self, webhook_payload) or async
def signature(webhook_payload) depending on the bound/unbound method) and
implement its return logic against the explicit parameter (webhook_payload)
instead of inspecting args/kwargs; update the mock assignment from
mock_fetch.side_effect = _mock_fetch_metadata to using
AsyncMock(side_effect=...) or AsyncMock(return_value=...) bound to
MetadataCoordinator.get_metadata_from_webhook so the test behavior follows the
actual API.
In `@tests/test_error_recovery.py`:
- Around line 125-149: The test_concurrent_error_handling test is sequential and
doesn't exercise real concurrency; change it to send the five requests in
parallel (e.g., use ThreadPoolExecutor or pytest-asyncio with an async test
client) instead of looping calling self.client.post synchronously, ensure the
mocked coordinator handler used by /webhook/audiobook-requests blocks long
enough (use an Event or Barrier in the mock) so requests overlap, and add an
in-flight counter or synchronized probe to assert at least two requests were
concurrently active while still asserting all five complete and no unhandled
exceptions occur.
In `@tests/test_integration.py`:
- Around line 29-46: The async method
MetadataCoordinator.get_metadata_from_webhook is being patched with plain
patch() which yields a synchronous MagicMock causing TypeError when awaited;
update tests/test_integration.py to import AsyncMock from unittest.mock and
replace each
patch("src.metadata_coordinator.MetadataCoordinator.get_metadata_from_webhook")
usage with patch(..., new_callable=AsyncMock) (for all four occurrences) so the
mock is awaitable and mock return values are applied correctly.
In `@tests/test_security.py`:
- Around line 203-211: The test currently tolerates server errors and swallows
unexpected exceptions: in the block that posts via
self.client.post("/webhook/audiobook-requests", json=malicious_json,
headers={"X-Autobrr-Token": "test_token"}) remove 500 from the allowed
resp.status_code list so only expected client-side validation codes remain
(e.g., 200, 400, 422), and tighten the exception handling by removing the broad
try/except or replacing it with a specific exception assertion (e.g., assert
raises JSONDecodeError or let the test fail on any uncaught Exception) so
unexpected server crashes cause the test to fail. Ensure changes target the resp
assertion and the surrounding try/except block in tests/test_security.py.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c16c99c-c197-462a-8f42-87d1aa383720
📒 Files selected for processing (53)
.env.exampleMakefileREADME.mdconfig/config.yaml.exampledocs/PR1_REVIEW_FIXES.mddocs/SYSTEM_COMPLETION_SUMMARY.mddocs/api/config-reference.mddocs/user-guide/configuration.mddocs/vendor/audible/.env.exampledocs/vendor/audible/README.mddocs/vendor/audible/authentication.mddocs/vendor/audible/config/config.yaml.exampledocs/vendor/audible/docs/README.mddocs/vendor/audible/docs/SYSTEM_COMPLETION_SUMMARY.mddocs/vendor/audible/docs/api/config-reference.mddocs/vendor/audible/docs/user-guide/configuration.mddocs/vendor/audible/docs/user-guide/troubleshooting.mddocs/vendor/audible/examples.mddocs/vendor/audible/external-api.mddocs/vendor/audible/getting-started.mddocs/vendor/audible/overview.mddocs/vendor/audible/raw/README.mddocs/vendor/audible/raw/auth/authentication.rst.txtdocs/vendor/audible/raw/auth/authorization.rst.txtdocs/vendor/audible/raw/auth/register.rst.txtdocs/vendor/audible/raw/index.rst.txtdocs/vendor/audible/raw/intro/getting_started.rst.txtdocs/vendor/audible/raw/intro/install.rst.txtdocs/vendor/audible/raw/manifest.jsondocs/vendor/audible/raw/marketplaces/marketplaces.rst.txtdocs/vendor/audible/raw/misc/advanced.rst.txtdocs/vendor/audible/raw/misc/async.rst.txtdocs/vendor/audible/raw/misc/changelog.md.txtdocs/vendor/audible/raw/misc/examples.rst.txtdocs/vendor/audible/raw/misc/external_api.rst.txtdocs/vendor/audible/raw/misc/load_save.rst.txtdocs/vendor/audible/raw/misc/logging.rst.txtdocs/vendor/audible/raw/modules/audible.rst.txtrequirements.txtsrc/audible_client.pysrc/audible_scraper.pysrc/main.pysrc/metadata.pytests/conftest.pytests/test_audible_client.pytests/test_audible_scraper.pytests/test_end_to_end.pytests/test_error_recovery.pytests/test_integration.pytests/test_main_integration.pytests/test_metadata_extended.pytests/test_security.pytests/test_utils_metadata.py
- Updated paths in SYSTEM_COMPLETION_SUMMARY.md for Audible scraper. - Clarified auth_file requirement in configuration.md for authenticated Audible lookups. - Enhanced security notes in .env.example regarding file permissions. - Improved README.md with CI badges and updated security status. - Fixed minor typos and formatting issues in various documentation files. - Updated example configurations and added clarity to the installation instructions. - Enhanced error handling and logging in audible_client.py and audible_scraper.py. - Improved test coverage for Audible client and scraper functionalities. - Refactored metadata.py to handle series sequence normalization more robustly. - Adjusted test cases for better async handling and error recovery. Hardens optional Audible backend and CI Keeps CI aligned with currently supported Python versions and adds a dedicated install-path verification so optional backend packaging regressions fail fast. Makes the backend dependency optional at runtime, pins core support packages for reproducible no-deps installs, improves client-close error handling, and avoids closing injected shared providers. Improves metadata normalization for numeric series values and scalar author fallback, then updates tests and docs to reflect authenticated-lookup requirements, stronger security expectations, and clearer setup guidance. Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/user-guide/configuration.md (1)
156-156:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCall
load_config()in the validation command.
- Issue: Line 156 only imports
load_configand prints success.- Impact: Broken configs can be reported as valid.
- Fix: Invoke
load_config()before printing.- Test: Intentionally break
config/config.yaml; this command should fail non-zero.Suggested doc patch
-python -c "from src.config import load_config; print('✅ Config valid')" +python -c "from src.config import load_config; load_config(); print('✅ Config valid')"🤖 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 `@docs/user-guide/configuration.md` at line 156, The validation command currently only imports load_config and prints success; change it to call load_config() and propagate failures by exiting non‑zero on exception. Update the one‑liner to invoke load_config() (from src.config import load_config; load_config()) and ensure any exception is not swallowed so the process returns a non‑zero exit code (or explicitly catch exceptions and call sys.exit(1) with an error message) so broken configs cause the command to fail.src/audible_scraper.py (1)
282-287:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAccept ISO English codes before dropping search hits.
- Issue: Lines 282-287 and 304-307 only treat
"english"as English.- Impact: Products returned with values like
"en"get filtered out, so valid English matches disappear from both the primary results and the Audnex fallback.- Fix: Normalize language through a small allowlist/helper, e.g.
{"english", "en", "en-us", "en-gb"}.- Test: Add coverage with an Audible/Audnex product whose
languageis"en"and assert it is retained.Possible fix
+ english_languages = {"english", "en", "en-us", "en-gb"} + for product in products: # Only include English books language = product.get("language", "").lower() - if language and language != "english": + if language and language not in english_languages: log.debug("audible.search.skip_non_english", language=language) continue @@ - if metadata and metadata.get("language", "").lower() == "english": + if metadata and metadata.get("language", "").lower() in english_languages: audnex_book = self._product_to_book(metadata)As per coding guidelines,
**/*.py: Focus on: Error handling patterns (especially for API calls and file operations).Also applies to: 304-307
🤖 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/audible_scraper.py` around lines 282 - 287, Normalize and accept common ISO English codes instead of only "english": add an allowlist set like {"english","en","en-us","en-gb"} and a small helper (e.g., is_english_language(language: str)) that normalizes and checks membership; replace the current check in the products loop (the language = product.get("language", "").lower() block that logs "audible.search.skip_non_english") and the similar block at the later location (around the other 304-307 check) to call this helper and only skip when it returns False; also add a unit test that constructs an Audible/Audnex product with language "en" and asserts it is retained.
♻️ Duplicate comments (2)
docs/vendor/audible/raw/README.md (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix manifest link target in README (Line 5).
- Issue: Link path is written as
docs/vendor/audible/raw/manifest.jsonfrom a file already inside that directory.- Impact: The rendered relative URL can resolve incorrectly and break navigation in GitHub/docs viewers.
- Fix: Use a directory-relative link:
manifest.json.- Test: Open the README on GitHub and click manifest.json; it should resolve to
docs/vendor/audible/raw/manifest.json.Suggested patch
-Use [manifest.json](docs/vendor/audible/raw/manifest.json) to see the synced source URLs and local file paths. +Use [manifest.json](manifest.json) to see the synced source URLs and local file paths.🤖 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 `@docs/vendor/audible/raw/README.md` at line 5, Update the README link so it uses a directory-relative target instead of repeating the current path: replace the link text that points to "docs/vendor/audible/raw/manifest.json" with just "manifest.json" in the README (the sentence "Use [manifest.json](...) to see the synced source URLs and local file paths."); this ensures the rendered link correctly resolves to the manifest in the same directory when viewed on GitHub or docs viewers.docs/vendor/audible/raw/misc/examples.rst.txt (1)
12-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDefine
filenamebeforeAuthenticator.from_file(...)in both examples.
- Issue: Line 12 and Line 27 use
filenamewithout defining it.- Impact: Snippets fail immediately with
NameErrorwhen users copy/paste.- Fix: Add an explicit file path variable in each standalone snippet (or inline string literal).
- Test: Paste each snippet into a REPL/script and verify execution proceeds past auth initialization.
Suggested patch
Print number of books for every marketplace:: import audible + filename = "path/to/credentials.json" auth = audible.Authenticator.from_file(filename) client = audible.Client(auth) country_codes = ["de", "us", "ca", "uk", "au", "fr", "jp", "it", "in"] @@ Get listening statistics aggregated month-over-month from 2021-03 to 2021-06:: import audible + filename = "path/to/credentials.json" auth = audible.Authenticator.from_file(filename) with audible.Client(auth=auth) as client: stats = client.get(Also applies to: 27-27
🤖 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 `@docs/vendor/audible/raw/misc/examples.rst.txt` at line 12, The examples call audible.Authenticator.from_file(filename) but never define filename, causing NameError; before each call to Authenticator.from_file (both occurrences around the examples using Authenticator.from_file and any related auth snippets) add a definition for filename (e.g., filename = "path/to/credentials.json") or replace filename with an inline string literal so the snippet is self-contained and can be pasted into a REPL/script and run past auth initialization.
🤖 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 `@docs/vendor/audible/docs/user-guide/troubleshooting.md`:
- Around line 69-71: Replace the inline literal token in the curl example that
calls the /webhook/test endpoint with a safe reference: change the Authorization
header to use the environment variable (e.g., $AUTOBRR_TOKEN) or show a secure
prompt flow using read -rsp to populate AUTOBRR_TOKEN before running curl;
update the example in the docs to reference AUTOBRR_TOKEN and add a short note
to run the command after exporting/reading the variable so users can verify the
request succeeds without exposing the token.
In `@docs/vendor/audible/raw/marketplaces/marketplaces.rst.txt`:
- Around line 86-87: Replace the ungrammatical sentence fragment "How to use
these marketplace with a previous version read `this comment <...>`_." with a
clear, direct instruction such as "To use this marketplace with an earlier
Audible version, see `this comment
<https://github.com/mkb79/Audible/issues/194#issuecomment-1728896926>`_." Update
the marketplace note in the same paragraph where the original fragment appears
so it reads fluently, then rebuild the docs to confirm the rendered output
displays the corrected sentence.
In `@docs/vendor/audible/raw/misc/advanced.rst.txt`:
- Around line 60-62: Fix the malformed sentence in the response-conversion
description: replace the broken fragment "converts them to a and output them as
a Python dict" with a clear sentence such as "The client converts API JSON
responses into Python dicts and returns/outputs them." Update the line in the
same paragraph (the sentence containing "converts them to a and output them") so
the rendered doc reads smoothly and verify the section renders correctly.
In `@docs/vendor/audible/raw/misc/external_api.rst.txt`:
- Line 447: Fix the typo in the response sentence "For a succesful request,
returns JSON body with `content_url`." by changing "succesful" to "successful"
so it reads "For a successful request, returns JSON body with `content_url`.";
update the string in docs/vendor/audible/raw/misc/external_api.rst.txt (search
for the exact sentence) and then rebuild the docs to verify the rendered text
shows the corrected word.
- Around line 591-592: Remove the duplicate parameter entry by deleting one of
the consecutive ":query category_image_variants:" lines so only a single ":query
category_image_variants:" remains in the parameters list; locate the repeated
token "category_image_variants" in the external_api.rst.txt block and remove the
redundant occurrence to ensure the parameter list renders cleanly.
In `@requirements.txt`:
- Around line 27-31: The project currently bypasses Python version checks with
--ignore-requires-python for the mkb79/Audible dependency (pinned commit) which
may break at runtime on Python 3.14; update the build to restrict supported
Python to 3.11–3.13 (remove or stop using --ignore-requires-python in the
Makefile/requirements handling and ensure metadata requires-python remains
<=3.13) and add a CI matrix job that runs on each supported interpreter (3.11,
3.12, 3.13) that imports the Audible client and exercises its authenticated
initialization path (e.g., the module/class used to create the authenticated
client in your repo—call its init/connect method) to validate runtime behavior
before allowing 3.14 usage.
- Line 20: Update the Pillow requirement to eliminate vulnerable versions: in
requirements.txt replace the current "Pillow>=9.4.0" constraint with
"Pillow>=12.2.0" so the minimum installed Pillow is 12.2.0 or later; after
changing the requirement, re-run your dependency scanning
(OSV/dependency-review) and any CI dependency resolution to verify the resolved
dependency set contains no Pillow CVEs.
In `@src/audible_client.py`:
- Around line 97-120: Serialize teardown by using the same creation lock and a
shutdown flag: add a boolean attribute (e.g., _shutting_down) and set it under
the existing lock before closing; in _close_all_clients() and aclose() acquire
the same lock used by get_client(), set _shutting_down=True, then iterate and
await client.close() while still synchronized, clear _clients, and release the
lock; update get_client() to check _shutting_down under the lock and raise or
refuse creation if shutdown is in progress so no new clients are cached during
teardown; add a concurrent race test that runs get_client("us") and aclose()
simultaneously and asserts _clients is empty after completion.
In `@src/audible_scraper.py`:
- Around line 342-349: The ASIN lookup is using a different request shape than
search_by_title_author; change the audible_client.get call (inside
_get_audible_library_client usage in this block) to call the same endpoint
string with a leading slash (use self.search_endpoint or
"/1.0/catalog/products") and pass the ASIN and response_groups inside a params
dict (e.g. params={'asin': asin, 'response_groups':
'contributors,media,product_attrs,product_desc,product_details,product_extended_attrs,series,rating,category_ladders'})
instead of sending response_groups as a keyword argument and embedding ASIN in
the path, so the request matches search_by_title_author's parameter handling.
In `@src/metadata.py`:
- Line 163: The current expression for "publishedYear" calls
release_date.split("-") and can raise AttributeError if release_date is a
non-string truthy value; change the logic where "publishedYear" is computed to
only call split when isinstance(release_date, str) (e.g., use
release_date.split("-")[0] if isinstance(release_date, str) and release_date
else item.get("publishedYear") or None) so non-string values fall back to
item.get("publishedYear") or None; also add unit tests covering non-string
releaseDate (e.g., integer timestamp and other types) to ensure normalization
doesn't crash.
- Around line 153-156: The current check using str(runtime_length_min).isdigit()
rejects decimal strings like "360.5", so change the validation for
runtime_length_min in metadata.py to accept integers and decimals: attempt to
cast runtime_length_min to float (e.g., float(runtime_length_min)) inside a
try/except, then set duration = int(float_value) or round as desired (truncating
via int is fine) and only fall back to 0 on ValueError/TypeError; update any
code paths that read runtimeLengthMin to use the same parsing logic and add unit
tests asserting behavior for inputs 360, "360.5", and 360.5.
---
Outside diff comments:
In `@docs/user-guide/configuration.md`:
- Line 156: The validation command currently only imports load_config and prints
success; change it to call load_config() and propagate failures by exiting
non‑zero on exception. Update the one‑liner to invoke load_config() (from
src.config import load_config; load_config()) and ensure any exception is not
swallowed so the process returns a non‑zero exit code (or explicitly catch
exceptions and call sys.exit(1) with an error message) so broken configs cause
the command to fail.
In `@src/audible_scraper.py`:
- Around line 282-287: Normalize and accept common ISO English codes instead of
only "english": add an allowlist set like {"english","en","en-us","en-gb"} and a
small helper (e.g., is_english_language(language: str)) that normalizes and
checks membership; replace the current check in the products loop (the language
= product.get("language", "").lower() block that logs
"audible.search.skip_non_english") and the similar block at the later location
(around the other 304-307 check) to call this helper and only skip when it
returns False; also add a unit test that constructs an Audible/Audnex product
with language "en" and asserts it is retained.
---
Duplicate comments:
In `@docs/vendor/audible/raw/misc/examples.rst.txt`:
- Line 12: The examples call audible.Authenticator.from_file(filename) but never
define filename, causing NameError; before each call to Authenticator.from_file
(both occurrences around the examples using Authenticator.from_file and any
related auth snippets) add a definition for filename (e.g., filename =
"path/to/credentials.json") or replace filename with an inline string literal so
the snippet is self-contained and can be pasted into a REPL/script and run past
auth initialization.
In `@docs/vendor/audible/raw/README.md`:
- Line 5: Update the README link so it uses a directory-relative target instead
of repeating the current path: replace the link text that points to
"docs/vendor/audible/raw/manifest.json" with just "manifest.json" in the README
(the sentence "Use [manifest.json](...) to see the synced source URLs and local
file paths."); this ensures the rendered link correctly resolves to the manifest
in the same directory when viewed on GitHub or docs viewers.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 164be5d0-e586-4625-b808-cf9ce81578b8
📒 Files selected for processing (34)
.circleci/config.yml.env.example.github/workflows/ci.ymldocs/SYSTEM_COMPLETION_SUMMARY.mddocs/user-guide/configuration.mddocs/vendor/audible/.env.exampledocs/vendor/audible/README.mddocs/vendor/audible/config/config.yaml.exampledocs/vendor/audible/docs/README.mddocs/vendor/audible/docs/user-guide/configuration.mddocs/vendor/audible/docs/user-guide/troubleshooting.mddocs/vendor/audible/examples.mddocs/vendor/audible/raw/README.mddocs/vendor/audible/raw/auth/authorization.rst.txtdocs/vendor/audible/raw/intro/getting_started.rst.txtdocs/vendor/audible/raw/intro/install.rst.txtdocs/vendor/audible/raw/marketplaces/marketplaces.rst.txtdocs/vendor/audible/raw/misc/advanced.rst.txtdocs/vendor/audible/raw/misc/async.rst.txtdocs/vendor/audible/raw/misc/examples.rst.txtdocs/vendor/audible/raw/misc/external_api.rst.txtdocs/vendor/audible/raw/misc/load_save.rst.txtpyproject.tomlrequirements.txtsrc/audible_client.pysrc/audible_scraper.pysrc/metadata.pytests/conftest.pytests/test_audible_client.pytests/test_audible_scraper.pytests/test_end_to_end.pytests/test_error_recovery.pytests/test_metadata_extended.pytests/test_security.py
…t shutdown handling, and update documentation for configuration and troubleshooting Co-authored-by: Copilot <copilot@github.com>
…r auth settings and enhance test coverage for explicit auth configurations Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/user-guide/configuration.md`:
- Around line 117-125: The docs mention "the current Python 3.14 environment"
but the project requires ">=3.11,<3.14"; update the sentence in
docs/user-guide/configuration.md that follows the Audible installation note to
instead reference the supported Python range "3.11–3.13" (or "Python >=3.11,
<3.14") and keep the surrounding references to AUDIBLE_AUTH_FILE,
AUDIBLE_AUTH_FILE_PASSWORD, and Authenticator.from_file(...) intact; then grep
the repository docs for any other occurrences of "3.14" and replace them with
the correct supported range so the documentation matches pyproject.toml and CI.
In `@docs/vendor/audible/docs/user-guide/troubleshooting.md`:
- Around line 229-231: Replace the literal placeholder
"YOUR_DISCORD_WEBHOOK_URL" in the curl example with an environment variable
reference (e.g., use an ENV-style token like $DISCORD_WEBHOOK or
{{DISCORD_WEBHOOK}} consistent with the Pushover example) and add a one-line
warning immediately after the snippet telling users not to paste or share their
real webhook URL in public or support channels; update the example line that
contains YOUR_DISCORD_WEBHOOK_URL so readers are guided to use the env variable
instead and see the explicit caution.
In `@docs/vendor/audible/raw/marketplaces/marketplaces.rst.txt`:
- Around line 85-87: Inside the '.. note::' block in marketplaces.rst.txt the
middle line "To use this marketplace with an earlier Audible version, see `this
comment
<https://github.com/mkb79/Audible/issues/194#issuecomment-1728896926>`_." is
indented less than the surrounding note lines; fix it by increasing that line's
leading whitespace to match the other lines in the same note directive so all
three lines are the same indentation and remain part of the note body, then
rebuild the docs to confirm the note renders as a single paragraph.
In `@docs/vendor/audible/raw/misc/examples.rst.txt`:
- Around line 26-30: The second example has an extra leading space before the
assignment to filename which causes indentation errors when copied; edit the
snippet so the line assigning filename aligns with the surrounding lines (remove
the extra leading whitespace) where the variable filename and the call
audible.Authenticator.from_file(filename) appear, ensuring the subsequent with
audible.Client(auth=auth) as client: line remains at the same indentation level.
- Around line 10-14: The example snippet has inconsistent indentation causing an
IndentationError: remove the extra leading whitespace before the filename
assignment so that the line "filename = ..." is aligned with "import audible",
"auth = audible.Authenticator.from_file(filename)" and "client =
audible.Client(auth)"; verify by copying the adjusted snippet into a Python REPL
and confirming that the variable filename and subsequent calls to
audible.Authenticator.from_file and audible.Client execute without syntax
errors.
In `@src/audible_scraper.py`:
- Around line 99-104: The _is_english_language function currently checks
membership in a small set and will miss valid English locale variants; update
src/audible_scraper.py's _is_english_language(language: str | None) to first
validate type as before then normalize and return True for any language string
where .strip().lower() either equals "english" or startswith "en" (e.g., use
startswith("en") to cover "en-au", "en-ca", "en-in", etc.); update or add unit
tests to include "en-au" and "en-ca" (and a non-English negative case) to verify
the behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 613a74eb-6c23-440a-b9d7-5803ef6b4360
📒 Files selected for processing (17)
.github/workflows/ci.ymlMakefiledocs/user-guide/configuration.mddocs/vendor/audible/docs/user-guide/troubleshooting.mddocs/vendor/audible/raw/README.mddocs/vendor/audible/raw/marketplaces/marketplaces.rst.txtdocs/vendor/audible/raw/misc/advanced.rst.txtdocs/vendor/audible/raw/misc/examples.rst.txtdocs/vendor/audible/raw/misc/external_api.rst.txtpyproject.tomlrequirements.txtsrc/audible_client.pysrc/audible_scraper.pysrc/metadata.pytests/test_audible_client.pytests/test_audible_scraper.pytests/test_metadata_extended.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/audible_client.py`:
- Around line 75-77: The warning currently logs the full auth path
(self.auth_file) which may leak user/home info; modify the block that checks
auth_path.exists() so the log call uses a redacted hint (e.g., auth_path.name or
a fixed placeholder) instead of the absolute path from self.auth_file, and
update/add a unit test asserting the emitted warning text does not contain the
full absolute path string; look for the auth_path and self.auth_file usage in
the missing-file branch to implement the change.
In `@tests/test_audible_client.py`:
- Around line 55-65: Update the
test_get_client_returns_none_without_decrypt_password test to ensure the
missing-password short-circuit truly runs by patching the external Audible
entrypoints and asserting they are not invoked: mock Authenticator.from_file and
httpx.AsyncClient (or the AsyncClient import used by AudibleClientProvider)
before calling AudibleClientProvider.get_client("us"), then assert both mocked
callables were never awaited/called so the test fails if the early return in
AudibleClientProvider.get_client is removed; reference the test name
test_get_client_returns_none_without_decrypt_password and the provider method
get_client as the targets to change.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c38d0e89-beaf-4f6b-9fb3-c457b0bddd6c
📒 Files selected for processing (2)
src/audible_client.pytests/test_audible_client.py
…ng for missing auth file path and add tests for language validation
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/vendor/audible/docs/user-guide/configuration.md`:
- Line 196: The checklist item currently states AUDIBLE_AUTH_FILE and
AUDIBLE_AUTH_FILE_PASSWORD set unconditionally; change the text to make these
environment variables conditional (e.g., "If using authenticated Audible
lookups: AUDIBLE_AUTH_FILE and AUDIBLE_AUTH_FILE_PASSWORD set") so it matches
the earlier description of auth_file as optional; update the checklist line
referencing AUDIBLE_AUTH_FILE and AUDIBLE_AUTH_FILE_PASSWORD to include the
conditional qualifier used elsewhere ("if using authenticated Audible lookups"
or "if using auth_file") and ensure wording matches line 64's "Optional
encrypted auth file."
In `@tests/test_audible_scraper.py`:
- Around line 155-159: Update the test function
test_is_english_language_accepts_common_locale_variants to include edge cases
for AudibleScraper._is_english_language: add assertions that passing None
returns False, passing an empty string "" returns False, and passing an
uppercase locale like "EN-US" returns True (to validate normalization). Keep
these assertions alongside the existing checks so the test covers both common
variants and boundary inputs.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 76e2729a-68c0-4f64-b475-35b1fc51f313
📒 Files selected for processing (7)
docs/user-guide/configuration.mddocs/vendor/audible/docs/user-guide/configuration.mddocs/vendor/audible/docs/user-guide/troubleshooting.mdsrc/audible_client.pysrc/audible_scraper.pytests/test_audible_client.pytests/test_audible_scraper.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
What it improves
Quality and test impact
Summary by CodeRabbit
New Features
Configuration
Documentation
Refactor
Tests
Chores