Handle newer Goodreads export formats while still allowing old ones

[IgnisDa]

The three provider id_from_isbn methods silently swallowed HTTP request and JSON parse errors via .ok()? before returning None, making it impossible to diagnose why an ISBN lookup failed.

Replace the .ok()? calls with explicit match arms that log the actual error via tracing::warn! before returning None. Also add #[tracing::instrument] to the coordinating function get_identifier_from_book_isbn so the ISBN value is captured in the span and each provider failure is logged at the call site.

Summary by CodeRabbit

• Bug Fixes
  • Improved Goodreads imports by reliably cleaning and validating ISBN values, including quoted and formula-formatted entries.
  • Added support for multiple Goodreads date formats when importing read dates.
  • Prevented invalid date values from interrupting the import process.
  • Adjusted record processing to improve import stability.

[coderabbitai]

Walkthrough

Goodreads imports now limit concurrent processing, normalize and validate ISBN values, and safely parse multiple date_read formats. A Trakt list-item fetch call was reformatted without changing behavior.

Changes

Goodreads import processing

Layer / File(s)	Summary
Book record processing crates/services/importer/goodreads/src/lib.rs	Processing concurrency is reduced, ISBN values are cleaned and validated, and date_read supports year-first and month-first formats without unwrapping parse failures.

Trakt request formatting

Layer / File(s)	Summary
List-item fetch formatting crates/services/importer/trakt/src/lib.rs	The list-item fetch_json call is compacted while preserving its URL, arguments, and result handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main Goodreads import changes, especially support for newer export date formats while preserving older ones.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch debug-issues

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@crates/utils/dependent/provider/src/lib.rs`:
- Line 160: Update the tracing instrument annotation on the provider function to
explicitly record the `isbn` argument value, rather than only declaring an empty
field. Preserve `skip_all` and ensure nested provider warnings retain the ISBN
context.

🪄 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 Plus

Run ID: f7c2c2a6-274c-4565-9c73-f665aa9b4e52

📥 Commits

Reviewing files that changed from the base of the PR and between ae3d965 and 337ee33.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (7)

• crates/providers/google-books/Cargo.toml
• crates/providers/google-books/src/lib.rs
• crates/providers/hardcover/src/base.rs
• crates/providers/openlibrary/src/client.rs
• crates/services/importer/trakt/src/lib.rs
• crates/utils/dependent/provider/Cargo.toml
• crates/utils/dependent/provider/src/lib.rs

[coderabbitai]

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 `@crates/services/importer/goodreads/src/lib.rs`:
- Around line 153-162: Guard the ended_on assignment in the date_read parsing
block by handling an empty seen_history before calling first_mut().unwrap(). For
read_count == 0, reject the inconsistent row as InputTransformation or otherwise
follow the existing empty-history handling path; only assign ended_on when a
first history entry exists.
- Around line 107-120: Update the ISBN validation near the `isbn` normalization
and before `get_identifier_from_book_isbn` so non-empty values must contain
exactly 13 ASCII digits, or apply the repository’s intended ISBN check-digit/GS1
validation. Preserve the existing `ImportFailedItem` error path and prevent
malformed values such as `123` from reaching provider lookup.

🪄 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 Plus

Run ID: cc7801fb-7698-4b7d-9494-35f7529727e7

📥 Commits

Reviewing files that changed from the base of the PR and between 337ee33 and f951b38.

📒 Files selected for processing (1)

• crates/services/importer/goodreads/src/lib.rs