Repository navigation
Studio: extract ebooks, Office files and email in Data Recipes document seeds - #14499
danielhanchen wants to merge 6 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
[skip ci]
|
@codex review |
1 similar comment
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fd7c25b8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ext != ".pdf": | ||
| return _extract_text_from_file(file_path, ext) | ||
| # A large spreadsheet must not stall the event loop. | ||
| return await to_thread.run_sync(_extract_text_from_file, file_path, ext) |
There was a problem hiding this comment.
When several users upload large Office or ebook files concurrently, this call submits every extraction to AnyIO's shared 40-thread pool without a dedicated limiter. Each accepted file may be 500 MB, and archive parsers can inflate substantially beyond that, so concurrent requests can exhaust memory while also occupying every worker used by unrelated synchronous endpoints. Use a small extraction-specific CapacityLimiter, analogous to the PDF limiter, rather than allowing these resource-intensive parses to consume the default pool.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 945dedd: non-PDF extraction now runs under its own CapacityLimiter(2), like PDFs.
| "width": 420, | ||
| "node_type": "markdown_note", | ||
| "name": "note_1", | ||
| "markdown": "### Books & Documents to Training Text\nTurn your own ebooks and documents into a plain `text` dataset for continued pretraining or style tuning. No model is needed: every row is a chunk of your files.\n\n**1. Upload files in the seed block.** EPUB, MOBI, FB2, PDF, Word (.docx, .doc), Excel (.xlsx, .xls), PowerPoint (.pptx, .ppt), OpenDocument, RTF, HTML, email (.eml, .msg), TXT and Markdown are extracted on your machine. Images, styles and navigation pages are dropped; paragraphs and chapter order are kept. DRM-protected books cannot be read.\n\n**2. Pick a chunk size.** Chunk size and overlap count characters (about 4 per token). 6000 characters is roughly 1500 tokens; raise it toward your training context length. Chunks break at paragraph and sentence boundaries where possible.\n\n**3. Set rows and Run.** Ordered sampling walks the books from start to end. Set rows to the chunk count shown in the seed preview to export each chunk once.\n\nOutput columns: `text` (the chunk) and `source_file` (the original file name). Train on `text` with the dataset text field, or add an LLM column (see the PDF Document QA recipe) to turn each chunk into question and answer pairs.", |
There was a problem hiding this comment.
For an ordinary book producing more than 10 chunks, the seed inspection request returns only its 10-row preview and the UI displays neither a total nor any rows beyond that limit, yet this recipe defaults to 10 output rows and tells users to set the run size to the count shown in that preview. Following these instructions therefore exports only the first 10 ordered chunks and silently omits the rest of the book. Return and display the total chunk count, or provide an all-chunks run mode/default, before directing users to use that count.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 945dedd: the document seed inspect now returns total_rows, the Preview tab shows "N chunks in total", and the recipe note points at that number.
danielhanchen
added
affects UI#
new-feature#
security#
…he seed preview [skip ci]
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 945dedd1cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| resolved_paths: response.resolved_paths ?? [], | ||
| seed_columns: response.columns, | ||
| seed_preview_rows: response.preview_rows ?? [], | ||
| seed_total_rows: response.total_rows ?? undefined, |
There was a problem hiding this comment.
When an inspected document recipe is autosaved and the page is reloaded, this value is lost: buildRecipePayload persists seed_preview_rows and the unstructured file metadata but not seed_total_rows, and the importer likewise never restores it. The preserved preview therefore reappears without the total needed by the new Books & Documents instructions, and the unstructured dialog has no explicit Load button to recover it without changing or closing the source configuration.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0fd3598: seed_total_rows is saved in the recipe ui block and restored by the importer, with a round-trip test.
| _, rows = materialize_multi_file_unstructured_seed( | ||
| file_entries = file_entries, | ||
| chunk_size = _to_int(chunk_size, DEFAULT_CHUNK_SIZE), | ||
| chunk_overlap = _to_int(chunk_overlap, DEFAULT_CHUNK_OVERLAP), | ||
| ) |
There was a problem hiding this comment.
For a large uploaded corpus, the route has already called build_multi_file_preview_rows, which invokes materialize_multi_file_unstructured_seed, before calling this helper. This second invocation reads the entire cached Parquet file and converts every row to dictionaries again (or does so immediately after the first invocation created the cache), doubling the O(n) work merely to obtain len(rows); with uploads allowed up to 1 GB and chunk sizes as small as one character, inspection latency becomes unnecessarily severe. Return the count from the first materialization or read it from Parquet metadata instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0fd3598: the preview and the total now come from one materialization (build_multi_file_preview).
…tal across reloads [skip ci]
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fd3598561
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {mode === "unstructured" && config.seed_total_rows | ||
| ? ` - ${config.seed_total_rows} chunks in total, ${previewRows.length} shown` | ||
| : null} |
There was a problem hiding this comment.
After a document has been inspected, changing the chunk size/overlap or removing a file leaves seed_total_rows unchanged while this line continues presenting it as the current total. Reinspection only happens after an upload finishes or after the dialog closes, so a user can switch directly to Preview, follow the template's instruction, and choose an incorrect export row count. Clear the total and preview when these inputs change, or recompute them before displaying the count.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 083c886: changing chunk size, overlap or the file list now clears the total until the seed is inspected again.
| "width": 420, | ||
| "node_type": "markdown_note", | ||
| "name": "note_1", | ||
| "markdown": "### Books & Documents to Training Text\nTurn your own ebooks and documents into a plain `text` dataset for continued pretraining or style tuning. No model is needed: every row is a chunk of your files.\n\n**1. Upload files in the seed block.** EPUB, MOBI, FB2, PDF, Word (.docx, .doc), Excel (.xlsx, .xls), PowerPoint (.pptx, .ppt), OpenDocument, RTF, HTML, email (.eml, .msg), TXT and Markdown are extracted on your machine. Images, styles and navigation pages are dropped; paragraphs and chapter order are kept. DRM-protected books cannot be read.\n\n**2. Pick a chunk size.** Chunk size and overlap count characters (about 4 per token). 6000 characters is roughly 1500 tokens; raise it toward your training context length. Chunks break at paragraph and sentence boundaries where possible.\n\n**3. Set rows and Run.** Ordered sampling walks the books from start to end. After you load the files, the seed Preview tab shows how many chunks they make in total; set rows to that number to export each chunk once (more rows start again from the first chunk).\n\nOutput columns: `text` (the chunk) and `source_file` (the original file name). Train on `text` with the dataset text field, or add an LLM column (see the PDF Document QA recipe) to turn each chunk into question and answer pairs.", |
There was a problem hiding this comment.
In browser and managed-account deployments, uploadUnstructuredFile sends the document to /seed/upload-unstructured-file, where the backend saves and extracts it under the server's upload root; extraction does not run in the browser. Saying these files are extracted “on your machine” can therefore give users a false privacy expectation before they upload sensitive books, email, or Office documents. Describe this as backend/server-side extraction, or qualify the claim specifically for local desktop deployments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 083c886: the note now says the Studio server you upload to extracts the files.
|
@codex review |
1 similar comment
|
@codex review |
This branch has not been deployed
1 participant
Fixes #14377
Summary
Data Recipes document seeds (the "Document file" block, also used by the PDF Document QA recipe) now accept ebooks, Office files, OpenDocument, RTF, HTML and email, not just PDF, DOCX, TXT and MD. There is also a new Books & Documents to Text recipe, listed with the GitHub Crawler, that turns uploaded books and documents into a plain
textdataset for continued pretraining. It needs no model.No new packages are installed. Every new format is read either by the stdlib readers Studio already uses for chat and RAG attachments (
core/rag/office_formats.py), or by PyMuPDF, which Studio already installs for PDFs (used here for.mobiand.fb2).Formats
.pdf,.docx,.txt,.md.epub.mobi,.fb2.doc,.docm,.dotx,.dotm.xlsx,.xlsm,.xltx,.xltm,.xls,.xlt.pptx,.pptm,.potx,.potm,.ppsx,.ppsm,.ppt,.pps.odt,.ott,.ods,.ots,.odp,.otp.rtf,.html,.htm,.xhtml,.xht,.markdown.eml,.msg,.mht,.mhtmlI picked these from what people usually have on disk, and from what the chat and RAG attachment paths already parse.
Left out:
.azw3(KF8). MuPDF opens a Gutenberg.azw3as one empty page, and store-bought Kindle files carry DRM anyway.Changes
routes/data_recipe/seed.py:UNSTRUCTURED_ALLOWED_EXTSlists the new types.parsers.parse..mobiand.fb2go through the PDF process pool, because MuPDF is not thread-safe.total_rows, the full chunk count, computed in the same pass as the preview. The seed Preview tab shows "N chunks in total, 10 shown", and the count is saved with the recipe, so users can set rows to export every chunk once.pymupdfis named in the error, the same waypython-docxandpymupdf4llmalready are.core/rag/parsers.py: addsmupdf_document_text, which joins MuPDF text blocks into paragraphs on one tall page, so a paragraph is not split at every visual line or page break.native_path_policy.rs.Tests
test_rag_office_formats.pyuploads and extracts.pymupdfnames the package.source_fileandtextcolumns.test_differential.pyagainst the merge base: 4 FIXED, 1 NEW_API, 144 UNCHANGED, 0 HEAD_FAIL. All new tests fail onmainand pass here.studio/backendteststest_data_recipe_seed.py,test_rag_office_formats.pyandtest_data_recipe_books_template.py: 285 passed.npm run i18n:check:strict,npm run typecheck,npm test(13230 passed, 0 failed) andnpm run buildall pass.Before / after
Fresh Studio installs of the merge base (
dff0fb917) and this branch (0fd7c25b8). The Learning Recipes cards gain Books & Documents to Text. On the same two servers, uploading a small.epuband.fb2to the recipe document seed changed from400 Unsupported file type: .epub. Allowed: .docx, .md, .pdf, .txtto200 status ok.