Skip to content

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Jul 28, 2025

This PR fixes the failing tests in PR #6278 by:

  1. Updating the snapshot test that was failing due to the documentation changes
  2. Fixing the TypeScript error in bedrock.ts by changing from token to credentials structure
  3. Updating the bedrock-reasoning test to match the new credentials structure

All tests are now passing locally.


Important

Fixes failing tests and enhances Gemini provider with new options, while improving UI for message queueing.

  • Tests:
    • Update snapshot test for documentation changes.
    • Fix TypeScript error in bedrock.ts by changing from token to credentials structure.
    • Update bedrock-reasoning.spec.ts to match new credentials structure.
  • Gemini Provider:
    • Add enableUrlContext and enableGrounding options in provider-settings.ts.
    • Update GeminiHandler in gemini.ts to support new options.
    • Add tests for GeminiHandler in gemini-handler.spec.ts.
  • UI Enhancements:
    • Add QueuedMessages component in ChatView.tsx to handle message queueing.
    • Update ChatTextArea.tsx to allow sending messages even when disabled, for queueing.
    • Add new i18n strings for queued messages in multiple locales.

This description was created by Ellipsis for 3f21a7c. You can customize this summary. It will automatically update as commits are pushed.

roomote bot and others added 3 commits July 27, 2025 14:32
* feat: Add support for message queueing

* fix: address PR review feedback for message queueing feature

- Restore original ChatView tests from main branch
- Fix broken test by updating ChatTextArea mock
- Add comprehensive tests for message queueing (simplified due to mocking constraints)
- Fix race condition using useRef and setTimeout in queue processing
- Extract QueuedMessage interface to shared types.ts file
- Replace inline styles with Tailwind classes in QueuedMessages
- Add i18n support for 'Queued Messages:' text
- Add keyboard navigation for removing queued messages
- Add JSDoc for fromQueue parameter in handleSendMessage

* refactor: move QueuedMessage interface to packages/types

- Move QueuedMessage interface from local types.ts to packages/types/src/message.ts
- Update imports in ChatView.tsx and QueuedMessages.tsx to use @roo-code/types
- Remove local types.ts file to follow codebase conventions

* fix: add id field when creating queued messages

- Generate unique id using timestamp when adding messages to queue
- Fixes TypeScript error after moving QueuedMessage interface

* Stop disabling sending

* Translations

* Fix tests

* Improved styling

* Remove unused string

* Test cleanup

* fix: address message queueing issues

- Fix race condition in queue processing by re-checking queue state inside setTimeout
- Add error handling for queue operations with retry mechanism
- Replace array index with stable message.id for React keys in QueuedMessages
- Generate more unique IDs using timestamp + random component

* feat: add inline editing for queued messages

- Add ability to edit queued messages by clicking on them
- Support Enter to save and Escape to cancel edits
- Add textarea that auto-resizes based on content
- Add hover effect to indicate messages are editable
- Add translation for click to edit tooltip

* feat: add scrollbar and fix height for queued messages

- Add max-height of 300px with scrollbar to queue container
- Add flex-shrink-0 to prevent message items from being squished
- Ensure consistent height for message items when multiple messages are queued

* feat: add translations for queued message edit tooltip

- Add 'queuedMessages.clickToEdit' translation key to all 18 language files
- Provides localized tooltip text for the click-to-edit functionality

* fix: improve message queue processing reliability

- Fix race condition by removing nested setState and setTimeout
- Add retry limit (3 attempts) to prevent infinite loops
- Add proper cleanup on component unmount
- Clear queue when starting new task
- Prevent queue processing during API errors
- Fix ESLint warnings for React hooks dependencies

---------

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Daniel Riccio <[email protected]>
Co-authored-by: Matt Rubens <[email protected]>
…rch (#5959)

* feat: Adding more settings and control over Gemini

- with topP, topK, maxOutputTokens
- allow users to enable URL context and Grounding Research

* feat: Adding parameter titles and descriptions + translation to all languages

* feat: adding more translations

* feat: adding `contextLimit` implementation from `maxContextWindow` PR + working with profile-specific thresholding

* feat: max value for context limit to model's limit + converting description and titles to settings for translation purposes

* feat: all languages translated

* feat: changing profile-specific threshold in context management setting will also change in Gemini context management

- sync between Context Management Settting <-> Gemini Context Management with regards to thresholding

* feat: max value of maxOutputTokens is model's maxTokens + adding more tests

* feat: improve unit tests and adding `data-testid` to slider and checkbox components

* fix: small changes in geminiContextManagement descriptions + minor fix

* fix: Switching from "Gemini Context Management" to "Token Management

- better naming and correct purpose

* fix: input field showed NaN -> annoying UX

* fix: Removing redundant "tokens" after the "set context limit"'s checkbox + removing the lengthy description

* fix: Changing the translation to be consistent with the english one

* fix: more translations

* fix: translations

* fix: removing contextLimit and token management related code

- due to the decision in: #3717

* fix: removing `contextLimit` test and removing token management in translations

* fix: changing from `Advanced Features` to `Tools` to be consistent with Gemini docs/AI studio

* fix: adding `try-catch` block for `generateContentStream`

* feat: Include citations + improved type safety

* feat: adding citation for streams (generateContextStream)

* fix: set default values for `topP`, `topK` and `maxOutputTokens`

* fix: changing UI/UX according to the review/feedback from `daniel-lxs`

* fix: updating the `Gemini.spec.tsx` unit test

- testing when it is hidden
- testing when users click on the collapsible trigger and model configuration appears

* fix: more changes from the feedback/review from `daniel-lxs`

* fix: adding sources at the end of the stream to preserve

* fix: change the description for grounding with google search and url context

* fix: adding translations

* fix: removing redundant extra translations - a mistake made by the agent

* fix: remove duplicate translation keys in geminiSections and geminiParameters

- Fixed duplicate keys in 13 localization files (es, fr, hi, id, it, ja, ko, nl, pl, pt-BR, ru, tr, vi)
- Removed second occurrence of geminiSections and geminiParameters keys
- Kept first occurrence which contains more comprehensive descriptions
- All JSON files validated for syntax correctness
- Translation completeness verified with missing translations script

Resolves duplicate key issue identified in PR #4895

* fix: delete topK, topP and maxOutputTokens from Gemini

* fix: deleting topK, topP and maxOutputTokens from translations/locales

* fix: adjust spacing between labels and descriptions + sentence casing

* fix: adding maxOutputTokens back and removing unknown type

* fix: internalizing error Gemini error message

* fix: updating tests in Gemini and Vertex to adjust to the new error logging

* fix: address PR review feedback for Gemini tools feature

- Fix Hindi translation grammatical error in settings.json
- Internationalize 'Sources:' string and error messages in gemini.ts
- Add comprehensive error scenario tests to gemini-handler.spec.ts
- Remove unused currentModelId prop from Gemini component
- Update all locale files with new translation keys

---------

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: Matt Rubens <[email protected]>
Co-authored-by: Daniel Riccio <[email protected]>
- Updated multi-file-search-replace.ts to emphasize PRECISE, TARGETED modifications
- Updated multi-search-replace.ts with similar clarification
- Updated rules.ts to describe apply_diff as for 'surgical edits - targeted changes'
- Fixed bedrock.ts TypeScript error by using credentials instead of token
- Updated bedrock-reasoning test to match new credentials structure
- Updated snapshots

These changes help users understand that apply_diff is for specific code modifications, not broad rewrites
Copilot AI review requested due to automatic review settings July 28, 2025 15:55
@hannesrudolph hannesrudolph requested review from cte, jr and mrubens as code owners July 28, 2025 15:55
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jul 28, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the failing tests identified in PR #6278 by addressing three main issues: snapshot mismatches due to documentation changes, TypeScript errors in bedrock credential handling, and ensuring proper test mocking for new features.

Key Changes

  • Updated snapshot test to reflect new apply_diff tool description
  • Fixed bedrock authentication to use proper credentials structure instead of token
  • Updated test mocks to handle new message queueing and Gemini parameter features

Reviewed Changes

Copilot reviewed 74 out of 74 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Various i18n locale files Added translations for Gemini parameters and queued messages UI
webview-ui/src/components/settings/providers/Gemini.tsx Added new checkboxes for URL context and grounding search parameters
webview-ui/src/components/chat/QueuedMessages.tsx New component for displaying and managing queued messages
webview-ui/src/components/chat/ChatView.tsx Integrated message queueing functionality
webview-ui/src/components/chat/ChatTextArea.tsx Updated to support message queueing when sending is disabled
src/api/providers/gemini.ts Added support for URL context and grounding search tools with citation extraction
src/api/providers/bedrock.ts Fixed credentials structure from token to proper AWS credentials format
src/core/prompts/sections/rules.ts Updated apply_diff description for clarity
Various test files Updated mocks and expectations to handle new features
Comments suppressed due to low confidence (1)

src/core/sliding-window/tests/sliding-window.spec.ts:252

  • [nitpick] Removed blank line breaks the logical grouping of test data and test case. The blank line helps separate the test data setup from the actual test case, improving readability.
		]

{
type: "ask",
ask: testCase.ask,
ask: testCase.ask as any,
Copy link

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

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

Using 'as any' type assertion bypasses TypeScript's type checking. Consider defining proper types or using a more specific type assertion to maintain type safety.

Suggested change
ask: testCase.ask as any,
ask: testCase.ask,

Copilot uses AI. Check for mistakes.
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 28, 2025
@daniel-lxs daniel-lxs closed this Jul 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 30, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants