LRriver commented on PR #356: URL: https://github.com/apache/hugegraph-ai/pull/356#issuecomment-4631921416
Thanks for the updates. The PDF upload implementation itself looks good to me: it adds `pypdf`-based extraction in the shared `read_documents()` path, handles mixed-case suffixes, returns clear Gradio errors for encrypted/unreadable/no-text PDFs, updates the upload copy, and adds coverage for TXT/DOCX/PDF plus both `build_vector_index()` and `extract_graph()` entrypoints. I have one blocking scope/dependency concern before merge: `hugegraph-llm/pyproject.toml` also changes `pycgraph` from `3.2.2` to `3.2.4`, including the aarch64 git source tag. This does not appear related to #345 or PDF upload support. `pycgraph` is the workflow engine used across many HugeGraph-LLM flows, so this broadens the PR blast radius beyond the document upload path. The current tests verify the PDF/document behavior, but they do not establish that the scheduler/flow dependency upgrade is safe across RAG, graph extraction, Text2Gremlin, import/update flows, and the aarch64 source path. Please either revert the `pycgraph` changes from this PR, or explain why PDF support requires this upgrade and add the corresponding broader flow/integration verification. My preference is to keep this PR focused on PDF upload support and move any `pycgraph` upgrade to a separate PR. Non-blocking improvement: the PDF tests are already sufficient for the main upload path, but they could be strengthened with a real encrypted PDF fixture, e.g. using `PdfWriter.encrypt()`, and a multi-page PDF test that asserts page-order-preserving extraction. This would make the issue’s page-by-page/stable-order acceptance criteria more explicit, but I do not think it needs to block this PR once the unrelated dependency bump is resolved. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
