Add anydoc file input processing - #11
Conversation
There was a problem hiding this comment.
Pull request overview
Adds local AnyDoc-based preprocessing so espipe can ingest common binary document formats (PDF/Office/ODF/RTF/EPUB) by converting them to Markdown while preserving the existing file-document JSON shape, metadata rules, and discovery behavior.
Changes:
- Route supported local non-text formats through
anydoc::to_markdownand reuse the existing Markdown file-document builder. - Add fixtures plus unit and CLI integration tests covering successful conversions, mixed inputs/globs, and conversion failures (malformed + image-only PDFs).
- Update CLI/README and add OpenSpec change artifacts documenting requirements and behavior.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/input.rs |
Adds AnyDoc routing for recognized non-CSV formats and refactors Markdown document construction for reuse by converted inputs. |
tests/file_output.rs |
Adds CLI integration tests and base64 fixture decoding helper for AnyDoc scenarios. |
tests/fixtures/anydoc/sample.rtf |
Adds an RTF fixture for AnyDoc conversion tests. |
tests/fixtures/anydoc/sample.docx.base64 |
Adds a DOCX fixture (base64) for AnyDoc conversion tests. |
tests/fixtures/anydoc/image-only.pdf.base64 |
Adds an image-only PDF fixture (base64) to validate OCR-required failure behavior. |
README.md |
Documents AnyDoc local document support, --content, and recursive glob examples; updates CLI usage. |
openspec/changes/add-anydoc-input/tasks.md |
Adds task checklist for the AnyDoc input change. |
openspec/changes/add-anydoc-input/specs/file-document-import/spec.md |
Updates file-document-import requirements to incorporate AnyDoc conversion and multi-input semantics. |
openspec/changes/add-anydoc-input/specs/anydoc-input/spec.md |
Adds new AnyDoc-specific requirements and scenarios. |
openspec/changes/add-anydoc-input/proposal.md |
Records motivation, scope, and impact of adding AnyDoc preprocessing. |
openspec/changes/add-anydoc-input/design.md |
Documents routing decisions, error behavior, and trade-offs for AnyDoc conversion. |
openspec/changes/add-anydoc-input/.openspec.yaml |
Adds OpenSpec metadata for the change set. |
Cargo.toml |
Adds anydoc dependency and includes AnyDoc fixtures in the package include list. |
Cargo.lock |
Locks the AnyDoc dependency and its transitive dependencies. |
💡 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.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/file_output.rs:29
- This helper takes
&PathBuf, which is more restrictive than needed. Accepting&Pathmakes the helper usable with any path-like value and is idiomatic for filesystem APIs.
fn write_base64_fixture(name: &str, path: &PathBuf) {
src/input.rs:1000
- This test helper takes
&PathBuf, which is unnecessarily restrictive. Prefer&Pathfor filesystem helpers so callers can pass any path-like value without allocating aPathBuf.
fn write_base64_fixture(source: &str, path: &PathBuf) {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/input.rs:1210
- Test name says it “preserves order”, but the assertions expect deterministic path sorting (PDF comes before RTF even though the URIs are passed as [rtf, pdf]). Renaming the test makes the intent match the behavior being verified.
#[test]
fn anydoc_mixed_file_import_preserves_order_and_file_metadata() {
let pdf = fixture_path("anydoc/sample.pdf");
let rtf = fixture_path("anydoc/sample.rtf");
let values = collect_values(open_input_values(vec![uri(&rtf), uri(&pdf)], "body").unwrap());
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (1)
README.md:96
- The CLI now accepts 2+ positional arguments (inputs followed by output), but this README section still says “Both positional arguments…”, which is now inaccurate and can confuse users reading the input/output rules.
-q, --quiet Quiet mode, don't print runtime summary
-z, --uncompressed Disable request body gzip compression
--content <CONTENT> Content subfield name for file imports [default: body]
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/fixtures/anydoc/sample.pdf.txt:36
- This PDF fixture is stored as a
.txtfile. Git checkouts with line-ending normalization (e.g., CRLF conversion) can mutate the byte offsets in thexreftable /startxref, which can make the PDF invalid for some parsers and cause cross-platform test flakiness. Consider storing the PDF as a binary.pdffixture or as base64 (likeimage-only.pdf.base64) and decoding it during tests.
%PDF-1.4
1 0 obj
<< /Type /Catalog /Pages 2 0 R >>
endobj
2 0 obj
<< /Type /Pages /Kids [3 0 R] /Count 1 >>
endobj
3 0 obj
<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>
endobj
4 0 obj
<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>
endobj
5 0 obj
<< /Length 41 >>
stream
BT
/F1 18 Tf
72 720 Td
(Hello PDF) Tj
ET
endstream
endobj
xref
0 6
0000000000 65535 f
0000000009 00000 n
0000000058 00000 n
0000000115 00000 n
0000000241 00000 n
0000000311 00000 n
trailer
<< /Size 6 /Root 1 0 R >>
startxref
401
%%EOF
src/input.rs:477
read_anydoc_file_documentformats the anydoc error into a string (eyre!("{}: {err}")), which drops the original error as a source and can reduce diagnostic detail (error chain/backtrace) in stderr. Prefer adding path context while preserving the underlying error as the source.
fn read_anydoc_file_document(
path: &Path,
content_field: &str,
include_file_metadata: bool,
) -> Result<Vec<Box<RawValue>>> {
let markdown = anydoc::to_markdown(path).map_err(|err| eyre!("{}: {err}", path.display()))?;
read_markdown_text_document(path, &markdown, content_field, include_file_metadata)
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/input.rs:856
add_origin_to_rawunconditionally inserts anoriginfield, which will silently overwrite any existingoriginfield in the incoming JSON record (e.g., user-provided NDJSON/JSON/Toon documents). This is data loss and can also allow spoofing/confusion about provenance; consider treatingoriginas a reserved field and returning an error if it already exists.
let Value::Object(mut document) = serde_json::from_str(raw.get())? else {
return Err(eyre!("Input document must be a JSON object"));
};
document.insert("origin".to_string(), origin.clone().into_value());
RawValue::from_string(Value::Object(document).to_string()).map_err(Into::into)
src/input.rs:778
add_origin_metadatawill overwrite an existing top-leveloriginfield when importing JSON/NDJSON/Toon as file-documents (multi-file or glob cases). That silently changes user content. Consider makingorigina reserved field for injected metadata and failing fast (or otherwise namespacing) when an input document already containsorigin.
This issue also appears on line 852 of the same file.
fn add_origin_metadata(document: &mut Map<String, Value>, origin: Option<&OriginMetadata>) {
if let Some(origin) = origin {
document.insert("origin".to_string(), origin.clone().into_value());
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/input.rs:779
add_origin_metadataunconditionally inserts anoriginfield, which will silently overwrite any existingoriginvalue already present in JSON/NDJSON/Toon documents (data loss / hard-to-debug behavior). Consider treatingoriginas reserved and failing fast on conflicts, or at minimum avoid overwriting an existingoriginkey.
fn add_origin_metadata(document: &mut Map<String, Value>, origin: Option<&OriginMetadata>) {
if let Some(origin) = origin {
document.insert("origin".to_string(), origin.clone().into_value());
}
}
src/input.rs:802
origin_from_urican produce surprising metadata for URIs that end with a trailing slash (emptyfilename) and for root-path URIs likehttps://example.com/(currently yieldsorigin.pathof./becausePath::new("/").parent()isNone). Normalizing trailing slashes and handling the root path explicitly will makeorigin.path/origin.filenamemore consistent.
fn origin_from_uri(uri: &UriRef<String>) -> OriginMetadata {
let path = uri.path().as_str();
let path_ref = Path::new(path);
OriginMetadata {
scheme: uri
src/input.rs:856
add_origin_to_rawparses and re-serializes every remote CSV/NDJSON/Toon record in order to attachorigin. This changes the exact JSON bytes (whitespace/key order), drops duplicate keys, and adds noticeable CPU overhead for large streams. If this tool is expected to handle high-volume NDJSON, consider a cheaper injection approach (e.g., inserting before the final}) or attaching origin at the output layer where parsing already occurs.
fn add_origin_to_raw(raw: Box<RawValue>, origin: Option<&OriginMetadata>) -> Result<Box<RawValue>> {
let Some(origin) = origin else {
return Ok(raw);
};
let Value::Object(mut document) = serde_json::from_str(raw.get())? else {
return Err(eyre!("Input document must be a JSON object"));
};
document.insert("origin".to_string(), origin.clone().into_value());
RawValue::from_string(Value::Object(document).to_string()).map_err(Into::into)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/input.rs:869
add_origin_to_rawoverwrites any existing top-leveloriginfield in remote NDJSON/CSV/Toon inputs. That can silently discard user-providedorigindata and make output depend on implicit reserved-field rules. Prefer rejecting conflicting inputs (or otherwise avoiding overwrites) so ingestion doesn’t mutate user payloads unexpectedly.
fn add_origin_to_raw(raw: Box<RawValue>, origin: Option<&OriginMetadata>) -> Result<Box<RawValue>> {
let Some(origin) = origin else {
return Ok(raw);
};
let Value::Object(mut document) = serde_json::from_str(raw.get())? else {
return Err(eyre!("Input document must be a JSON object"));
};
document.insert("origin".to_string(), origin.clone().into_value());
RawValue::from_string(Value::Object(document).to_string()).map_err(Into::into)
src/input.rs:779
add_origin_metadataunconditionally inserts anoriginfield, which will overwrite any existing top-leveloriginfield in imported JSON/NDJSON/Toon file-documents (silent data loss / unexpected schema mutation). Consider only inserting whenoriginis absent (or otherwise handling conflicts explicitly).
This issue also appears on line 861 of the same file.
fn add_origin_metadata(document: &mut Map<String, Value>, origin: Option<&OriginMetadata>) {
if let Some(origin) = origin {
document.insert("origin".to_string(), origin.clone().into_value());
}
}
src/input.rs:118
Input::read_lineaddsoriginby deserializing each producedRawValueback intoValueand serializing again. For large remote CSV/NDJSON/Toon inputs this adds a full extra parse+serialize per record. Consider pushingorigininjection down intoread_csv_line/read_json_line/read_toon_document(or returning a structured map) so each record is serialized only once.
add_origin_to_raw(raw, origin.as_ref())
}
Input::FileCsv { reader, origin, .. } => {
let raw = read_csv_line(reader)?;
add_origin_to_raw(raw, origin.as_ref())
Summary
Verification