feat: add flag metadata defaulting and immutability#221
Conversation
….15.1) - ResolutionDetails#flag_metadata now defaults to a frozen empty hash when nil, ensuring consumers always get a hash (spec 1.4.14) - flag_metadata is always frozen to guarantee immutability (spec 1.4.15.1) - Add spec tests for Requirement 1.4.14 and 1.4.15.1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jose Colella <jose.colella@gusto.com>
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 enhances the ResolutionDetails structure to ensure strict compliance with OpenFeature specification requirements 1.4.14 and 1.4.15.1. It specifically addresses the behavior of flag_metadata, guaranteeing that it defaults to an empty, frozen hash and remains immutable, thereby improving the robustness and predictability of flag evaluation results. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request correctly implements the defaulting and immutability requirements for flag_metadata in ResolutionDetails according to the OpenFeature specification. The changes are well-tested. I have one suggestion to make the implementation more concise and idiomatic.
| raw = self[:flag_metadata] | ||
| if raw.nil? | ||
| EMPTY_FLAG_METADATA | ||
| elsif raw.frozen? | ||
| raw | ||
| else | ||
| raw.freeze | ||
| end |
There was a problem hiding this comment.
This logic can be simplified. The elsif raw.frozen? check is redundant because calling freeze on an already frozen object is a no-op that returns the object itself. You can make this method more concise and idiomatic by using the || operator for defaulting.
(self[:flag_metadata] || EMPTY_FLAG_METADATA).freezeCalling .freeze directly on the caller's hash was a hidden side effect. Use .dup.freeze to return a frozen copy instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jose Colella <jose.colella@gusto.com>
|
Re: simplification suggestion from gemini-code-assist The suggested metadata = { "scope" => "user" }
ResolutionDetails.new(flag_metadata: metadata)
# After accessing .flag_metadata, metadata is now frozen — caller can't mutate itThe current code uses 🤖 Jose's AI agent |
|
Thank you for the detailed explanation, |
Summary
ResolutionDetails#flag_metadatanow defaults to a frozen empty hash{}whennil, per spec requirement 1.4.14flag_metadatais always frozen to guarantee immutability, per spec requirement 1.4.15.1Spec References
Part of the spec compliance roadmap: #220
Test plan
bundle exec rspec spec/specification/flag_evaluation_api_spec.rb— all passbundle exec rspec— full suite passes (0 failures)bundle exec standardrb— no offenses🤖 Jose's AI agent