LW-406 [FIX] send whisper params under the names the service reads - #34
Open
chandrasekharan-zipstack wants to merge 2 commits into
Open
LW-406 [FIX] send whisper params under the names the service reads#34chandrasekharan-zipstack wants to merge 2 commits into
chandrasekharan-zipstack wants to merge 2 commits into
Conversation
line_spitter_strategy and filename were sent under query keys the service does not read, so the values were silently dropped and the service defaults were used instead. page_seperator works but is misspelled. Adds line_splitter_strategy, file_name and page_separator. The old names are still accepted and warn. filename and page_seperator forward to their replacements; line_spitter_strategy does not, since applying it now would silently change extraction output for callers who set it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dra3Xevzb5oYtMz9fhj8iG
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dra3Xevzb5oYtMz9fhj8iG
Contributor
|
| Filename | Overview |
|---|---|
| src/unstract/llmwhisperer/client_v2.py | Adds alias resolution, deprecation warnings, conflict validation, and corrected service query keys without an unacknowledged actionable defect. |
| tests/unit/client_v2_test.py | Covers corrected query keys, unchanged effective defaults, deprecated aliases, intentionally ignored input, and conflicting spellings. |
| tests/integration/client_v2_test.py | Verifies that the corrected line-splitter key reaches the service validation path. |
Reviews (1): Last reviewed commit: "test: verify line_splitter_strategy reac..." | Re-trigger Greptile
This was referenced Aug 11, 2026
johnyrahul
approved these changes
Aug 11, 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
2 participants
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()gainsline_splitter_strategy,file_nameandpage_separator. The existingline_spitter_strategy,filenameandpage_seperatorstill work, log a warning and raiseDeprecationWarning.Why
LW-406. Two of these params were sent under query keys the service does not read. Flask drops unknown params, so the request returned 200 and the caller believed the setting applied:
line_spitter_strategyline_spitter_strategyline_splitter_strategyleft-priorityfilenamefilenamefile_namesample.pdfpage_seperatorpage_seperatorpage_seperatorRemoving the old kwargs would raise
TypeErrorin code that runs today, so they stay until a future major.How
_resolve_deprecated_param()picks between the two spellings.filenameandpage_seperatorforward to their replacements. Worst case forfilenameis that a report shows the real filename instead ofsample.pdf;page_seperatoralready worked, so forwarding keeps it working.line_spitter_strategystays dead — it warns, and its value is still not applied. Forwarding it would silently change extraction output on a patch upgrade for anyone who setmid-priorityorright-priority. Callers opt in by renaming.Passing both spellings of the same param raises
LLMWhispererClientExceptionrather than silently picking one.New params are appended at the end of the signature so positional callers are unaffected, and the deprecated ones default to
Noneso "passed" is distinguishable from "left at the default".Relevant Docs
llmwhisperer-docsRelated Issues or PRs
page_separatorsupport, blocks this PRDependencies Versions / Env Variables
Notes on Testing
8 unit tests cover corrected names on the wire, unchanged defaults, forwarding, the dead param, and the both-spellings error.
One integration test proves
line_splitter_strategyactually reaches the service: an unknown value now returns400 Invalid line splitter strategy selected, which can only happen if the param arrives. Verified live againstllmwhisperer-api.globe.unstract.com. Costs no pages — validation rejects before processing.Not covered automatically:
file_name—whisper-detaildoes not exposedocument_name, so a round trip is not observable through the API. Needs a manual check of the usage report.page_separator— needs #721 deployed to the test environment first.Behaviour changes worth calling out in the release notes
line_splitter_strategyused to be silently ignored and now returns a hard 400.file_namenow reaches usage reports, which previously always recordedsample.pdf.Screenshots
n/a
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Dra3Xevzb5oYtMz9fhj8iG