[ALF] Add index adjustment for UTF-8 indices#8056
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements UTF-8 to UTF-16 index conversion for citations and grounding metadata, ensuring accurate text segment mapping in the public API. Key changes include the introduction of a convertUtf8IndexToUtf16 utility, updates to the Candidate model hierarchy, and the addition of comprehensive unit and instrumentation tests. Feedback highlights a potential out-of-bounds error when handling malformed surrogate pairs, performance inefficiencies due to redundant content scans, and compatibility issues with android.util.Log in non-Android test environments.
| val c = text[i].code | ||
| progress += | ||
| when { | ||
| c < 0x80 -> 1 // ASCII |
There was a problem hiding this comment.
Char has methods like isLowSurrogate https://kotlinlang.org/api/core/kotlin-stdlib/kotlin/-char/#-1623537334%2FFunctions%2F1935454986 can we use those?
There was a problem hiding this comment.
Great catch, we should be able to use them for some cases, but at a quick glance there's not methods for all of these cases, would you prefer we mix and match direct checking and method usage or remain consistent?
There was a problem hiding this comment.
I think using the ones in Char and adding our own as extension functions can help with clarity. Without some decent understanding of encoding it's not easy to follow the code (IMO).
| segment.startIndex shouldBeGreaterThanOrEqual 0 | ||
| segment.startIndex shouldBeLessThanOrEqual segment.endIndex |
There was a problem hiding this comment.
shouldBeBetween ? https://kotest.io/docs/assertions/core-matchers.html
| segment.partIndex shouldBeGreaterThanOrEqual 0 | ||
| segment.partIndex shouldBeLessThan candidate.content.parts.size |
| segment.partIndex shouldBeGreaterThanOrEqual 0 | ||
| segment.partIndex shouldBeLessThan candidate.content.parts.size | ||
| val part = candidate.content.parts[segment.partIndex] | ||
| part::class shouldBe TextPart::class |
There was a problem hiding this comment.
shouldBeInstanceOf ? https://kotest.io/docs/assertions/core-matchers.html
| } | ||
| } | ||
|
|
||
| internal fun convertUtf8IndexToUtf16(content: Content, originalIndex: Int): Int { |
There was a problem hiding this comment.
Add a comment to help understand the logic below
2 participants
The AI Logic endpoints return citation indices based on UTF-8 bytes, but Java and Kotlin use UTF-16 natively. This means that the provided indices are often offset from actual content and can even point out of bounds, making them very difficult to use.
Applies both to citation metadata and grounding.
Testing was added for all validation to ensure that grounding indices match completely with what is provided, currently passing all extant testing. Further testing was added to force grounding with citation using strings which will differ in length in UTF-8 and UTF-16 (the degree symbol and accented letters are multi-byte unicode characters).
Manual testing was done for accurate indices in practice, exhaustive testing is difficult, as citation is generally rarer. Grounding is easy to force and test.