Skip to content

fix: replace unit test with integration test for proactive context compression - #1377

Closed
oyi77 wants to merge 42 commits into
diegosouzapw:mainfrom
oyi77:feat/dynamic-context-compaction-and-timeout-fixes
Closed

oyi77 wants to merge 42 commits into
diegosouzapw:mainfrom
oyi77:feat/dynamic-context-compaction-and-timeout-fixes

Conversation

@oyi77

@oyi77 oyi77 commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add proactive compression logic to chatCore.ts (85% threshold check)
  • Make contextManager.ts reserveTokens configurable via options
  • Replace flawed unit test with proper integration test
  • New test validates compression via handleChatCore (real request flow)
  • All 4 test cases pass: proactive trigger, no-trigger, structure preservation, tool handling

Changes

Modified Files

  • open-sse/handlers/chatCore.ts - Added proactive compression check before requests
  • open-sse/services/contextManager.ts - Made reserveTokens configurable
  • tests/integration/chatcore-compression-integration.test.ts - New integration test (4 test cases)
  • tests/unit/chatcore-compression-integration.test.mjs - Deleted (flawed approach)

Test Results

All 4 integration test cases pass:
✅ Compression triggered when context exceeds 85% threshold
✅ No compression when context is below threshold
✅ Message structure preserved during compression
✅ Tool messages handled correctly during compression

Fixes the failing test that was blocking PRs #1363 and #1368.

clousky2020 and others added 30 commits April 16, 2026 20:27
- Fix duplicate routing strategy guidance texts in zh-CN.json
- Fix strategy recommendations duplicates
- Add useMemo import in playground/page.tsx
- Add hardcoded string localization in ProxyConfigModal.tsx
- Replace dangerouslySetInnerHTML with t.rich in OAuthModal.tsx
- Replace dangerouslySetInnerHTML with t.rich in PricingModal.tsx
- Add useMemo in RequestLoggerV2.tsx for performance

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add cacheSource and tps to the translated columns array and their
corresponding translation keys in en.json and zh-CN.json.
…-entrypoint

fix(cli): resolve Node 22 TS entrypoint incompatibility
…d missing thoughtSignatures for Antigravity translator
- Fixed proxyFetch regex incomplete escape
- Updated contextManager regex to avoid Polynomial ReDoS (using [^]*?)
- Removed redundant incomplete sanitization replace in page.tsx
- Fixed perplexity-web missing flags (i) in regex and used [^]*?
- Renamed callLogArtifact sha256 to artifactHash to fix false positive password hash alert
Add dedicated batch test modes for web-cookie, search, and audio
providers in the dashboard, API route, and request validation so
category-level testing targets the correct connections.

Rename legacy qoder refresh and usage helpers from iflow to qoder
for consistency, and tighten regex handling in response cleaning,
thinking compression, and proxy matching to address edge cases and
static analysis findings.

Also update related tests, typing fixes, and README star history
embeds.
Add one-off database inspection and cleanup scripts under
`scripts/scratch/` for local debugging and maintenance work.

Document root cleanliness and file placement expectations for AI
assistants in `GEMINI.md` to keep temporary scripts and tests out of
the project root.
diegosouzapw and others added 12 commits April 16, 2026 16:47
Proactively compress oversized contexts before sending to upstream providers,
preventing context_length_exceeded errors. Compression triggers at 85% of
model's context limit using the existing 3-layer compressContext() function.

- Import compressContext, estimateTokens, getTokenLimit from contextManager
- Add compression check after translation, before executor dispatch
- Estimate tokens and compare against 85% threshold of model's context limit
- Apply 3-layer compression (trim tools, compress thinking, purify history)
- Log compression events with before/after token counts and layers applied
- Audit compression events for observability
- Add unit tests verifying integration behavior

Closes diegosouzapw#1290
- Replace hardcoded 8000 token limit with dynamic calculation from models.dev
- Use 40% of model's context window for history (safety margin)
- Parse model string to get provider and model name
- Query getTokenLimit() which checks models.dev DB for actual context limits
- Fallback to 8000 tokens if model not found
- Add warning log when history too large even with single message

This ensures compaction works with any model size:
- Small models (8K context): ~3.2K tokens for history
- Medium models (128K context): ~51K tokens for history
- Large models (1M+ context): ~400K+ tokens for history

Fixes 'Conversation history too large to compact' errors by adapting to model capabilities.
Problem: When a combo has mixed context sizes (e.g., gpt-4o-mini 128K, gemini-3-pro 1M),
using only the first model's limit could cause compaction to fail on fallback models.

Solution:
- Pass all combo target models to selectMessagesForSummary()
- Calculate minimum context limit across all targets
- Use the smallest limit to ensure compatibility with any fallback

Example combo: [gpt-4o-mini (128K), claude-opus (200K), gemini-3-pro (1M)]
- Before: Used 128K limit (first model only)
- After: Uses 128K limit (minimum across all targets)
- Result: Compacted history works with any model in the combo

This ensures handoff context is compatible with all potential fallback models.
- Add missing crypto imports in cursor.ts and perplexity-web.ts
- Fix memory FTS5 to use rowid instead of memory_id for correct JOIN
- Fix dangerous JSON string truncation in perplexity-web.ts by truncating history array before stringify
- Fix timeout resource leak in contextHandoff.ts by clearing timeout
- Fix encryption security issue by throwing error instead of falling back to plaintext

Addresses all high and medium priority issues from gemini-code-assist review.
- Add missing columns to call_logs table in SCHEMA_SQL: has_request_body, has_response_body, request_summary, error_summary, detail_state, artifact_relpath, artifact_size_bytes, artifact_sha256
- Add missing indexes: idx_call_logs_requested_model, idx_call_logs_request_type, idx_cl_combo_target
- Update insertCallLog test helper to include has_response_body and detail_state columns
- Fixes 'no such column: current.detail_state' error in call-log-file-rotation.test.ts
- All 5 tests in call-log-file-rotation.test.ts now pass
The rotateCallLogs function was using getCallLogsTableMaxRows() which reads
CALL_LOGS_TABLE_MAX_ROWS env var, but tests set CALL_LOG_MAX_ENTRIES.
Changed to use getCallLogMaxEntries() consistently for both database and
filesystem rotation.
…tion test

The test was creating artifact files but not linking them to database rows
via artifact_relpath column. This caused cleanupOrphanCallLogFiles to delete
them as orphans. Now the test helper includes artifact_relpath and passes it
when inserting call logs.

All 5 tests now pass.
…mpression

- Add proactive compression logic to chatCore.ts (85% threshold check)
- Make contextManager.ts reserveTokens configurable via options
- Replace flawed unit test with proper integration test
- New test validates compression via handleChatCore (real request flow)
- All 4 test cases pass: proactive trigger, no-trigger, structure preservation, tool handling
@oyi77
oyi77 requested a review from diegosouzapw as a code owner April 17, 2026 23:00
@oyi77

oyi77 commented Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

Closing to create a clean PR with only the compression test fix.

@oyi77 oyi77 closed this Apr 17, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates OmniRoute to version 3.6.8, introducing Node.js 24 LTS support and the Claude Opus 4.7 model. Key enhancements include proactive context compression, cache-aware system prompt handling for GPT-5 models, and persistent Antigravity credit tracking. Feedback highlights that the base database schema is out of sync with new migrations, which will cause runtime errors on fresh installs. Furthermore, the proactive compression logic requires a safety margin to be effective, and migration 026 risks data loss by nulling log bodies. Additional concerns include potential memory pressure from SSE buffering in the Antigravity executor and violations of project organization rules regarding build artifacts and root-level scripts.

Comment thread src/lib/db/core.ts
Comment on lines +191 to +198
has_request_body INTEGER DEFAULT 0,
has_response_body INTEGER DEFAULT 0,
request_summary TEXT,
error_summary TEXT,
detail_state TEXT DEFAULT 'none',
artifact_relpath TEXT,
artifact_size_bytes INTEGER DEFAULT NULL,
artifact_sha256 TEXT DEFAULT NULL,
has_request_body INTEGER DEFAULT 0,
has_response_body INTEGER DEFAULT 0,
has_pipeline_details INTEGER DEFAULT 0,
request_summary TEXT
artifact_size_bytes INTEGER,
artifact_sha256 TEXT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The SCHEMA_SQL is out of sync with the changes introduced in migration 026_simplify_call_logs.sql. Fresh installations using this schema will still include the deprecated artifact columns (artifact_relpath, artifact_size_bytes, artifact_sha256) and will be missing the new inline storage columns (request_body, response_body, error). This discrepancy will cause runtime errors on fresh installs when the application attempts to write to or read from the missing inline storage columns.

Comment on lines 1402 to 1406
provider,
model: effectiveModel,
maxTokens: contextLimit,
reserveTokens: 0,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The proactive compression logic is ineffective for contexts that are between the 85% threshold and the 100% hard limit. By passing reserveTokens: 0, the targetTokens in compressContext becomes equal to the full contextLimit. Consequently, compressContext will return compressed: false for any context that is large but still under the hard limit, defeating the purpose of proactive compression which is to leave room for the response.

To fix this, reserveTokens should be set to a value that provides a safety margin, such as the difference between the limit and the threshold.

      const compressionResult = compressContext(translatedBody, {
        provider,
        model: effectiveModel,
        maxTokens: contextLimit,
        reserveTokens: contextLimit - threshold,
      });

Comment on lines +34 to +36
NULL AS request_body,
NULL AS response_body,
NULL AS error,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This migration explicitly sets request_body, response_body, and error to NULL for all existing records. While much of this data may have been offloaded to external artifacts, any data that was stored inline (e.g., for smaller requests or before offloading was implemented) will be permanently lost. It is safer to select these columns directly from the old table to preserve any existing inline data.

Suggested change
NULL AS request_body,
NULL AS response_body,
NULL AS error,
request_body,
response_body,
error,

Comment on lines +718 to +728
let sseBuffer = "";
const passThrough = new TransformStream({
transform(chunk, controller) {
controller.enqueue(chunk);
// Accumulate text to scan for remainingCredits
try {
const text = new TextDecoder().decode(chunk, { stream: true });
sseBuffer += text;
} catch {
/* decoding best-effort */
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The sseBuffer accumulates the entire response body in memory to extract remainingCredits during the flush phase. For very large streaming responses, this could lead to significant memory pressure or OOM errors. Since the credits information is typically contained within specific SSE chunks, consider processing the chunks incrementally or only retaining a small buffer of the most recent data to extract the required fields.

Comment thread run-responses-test.js
Comment on lines +1 to +7
import { test } from "node:test";
import { execSync } from "child_process";
console.log("running test...");
execSync(
"node --import tsx/esm --test tests/integration/chat-pipeline.test.ts --test-name-pattern='chat pipeline serves repeated /v1/responses requests'",
{ stdio: "inherit" }
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This new script is placed in the project root, which violates the organization rules defined in the newly added GEMINI.md (lines 6-13). According to those rules, all maintenance, debugging, or experimental scripts must be placed strictly inside the scripts/ directory to maintain project cleanliness.

Comment thread src/lib/dataPaths.js
Comment on lines +1 to +59
"use strict";
var __importDefault =
(this && this.__importDefault) ||
function (mod) {
return mod && mod.__esModule ? mod : { default: mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
exports.APP_NAME = void 0;
exports.getLegacyDotDataDir = getLegacyDotDataDir;
exports.getDefaultDataDir = getDefaultDataDir;
exports.resolveDataDir = resolveDataDir;
exports.isSamePath = isSamePath;
const path_1 = __importDefault(require("path"));
const os_1 = __importDefault(require("os"));
exports.APP_NAME = "omniroute";
function safeHomeDir() {
try {
return os_1.default.homedir();
} catch {
return process.cwd();
}
}
function normalizeConfiguredPath(dir) {
if (typeof dir !== "string") return null;
const trimmed = dir.trim();
if (!trimmed) return null;
return path_1.default.resolve(trimmed);
}
function getLegacyDotDataDir() {
return path_1.default.join(safeHomeDir(), `.${exports.APP_NAME}`);
}
function getDefaultDataDir() {
const homeDir = safeHomeDir();
if (process.platform === "win32") {
const appData = process.env.APPDATA || path_1.default.join(homeDir, "AppData", "Roaming");
return path_1.default.join(appData, exports.APP_NAME);
}
// Support XDG on Linux/macOS when explicitly configured.
const xdgConfigHome = normalizeConfiguredPath(process.env.XDG_CONFIG_HOME);
if (xdgConfigHome) {
return path_1.default.join(xdgConfigHome, exports.APP_NAME);
}
return getLegacyDotDataDir();
}
function resolveDataDir({ isCloud = false } = {}) {
if (isCloud) return "/tmp";
const configured = normalizeConfiguredPath(process.env.DATA_DIR);
if (configured) return configured;
return getDefaultDataDir();
}
function isSamePath(a, b) {
if (!a || !b) return false;
const normalizedA = path_1.default.resolve(a);
const normalizedB = path_1.default.resolve(b);
if (process.platform === "win32") {
return normalizedA.toLowerCase() === normalizedB.toLowerCase();
}
return normalizedA === normalizedB;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This file appears to be compiled CommonJS output from TypeScript (indicated by "use strict", __importDefault, and exports usage). In a TypeScript project, generated build artifacts should not be committed to the source tree. This logic should be implemented in a .ts file, and the compiled output should be handled by the build process and ignored via .gitignore.

@oyi77
oyi77 deleted the feat/dynamic-context-compaction-and-timeout-fixes branch April 18, 2026 07:20
This was referenced Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants