fix: avoid validating forest car twice in AnyCar::new

[hanabi1224]

Summary of changes

Came accross this while investigating #6733 (but this does not fix the issue)

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

• I have performed a self-review of my own code,
• I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
• I have added tests that prove my fix is effective or that my feature works (if possible),
• I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

• I have read and agree to the CONTRIBUTING document.
• I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

• Refactor
  • Simplified CAR detection and construction flow for clearer handling, including forest-format CARs.
• New Features
  • Added a public helper to load a single CAR file directly.
• Bug Fixes
  • Improved error reporting when loading CAR files to include the file path in failure messages.
• Tests
  • Relaxed test assertion to tolerate minor formatting differences in error output.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a3de6fa0-2b6d-49bc-a036-cd3758bf83a8

📥 Commits

Reviewing files that changed from the base of the PR and between eeb6a22 and 0a26121.

📒 Files selected for processing (1)

• tests/tool_tests.rs

---

Walkthrough

Refactors CAR validation/construction: ForestCar now validates first and exposes validate_car plus new_from_validation_result; AnyCar::new uses validate_car/new_from_validation_result to detect/construct .forest.car.zst; ManyCar adds a read_only_file helper that wraps file-load errors with context. Tests relax an stderr exact-match to a contains check.

Changes

Cohort / File(s)	Summary
ForestCar refactor src/db/car/forest.rs	Extracted validation from ForestCar::new; added pub(super) fn new_from_validation_result(reader, (header, index_start_pos, index_size_bytes)) to construct from validation output; changed validate_car visibility to pub(super) for reuse.
AnyCar integration src/db/car/any.rs	AnyCar::new now calls ForestCar::validate_car to detect/parse .forest.car.zst and constructs forest CAR via new_from_validation_result(...); simplified plain CAR conversion using .into().
ManyCar helper & error context src/db/car/many.rs	Added pub fn read_only_file(&self, file: impl AsRef<Path>) -> anyhow::Result<()> and delegated per-file loading in read_only_files to this helper, adding contextual error messages on open/load failures.
Test update tests/tool_tests.rs	Relaxed stderr assertion in export_empty_archive test from exact equality to substring containment (predicate::str::contains(...)).

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: support large index in forest car #6690 — similar changes to ForestCar construction/validation and validate_car visibility.

Suggested reviewers

• LesnyRumcajs
• akaladarshi

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: avoid validating forest car twice in AnyCar::new' directly and clearly describes the main change: refactoring AnyCar::new to use a validation result from ForestCar::validate_car instead of calling ForestCar::is_valid and then ForestCar::new, which were redundantly validating the CAR file.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 hm/fix-dup-validation-in-anycar

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch hm/fix-dup-validation-in-anycar

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/db/car/many.rs (1)

133-140: Use .with_context() to preserve the error chain.

The current pattern with map_err(|e| anyhow::anyhow!("...: {e}", ...)) loses the original error chain because {e} only captures the Display output, not the underlying causes. Using .with_context() preserves the full error chain for better debugging.

♻️ Suggested refactor

     pub fn read_only_file(&self, file: impl AsRef<Path>) -> anyhow::Result<()> {
-        (|| {
-            self.read_only(AnyCar::new(EitherMmapOrRandomAccessFile::open(
-                file.as_ref(),
-            )?)?)
-        })()
-        .map_err(|e| anyhow::anyhow!("failed to load CAR at {}: {e}", file.as_ref().display()))
+        self.read_only(AnyCar::new(EitherMmapOrRandomAccessFile::open(
+            file.as_ref(),
+        )?)?)
+        .with_context(|| format!("failed to load CAR at {}", file.as_ref().display()))
     }

As per coding guidelines: "Use anyhow::Result<T> for most operations and add context with .context() when errors occur".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/db/car/many.rs` around lines 133 - 140, The current read_only_file
implementation replaces the original error chain by mapping errors into a new
anyhow error string; update read_only_file so that failures from
EitherMmapOrRandomAccessFile::open, AnyCar::new, or self.read_only are
propagated with context instead of being recreated: call .context(...) (or
.with_context(...)) on the fallible operations inside read_only_file (around the
EitherMmapOrRandomAccessFile::open and/or the outer closure) to attach the
"failed to load CAR at <path>" message while preserving the original error chain
for read_only, AnyCar::new and EitherMmapOrRandomAccessFile::open.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/db/car/many.rs`:
- Around line 133-140: The current read_only_file implementation replaces the
original error chain by mapping errors into a new anyhow error string; update
read_only_file so that failures from EitherMmapOrRandomAccessFile::open,
AnyCar::new, or self.read_only are propagated with context instead of being
recreated: call .context(...) (or .with_context(...)) on the fallible operations
inside read_only_file (around the EitherMmapOrRandomAccessFile::open and/or the
outer closure) to attach the "failed to load CAR at <path>" message while
preserving the original error chain for read_only, AnyCar::new and
EitherMmapOrRandomAccessFile::open.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 011e7b61-2647-465c-9eeb-dd9f7a1a61a8

📥 Commits

Reviewing files that changed from the base of the PR and between 33205b3 and eeb6a22.

📒 Files selected for processing (1)

• src/db/car/many.rs

[codecov]

Codecov Report

❌ Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.71%. Comparing base (9dc4711) to head (0a26121).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/db/car/many.rs	75.00%	0 Missing and 2 partials ⚠️
src/db/car/any.rs	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/db/car/forest.rs	83.18% <100.00%> (+0.30%)	⬆️
src/db/car/any.rs	67.64% <66.66%> (ø)
src/db/car/many.rs	67.40% <75.00%> (+0.73%)	⬆️

... and 4 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9dc4711...0a26121. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.