misc: Support property separator with xml format in EBNFComposer#8418
misc: Support property separator with xml format in EBNFComposer#8418CatherineSue wants to merge 3 commits intomainfrom
Conversation
- Added PROPERTY_SEPARATOR_MAP to handle format-specific separators - Fixed hardcoded comma separators that were incorrectly applied to XML - Updated both required and optional property handling
There was a problem hiding this comment.
Summary of Changes
Hello @CatherineSue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the EBNF generation logic by introducing a flexible mechanism for defining property separators based on the output format. The primary goal is to correctly support XML-based function call formats, which require newline characters as property separators, while maintaining compatibility with existing JSON and Pythonic formats that use commas. This change improves the adaptability of the EBNF composer for various model outputs.
Highlights
- Configurable Property Separators: Introduced a new
PROPERTY_SEPARATOR_MAPwithin theEBNFComposerclass. This map allows defining specific separators for properties based on the target function call format (e.g.,", "forpythonicandjson, and"\n"forxml). - Dynamic Separator Application: Modified the
build_ebnffunction to dynamically retrieve the appropriate property separator fromPROPERTY_SEPARATOR_MAPbased on thefunction_format. This ensures that both required and optional properties are joined using the correct separator for the given format, specifically addressing the need for newline separators in XML.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for format-specific property separators in the EBNF composer, which improves handling formats like XML. The implementation is clear and directly addresses the problem. I suggest removing the hardcoded default value to improve maintainability.
|
It seems ut-backend-1 (5) keeps failing due to OOM: |
|
#8224 implements the same support by adding a |
|
Close this PR as it is a duplicate of #8224 |
Motivation
Currently, we only use ", " to concatenate the ebnf strings between different tool calls. This is because in both json or ast/pythonic format functions, they use , to between different arguments, such as:
"parameters": {"a": 2, "b": 2},add(a=1,b=2). However, since Qwen3-coder and other models may start to use XML tags for the function format, we should use\nin this case.NOTE: the separator currently doesn't break any parsing. Because usually with full XML-ish format strings, we use regex to find all tags like <parameter=xxx> . The actually separator between them doesn't matter.
This PR is purely serving as a improvement.
Modifications
Checklist