Add contextualData to #request-generate#9
Conversation
1. Add `perPartitionMetadata` to `requestMap`. 2. Add `contextualData` to `perPartitionMetadata`. 3. Align naming rules: * Use lower case with space for pass-in arguments. * Use camel case for variables in request generation.
There was a problem hiding this comment.
Hello @xtlsheep, 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!
Summary of Changes
This pull request introduces contextualData to the #request-generate process within the Protected Audience Key-Value Services specification. It modifies the request generation algorithm to include contextual signals configuration. The changes involve adding perPartitionMetadata to requestMap, incorporating contextualData into perPartitionMetadata, and aligning naming conventions for arguments and variables.
Highlights
- Feature Addition: Adds
contextualDatato the request generation process, allowing for contextual signals configuration. - Data Structure Modification: Introduces
perPartitionMetadatatorequestMapto encapsulate contextual data. - Naming Convention Alignment: Standardizes naming rules by using lowercase with spaces for pass-in arguments and camel case for variables in request generation.
Changelog
- draft-ietf-protected-audience-key-value-services.md
- Added
contextual datamap to the algorithm input, defining its structure with keys as strings and values as lists ofindex. - Modified the request generation algorithm to include
perPartitionMetadatainrequestMap. - Implemented logic to populate
contextualDatabased on the inputcontextual datamap, creating a list of data maps with 'ids' and 'value' fields. - Updated
requestMapto includeperPartitionMetadata. - Corrected
compression group idreference on line 381 tocompression group idon line 389.
- Added
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 is currently in preview and 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 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.
A map of requests takes form,
With data to weather the norm,
Contextual signals arise,
In encoded CBOR skies,
A new standard to perform.
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
The pull request introduces contextual data to the request generation process, enhancing the flexibility of the Key Value Service. The changes seem well-structured and align with the overall goals of the project. However, there are a few areas where improvements can be made to enhance clarity and maintainability.
Summary of Findings
- Naming Consistency: The pull request aims to align naming rules, but inconsistencies remain. Specifically, the use of
compression group id(with spaces) in the input parameters andcompressionGroupId(camel case) in the code could lead to confusion. Consistent naming is crucial for maintainability. - Clarity of 'contextual data' Processing: The logic for processing
contextual datainvolves multiple nested loops and map creations. While functional, it could benefit from improved readability and potentially be refactored into smaller, more manageable functions.
Merge Readiness
The pull request is almost ready for merging. Addressing the naming inconsistencies and improving the clarity of the contextual data processing logic would significantly enhance the code's quality. I am unable to directly approve the pull request, and other reviewers should review and approve this code before merging. I recommend that the pull request not be merged until the high severity issues are addressed.
| 1. Let `partitions` be an empty array. | ||
| 1. Let `perPartitionMetadata` be an empty map. | ||
| 1. For each `group` in `compression groups`: | ||
| 1. For each `partition` in `group`'s `partitions`: |
There was a problem hiding this comment.
This is still messed up if you click on the rich diff view: https://screenshot.googleplex.com/9vrRvrnx7iqwZzU
I think it's an indentation error? Needs to be probably 4 spaces, see the "Parsing a request" section below.
There was a problem hiding this comment.
Yeah you are correct, it was due to the indentation. Fixed.
perPartitionMetadatatorequestMap.contextualDatatoperPartitionMetadata.