Conversation
Summary of ChangesHello, 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 refines the expression evaluation engine by standardizing the return types of FEEL expressions to native Go types, enhancing interoperability and simplifying downstream consumption of results. It also introduces a more flexible mechanism for handling and extracting values from various context structures, improving the overall robustness of context-aware functions. These changes incorporate and build upon previous work, ensuring a more consistent and predictable API. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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. 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
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the library to return native Go types from evaluation functions instead of custom FEEL types, significantly improving usability. While the core logic involving the new unwrapFEELValue function and getContextMap helper, along with extensive test updates, looks good, two potential Denial of Service (DoS) vulnerabilities were identified. These stem from recursive calls and pointer dereferencing without depth limits or cycle detection, which could lead to stack overflows or infinite loops with circular data structures. Additionally, there is one minor suggestion to improve a test helper.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the FEEL engine to return native Go types instead of internal FEEL-specific types, which is a great enhancement for the library's usability. The core change is the unwrapFEELValue function that recursively converts FEEL types to their Go equivalents. The tests have been updated accordingly. I've found a couple of areas for improvement. There's a high-severity issue with how year-month durations are handled, leading to a lossy conversion and incorrect results. Additionally, there's a medium-severity issue in the new getContextMap helper where it silently ignores map entries with non-string keys. Addressing these points will make the new functionality more robust and correct.
This PR superseeds #26 from @trojanc with including some minor code fixes after merge.
Original PR description
This PR attempts to address #3 and returns go types with EvalString and EvalStringWithScope
This MR also adds a small fix to have tck tests run
Assumptions made: