Skip to content

fix: handle square bracket HTML entities in Gemini responses #7577

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

Jump to bottom
Merged
merged 1 commit into from
Sep 16, 2025
Merged

fix: handle square bracket HTML entities in Gemini responses #7577

merged 1 commit into from
Sep 16, 2025

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Sep 1, 2025 •
edited by ellipsis-dev bot
Loading

Description

This PR fixes an issue where square brackets and array indices were being stripped from C# code when using Google Gemini models (specifically gemini-2.5-pro).

Problem

When using Gemini API with Roo Code to generate C# code containing arrays, the square brackets were missing from the output. For example:

  • Expected: string[] myArray = new string[5];
  • Actual: string myArray = new string5;

Solution

The issue was that Gemini was escaping square brackets as HTML entities ([, ], [, ]), but our unescapeHtmlEntities function wasn't handling these specific entities.

This PR adds support for unescaping both numeric and named HTML entities for square brackets in the unescapeHtmlEntities function.

Changes

  • Added support for [, ], [, and ] entities in src/utils/text-normalization.ts
  • Added comprehensive tests for square bracket entity handling
  • Tests include the exact scenario from the bug report (C# array syntax)

Testing

  • All existing tests pass ✅
  • New tests added for square bracket entity handling ✅
  • Tested with the specific C# array syntax from the issue ✅

Fixes #7576

Important

Fixes missing square brackets in C# code by updating unescapeHtmlEntities to handle specific HTML entities.

  • Behavior:
    • Fixes issue with missing square brackets in C# code generated by Gemini API by updating unescapeHtmlEntities in text-normalization.ts.
    • Supports unescaping [, ], [, and ] entities.
  • Testing:
    • Adds tests for numeric and named square bracket entities in text-normalization.spec.ts.
    • Includes test for C# array syntax with escaped square brackets.

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

@roomote
fix: handle square bracket HTML entities in Gemini responses
5b2fd3f 
- Added support for [, ], [, and ] entities
- Fixes issue where Gemini API responses were missing square brackets in C# array syntax
- Added comprehensive tests for square bracket entity handling

Fixes #7576
@roomote roomote bot requested review from cte, jr and mrubens as code owners September 1, 2025 14:38
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Sep 1, 2025
roomote[bot]
roomote bot commented Sep 1, 2025
View reviewed changes
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Reviewing my own code because apparently I trust no one, not even myself.

src/utils/text-normalization.ts
.replace(/"/g, '"')
.replace(/'/g, "'")
.replace(/'/g, "'")
.replace(/[/g, "[")
Copy link
Contributor Author

@roomote roomote bot Sep 1, 2025

Choose a reason for hiding this comment

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

Is the order of replacements intentional here? I notice that the ampersand replacement is placed last, which seems correct since replacing it first could interfere with other entity replacements. Would it be worth adding a comment explaining why it must be last to prevent future reordering mistakes?

src/utils/text-normalization.ts
.replace(/[/g, "[")
.replace(/]/g, "]")
.replace(/[/g, "[")
.replace(/]/g, "]")
Copy link
Contributor Author

@roomote roomote bot Sep 1, 2025

Choose a reason for hiding this comment

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

While this PR fixes the square bracket issue, are there other HTML entities that Gemini might escape that we're not handling? For example: parentheses (( and )), curly braces ({ and }), or other mathematical/programming symbols. Might be worth investigating what other entities Gemini commonly escapes in code contexts.

src/utils/__tests__/text-normalization.spec.ts
expect(unescapeHtmlEntities(undefined as unknown as string)).toBe(undefined)
})

it("unescapes square bracket entities (numeric)", () => {
Copy link
Contributor Author

@roomote roomote bot Sep 1, 2025

Choose a reason for hiding this comment

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

Would it make sense to group all the square bracket tests together in a nested describe block for better organization? This would make the test structure clearer and easier to navigate.

src/utils/__tests__/text-normalization.spec.ts
expect(unescapeHtmlEntities("matrix[i][j]")).toBe("matrix[i][j]")
})

it("handles C# array syntax with escaped square brackets", () => {
Copy link
Contributor Author

@roomote roomote bot Sep 1, 2025

Choose a reason for hiding this comment

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

Great test coverage! This test case directly addresses the reported issue with C# array syntax. The test is well-documented with a comment explaining its purpose.

@roomote roomote bot mentioned this pull request Sep 1, 2025
Closed
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 1, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 2, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Sep 2, 2025
daniel-lxs
daniel-lxs approved these changes Sep 2, 2025
View reviewed changes
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 2, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Sep 2, 2025
@mrubens mrubens merged commit cde738a into main Sep 16, 2025
31 checks passed
@mrubens mrubens deleted the fix/gemini-square-brackets-7576 branch September 16, 2025 15:53
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Sep 16, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrubens mrubens mrubens approved these changes

@daniel-lxs daniel-lxs daniel-lxs approved these changes

@cte cte Awaiting requested review from cte cte is a code owner

@jr jr Awaiting requested review from jr jr is a code owner

Assignees

No one assigned

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Roo Code Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug with array square brackets for Roo + gemini-2.5-pro

5 participants

@mrubens @daniel-lxs @hannesrudolph @roomote