Skip to content

fix(ingestion): normalize artifact stat errors - #462

Merged
johnjhughes merged 2 commits into
mainfrom
fix/materializer-artifact-oserror
Aug 6, 2026
Merged

johnjhughes merged 2 commits into
mainfrom
fix/materializer-artifact-oserror

Conversation

@johnjhughes

@johnjhughes johnjhughes commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Validation

  • uv run ruff format --check .
  • uv run ruff check .
  • uv run pyright
  • uv run pytest -q tests/test_cohort_document_materializer.py -k materializer_artifact_validation (8 passed)
  • full suite before the final disjoint latest-main rebase: 6,137 passed, 13 skipped
  • hosted CI is the authoritative combined full-suite proof for published head 5bf15922

Bead: LegalForecastBench-4i0g


Note

Low Risk
Narrow change to artifact path validation error normalization in the cohort document materializer; no auth, data, or workflow logic changes.

Overview
require_materializer_artifact now treats any lstat() OSError (not only FileNotFoundError) as failed metadata lookup, so inaccessible artifacts surface the same MaterializerArtifactValidationError as missing or non-regular files instead of leaking the underlying OS error.

A regression test uses a path stub that raises PermissionError on lstat() and asserts the unchanged CLI-facing message and CohortDocumentMaterializationError contract.

Reviewed by Cursor Bugbot for commit 5bf1592. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI review requested due to automatic review settings August 6, 2026 02:38
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@secure-gate-dev-token-broker[bot], you've reached your PR review limit, so we couldn't start this review.

Next review available in: 25 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 951e623b-c582-4dff-b1bb-7ae910db08cd

📥 Commits

Reviewing files that changed from the base of the PR and between 6cb3051 and 062bd97.

📒 Files selected for processing (2)
  • legalforecast/ingestion/cohort_document_materializer.py
  • tests/test_cohort_document_materializer.py

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

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens artifact-path validation in the cohort document materializer by normalizing lstat() failures into the existing MaterializerArtifactValidationError, ensuring the CLI boundary and user-facing error message remain consistent (per the contract introduced in #454).

Changes:

  • Treat any path.lstat() OSError as “no metadata available” so missing/inaccessible artifacts consistently raise the regular-file validation error.
  • Add a regression test that simulates an unreadable artifact (permission failure) and asserts the normalized CohortDocumentMaterializationError message.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
legalforecast/ingestion/cohort_document_materializer.py Broadens lstat() exception handling to normalize all OSError failures into the existing artifact validation error path.
tests/test_cohort_document_materializer.py Adds a regression test for inaccessible artifact metadata (permission-style lstat() failure) to lock in the normalized error message/contract.

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

Comment thread tests/test_cohort_document_materializer.py
Copilot AI review requested due to automatic review settings August 6, 2026 02:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@johnjhughes
johnjhughes merged commit de02b82 into main Aug 6, 2026
10 checks passed
@johnjhughes
johnjhughes deleted the fix/materializer-artifact-oserror branch August 6, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants