Repository navigation
fix(compression): restore boundary whitespace around preserved spans in llmlingua (#13455) - #13794
Closed
diegosouzapw wants to merge 1 commit into
Closed
diegosouzapw wants to merge 1 commit into
diegosouzapw wants to merge 1 commit into
Conversation
…in llmlingua (#13455) Root cause: compressMessageText() splits a message into alternating prose / preserved segments and re-joins them with a bare parts.join("") after each prose segment is sent through the backend. The real ONNX worker backend rebuilds output from surviving wordpieces after tokenization, which trims/normalizes segment boundaries as a side effect and never promised to preserve the caller's leading/trailing whitespace, so a prose segment sitting directly against a preserved placeholder (inline code, URL, etc.) got glued to it with no space. Fix: compressProseText() now captures the original segment's exact leading/trailing whitespace before calling the backend and reinserts it around the backend's result when the result doesn't already carry equivalent boundary whitespace, and the accept/reject length gate compares post- restoration length so a segment whose only "change" was losing its own whitespace is correctly treated as a no-op. Scope: this PR fixes only the boundary-whitespace loss. Case flattening and negation/stopword dropping WITHIN surviving prose is the default TinyBERT checkpoint's own token-pruning decision inside the third-party ONNX model (no per-word "never prune this" hook exists there) and is left as a follow-up per the plan-file's scope note. Regression test: tests/unit/compression/llmlingua-whitespace-restitch.test.ts
Collaborator
Author
|
Closing in favour of #13768 by @lorenzozanee, which covers the same llmlingua fidelity area for #13455 and was opened first. This PR came out of a parallel maintainer sweep that was already running when #13768 appeared. The contributor's PR keeps the credit and goes through review. One thing worth carrying over into the review of #13768: the core symptom in #13455 is boundary whitespace disappearing around preserved spans ( |
diegosouzapw
deleted the
fix/13455-llmlingua-lowercase-negation-whitespace
branch
September 16, 2026 01:21
This was referenced Sep 21, 2026
Merged
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.
Refs #13455
Root cause
Issue #13455 bundled three distinct symptoms with three different fixability profiles:
<system-reminder>/envelope destruction — already fixed by fix(compression): preserve instruction blocks from lossy engines #13523 (landed one daybefore this analysis), unrelated to this PR.
in plain prose — this PR's fix.
compressMessageText()inopen-sse/services/compression/engines/llmlingua/index.tssplits a message intoalternating prose/preserved segments and re-joins them with a bare
parts.join("")after each prose segment is sent through the compression backend. The real ONNX
worker backend rebuilds output from surviving wordpieces after tokenization, which
trims/normalizes segment boundaries as a side effect — it never promised to preserve
the caller's leading/trailing whitespace. Whenever a prose segment sat directly
against a preserved placeholder, the bare join glued the two together with no space
(
"NEVER run \rm -rf` on the target host"→ `` "runrm -rf`target host" ``).default TinyBERT checkpoint's own per-token pruning decision, made inside the
third-party
@atjsh/llmlingua-2ONNX model with no token-span/offset-mapping API tohook into. Not fixable purely in OmniRoute's own code without either pre-tombstoning
individual negation words (fragmenting prose for every stacked engine, unreviewed
blast radius), switching the default model to an unverified cased checkpoint, or
forking the third-party reconstruction logic — each a real design decision the owner
needs to make. Left for a follow-up rather than bundled into this fix.
Fix
compressProseText()now captures the original segment's leading/trailing whitespacebefore calling the backend, and reinserts that exact original whitespace run (not a
synthesized single space) around the backend's result if the result doesn't already carry
equivalent boundary whitespace — so a segment that bordered a newline stays on its own
line instead of collapsing onto one. The accept/reject "did this actually get shorter"
gate in the same function now compares using the post-restoration length, and rejects a
segment whose only change would be reattaching its own stripped whitespace (no real
compression happened).
Scoped entirely to
open-sse/services/compression/engines/llmlingua/index.ts— nochanges to
preservation.ts(the already-shipped #13523 envelope fix) or any otherengine.
Regression test
tests/unit/compression/llmlingua-whitespace-restitch.test.ts(5 cases: the already-fixedenvelope regression guard, the reported glued-segment shape, a preserved block at the
very start of the message, one at the very end, and a newline boundary before a fenced
block).
RED on unfixed code (4/5 failing):
GREEN after the fix (5/5):
Existing llmlingua suites re-run clean (no regression):
llmlingua-failopen.test.ts,llmlingua-ultra-entry.test.ts,llmlingua-model-store.test.ts,llmlingua-worker-resolution.test.ts,llmlingua-worker-spawn-12822.test.ts,llmlingua-worker.test.ts,ultra-slm-tier.test.ts— 29/29 pass, 0 fail.Gates run
npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>— clean (0 errors).npx prettier --check <changed files>— all matched files use Prettier code style.npm run check:open-sse-typecheck— OK, 0 errors, all within frozen baseline.node scripts/check/check-file-size.mjs— no new✗on touched files; the pre-existing✗entries (open-sse/utils/stream.ts,open-sse/handlers/imageGeneration.ts,tests/unit/batch_api.test.ts,tests/unit/image-generation-handler.test.ts) are base drift, untouched by this PR.node scripts/check/check-complexity.mjs— OK, 2826 violations vs baseline 3218 (ratchet respected).node scripts/check/check-cognitive-complexity.mjs— OK, 1276 violations vs baseline 1437 (ratchet respected).node scripts/check/check-test-discovery.mjs— OK, new test file discovered (5799 test files, 32 collectors).Not covered here
Per the plan-file's explicit scope decision, this PR fixes ONLY the boundary-whitespace
loss described above. It does not address:
SSH→ssh) or negation/stopword dropping within survivingprose text — that is the default TinyBERT checkpoint's own token-pruning decision made
inside the third-party ONNX model, with no per-word "never prune this" hook exposed
the way the rule-based
ultraengine got in fix(backend): ultra heuristic prunes polarity/negation words and collapses newlines, inverting instructions #13454. Fixing it needs either mappingoutput back onto the original text's exact character spans, or switching the default
to a verified cased checkpoint — both real design decisions outside a single-issue
bug-fix PR. Recommend tracking as its own follow-up issue.