pjandro/OmniRoute
1
0
Fork 0
mirror of https://github.com/diegosouzapw/OmniRoute.git synced 2026-09-17 20:32:25 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix(compression): skip RTK dedup and truncation for non-shell tool results (#13521)

Browse Source 
* fix(db): union customModels with syncedAvailableModels in dispatch path

* fix(compression): skip RTK dedup and truncation for non-shell tool results

RTK's line deduplication and truncation were applied to all tool results
including non-shell tools (read, grep, glob, edit, write). This collapsed
structurally meaningful repeated lines in file content (e.g. JSON closing
braces, repeated key names), silently corrupting what the model received.

Now skipFilters (set for non-shell tools) and isDocumentLikeRead both
gate dedup and truncation, so file content survives byte-identical.

Fixes #13388

* fix(compression): restrict RTK truncation skip to document-like reads

The non-shell truncation-skip (options.skipFilters) disabled the generic
line/char cap for every non-shell tool result, including grep/glob/search
output that #4559 deliberately did NOT exempt. Only isDocumentLikeRead now
gates the generic truncation cap; the broader skip stays for dedup, which
is the operation that actually corrupts structured JSON content.

Also drops docs/omniroute-pr-body.md, an out-of-scope file carried by an
unrelated commit on this branch, and fixes the regression test's broken
relative import (tests/unit/compression -> open-sse is 3 levels up, not
2 — this is why the test file could not even load before this commit),
adjusts its truncation fixture to a genuinely document-like (non-JSON)
read so it actually exercises the isDocumentLikeRead exemption, and adds
a negative case asserting large non-shell grep output still gets
truncated by the generic cap.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* test(compression): make the RTK preservation test an actual regression guard

The four cases all passed against the tip WITHOUT this branch's fix, so they
guarded nothing — the next RTK refactor could reopen #13388 in silence.

Two causes, both fixed here:

- The fixture's repeated lines were not consecutive, and deduplicateRepeatedLines
  only collapses consecutive runs, so dedup never ran on it. Replaced with a
  matrix of identical rows, where collapsing them CORRUPTS the data rather than
  just reformatting it — which is the damage the fix prevents.
- Both central assertions sat inside `if (result.stats)`. With the old fixture
  the engine reported no stats, so the assertion bodies were skipped entirely
  and the test passed by doing nothing. They now run unconditionally.

Verified in both directions on the current tip:

  with this branch's fix   → tests 4 | pass 4 | fail 0
  against the tip's engine → tests 4 | pass 3 | fail 1
                             ✖ RTK should NOT dedup file content from a
                               non-shell 'read' tool

---------

Co-authored-by: Forge <forge@kooshapari.local>
Co-authored-by: KooshaPari <kooshapari@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
Koosha Paridehpour
2026-09-17 08:38:16 -07:00
committed by GitHub
parent 7a0b0c64fb
commit f5ff7c1e1b
2 changed files with 269 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
open-sse/services/compression/engines/rtk/index.ts
View File

@@ -130,8 +130,7 @@ function mergeRtkConfig(base?: Partial<RtkConfig>, override?: Record<string, unk
? Math.max(1, Math.floor(merged.rawOutputMaxFiles))
: DEFAULT_RTK_CONFIG.rawOutputMaxFiles,
rawOutputMaxAgeDays:
typeof merged.rawOutputMaxAgeDays === "number" &&
Number.isFinite(merged.rawOutputMaxAgeDays)
typeof merged.rawOutputMaxAgeDays === "number" && Number.isFinite(merged.rawOutputMaxAgeDays)
? Math.max(1, Math.floor(merged.rawOutputMaxAgeDays))
: DEFAULT_RTK_CONFIG.rawOutputMaxAgeDays,
};
@@ -260,7 +259,10 @@ export function processRtkText(
if (config.enabledFilters.length === 0 || config.enabledFilters.includes(filter.id)) {
const filtered = applyLineFilter(result, {
...filter,
maxLines: effectiveMaxLines(filter.maxLines || config.maxLinesPerResult, config.intensity),
maxLines: effectiveMaxLines(
filter.maxLines || config.maxLinesPerResult,
config.intensity
),
});
result = filtered.text;
if (filtered.appliedRules.length > 0) {
@@ -309,7 +311,13 @@ export function processRtkText(
}
}
const deduped = deduplicateRepeatedLines(result, { threshold: config.deduplicateThreshold });
// #13388: skip dedup for non-shell tool results (file reads, grep, glob, etc.)
// where repeated structural lines are semantically meaningful. Also skip when
// the content is a document-like read to avoid false-positive dedup on code files.
const shouldSkipDedup = options.skipFilters || isDocumentLikeRead;
const deduped = shouldSkipDedup
? { text: result, collapsed: 0 }
: deduplicateRepeatedLines(result, { threshold: config.deduplicateThreshold });
if (deduped.collapsed > 0) {
result = deduped.text;
techniquesUsed.push("rtk-dedup");
@@ -338,6 +346,10 @@ export function processRtkText(
});
// #4559: skip the generic line/char hard-cap for document/file reads (see
// isDocumentLikeRead above) so the middle of a code/prose read is not dropped.
// Non-shell results that are NOT document-like (grep/glob/search output) still
// get the generic cap — #13388 only exempted dedup, which is what corrupts
// structured JSON; unlimited truncation-skip would reopen the problem #4559 fixed
// for a different class of tools.
const truncated = isDocumentLikeRead
? { text: result, truncated: false, droppedLines: 0 }
: smartTruncate(result, {

253
tests/unit/compression/rtk-file-content-preservation.test.ts Normal file
View File

@@ -0,0 +1,253 @@
/**
* Tests for #13388: RTK should not collapse file-content tool results.
*
* When a non-shell tool (read, grep, glob, edit, write) returns file content,
* RTK's line deduplication should NOT collapse structurally meaningful repeated
* lines (e.g. `},`, `"models": [`, `]` in JSON files) — that is what corrupts
* structured content.
*
* The generic line/char truncation cap (#4559) is a separate concern: it stays
* in effect for non-shell results that are not document-like reads (e.g. large
* grep/glob output), so those outputs can still be bounded. Only document-like
* reads (see `isDocumentLikeRead`) are exempt from truncation, same as before
* #13388.
*
* Before the fix, RTK applied dedup to all tool results including non-shell
* tool outputs, silently corrupting file content.
*/
import { test } from "node:test";
import assert from "node:assert/strict";
import { applyRtkCompression } from "../../../open-sse/services/compression/engines/rtk/index.ts";
// A JSON file with structurally meaningful repeated lines that should NOT
// be collapsed by deduplication.
const JSONC_FILE_CONTENT = `{
"models": [
{
"id": "gpt-4o",
"name": "GPT-4o",
"contextLength": 128000
}
],
// Consecutive identical lines whose collapse silently CORRUPTS the data:
// dropping rows changes the matrix, it does not just change formatting.
"embeddingSeed": [
[0, 0, 0, 0],
[0, 0, 0, 0],
[0, 0, 0, 0],
[0, 0, 0, 0],
[0, 0, 0, 0],
[0, 0, 0, 0]
]
}`;
test("RTK should NOT dedup file content from a non-shell 'read' tool", () => {
const body = {
model: "codex/gpt-5",
messages: [
// The assistant asked to read a file
{
role: "assistant",
tool_calls: [
{
id: "call_read_1",
type: "function",
function: { name: "read", arguments: '{"path": "models.json"}' },
},
],
},
// The tool result contains the file content
{
role: "tool",
tool_call_id: "call_read_1",
content: JSONC_FILE_CONTENT,
},
],
};
const result = applyRtkCompression(body, {
config: {
enabled: true,
applyToToolResults: true,
deduplicateThreshold: 2,
},
});
// The content must NOT carry dedup markers. This assertion runs
// unconditionally: guarding it behind `if (result.stats)` made the whole
// check vanish whenever the engine reported no stats, which is exactly the
// case this test has to catch.
{
// Verify no dedup markers appear in the output
const output = JSON.stringify(result.body);
assert.ok(
!output.includes("[line repeated"),
"RTK should NOT insert dedup markers into file content from a non-shell tool"
);
assert.ok(
!output.includes("[rtk:dropped"),
"RTK should NOT insert drop markers into file content from a non-shell tool"
);
}
});
// Prose/source-like content that is genuinely "document-like" per `isDocumentLikeRead`
// (no command detected, not classified as a known log/JSON/error type). A pure JSON
// blob does NOT qualify — it matches the `json-output` detector — so this fixture
// intentionally avoids a bare `{...}`/`[...]` shape.
const DOCUMENT_LIKE_SOURCE_CONTENT = `import { registerHandler } from "./registry";
export function handler1() {
registerHandler("one", () => ({ status: "ok" }));
}
export function handler2() {
registerHandler("two", () => ({ status: "ok" }));
}
export function handler3() {
registerHandler("three", () => ({ status: "ok" }));
}
// FINAL_HANDLER_MARKER
export function finalHandler() {
registerHandler("final", () => ({ status: "ok" }));
}
`;
test("RTK should NOT truncate document-like file content from a non-shell 'read' tool", () => {
// Build a large file content that would exceed maxCharsPerResult
const largeContent = DOCUMENT_LIKE_SOURCE_CONTENT.repeat(20);
const body = {
model: "codex/gpt-5",
messages: [
{
role: "assistant",
tool_calls: [
{
id: "call_read_2",
type: "function",
function: { name: "read", arguments: '{"path": "handlers.ts"}' },
},
],
},
{
role: "tool",
tool_call_id: "call_read_2",
content: largeContent,
},
],
};
const result = applyRtkCompression(body, {
config: {
enabled: true,
applyToToolResults: true,
maxCharsPerResult: 1000, // Very low limit that would truncate file content
maxLinesPerResult: 5,
},
});
// Runs unconditionally on purpose: a `if (result.stats)` guard here silently
// skipped the whole check whenever the engine reported no stats.
{
const output = JSON.stringify(result.body);
assert.ok(
output.includes("FINAL_HANDLER_MARKER"),
"RTK should NOT truncate file content — the tail of the file must survive"
);
assert.ok(
!output.includes("[rtk:dropped"),
"RTK should NOT drop lines from file content of a non-shell tool"
);
}
});
test("RTK SHOULD still truncate large non-shell grep output (not a document-like read)", () => {
// #13388 only exempts dedup for non-shell tools; the generic truncation cap
// must still apply to non-document-like output such as grep results, or a
// large enough match list could blow the context budget unbounded.
const grepOutput = Array.from(
{ length: 500 },
(_, i) => `src/file${i}.ts:${i}: match line ${i}`
).join("\n");
const body = {
model: "codex/gpt-5",
messages: [
{
role: "assistant",
tool_calls: [
{
id: "call_grep_1",
type: "function",
function: { name: "grep", arguments: '{"pattern": "match"}' },
},
],
},
{
role: "tool",
tool_call_id: "call_grep_1",
content: grepOutput,
},
],
};
const result = applyRtkCompression(body, {
config: {
enabled: true,
applyToToolResults: true,
maxCharsPerResult: 1000,
maxLinesPerResult: 20,
},
});
assert.equal(result.compressed, true, "RTK should still truncate large grep output");
const output = JSON.stringify(result.body);
assert.ok(
!output.includes("src/file250.ts:250:"),
"A middle grep match should be dropped by the generic truncation cap"
);
});
test("RTK SHOULD still dedup and truncate shell command output", () => {
const shellOutput = Array(50)
.fill("npm WARN deprecated package@1.0.0: use package@2.0.0 instead")
.join("\n");
const body = {
model: "codex/gpt-5",
messages: [
{
role: "assistant",
tool_calls: [
{
id: "call_bash_1",
type: "function",
function: {
name: "bash",
arguments: '{"command": "npm install"}',
},
},
],
},
{
role: "tool",
tool_call_id: "call_bash_1",
content: shellOutput,
},
],
};
const result = applyRtkCompression(body, {
config: {
enabled: true,
applyToToolResults: true,
deduplicateThreshold: 2,
},
});
// Shell output SHOULD be deduped
assert.equal(result.compressed, true, "RTK should still compress shell command output via dedup");
});