Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the DailyPaper push functionality by overhauling its integration with the MinerU Cloud API. The primary goal is to improve the reliability and configurability of figure extraction from PDFs, ensuring a more robust and maintainable system for generating daily paper reports. This refactor moves to a more modern, asynchronous API pattern, which allows for better handling of long-running extraction tasks and provides clearer error feedback. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the MinerU integration to standardize on the v4 async task API, enhancing robustness with a new MineruClient and externalized configuration. However, it introduces security concerns related to the handling of external ZIP artifacts, specifically potential Denial of Service (DoS) via Zip Bombs and Server-Side Request Forgery (SSRF) due to insufficient validation of the ZIP download URL. It is recommended to implement size limits for downloads and decompression, and validate external URLs against an allow-list. Additionally, consider removing some redundant type casting for improved code clarity.
| response = client.get(zip_url) | ||
| response.raise_for_status() | ||
|
|
||
| with zipfile.ZipFile(io.BytesIO(response.content), "r") as zf: |
There was a problem hiding this comment.
The _extract_figures_from_zip method downloads a ZIP file from an external URL and processes it using zipfile.ZipFile. It reads the content of a markdown file from the ZIP into memory without any size or decompression limits. An attacker could provide a malicious ZIP file (Zip Bomb) that expands to an enormous size, leading to excessive memory consumption and potentially crashing the application (Denial of Service). Consider implementing a limit on the size of the downloaded content and the decompressed data.
|
|
||
| def _extract_figures_from_zip(self, client: httpx.Client, zip_url: str) -> List[Figure]: | ||
| """Download MinerU result zip and parse figures from generated markdown.""" | ||
| response = client.get(zip_url) |
There was a problem hiding this comment.
The _extract_figures_from_zip method performs a GET request to zip_url, which is obtained from the MinerU API response. There is no validation to ensure that zip_url points to a trusted domain. An attacker who can influence the API response (e.g., by compromising the API or if the user is tricked into using a malicious API base URL) could use this to perform Server-Side Request Forgery (SSRF) attacks against internal network resources. It is recommended to validate that the zip_url belongs to an expected allow-list of domains.
| figures_max_items=max(1, int(figures_max_items)), | ||
| mineru_api_base_url=mineru_api_base_url, | ||
| mineru_model_version=mineru_model_version, | ||
| mineru_max_wait_seconds=max(5.0, float(mineru_max_wait_seconds)), |
There was a problem hiding this comment.
The float() cast here is redundant because mineru_max_wait_seconds is already a float, as returned by _parse_float_env on line 218. You can remove the cast for cleaner code.
This redundant cast also appears on lines 255, 410, and 469.
| mineru_max_wait_seconds=max(5.0, float(mineru_max_wait_seconds)), | |
| mineru_max_wait_seconds=max(5.0, mineru_max_wait_seconds), |
There was a problem hiding this comment.
Pull request overview
This PR hardens the DailyPaper → MinerU integration by standardizing on MinerU v4’s async task API and wiring runtime configuration through the CLI and ARQ worker, while preserving a safety rule that prevents publishing ZIP-artifact pseudo URLs as main_figure.
Changes:
- Refactor
MineruClientto use v4 async task create/poll flow and parse figures either from task payload or downloaded ZIP markdown. - Wire MinerU runtime knobs (
MINERU_API_BASE_URL,MINERU_MODEL_VERSION,MINERU_MAX_WAIT_SECONDS) through CLI and ARQ worker into the DailyPaper workflow. - Add/adjust unit tests and update config/docs templates to reflect the new integration behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/paperbot/infrastructure/extractors/mineru_client.py |
Switches to v4 task API, adds URL validation, polling, ZIP download + markdown parsing, unified response/error handling. |
src/paperbot/application/workflows/dailypaper.py |
Passes MinerU config into MineruClient and blocks ZIP-artifact URLs from becoming main_figure. |
src/paperbot/infrastructure/queue/arq_worker.py |
Adds env parsing and job wiring for MinerU base URL/model/max-wait seconds. |
src/paperbot/presentation/cli/main.py |
Adds CLI flags and env fallbacks for MinerU base URL/model/max-wait seconds. |
tests/unit/test_mineru_client.py |
Adds tests for source URL validation and markdown ZIP ref parsing. |
tests/unit/test_dailypaper.py |
Adds test for publishable figure URL filter (reject .zip artifacts). |
tests/unit/test_arq_daily_papers.py |
Extends cron enqueue test to assert MinerU config is propagated. |
env.example |
Documents MinerU v4 base URL/model/max-wait env vars. |
README.md |
Updates feature/status text to reflect MinerU/Apprise integration state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| host = (parsed.netloc or "").lower() | ||
| if any(host == hint or host.endswith(f".{hint}") for hint in _UNSUPPORTED_HOST_HINTS): | ||
| raise ValueError( | ||
| "MinerU Cloud may timeout on github/aws URLs; provide a publicly " | ||
| "accessible non-github/non-aws URL" | ||
| ) |
There was a problem hiding this comment.
_validate_source_url() uses parsed.netloc for host checks; netloc includes ports/credentials (e.g., "bucket.s3.amazonaws.com:443"), which will bypass the unsupported host detection. Use parsed.hostname (or split off port) for the github/aws timeout guard so it reliably blocks those hosts.
| err_msg = str(detail.get("err_msg") or "task failed") | ||
| raise RuntimeError(err_msg) | ||
|
|
||
| time.sleep(self._poll_interval_seconds) | ||
|
|
There was a problem hiding this comment.
_poll_until_done() uses time.sleep() inside the polling loop. This client is invoked from the async ARQ job path (daily_papers_job -> extract_figures_for_report), so time.sleep() will block the event loop and reduce worker concurrency. Consider making the MinerU client async (httpx.AsyncClient + await asyncio.sleep) or running the blocking extraction in a thread (e.g., asyncio.to_thread).
| def test_validate_source_url_requires_http(): | ||
| client = MineruClient(api_key="test-key") | ||
| try: | ||
| client._validate_source_url("file:///tmp/a.pdf") | ||
| assert False, "expected ValueError for non-http source" | ||
| except ValueError as exc: | ||
| assert "http(s)" in str(exc) |
There was a problem hiding this comment.
These exception assertions use a try/except + assert False pattern. The rest of the unit tests in this repo commonly use pytest.raises(..., match=...) for this, which gives clearer failure output and avoids accidentally passing when an unexpected exception is raised earlier/later.
| def test_validate_source_url_rejects_github_and_aws(): | ||
| client = MineruClient(api_key="test-key") | ||
| for url in ( | ||
| "https://github.com/a/b/raw/main/paper.pdf", | ||
| "https://raw.githubusercontent.com/a/b/main/paper.pdf", | ||
| "https://bucket.s3.amazonaws.com/paper.pdf", | ||
| ): | ||
| try: | ||
| client._validate_source_url(url) | ||
| assert False, f"expected ValueError for URL: {url}" | ||
| except ValueError as exc: | ||
| assert "github/aws" in str(exc) |
There was a problem hiding this comment.
Same here: prefer pytest.raises(...) instead of manual try/except + assert False when validating that github/aws URLs are rejected; it keeps the test more idiomatic and makes failures easier to diagnose.
| def _extract_figures_from_zip(self, client: httpx.Client, zip_url: str) -> List[Figure]: | ||
| """Download MinerU result zip and parse figures from generated markdown.""" | ||
| response = client.get(zip_url) | ||
| response.raise_for_status() | ||
|
|
||
| with zipfile.ZipFile(io.BytesIO(response.content), "r") as zf: | ||
| md_members = [name for name in zf.namelist() if name.lower().endswith(".md")] |
There was a problem hiding this comment.
_extract_figures_from_zip() reads the entire ZIP into memory via response.content + BytesIO. Given the documented input limit (up to 200MB PDFs) the resulting ZIP can be large, which risks high memory usage / OOM in the worker. Prefer streaming the response to a temporary file (httpx.stream) and opening ZipFile from disk, or otherwise enforce a maximum download size.
* Feat/daily push epic 179 (#218) * feat(push): implement Epic #179 — daily push optimization Multi-channel push infrastructure with structured digest cards: - #187: Integrate HF upvotes into Judge scoring prompt - #180: Add digest_card extraction (highlight/method/finding/tags) with new LLM prompt, email template rendering, markdown output - #181: MinerU Cloud API client for PDF figure extraction - #182: Apprise multi-channel push layer with YAML config - #183-185: Channel formatters (Telegram MarkdownV2, Discord Rich Embed, WeCom markdown+news, Feishu/Lark interactive card) - #186: RSS 2.0 + Atom feed endpoints (/api/feed/daily.xml, .atom) Also creates follow-up issues #190 (Push Channel Settings UI) and #191 (Dashboard Digest Card Enhancement). 56 new tests, all passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: integrate MinerU figure extraction into CLI and ARQ worker daily-paper workflow Refs #179 * fix(daily-push): enable MinerU flags in ARQ cron payload (refs #181) * feat(push): wire channel formatters into apprise digest delivery (refs #182 #183 #184 #185) * feat(feed): add keyword RSS, feed cache, and figure enclosure support (refs #186) * feat(mineru): add extraction result cache for daily push figure pipeline (refs #181) * feat(push): add telegram bot command subscription endpoint (refs #191) * fix(push): add retry/backoff for channel delivery failures (refs #191) * feat(hf): add trending modes, cache, and arxiv identity dedupe (refs #193) * feat(push): add idempotency and error-code mapping for channel delivery (refs #191) * test(push): add mock channel e2e and failure-injection coverage (refs #191) * docs(push): add audit script and ops runbook for acceptance closure (refs #192) * docs: update README for AgentSwarm execution transition * docs: archive stale docs and rename AgentSwarm todo * docs: add AgentSwarm design proposal + web Playwright e2e setup - Add docs/proposals/agentswarm-design.md covering multi-agent orchestration architecture, adapter pattern, session management, skills ecosystem, and OpenCode/OpenClaw/oMo integration (refs #197, #214) - Add Playwright e2e test scaffolding for web dashboard (smoke, papers, research specs) - Update web/.gitignore for Playwright artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors (#223) * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 支持内联标题/摘要，以绕过 PaperInputRouter 获取 studio 论文 ID - 使上下文包生成 SSE 流期间的数据库持久化操作不再致命 - 从库中导入重复论文时显示错误消息 - 将左侧文件面板宽度缩小至约 18%（默认宽度）；移除默认模板文件 - 为 Claude CLI 添加 --verbose 标志，以兼容 stream-json 输出 Related to #222 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 --------- Co-authored-by: Jerry Zhang <1772030600@qq.com> * fix(web): correct package.json scripts JSON syntax * chore(gitignore): block nested .env secrets and keep examples * docs(env): add MinerU key and figure extraction env templates * refactor(mineru): standardize on v4 task API for daily push (#229) * fix(mineru): support v4 async task extraction flow * refactor(mineru): standardize on v4 task API for daily push (#179) * feat(push): render MinerU main figure in email via inline fallback * docs(readme): update email push demo screenshot * docs(readme): refresh Phase 6 daily push descriptions * fix(security): harden runbook allowed-dir path validation * fix(review): resolve security and reliability findings from AI review * fix(codeql): harden studio chat project_dir normalization --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Boyu liu <oor2020@163.com>
* Feat/daily push epic 179 (#218) * feat(push): implement Epic #179 — daily push optimization Multi-channel push infrastructure with structured digest cards: - #187: Integrate HF upvotes into Judge scoring prompt - #180: Add digest_card extraction (highlight/method/finding/tags) with new LLM prompt, email template rendering, markdown output - #181: MinerU Cloud API client for PDF figure extraction - #182: Apprise multi-channel push layer with YAML config - #183-185: Channel formatters (Telegram MarkdownV2, Discord Rich Embed, WeCom markdown+news, Feishu/Lark interactive card) - #186: RSS 2.0 + Atom feed endpoints (/api/feed/daily.xml, .atom) Also creates follow-up issues #190 (Push Channel Settings UI) and #191 (Dashboard Digest Card Enhancement). 56 new tests, all passing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: integrate MinerU figure extraction into CLI and ARQ worker daily-paper workflow Refs #179 * fix(daily-push): enable MinerU flags in ARQ cron payload (refs #181) * feat(push): wire channel formatters into apprise digest delivery (refs #182 #183 #184 #185) * feat(feed): add keyword RSS, feed cache, and figure enclosure support (refs #186) * feat(mineru): add extraction result cache for daily push figure pipeline (refs #181) * feat(push): add telegram bot command subscription endpoint (refs #191) * fix(push): add retry/backoff for channel delivery failures (refs #191) * feat(hf): add trending modes, cache, and arxiv identity dedupe (refs #193) * feat(push): add idempotency and error-code mapping for channel delivery (refs #191) * test(push): add mock channel e2e and failure-injection coverage (refs #191) * docs(push): add audit script and ops runbook for acceptance closure (refs #192) * docs: update README for AgentSwarm execution transition * docs: archive stale docs and rename AgentSwarm todo * docs: add AgentSwarm design proposal + web Playwright e2e setup - Add docs/proposals/agentswarm-design.md covering multi-agent orchestration architecture, adapter pattern, session management, skills ecosystem, and OpenCode/OpenClaw/oMo integration (refs #197, #214) - Add Playwright e2e test scaffolding for web dashboard (smoke, papers, research specs) - Update web/.gitignore for Playwright artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors (#223) * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 支持内联标题/摘要，以绕过 PaperInputRouter 获取 studio 论文 ID - 使上下文包生成 SSE 流期间的数据库持久化操作不再致命 - 从库中导入重复论文时显示错误消息 - 将左侧文件面板宽度缩小至约 18%（默认宽度）；移除默认模板文件 - 为 Claude CLI 添加 --verbose 标志，以兼容 stream-json 输出 Related to #222 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 * fix(studio): fix context pack generation, duplicate import UX, and CLI chat errors - 修改Gemini AI review代码问题 --------- Co-authored-by: Jerry Zhang <1772030600@qq.com> * fix(web): correct package.json scripts JSON syntax * chore(gitignore): block nested .env secrets and keep examples * docs(env): add MinerU key and figure extraction env templates * refactor(mineru): standardize on v4 task API for daily push (#229) * fix(mineru): support v4 async task extraction flow * refactor(mineru): standardize on v4 task API for daily push (#179) * feat(push): render MinerU main figure in email via inline fallback * docs(readme): update email push demo screenshot * docs(readme): refresh Phase 6 daily push descriptions * fix(security): harden runbook allowed-dir path validation * fix(review): resolve security and reliability findings from AI review * fix(codeql): harden studio chat project_dir normalization * docs(readme): restructure README for professional presentation - Add centered hero header with badges (CI, Roadmap, License, Python, Next.js) - Replace verbose 18-row feature table with categorized bullet lists - Move Roadmap Phase 1-6 content to pinned issue #232 - Move module maturity matrix to Roadmap #232 - Consolidate screenshots into collapsible sections - Remove inline API endpoint table (40+ rows) and directory tree - Add Contributing section with Roadmap link - Streamline Getting Started with collapsible config details - Add repo topics for GitHub discoverability References: vLLM, Docling, PaperQA2, R2R README patterns Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Boyu liu <oor2020@163.com>
Summary
This PR delivers the final MinerU integration hardening for DailyPaper push:
POST /extract/taskGET /extract/task/{task_id}/extractfallback path.MineruClientfor clearer flow:figures+ captionsMINERU_API_BASE_URLMINERU_MODEL_VERSIONMINERU_MAX_WAIT_SECONDS.zip#...pseudo image URLs asmain_figure.README.md,env.example) and tests.Linked Work
fd7e1c970efd76Validation
py_compile) on modified Python files.pytestcurrently exits with code 139 in this machine environment (process-level crash, no assertion output). Recommend CI runner regression as source of truth.