LW-406 [FIX] send page_separator under both spellings - #36
Merged
Conversation
2.8.0 renamed the wire param from `page_seperator` to `page_separator`, which only works against a service that reads the corrected name (v2.64.2 and later). Against an older service the key is dropped and the caller's page separator silently reverts to `<<<` with a 200 response. Sending both keys makes the client version-agnostic for the deprecation window: newer services prefer `page_separator`, older ones read `page_seperator`, and neither errors on the key it does not recognise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EVoa8BMCNnhdkATd7A9U4J
Contributor
|
| Filename | Overview |
|---|---|
| src/unstract/llmwhisperer/client_v2.py | Duplicates the resolved page-separator value under both wire-level spellings without changing the public signature or argument-resolution behavior. |
| tests/unit/client_v2_test.py | Verifies that both spellings carry identical values for default, explicit, and deprecated inputs. |
Reviews (1): Last reviewed commit: "fix: send page_separator under both spel..." | Re-trigger Greptile
Contributor
|
johnyrahul
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
whisper()now sends the page separator value under bothpage_separatorandpage_seperator. One line in theparamsdict, plus tests.The Python signature is unchanged — this is a wire-level fix only.
Why
2.8.0 renamed the wire param from
page_seperatortopage_separator. The service only learned to read the corrected name in v2.64.2 (Zipstack/unstract-llm-whisperer#721). Against any older service, 2.8.0 drops the key entirely and the caller's page separator silently reverts to<<<— HTTP 200, no error, no warning.That constraint is real for self-hosted and on-prem installs, which pick their own service version and supply their own
base_url. Nothing in the shipped artifact records the requirement: it lives only in the #34 PR body, which is invisible to anyone runningpip install -U llmwhisperer-client.Note the asymmetry with the other two params 2.8.0 renamed. For
line_splitter_strategyandfile_namethe client was the one misspelling, so the corrected names are what the service has always read and 2.8.0 is a strict improvement at any service version. Forpage_separatorthe service carried the typo, so the corrected name is the one that only recent builds understand. It is the only param where the rename can regress.How
Emitting both keys makes the client version-agnostic and removes the version constraint rather than documenting it:
page_separatorand ignores the extra key.page_seperatorand ignores the one it does not know.Flask drops unrecognised query params, so neither side errors on the key it does not read. The old key can come out at the next major, alongside the deprecated kwargs.
Notes on Testing
33 passed, offline, no network.page_separator=.test_whisper_deprecated_page_seperator_is_forwardedextended to assert both keys.test_whisper_sends_corrected_param_namesdropped itsassert "page_seperator" not in query, which this change deliberately invalidates. Theline_spitter_strategyandfilenameabsence assertions are untouched — those keys are still correctly gone.Release
Patch bump.
__version__is not touched here — the release workflow owns it viaworkflow_dispatch.Related
page_separatorsupport, released in v2.64.2Still open from the #34 review, not addressed here
file_nameis sent as an empty string on every default call, and the service'srequest.args.get("file_name", "sample.pdf")returns""for a present-but-empty key, sodocument_namelands blank in usage records for every caller who never set it. Worth folding into the same patch release if you want it — say the word and I will add it.🤖 Generated with Claude Code
https://claude.ai/code/session_01EVoa8BMCNnhdkATd7A9U4J