Merge branch 'main' into thought_filter_refinement

packages/core/src/core/geminiChat.test.ts

@@ -22,6 +22,7 @@ import {
   stripToolCallIdPrefixes,
   type HistoryTurn,
   coalesceConsecutiveRoles,
+  stripThoughts,
 } from './geminiChat.js';
 import {
   type CompletedToolCall,
@@ -2282,6 +2283,110 @@ describe('GeminiChat', () => {
         text: 'actual conversational response',
       });
     });
+
+    it('should completely filter out thought parts from getHistoryTurns when context management is disabled but model is gemini-2/modern', () => {
+      vi.mocked(mockConfig.isContextManagementEnabled).mockReturnValue(false);
+      vi.mocked(mockConfig.getModel).mockReturnValue('gemini-2.5-pro');
+
+      chat.setHistory([
+        {
+          role: 'user',
+          parts: [{ text: 'hello' }],
+        },
+        {
+          role: 'model',
+          parts: [
+            { text: 'internal monologue', thought: true } as unknown as Part,
+            { text: 'actual conversational response' },
+          ],
+        },
+      ]);
+
+      const turns = chat.getHistoryTurns(true);
+
+      expect(turns).toHaveLength(2);
+      const modelTurn = turns[1];
+      expect(modelTurn.content.parts).toHaveLength(1);
+      expect(modelTurn.content.parts![0]).toEqual({
+        text: 'actual conversational response',
+      });
+    });
+
+    it('should completely filter out thought parts from getHistoryTurns when model supports modern features', () => {
+      vi.mocked(mockConfig.isContextManagementEnabled).mockReturnValue(false);
+      vi.mocked(mockConfig.getModel).mockReturnValue('gemini-3.1-pro-preview');
+
+      chat.setHistory([
+        {
+          role: 'user',
+          parts: [{ text: 'hello' }],
+        },
+        {
+          role: 'model',
+          parts: [
+            { text: 'internal monologue', thought: true } as unknown as Part,
+            { text: 'actual conversational response' },
+          ],
+        },
+      ]);
+
+      const turns = chat.getHistoryTurns(true);
+
+      expect(turns).toHaveLength(2);
+      const modelTurn = turns[1];
+      expect(modelTurn.content.parts).toHaveLength(1);
+      expect(modelTurn.content.parts![0]).toEqual({
+        text: 'actual conversational response',
+      });
+    });
+
+    it('should completely filter out model turns that end up with empty parts after stripping thoughts', () => {
+      vi.mocked(mockConfig.isContextManagementEnabled).mockReturnValue(false);
+      vi.mocked(mockConfig.getModel).mockReturnValue('gemini-2.5-pro');
+
+      chat.setHistory([
+        {
+          role: 'user',
+          parts: [{ text: 'hello' }],
+        },
+        {
+          role: 'model',
+          parts: [
+            { text: 'internal monologue', thought: true } as unknown as Part,
+          ],
+        },
+      ]);
+
+      const turns = chat.getHistoryTurns(true);
+
+      // Since the model turn contains only a thought part, it should be filtered out entirely.
+      expect(turns).toHaveLength(1);
+      expect(turns[0].content.role).toBe('user');
+    });
+
+    it('should coalesce consecutive user turns when an intermediate model turn is stripped', () => {
+      vi.mocked(mockConfig.isContextManagementEnabled).mockReturnValue(false);
+      vi.mocked(mockConfig.getModel).mockReturnValue('gemini-2.5-pro');
+
+      chat.setHistory([
+        { role: 'user', parts: [{ text: 'Question 1' }] },
+        {
+          role: 'model',
+          parts: [{ text: 'thinking...', thought: true } as unknown as Part],
+        },
+        { role: 'user', parts: [{ text: 'Question 2' }] },
+      ]);
+
+      const turns = chat.getHistoryTurns(true);
+
+      // The model turn contains only a thought part, so it is stripped.
+      // The two adjacent user turns must be coalesced into one user turn.
+      expect(turns).toHaveLength(1);
+      expect(turns[0].content.role).toBe('user');
+      expect(turns[0].content.parts).toHaveLength(2);
+      expect(turns[0].content.parts![0].text).toBe('Question 1');
+      expect(turns[0].content.parts![1].text).toBe('Question 2');
+    });
   });
 
   describe('ensureActiveLoopHasThoughtSignatures', () => {
@@ -3196,4 +3301,94 @@ describe('GeminiChat', () => {
       expect(coalesceConsecutiveRoles(history)).toEqual(history);
     });
   });
+
+  describe('stripThoughts', () => {
+    it('should return empty history if empty array is passed', () => {
+      expect(stripThoughts([])).toEqual([]);
+    });
+
+    it('should strip thought parts and keep the turn if other parts remain', () => {
+      const history: HistoryTurn[] = [
+        {
+          id: '1',
+          content: {
+            role: 'model',
+            parts: [
+              { text: 'internal monologue', thought: true } as unknown as Part,
+              { text: 'visible response' },
+            ],
+          },
+        },
+      ];
+      expect(stripThoughts(history)).toEqual([
+        {
+          id: '1',
+          content: {
+            role: 'model',
+            parts: [{ text: 'visible response' }],
+          },
+        },
+      ]);
+    });
+
+    it('should completely remove a turn if all its parts are thought parts', () => {
+      const history: HistoryTurn[] = [
+        {
+          id: '1',
+          content: {
+            role: 'user',
+            parts: [{ text: 'hello' }],
+          },
+        },
+        {
+          id: '2',
+          content: {
+            role: 'model',
+            parts: [
+              { text: 'internal monologue', thought: true } as unknown as Part,
+            ],
+          },
+        },
+      ];
+      expect(stripThoughts(history)).toEqual([
+        {
+          id: '1',
+          content: {
+            role: 'user',
+            parts: [{ text: 'hello' }],
+          },
+        },
+      ]);
+    });
+
+    it('should preserve turns that do not have parts arrays', () => {
+      const history: HistoryTurn[] = [{ id: '1', content: { role: 'user' } }];
+      expect(stripThoughts(history)).toEqual(history);
+    });
+
+    it('should preserve top-level metadata when stripping thoughts', () => {
+      const history: HistoryTurn[] = [
+        {
+          id: '1',
+          content: {
+            role: 'model',
+            parts: [
+              { text: 'internal monologue', thought: true } as unknown as Part,
+              { text: 'visible response' },
+            ],
+          },
+          // top-level turn metadata
+          timestamp: '2026-07-23T00:00:00.000Z',
+          metadata: { some: 'value' },
+        } as unknown as HistoryTurn,
+      ];
+      const stripped = stripThoughts(history);
+      expect(stripped).toHaveLength(1);
+      expect(stripped[0]).toHaveProperty(
+        'timestamp',
+        '2026-07-23T00:00:00.000Z',
+      );
+      expect(stripped[0]).toHaveProperty('metadata', { some: 'value' });
+    });
+  });
 });

packages/core/src/core/geminiChat.ts

@@ -30,7 +30,11 @@ import {
   getRetryErrorType,
 } from '../utils/retry.js';
 import type { ValidationRequiredError } from '../utils/googleQuotaErrors.js';
-import { resolveModel, supportsModernFeatures } from '../config/models.js';
+import {
+  resolveModel,
+  supportsModernFeatures,
+  isGemini2Model,
+} from '../config/models.js';
 import { hasCycleInSchema } from '../tools/tools.js';
 import type { StructuredError } from './turn.js';
 import type { CompletedToolCall } from '../scheduler/types.js';
@@ -766,9 +770,10 @@ export class GeminiChat {
         abortSignal,
       };
 
-      let contentsToUse: Content[] = supportsModernFeatures(modelToUse)
-        ? [...contentsForPreviewModel]
-        : [...requestContents];
+      let contentsToUse: Content[] =
+        supportsModernFeatures(modelToUse) || isGemini2Model(modelToUse)
+          ? [...contentsForPreviewModel]
+          : [...requestContents];
 
       const hookSystem = this.context.config.getHookSystem();
       if (hookSystem) {
@@ -810,9 +815,10 @@ export class GeminiChat {
           );
           lastModelToUse = modelToUse;
           // Re-evaluate contentsToUse based on the new model's feature support
-          contentsToUse = supportsModernFeatures(modelToUse)
-            ? [...contentsForPreviewModel]
-            : [...requestContents];
+          contentsToUse =
+            supportsModernFeatures(modelToUse) || isGemini2Model(modelToUse)
+              ? [...contentsForPreviewModel]
+              : [...requestContents];
         }
         if (beforeModelResult.modifiedConfig) {
           Object.assign(config, beforeModelResult.modifiedConfig);
@@ -956,9 +962,16 @@ export class GeminiChat {
       ? extractCuratedHistory(this.agentHistory.get())
       : [...this.agentHistory.get()];
 
-    return this.context.config.isContextManagementEnabled()
-      ? scrubHistory(history)
-      : history;
+    if (this.context.config.isContextManagementEnabled()) {
+      return scrubHistory(history);
+    }
+
+    const model = this.context.config.getModel();
+    if (isGemini2Model(model) || supportsModernFeatures(model)) {
+      return coalesceConsecutiveRoles(stripThoughts(history));
+    }
+
+    return history;
   }
 
   /**
@@ -1503,3 +1516,22 @@ export function coalesceConsecutiveRoles(
   }
   return result;
 }
+
+export function stripThoughts(history: HistoryTurn[]): HistoryTurn[] {
+  return history
+    .map((turn) => {
+      if (!turn.content.parts) return turn;
+      const hasThought = turn.content.parts.some((p) => p && p.thought);
+      if (!hasThought) return turn;
+
+      const nonThoughtParts = turn.content.parts.filter((p) => p && !p.thought);
+      return {
+        ...turn,
+        content: {
+          ...turn.content,
+          parts: nonThoughtParts,
+        },
+      };
+    })
+    .filter((turn) => !turn.content.parts || turn.content.parts.length > 0);
+}

tools/caretaker-agent/cloudrun/triage-worker/main.py

@@ -9,21 +9,6 @@ from utils.validator import validate_triage_result
 from utils.egress import send_label_action, send_comment_action
 from db.issues_store import IssuesStore, ClaimAction, ReleaseAction
 
-FEATURE_CLOSED_COMMENT = (
-    "Thank you for bringing this to our attention. Right now, our "
-    "engineering team is focusing all resources on critical system "
-    "maintenance and core stability. Because of this, we don't have "
-    "immediate plans to address this specific issue. If you believe "
-    "this issue was misclassified, feel free to reopen it."
-)
-
-QUALITY_CLOSED_COMMENT = (
-    "Thank you for reaching out. We are closing this issue as it does "
-    "not contain a discernible description or actionable bug report for "
-    "our team to investigate. If you believe this was closed in error, "
-    "please feel free to open a new issue with complete reproduction details."
-)
-
 
 def main() -> None:
     """
@@ -99,13 +84,7 @@ def main() -> None:
             workable_spec = triage_result.get("workable_spec", {})
             
             if quality in ["SPAM", "EMPTY", "FEATURE"]:
-                print(f"[WORKER] Quality: {quality}. Leaving comment and applying auto-close label.")
-                if quality == "FEATURE":
-                    comment = FEATURE_CLOSED_COMMENT
-                else:  # SPAM or EMPTY
-                    comment = QUALITY_CLOSED_COMMENT
-
-                send_comment_action(owner, repo, issue_number, comment)
+                print(f"[WORKER] Quality: {quality}. Applying auto-close label.")
                 send_label_action(owner, repo, issue_number, ["auto-close"])
                 store.release_lock(
                     owner,

tools/caretaker-agent/cloudrun/triage-worker/tests/test_integration_main.py

@@ -214,15 +214,13 @@ class TestIntegrationMain(unittest.TestCase):
         self.assertIsNone(self.stored_data["lock"]["holder"])
 
     @patch("main.process_issue_triage")
-    @patch("main.send_comment_action")
     @patch("main.send_label_action")
-    def test_auto_close_flows(self, mock_send_label, mock_send_comment, mock_triage):
+    def test_auto_close_flows(self, mock_send_label, mock_triage):
         """Verifies end-to-end flow for auto-closed issues."""
         for quality in ["SPAM", "EMPTY", "FEATURE"]:
             self.mock_store.acquire_lock.reset_mock()
             self.mock_store.release_lock.reset_mock()
             mock_send_label.reset_mock()
-            mock_send_comment.reset_mock()
             mock_triage.reset_mock()
 
             self.stored_data = {
@@ -251,7 +249,6 @@ class TestIntegrationMain(unittest.TestCase):
             mock_send_label.assert_called_once_with(
                 "owner", "repo", 42, ["auto-close"]
             )
-            mock_send_comment.assert_called_once()
 
             self.assertEqual(self.stored_data["status"], "AUTO_CLOSE")
             self.assertIsNone(self.stored_data["lock"]["holder"])

tools/caretaker-agent/cloudrun/triage-worker/tests/test_main.py

@@ -79,35 +79,23 @@ class TestMainExecutionLoop(unittest.TestCase):
                 self.assertEqual(ctx.exception.code, 0)
 
     @patch("main.process_issue_triage")
-    @patch("main.send_comment_action")
     @patch("main.send_label_action")
-    def test_main_auto_close_quality_flow(
-        self, mock_send_label, mock_send_comment, mock_triage
-    ):
-        """SPAM, EMPTY, and FEATURE issues dispatch comment and auto-close label."""
-        for quality in ["SPAM", "EMPTY", "FEATURE"]:
-            with self.subTest(quality=quality):
-                self.mock_store.acquire_lock.reset_mock()
-                self.mock_store.release_lock.reset_mock()
-                mock_send_label.reset_mock()
-                mock_send_comment.reset_mock()
-                mock_triage.reset_mock()
+    def test_main_auto_close_quality_flow(self, mock_send_label, mock_triage):
+        """SPAM/EMPTY/FEATURE issues dispatch auto-close label."""
+        self.mock_store.acquire_lock.return_value = ClaimAction.PROCEED
+        output = json.dumps({"triage_metadata": {"quality": "SPAM"}})
+        mock_triage.return_value = (True, output)
 
-                self.mock_store.acquire_lock.return_value = ClaimAction.PROCEED
-                output = json.dumps({"triage_metadata": {"quality": quality}})
-                mock_triage.return_value = (True, output)
+        with self.assertRaises(SystemExit) as ctx:
+            main()
 
-                with self.assertRaises(SystemExit) as ctx:
-                    main()
-
-                self.assertEqual(ctx.exception.code, 0)
-                mock_send_comment.assert_called_once()
-                mock_send_label.assert_called_once_with(
-                    "owner", "repo", 42, ["auto-close"]
-                )
-                self.mock_store.release_lock.assert_called_once_with(
-                    "owner", "repo", 42, "exec-123", success=True, status="AUTO_CLOSE"
-                )
+        self.assertEqual(ctx.exception.code, 0)
+        mock_send_label.assert_called_once_with(
+            "owner", "repo", 42, ["auto-close"]
+        )
+        self.mock_store.release_lock.assert_called_once_with(
+            "owner", "repo", 42, "exec-123", success=True, status="AUTO_CLOSE"
+        )
 
     @patch("main.process_issue_triage")
     @patch("main.send_comment_action")