Skip to content

fix: keep the server_url path when splitting PDFs - #355

Merged
CyMule merged 2 commits into
mainfrom
fix/split-url-prefix
Oct 7, 2026
Merged

CyMule merged 2 commits into
mainfrom
fix/split-url-prefix

Conversation

@CyMule

@CyMule CyMule commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

PDF splitting now works when server_url includes a path, such as a self-hosted API behind /deployment.

server_url="https://host/deployment", split_pdf_page=True

Before: GET https://host/general/docs             → 404, partition fails
After:  GET https://host/deployment/general/docs  → 200, pages processed

Why

Before sending the split page requests, the SDK makes one extra request to the API. It built that URL from the host alone, dropping any path in server_url. It now builds that URL from the actual partition request, so a path in either the client's server_url or a per-call server_url is kept. The page requests themselves already used the right URL.

The helper that built the old URL has no other callers, so it is removed along with its test.

Validation

Offline tests cover sync and async calls, with and without a per-call server_url, and plain, nested, and URL-encoded paths. Unit tests, pylint, and mypy pass. Adds a CHANGELOG entry and bumps the version to 0.46.3.

@CyMule
CyMule marked this pull request as ready for review October 7, 2026 15:43
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T15:46:23.649166Z f2d0372 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Shadow auto-approve: would not auto-approve. This PR does not meet the repository auto-approval settings.

View guided diff | Re-trigger cubic

@CyMule CyMule changed the title Preserve deployment paths in split-PDF collection requests fix: keep the server_url path when splitting PDFs Oct 7, 2026

@cragwolfe cragwolfe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oracle Pro review of exact head f2d0372ce6c6b8f55f7950a6fb946ac084eff619.

No concrete actionable defects found. I verified the URL change against the SDK's partition URL construction and split hook: it preserves the encoded deployment prefix, uses the actual per-call request URL, and keeps routing state local to each request. The checked-in sync/async tests cover default and per-call URLs, nested and encoded paths, and split/unsplit requests. Existing memory and concurrency behavior is unchanged by this diff.

Exact-head CI: unit, integration, contract, platform integration, lint, CodeQL and security checks succeeded; Claude was skipped. No local tests were run. Oracle's completed browser verdict was SAFE TO MERGE; Pro was confirmed before submission and during processing. The CLI capture timed out, so the completed response was retained directly from the original browser conversation.

(authored by codex)

@CyMule
CyMule merged commit ba25d25 into main Oct 7, 2026
22 checks passed
@CyMule
CyMule deleted the fix/split-url-prefix branch October 7, 2026 20:16
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.

2 participants