Skip to content

fix: bound web_crawl memory by streaming with a byte cap - #5971

Merged
praisonai-triage-agent[bot] merged 2 commits into
mainfrom
claude/issue-5969-20261024
Oct 24, 2026
Merged

praisonai-triage-agent[bot] merged 2 commits into
mainfrom
claude/issue-5969-20261024

Conversation

@praisonai-triage-agent

Copy link
Copy Markdown
Contributor

Fixes #5969

What

_crawl_with_httpx downloaded and decoded the whole response before slicing to 50,000 chars, so a large or never-ending page could use memory without limit.

  • httpx path: use client.stream(...) and read at most _MAX_CRAWL_BYTES (5 MB) through iter_bytes, then decode only those bytes. Redirect and SSRF checks are the same as before: every hop is still validated, there are at most 5 redirects, and a 3xx with no Location still raises.
  • urllib fallback: response.read(_MAX_CRAWL_BYTES).
  • No new public parameters. The 50,000-char output limit is unchanged.

The issue also suggests rejecting a response early when its Content-Length is over the cap. I left that out: with the stream capped, memory is already bounded, and truncating still returns the first 50k chars of a large but legitimate page.

Verification

Reproduction from the issue (300 MB response):

  • before: RSS 23 → 1075 MB
  • after, httpx path: 36 → 53 MB
  • after, urllib path: 27 → 38 MB

All three return 50,000 chars. A small page reached through a redirect gives the same result as before.

New tests in tests/unit/tools/test_web_crawl_size_cap.py (they skip when httpx isn't installed) check that the body is consumed only up to the cap and that redirects still work. The existing web_crawl markdown and SSRF tests pass. In the full unit run, every failure comes from an optional dependency missing in CI (litellm, starlette, fastapi, ...); none are in crawl code.

🤖 Generated with Claude Code

…e whole body (fixes #5969)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7df129cf-4c18-4e37-83e4-27510bf598cc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MervinPraison

Copy link
Copy Markdown
Owner

@coderabbitai review

@MervinPraison

Copy link
Copy Markdown
Owner

/review

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review skipped.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@MervinPraison MervinPraison added pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet pipeline/final-claude-pending Reviews done; waiting for FINAL @claude labels Oct 24, 2026
@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the missing fallback test is now included.

Summary

This PR caps downloaded page content at 5 MiB while keeping the 50,000-character output limit.

  • The basic web crawler limits page input to 5 MiB before conversion.

Reviews (2) · Last reviewed commit: "test: cover web_crawl urllib fallback by..." · Reviewed by Greptile

Comment thread src/praisonai-agents/tests/unit/tools/test_web_crawl_size_cap.py Outdated
@MervinPraison

Copy link
Copy Markdown
Owner

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Review changes in this PR. Python SDK: praisonaiagents, praisonai. TypeScript SDK: src/praisonai-ts/. Do NOT modify src/praisonai-rust. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

MANDATORY READ (before reviewing):

  • Always read src/praisonai-agents/AGENTS.md
  • If this PR touches src/praisonai-ts/, also read src/praisonai-ts/AGENTS.md §2.1.2 (TS triage + PR review checklist)

Phase 1: Review per AGENTS.md

  1. Protocol-driven: check heavy implementations vs core SDK
  2. Backward compatible: ensure zero feature regressions
  3. Performance: no hot-path regressions
  4. SDK value: review in depth whether the change genuinely adds value to the SDK — never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
  5. Do not bloat the Agent class with additional params — only if absolutely required; we already support many params.
  6. Repo routing: agent-callable tools → PraisonAI-Tools; lifecycle plugins → PraisonAI-Plugins; optional sandbox backends → PraisonAI-Plugins (praisonai.sandbox entry point) — request changes if wrongly added to praisonaiagents/
    6b. External tool examples (examples/tools/external/**): If the diff is confined to that path, mark SDK value ❌ for monorepo merge — redirect the contributor to PraisonAI-Tools; do not Approve for merge unless the PR has the maintainer-accept-example label.
    6c. New vendor *_tools.py in core (src/praisonai-agents/praisonaiagents/tools/): If the PR adds a new *_tools.py for a third-party API/SaaS/dataset (e.g. FXMacroData), mark Repo routing ❌ and SDK value ❌ — redirect to MervinPraison/PraisonAI-Tools. Do not Approve because legacy bundled tools (Tavily/Exa/You.com/etc.) already in core are not precedent for new vendor modules. Request changes or recommend closing unless the PR has maintainer-accept-core-tools.

MANDATORY COMMENT FORMAT — include this Phase 1 table in your review comment:

Phase 1 — AGENTS.md review

Check Result
Protocol-driven / no heavy impl in core ✅ or ❌ + one-line rationale
Backward compatible ✅ or ❌ + one-line rationale
Performance (hot path) ✅ or ❌ + one-line rationale
SDK value ✅ or ❌ + one-line rationale (explicitly judge whether the change strengthens the SDK)
No Agent param bloat ✅ or ❌ + one-line rationale
Repo routing ✅ or ❌ + one-line rationale

For TypeScript PRs (src/praisonai-ts/), also add:
| TS types / parity / tests | ✅ or ❌ + one-line rationale (npm run build && npm test) |

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Also independently identify and fix any gaps or issues you find in the changed code — do not rely only on prior reviewer feedback
9. Push all code fixes directly to THIS branch (do NOT create a new PR)
10. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
11. Never Approve or write “Approve for merge” if any Phase 1 row is ❌, if rule 6b/6c applies, or if the change belongs in PraisonAI-Tools/PraisonAI-Plugins. Use Request changes, redirect to the external repo, and recommend closing the monorepo PR when routing is wrong.
12. Approve only when every Phase 1 row is ✅ (including Repo routing and SDK value) and blocking issues are resolved; otherwise request changes with clear action items

@MervinPraison MervinPraison added pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:cooldown Blocked: post-push or @claude cooldown pipeline/blocked:stale-final Blocked: FINAL stale after new commits and removed pipeline/final-claude-pending Reviews done; waiting for FINAL @claude pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet labels Oct 24, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

Final architecture review

Phase 1 — AGENTS.md review

Check Result
Protocol-driven / no heavy impl in core ✅ The fix stays inside the existing web_crawl_tools.py (a private helper and a constant). It adds no new module or dependency.
Backward compatible ✅ The tool returns the same output (it was already truncated to 50,000 characters). The SSRF check still runs on every redirect hop. A 3xx response without a Location header still raises, as it did before.
Performance (hot path) ✅ Memory use and HTML parsing are now capped at 5 MiB per page. Before, the whole response body was buffered.
SDK value ✅ This makes the tool more robust: a single huge or hostile response can no longer use up memory.
No Agent param bloat ✅ No new Agent params or exports. _MAX_CRAWL_BYTES is private, not a user setting.
Repo routing ✅ This fixes a bug in a tool that is already in core. It does not add a new vendor tool.

Phase 2 — Fixes pushed (d4b7b18c8)

Modified src/praisonai-agents/tests/unit/tools/test_web_crawl_size_cap.py only:

  • Greptile P2 (valid): added test_urllib_fallback_reads_only_up_to_cap. It forces ImportError for httpx, checks that response.read is called once with _MAX_CRAWL_BYTES, and checks the returned Markdown.
  • Moved pytest.importorskip("httpx") from module level into the mock_client fixture. Without httpx installed, the httpx tests are skipped but the fallback test still runs. Before, the whole module was skipped.

Skipped: no changes to production code. I reviewed the streaming and redirect logic and found no defects. Qodo was billing-blocked. CodeRabbit raised no actionable findings.

Tests: test_web_crawl_size_cap.py (3 tests) and test_web_crawl_markdown.py (11 tests) pass. Running all of tests/unit/tools gives 73 failed and 2 collection errors. The same failures happen without this change: the CI runner is missing pydantic and other dependencies.

Phase 3 — Verdict

✅ Approve. Every Phase 1 check passes and the only valid review finding is fixed.

@MervinPraison MervinPraison removed pipeline/blocked:cooldown Blocked: post-push or @claude cooldown pipeline/blocked:stale-final Blocked: FINAL stale after new commits labels Oct 24, 2026
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

Merge gate scan — eligible for assessment. Claude merge gate will assess and may auto-merge on an APPROVE verdict.

@praisonai-triage-agent praisonai-triage-agent Bot added pipeline/merge-ready Eligible for merge gate auto-merge claude-merge-gate-active and removed pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:ci Blocked: CI not green on HEAD labels Oct 24, 2026
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

MERGE_GATE_VERDICT: APPROVE

  • Scope: 2 files, +110/−12. Fixes a memory bug in the existing praisonaiagents/tools/web_crawl_tools.py: the httpx path now streams the response and stops at a private _MAX_CRAWL_BYTES (5 MiB) cap, and the urllib fallback uses read(cap). No new tool module, export, dependency or Agent param, and agent.py is unchanged.
  • Behaviour kept: every redirect hop is still SSRF-checked, there are still at most 5 redirects, and a 3xx with no Location header still raises. The 50,000-char output limit is unchanged.
  • Tests: new tests/unit/tools/test_web_crawl_size_cap.py covers the httpx byte cap, redirects, and the urllib fallback cap.
  • Reviews: the FINAL architecture review is present with every Phase 1 row ✅, including SDK value and Repo routing. No CHANGES_REQUESTED reviews; the only other review is a Greptile COMMENTED review scoring 5/5.
  • Policy: the author is app/praisonai-triage-agent, which is on the allowlist, and the branch is not from a fork. No blocking labels, sensitive paths or secrets in the diff.
  • Not checked: AGENTS.md could not be read in this run (no tool permission). The checks above are based on the diff and the gate checklist.

@praisonai-triage-agent
praisonai-triage-agent Bot merged commit 56262f8 into main Oct 24, 2026
44 checks passed
@praisonai-triage-agent

Copy link
Copy Markdown
Contributor Author

Merged by Claude PR merge gate (claude-merge-gate.yml).
Verdict: MERGE_GATE_VERDICT: APPROVE
SHA: d4b7b18
Method: merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merged-by-gate pipeline/merge-ready Eligible for merge gate auto-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

web_crawl's httpx path buffers the entire response body before truncating to 50,000 chars — a 300 MB page costs ~1 GB of RSS

1 participant