refactor(ingestion): extract materializer artifact guard

[johnjhughes]

Summary

• move the shared materializer artifact file guard into the importable materializer domain module
• preserve the CLI boundary with a thin CommandError adapter, unchanged messages, and retained exception causes
• add direct domain coverage for ordinary files, symlinks, missing paths, directories, and hardlinks

Validation

• uv run ruff format --check .
• uv run ruff check .
• uv run pyright
• uv run pytest -q tests/test_cohort_document_materializer.py (56 passed)
• uv run pytest -q (6113 passed, 13 skipped)

Bead: LegalForecastBench-cwdv.5

---

Note

Low Risk
Behavior-preserving refactor with expanded tests; validation rules and CLI messages are unchanged.

Overview
Moves cohort materializer artifact path validation (no symlink components, regular file, single hard link) from the CLI into require_materializer_artifact in cohort_document_materializer.

The CLI keeps _require_materializer_artifact as a thin wrapper that maps CohortDocumentMaterializationError to CommandError with the same user-facing messages and exception chaining.

Adds direct unit tests for valid files, symlinks, missing paths, directories, hardlinks, and the CLI error contract.

^{Reviewed by Cursor Bugbot for commit 0a37acd. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes

  • Improved validation of materialized document artifacts.
  • Invalid paths, symlinks, directories, missing files, and hardlinked files now produce consistent command errors.
  • Command-line errors now provide more reliable reporting while preserving the underlying failure context.

• Tests

  • Added coverage for valid files and rejected artifact types.
  • Verified metadata handling and consistent command-line error behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dfaebcf5-84cc-4c5f-8b07-cf9ddace1d06

📥 Commits

Reviewing files that changed from the base of the PR and between 2ba5e59 and ea73745.

📒 Files selected for processing (3)

• legalforecast/cli.py
• legalforecast/ingestion/cohort_document_materializer.py
• tests/test_cohort_document_materializer.py

🚧 Files skipped from review as they are similar to previous changes (3)

• legalforecast/cli.py
• tests/test_cohort_document_materializer.py
• legalforecast/ingestion/cohort_document_materializer.py

---

Walkthrough

The PR adds shared validation for materializer artifacts. The CLI uses this validator and converts validation failures to CommandError. Tests cover valid files, invalid paths, symlinks, hard links, metadata reuse, and error causes.

Changes

Materializer validation

Layer / File(s)	Summary
Centralized artifact validation legalforecast/ingestion/cohort_document_materializer.py, tests/test_cohort_document_materializer.py	Adds require_materializer_artifact and MaterializerArtifactValidationError. Tests cover regular files, symlinks, missing paths, directories, hard links, and lstat metadata reuse.
CLI validation integration legalforecast/cli.py, tests/test_cohort_document_materializer.py	Replaces local checks with the shared validator. Converts validation failures to CommandError and preserves the existing cause behavior for component-validation failures.

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

Possibly related PRs

• johnhughes3/LegalForecastBench#192: Introduced the materializer extended by this validation helper.
• johnhughes3/LegalForecastBench#451: Refactors related CLI path validation into shared ingestion validators.

Suggested reviewers: copilot

Poem

A rabbit checks each artifact path,
Symlinks leave the validation path.
Hard links fail, missing files too,
Regular files pass review.
The CLI reports each error clear,
And keeps its cause when needed near.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: extracting the materializer artifact guard into the ingestion module.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ 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 refactor/materializer-artifact-validation

---

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.}

[Copilot]

Pull request overview

Refactors cohort document materializer artifact path validation by moving the shared CLI guard into the ingestion domain module while adding targeted unit coverage for the new domain API and preserving CLI-facing error messages.

Changes:

• Introduces require_materializer_artifact() in cohort_document_materializer for validating artifact paths (non-symlink traversal, regular file, single-link).
• Updates CLI _require_materializer_artifact to delegate to the domain guard and adapt domain errors to CommandError.
• Adds unit tests covering valid artifacts and rejection cases (symlink, missing, directory, hardlink) plus a CLI adapter contract check.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
tests/test_cohort_document_materializer.py	Adds direct domain tests for artifact validation plus a CLI adapter contract test.
legalforecast/ingestion/cohort_document_materializer.py	Adds require_materializer_artifact domain guard for artifact validation.
legalforecast/cli.py	Replaces inline artifact validation with a thin wrapper around the new domain guard.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

legalforecast/ingestion/cohort_document_materializer.py:38

• require_materializer_artifact only treats FileNotFoundError from path.lstat() as “not a regular file”. Other OSErrors (e.g., PermissionError, NotADirectoryError) will now propagate, whereas the previous CLI implementation (Path.is_file()/is_symlink()) would treat these as a failed file check and emit the stable "must be a regular non-symlink file" message. Catching OSError here would better preserve the stated CLI/domain error contract.

    try:
        metadata = path.lstat()
    except FileNotFoundError:
        metadata = None

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

legalforecast/ingestion/cohort_document_materializer.py:38

• require_materializer_artifact only treats FileNotFoundError from path.lstat() as a validation failure. Other common OSError cases (e.g., PermissionError, NotADirectoryError, overly-long paths) will currently bubble up as raw OS exceptions, which would break the stated “preserve CLI boundary / unchanged messages” goal because the CLI adapter won’t convert them into a CommandError with the expected text.

    try:
        metadata = path.lstat()
    except FileNotFoundError:
        metadata = None