Skip to content

Conversation

@ochafik
Copy link
Contributor

@ochafik ochafik commented Jan 30, 2026

Summary

Improve performance when fetching PDFs from servers that don't support HTTP Range requests by caching the full file body on first request, eliminating redundant full-file downloads for subsequent chunk requests.

Changes

  • Added remoteFullBodyCache Map to store full PDF bodies from servers that return HTTP 200 instead of 206 Partial Content
  • Introduced sliceToChunk() helper function to extract requested byte ranges from cached or fetched full data
  • Enhanced readPdfRange() to:
    • Check cache before making remote requests
    • Detect HTTP 200 responses (indicating Range request was ignored)
    • Cache full body and slice to requested range when HTTP 200 is received
    • Continue normal HTTP 206 Partial Content handling for compliant servers

Implementation Details

  • Cache lookup uses normalized URL as key to ensure consistent cache hits
  • sliceToChunk() safely handles edge cases (offset beyond file size, clamped byte counts)
  • Maintains backward compatibility with Range-compliant servers while gracefully handling non-compliant ones
  • Reduces bandwidth usage for multi-chunk requests against servers without Range support

https://preview.claude.ai/code/session_01JutQT8VqbMmfC1n1438hjP

When a remote server ignores the Range header and returns HTTP 200
with the full body, readPdfRange previously passed the entire file
(potentially 10MB+) through as a single chunk. This bypassed the
512KB limit because:

1. The error check `!response.ok && status !== 206` short-circuits
   on 200 (ok is true), so the full body is read via arrayBuffer()
2. No Content-Range header on a 200 response leaves totalBytes at 0
3. hasMore becomes `offset + fullSize < 0` = false, so the client
   stops after one oversized message

Fix: detect HTTP 200, cache the full body in memory (to avoid
re-downloading on every subsequent chunk request), then slice to the
requested range. The 512KB per-message limit is now enforced for all
remote URLs regardless of Range request support.
@ochafik ochafik marked this pull request as draft January 30, 2026 18:21
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 30, 2026

Open in StackBlitz

@modelcontextprotocol/ext-apps

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/ext-apps@411

@modelcontextprotocol/server-arcade

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-arcade@411

@modelcontextprotocol/server-basic-react

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-basic-react@411

@modelcontextprotocol/server-basic-vanillajs

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-basic-vanillajs@411

@modelcontextprotocol/server-budget-allocator

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-budget-allocator@411

@modelcontextprotocol/server-cohort-heatmap

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-cohort-heatmap@411

@modelcontextprotocol/server-customer-segmentation

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-customer-segmentation@411

@modelcontextprotocol/server-map

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-map@411

@modelcontextprotocol/server-pdf

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-pdf@411

@modelcontextprotocol/server-scenario-modeler

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-scenario-modeler@411

@modelcontextprotocol/server-shadertoy

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-shadertoy@411

@modelcontextprotocol/server-sheet-music

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-sheet-music@411

@modelcontextprotocol/server-system-monitor

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-system-monitor@411

@modelcontextprotocol/server-threejs

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-threejs@411

@modelcontextprotocol/server-transcript

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-transcript@411

@modelcontextprotocol/server-video-resource

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-video-resource@411

@modelcontextprotocol/server-wiki-explorer

npm i https://pkg.pr.new/modelcontextprotocol/ext-apps/@modelcontextprotocol/server-wiki-explorer@411

commit: 7302d4e

- Add dual timeout strategy for cache cleanup:
  - 10s inactivity timeout (resets on each access)
  - 60s max lifetime (absolute timeout from creation)
- Add 50MB max size limit with both Content-Length and actual size checks
- Add unit tests for caching behavior including:
  - Cache on HTTP 200 response (no range support)
  - No cache on HTTP 206 response (range supported)
  - Slice cached data for subsequent range requests
  - Reject PDFs exceeding size limits
- Export getCacheSize() and clearCache() for testing
@ochafik ochafik marked this pull request as ready for review January 30, 2026 19:59
- Replace module-level global cache with createPdfCache() factory
- Each server instance now gets its own isolated cache
- Export PdfCache interface for type-safe usage
- Update tests to use per-test cache instances
- Add test verifying cache isolation between sessions
@ochafik ochafik merged commit bb4e5c5 into main Jan 30, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants