Parallelize multi-file imports and skip unreadable files - #17
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are a few user-facing and maintainability concerns (notably warning output formatting and a potential structured-input performance regression for single-file globs) that should be addressed or explicitly validated before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates espipe’s local batch-import behavior so per-file failures (read/convert/split) are treated as warnings and don’t abort processing of subsequent files, while preserving fatal errors for direct single-file imports. This aligns CLI behavior, tests, and documentation/specs around resilient batch ingestion.
Changes:
- Add
skip_errorshandling to local file-document and local--splitinputs so per-file failures warn-and-skip in multi-file/glob scenarios. - Extend CLI regression tests to ensure batch imports continue after unreadable/malformed/image-only PDF inputs, and adjust existing tests to reflect warning semantics.
- Document the batch recovery behavior in README and OpenSpec, and bump version/changelog to 0.6.1.
File summaries
| File | Description |
|---|---|
tests/file_output.rs |
Adds/updates CLI tests verifying per-file failures are skipped in batch imports while later files still ingest. |
src/input.rs |
Introduces skip_errors to batch file-document and local split readers; adjusts parsing paths and unit tests accordingly. |
README.md |
Documents warn-and-skip behavior for multi-file imports and image-only PDFs in batch contexts. |
openspec/specs/file-document-import/spec.md |
Specifies requirement that batch file errors do not abort later imports. |
openspec/specs/anydoc-input/spec.md |
Updates AnyDoc spec to require per-file recovery behavior in batch imports. |
CHANGELOG.md |
Adds 0.6.1 entry describing the batch-import failure-handling fix. |
Cargo.toml |
Bumps crate version to 0.6.1. |
Cargo.lock |
Updates lockfile version for espipe to 0.6.1. |
.gitignore |
Ignores additional agent-generated files. |
Review details
- Files reviewed: 7/9 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core ingestion/error-handling behavior across multiple input modes (file documents + local split), so a final human review is advisable despite added tests and specs.
Review details
- Files reviewed: 7/9 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The updated batch error-handling logic is consistent with the documented/spec’d behavior and is backed by targeted regression tests for the new warning-and-continue semantics.
Review details
- Files reviewed: 7/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The new file-count summary logic can report incorrect “from 0 files” (and can label remote origins as files), which undermines the accuracy of the completion summary.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/20 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces substantial new concurrency and user-visible CLI semantics changes in core ingestion paths that warrant final human review despite strong test/spec updates.
Review details
- Files reviewed: 18/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
--batch-sizevalues still win.Piped X of Y docs from Z filesso skipped files do not inflate document totals.Performance
On the 6,246-file NASA STI PDF collection, the release build emitted all 5,850 eligible documents to local NDJSON in 5.185 seconds. The ordered worker-pool version took 8.768 seconds, so completion-order delivery reduced application elapsed time by 40.9%.
Verification
cargo test: 190 passed, 5 ignoredcargo checkcargo fmt -- --checkgit diff --check