Skip to content

fix: correct stripBulkToolOutputs JSDoc to match AND conditions, fix test data to satisfy both thresholds #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ZachDreamZ merged 1 commit into from
Jul 1, 2026
Merged

fix: correct stripBulkToolOutputs JSDoc to match AND conditions, fix test data to satisfy both thresholds #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion extensions/engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
if (typeof c === "string") return c;
if (Array.isArray(c)) {
return c
.filter((block: any): boolean => block?.type === "text")

Check warning on line 30 in extensions/engine.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
.map((block: any): string => block.text ?? "")

Check warning on line 31 in extensions/engine.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
.join(" ");
}
return String(c ?? "");
Expand Down Expand Up @@ -968,10 +968,10 @@
if (msg.role !== "assistant") return msg;
if (!Array.isArray(msg.content)) return msg;

const hasThinking = msg.content.some((b: any) => b?.type === "thinking");

Check warning on line 971 in extensions/engine.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
if (!hasThinking) return msg;

const newContent = msg.content.filter((b: any) => b?.type !== "thinking");

Check warning on line 974 in extensions/engine.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
// If all content was thinking, keep a placeholder
if (newContent.length === 0) {
return { ...msg, content: "[reasoning stripped]" };
Expand All @@ -982,7 +982,8 @@

/**
* Strip bulk tool outputs (large file reads, directory listings).
* Only targets outputs > 100 lines or > 5000 characters of plain text.
* Only targets outputs with > 100 lines AND > 5000 characters of plain text.
* Both conditions must be met to avoid truncating compact single-line results.
*/
private stripBulkToolOutputs(messages: Message[]): Message[] {
return messages.map((msg) => {
Expand Down
10 changes: 8 additions & 2 deletions tests/engine-coverage.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ describe("microCompact", () => {

it("truncates bulk tool outputs when over budget", () => {
const engine = new UltraCompactEngine({ modelName: "gpt-4o" });
const bulkOutput = Array(150).fill("LongLineForTestingPadding ").join(String.fromCharCode(10));
// Must exceed both thresholds: content.length >= 5000 AND lines > 100
const longLine = "LongLineForTestingPaddingAndTruncationCheck "; // 47 chars
const bulkOutput = Array(150).fill(longLine).join(String.fromCharCode(10));
// 150 * 47 = 7050 chars + 149 newlines ≈ 7199 chars (>= 5000 ✓, > 100 lines ✓)
Comment on lines +119 to +121

@devin-ai-integration devin-ai-integration Bot Jul 1, 2026

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.

🟡 Incorrect character count in test comment overstates safety margin above threshold

The test comment claims the string is 47 characters (tests/engine-coverage.test.ts:119) but it is actually 44, making the total content 6749 chars (not 7199 as the comment on line 121 states).

Impact: A developer trusting the comment's margin when modifying the test string could unknowingly bring the total below the 5000-character threshold, breaking the test.

Verified character count mismatch via Node.js evaluation

The string "LongLineForTestingPaddingAndTruncationCheck " is evaluated by node -e to be 44 characters, not 47 as stated in the comment on tests/engine-coverage.test.ts:119. The resulting total is 150 * 44 + 149 = 6749, not 7199. While the test still passes both thresholds (>= 5000 chars and > 100 lines), the stated margin of safety is overstated by ~450 characters.

Suggested change
const longLine = "LongLineForTestingPaddingAndTruncationCheck "; // 47 chars
const bulkOutput = Array(150).fill(longLine).join(String.fromCharCode(10));
// 150 * 47 = 7050 chars + 149 newlines ≈ 7199 chars (>= 5000 ✓, > 100 lines ✓)
const longLine = "LongLineForTestingPaddingAndTruncationCheck "; // 44 chars
const bulkOutput = Array(150).fill(longLine).join(String.fromCharCode(10));
// 150 * 44 = 6600 chars + 149 newlines = 6749 chars (>= 5000 ✓, > 100 lines ✓)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const hugeText = "TextContentForTokenBudget".repeat(25000);
const msgs: Message[] = [
makeMsg("1", "tool", bulkOutput),
Expand Down Expand Up @@ -163,7 +166,10 @@ describe("microCompact", () => {

it("saves tokens when stripping bulk tool output", () => {
const engine = new UltraCompactEngine({ modelName: "gpt-4o" });
const bigTool = "A".repeat(7000);
// Must exceed both thresholds: content.length >= 5000 AND lines > 100
const longLine = "LineContentForTruncationCheckAndTokenSavingTest "; // 51 chars
const bigTool = Array(110).fill(longLine).join(String.fromCharCode(10));
// 110 * 51 = 5610 chars + 109 newlines ≈ 5719 chars (>= 5000 ✓, > 100 lines ✓)
Comment on lines +170 to +172

@devin-ai-integration devin-ai-integration Bot Jul 1, 2026

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.

🟡 Incorrect character count in test comment overstates safety margin above threshold

The test comment claims the string is 51 characters (tests/engine-coverage.test.ts:170) but it is actually 48, putting the real content total at only 5389 chars — much closer to the 5000-char threshold than the stated 5719.

Impact: A developer shortening the test string by as few as 4 characters (relying on the overstated margin) would silently break the test assertion.

Verified character count mismatch via Node.js evaluation

The string "LineContentForTruncationCheckAndTokenSavingTest " is 48 characters, not 51 as stated in the comment at tests/engine-coverage.test.ts:170. The actual total is 110 * 48 + 109 = 5389 chars, not 5719. This leaves only a 389-char buffer above the 5000-char threshold instead of the 719 chars the comment implies. The test works today, but the margin is overstated by nearly half.

Suggested change
const longLine = "LineContentForTruncationCheckAndTokenSavingTest "; // 51 chars
const bigTool = Array(110).fill(longLine).join(String.fromCharCode(10));
// 110 * 51 = 5610 chars + 109 newlines ≈ 5719 chars (>= 5000 ✓, > 100 lines ✓)
const longLine = "LineContentForTruncationCheckAndTokenSavingTest "; // 48 chars
const bigTool = Array(110).fill(longLine).join(String.fromCharCode(10));
// 110 * 48 = 5280 chars + 109 newlines = 5389 chars (>= 5000 ✓, > 100 lines ✓)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const hugeText = "TextContentForTokenBudget".repeat(25000);
const msgs: Message[] = [
makeMsg("1", "tool", bigTool),
Expand Down
Loading