fix(compression): skip RTK dedup and truncation for non-shell tool results by KooshaPari · Pull Request #13521 · diegosouzapw/OmniRoute

KooshaPari · 2026-09-13T06:14:18Z

Problem

RTK's line deduplication and smart 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 "models": [ — silently corrupting what the model received. A 714-line JSON config was reduced to ~4 lines plus [line repeated 2x] markers.

Root Cause

In processRtkText(), the dedup and truncation steps ran unconditionally. The skipFilters flag (set for non-shell tools via SHELL_TOOL_NAME_RE) only skipped filter matching but not the subsequent dedup/truncation passes.

Fix

deduplicateRepeatedLines() is now skipped when skipFilters or isDocumentLikeRead is true
smartTruncate() is now skipped when skipFilters is true (extending the existing isDocumentLikeRead guard)
Shell command output (bash, terminal, execute_command) continues to be deduped and truncated as before

Test Impact

New test: tests/unit/compression/rtk-file-content-preservation.test.ts
Tests: file content from read tool survives, no dedup markers, no truncation, shell output still deduped

…sults 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 diegosouzapw#13388

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

diegosouzapw · 2026-09-15T02:00:06Z

Thanks for #13388. Holding for scope.

skipFilters is true for every non-shell tool result, so this also disables the generic line/char cap for grep, glob and search results. Those are exactly the outputs that can run to thousands of lines, and #4559 deliberately exempted only document-like reads.

The branch also carries an unrelated commit ("union customModels with syncedAvailableModels") and docs/omniroute-pr-body.md.

Could you narrow it to skipping dedup (not truncation) for non-shell results, drop the extra commit and file, and keep the regression test?

diegosouzapw · 2026-09-15T11:57:56Z

Thanks for #13388 — the dedup-corrupting-JSON diagnosis is spot on. skipFilters is true for
every non-shell tool result though, so this change also disables the generic line/char cap for
grep, glob and search output — exactly the outputs that can run to thousands of lines, and #4559
deliberately exempted only document-like reads from that cap. The branch also carries an
unrelated commit ("union customModels with syncedAvailableModels") and docs/omniroute-pr-body.md.
Could you narrow this to skipping dedup only (not truncation) for non-shell results, drop the
extra commit and file, and keep the regression test?

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 diegosouzapw#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>

…n guard The four cases all passed against the tip WITHOUT this branch's fix, so they guarded nothing — the next RTK refactor could reopen diegosouzapw#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

…rge wave (#14082) Every PR against release/v3.8.51 is born red on all four Unit Tests fast-path shards. Reproduced on the pure tip (1603c86): ~35 tests. This commit repairs the two large clusters plus the stale assertions sharing their root cause (17 tests); the rest is tracked separately. RTK (12 tests, real regression): #13521 gated dedup on skipFilters || isDocumentLikeRead but isDocumentLikeRead is true for ANY text whose type detection is unknown — not only non-shell tool results — so plain repeated tool output (the classic RTK case) stopped being deduplicated, processRtkText returned no stats, and rtkEngine.apply().stats.engine came back undefined. The intent of #13388 was the non-shell-tool skip, which resolveToolMeta already expresses as skipFilters; dedup now honours only that. The #13521 regression guard (rtk-file-content-preservation, tool named 'read' → skipFilters) still passes 4/4; rtk-engine is back to 9/9. Golden + stale assertions (5 tests, product changes never propagated): - tests/snapshots/provider/translate-path.json: xKiro (#12648) was added to the registry without regenerating the snapshot — purely additive entry. - providers-constants-split: APIKEY_PROVIDER_COUNT 241 → 242 (xKiro). - tests/snapshots/g13/combo-chatcore-public-seams.json and the three Content-Type assertions in chatcore-translation-paths / chat-route-coverage: #13419 deliberately made streaming responses declare 'text/event-stream; charset=utf-8' (Arabic/Persian mojibake) and updated the integration test but not these unit assertions. Before/after on the 26 base-red files: 266 tests, 23 → 18 failing here (the RTK suites were counted per-file in CI; per-test this is 17 fixed). The file-size ✗ on chatcore-translation-paths.test.ts is the pre-existing drift #14016 rebaselines — this commit keeps that file's line count unchanged.

…sults (diegosouzapw#13521) * 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 diegosouzapw#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 diegosouzapw#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 diegosouzapw#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>

…rge wave (diegosouzapw#14082) Every PR against release/v3.8.51 is born red on all four Unit Tests fast-path shards. Reproduced on the pure tip (3ce81a1): ~35 tests. This commit repairs the two large clusters plus the stale assertions sharing their root cause (17 tests); the rest is tracked separately. RTK (12 tests, real regression): diegosouzapw#13521 gated dedup on skipFilters || isDocumentLikeRead but isDocumentLikeRead is true for ANY text whose type detection is unknown — not only non-shell tool results — so plain repeated tool output (the classic RTK case) stopped being deduplicated, processRtkText returned no stats, and rtkEngine.apply().stats.engine came back undefined. The intent of diegosouzapw#13388 was the non-shell-tool skip, which resolveToolMeta already expresses as skipFilters; dedup now honours only that. The diegosouzapw#13521 regression guard (rtk-file-content-preservation, tool named 'read' → skipFilters) still passes 4/4; rtk-engine is back to 9/9. Golden + stale assertions (5 tests, product changes never propagated): - tests/snapshots/provider/translate-path.json: xKiro (diegosouzapw#12648) was added to the registry without regenerating the snapshot — purely additive entry. - providers-constants-split: APIKEY_PROVIDER_COUNT 241 → 242 (xKiro). - tests/snapshots/g13/combo-chatcore-public-seams.json and the three Content-Type assertions in chatcore-translation-paths / chat-route-coverage: diegosouzapw#13419 deliberately made streaming responses declare 'text/event-stream; charset=utf-8' (Arabic/Persian mojibake) and updated the integration test but not these unit assertions. Before/after on the 26 base-red files: 266 tests, 23 → 18 failing here (the RTK suites were counted per-file in CI; per-test this is 17 fixed). The file-size ✗ on chatcore-translation-paths.test.ts is the pre-existing drift diegosouzapw#14016 rebaselines — this commit keeps that file's line count unchanged.

Forge and others added 2 commits September 12, 2026 23:13

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

500bf5a

Copilot AI lite review requested due to automatic review settings September 13, 2026 06:14

KooshaPari requested a review from diegosouzapw as a code owner September 13, 2026 06:14

Copilot AI reviewed Sep 13, 2026

KooshaPari and others added 2 commits September 16, 2026 01:46

diegosouzapw merged commit f5ff7c1 into diegosouzapw:release/v3.8.51 Sep 17, 2026
3 of 7 checks passed

This was referenced Sep 18, 2026

fix(compression): drain the worker pool queue when worker construction throws #12755

Merged

fix(compression): repair the unit-test base-reds left by the 09-17 merge wave (RTK + goldens) #14082

Merged

Navigation Menu

Uh oh!

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

fix(compression): skip RTK dedup and truncation for non-shell tool results#13521
diegosouzapw merged 4 commits into
diegosouzapw:release/v3.8.51from
KooshaPari:fix-13388-rtk-file-content

KooshaPari commented Sep 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

diegosouzapw commented Sep 15, 2026

Uh oh!

diegosouzapw commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

KooshaPari commented Sep 13, 2026

Problem

Root Cause

Fix

Test Impact

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

diegosouzapw commented Sep 15, 2026

Uh oh!

diegosouzapw commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants