feat: Add EvaluationOptions structure

[NeaguGeorgiana23]

This PR

• Introduces the EvaluationOptions structure to enable per-invocation hook registration and evaluation hints according to the OpenFeature Hooks specification 4.5.1.
• Adds unit tests for EvaluationOptions in test/evaluation_options_test.cpp to ensure proper container initialization, ordering preservation, safe downcasting, hint storage, and copy/move semantics.

Related Issues

Fixes #111

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 401f27f3-c527-4c61-8cac-cf8030a6afe6

📥 Commits

Reviewing files that changed from the base of the PR and between 12c6b9a and c0212cf.

📒 Files selected for processing (5)

• openfeature/BUILD
• openfeature/evaluation_options.h
• test/BUILD
• test/hook_test.cpp
• test/openfeature_api_test.cpp

💤 Files with no reviewable changes (1)

• test/BUILD

🚧 Files skipped from review as they are similar to previous changes (2)

• openfeature/evaluation_options.h
• openfeature/BUILD

---

📝 Walkthrough

Walkthrough

Adds the public EvaluationOptions type and its Bazel target. Updates API tests to use the explicit openfeature namespace and renamed api_ fixture member. Retains equivalent namespace and BUILD formatting.

Changes

Hook and Evaluation APIs

Layer / File(s)	Summary
Evaluation options API and Bazel wiring openfeature/evaluation_options.h, openfeature/BUILD	Defines EvaluationOptions with general hooks and hook hints. Adds the corresponding Bazel C++ library target.
API test namespace and fixture updates test/openfeature_api_test.cpp, test/hook_test.cpp, test/BUILD	Updates API tests to use api_ and an explicit namespace. Retains equivalent namespace and BUILD closing lines.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• open-feature/cpp-sdk#112: Introduces the HookHints type used by EvaluationOptions.
• open-feature/cpp-sdk#114: Adds the HookHints API consumed by EvaluationOptions.
• open-feature/cpp-sdk#118: Introduces the GeneralHook and HookHints APIs used by EvaluationOptions.

Suggested reviewers: oxddr

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The PR also changes unrelated test namespace and API-member references that are not required for EvaluationOptions.	Remove or separately justify the unrelated test namespace and API-member changes, or link them to a specific requirement.
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary change: adding the EvaluationOptions structure.
Description check	✅ Passed	The description directly covers EvaluationOptions, its specification support, tests, and linked issue #111.
Linked Issues check	✅ Passed	The changes implement the EvaluationOptions portion requested by issue #111, including hook registration and evaluation hints.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (4)

test/evaluation_options_test.cpp (1)

5-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Include <utility> for std::move.

std::move is used on line 112, which strictly requires the <utility> header. Although it compiles currently due to transitive includes from other standard library headers, it's best practice to explicitly include the required headers.

♻️ Proposed fix

 `#include` <any>
 `#include` <memory>
 `#include` <string>
+#include <utility>
 `#include` <vector>

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/evaluation_options_test.cpp` around lines 5 - 9, Add the standard
<utility> header to the includes in evaluation_options_test.cpp so the std::move
usage in the test is explicitly supported, without changing the surrounding test
logic.

openfeature/flag_evaluation_details.cpp (1)

1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Include <utility> for std::move.

The code uses std::move extensively, which is defined in <utility>. While it may compile due to transitive inclusions from <string> or <optional>, it is best practice to include it explicitly.

♻️ Proposed fix

 `#include` <string>
 `#include` <string_view>
+#include <utility>
 
 `#include` "openfeature/error_code.h"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openfeature/flag_evaluation_details.cpp` around lines 1 - 12, Update the
includes in flag_evaluation_details.cpp to explicitly add the standard <utility>
header required by the std::move usages, leaving the existing includes and
implementation unchanged.

openfeature/flag_evaluation_details.h (1)

25-26: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Optimize constructor by taking resolution_details by value.

The wrapping constructor currently takes ResolutionDetails<T> by const&, which forces an internal copy of potentially expensive fields (like strings and the generic T mapping). Taking the argument by value and using std::move allows the compiler to optimize out copies when an rvalue is passed.

• openfeature/flag_evaluation_details.h#L25-L26: Change the parameter to take by value: ResolutionDetails<T> resolution_details.
• openfeature/flag_evaluation_details.cpp#L25-L29: Change the parameter to take by value and move it into the base constructor: : ResolutionDetails<T>(std::move(resolution_details)).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openfeature/flag_evaluation_details.h` around lines 25 - 26, Update the
FlagEvaluationDetails constructor in openfeature/flag_evaluation_details.h lines
25-26 to accept ResolutionDetails<T> by value, and update its definition in
openfeature/flag_evaluation_details.cpp lines 25-29 to accept the same value
parameter and move it into the ResolutionDetails<T> base constructor.

openfeature/hook.h (1)

35-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use const HookContext<T>& in Before for consistency.

The ctx parameter is passed by non-const reference in Before, but by const reference in After, Error, and Finally. Since any shared mutable state between stages should be managed internally (e.g. via a shared pointer to HookData), const HookContext<T>& is sufficient for all stages and provides a safer, consistent API contract.

• openfeature/hook.h#L35-L36: Change the parameter to const HookContext<T>& ctx.
• test/hook_test.cpp#L28-L29: Update the overridden TrackingHook::Before signature to match const HookContext<T>& ctx.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openfeature/hook.h` around lines 35 - 36, Change the Before method in
openfeature/hook.h at lines 35-36 to accept const HookContext<T>&, matching the
const contract used by After, Error, and Finally. Update the
TrackingHook::Before override in test/hook_test.cpp at lines 28-29 to use the
same const reference signature.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@openfeature/flag_evaluation_details.cpp`:
- Around line 1-12: Update the includes in flag_evaluation_details.cpp to
explicitly add the standard <utility> header required by the std::move usages,
leaving the existing includes and implementation unchanged.

In `@openfeature/flag_evaluation_details.h`:
- Around line 25-26: Update the FlagEvaluationDetails constructor in
openfeature/flag_evaluation_details.h lines 25-26 to accept ResolutionDetails<T>
by value, and update its definition in openfeature/flag_evaluation_details.cpp
lines 25-29 to accept the same value parameter and move it into the
ResolutionDetails<T> base constructor.

In `@openfeature/hook.h`:
- Around line 35-36: Change the Before method in openfeature/hook.h at lines
35-36 to accept const HookContext<T>&, matching the const contract used by
After, Error, and Finally. Update the TrackingHook::Before override in
test/hook_test.cpp at lines 28-29 to use the same const reference signature.

In `@test/evaluation_options_test.cpp`:
- Around line 5-9: Add the standard <utility> header to the includes in
evaluation_options_test.cpp so the std::move usage in the test is explicitly
supported, without changing the surrounding test logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: da405df6-b178-4523-a0ba-a41108057819

📥 Commits

Reviewing files that changed from the base of the PR and between b7e65b5 and 06bd51c.

📒 Files selected for processing (11)

• openfeature/BUILD
• openfeature/base_hook.h
• openfeature/evaluation_options.h
• openfeature/flag_evaluation_details.cpp
• openfeature/flag_evaluation_details.h
• openfeature/hook.cpp
• openfeature/hook.h
• test/BUILD
• test/evaluation_options_test.cpp
• test/flag_evaluation_details_test.cpp
• test/hook_test.cpp

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

openfeature/BUILD (1)

91-100: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Declare the missing base_hook Bazel target.

evaluation_options and the test targets depend on //openfeature:base_hook, but openfeature/BUILD only defines the base_hook.h header as an included file. Add a cc_library(name = "base_hook", ...) rule for openfeature/base_hook.h.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openfeature/BUILD` around lines 91 - 100, Define the missing Bazel target
`base_hook` in `openfeature/BUILD` as a `cc_library` exposing `base_hook.h`, so
`evaluation_options` and dependent test targets can resolve
`//openfeature:base_hook`.

Source: MCP tools

test/BUILD (1)

131-145: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Keep only one hook_test target.

test/BUILD declares name = "hook_test" twice for hook_test.cpp at lines 132 and 148. Bazel rejects duplicate target names in one package; merge the needed dependencies into one rule or remove the duplicate block.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/BUILD` around lines 131 - 145, Remove the duplicate hook_test
declaration in test/BUILD, retaining a single cc_test target for hook_test and
hook_test.cpp. Merge any unique dependencies from both declarations into the
remaining rule so Bazel has one valid target with all required inputs.

Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@openfeature/BUILD`:
- Around line 91-100: Define the missing Bazel target `base_hook` in
`openfeature/BUILD` as a `cc_library` exposing `base_hook.h`, so
`evaluation_options` and dependent test targets can resolve
`//openfeature:base_hook`.

In `@test/BUILD`:
- Around line 131-145: Remove the duplicate hook_test declaration in test/BUILD,
retaining a single cc_test target for hook_test and hook_test.cpp. Merge any
unique dependencies from both declarations into the remaining rule so Bazel has
one valid target with all required inputs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: db074e88-5f0b-49c8-b716-659ef15c8942

📥 Commits

Reviewing files that changed from the base of the PR and between 06bd51c and 12c6b9a.

📒 Files selected for processing (3)

• openfeature/BUILD
• test/BUILD
• test/evaluation_options_test.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• test/evaluation_options_test.cpp

[Copilot]

Pull request overview

Adds the EvaluationOptions data structure for per-invocation hooks and hook hints.

Changes:

• Adds EvaluationOptions and its Bazel target.
• Adds tests covering storage, ordering, casting, and copy/move behavior.
• Cleans up test namespace usage.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
openfeature/evaluation_options.h	Defines evaluation options.
openfeature/BUILD	Adds the library target.
test/evaluation_options_test.cpp	Tests the new structure.
test/BUILD	Registers the new tests.
test/openfeature_api_test.cpp	Scopes tests to the namespace.
test/hook_test.cpp	Non-functional formatting update.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[m-olko]

To be honest, I don't feel like creating unit test purely for struct is needed. The evaluation_options could even be inlined in other file in my opinion. The tests written here are only checking implementations of cpp internal libraries (maps assignment, array manipulation). As there is no logic written solely for the sdk, and this is just logical grouping of some parameters, I would say that writing tests for that is overkill.

Also do you have any examples of this being used in the code? I couldn't find any references in hanging pr's, and without seeing the purpose and usage, it is hard for me to comment on the quality of this code

[NeaguGeorgiana23]

To be honest, I don't feel like creating unit test purely for struct is needed. The evaluation_options could even be inlined in other file in my opinion. The tests written here are only checking implementations of cpp internal libraries (maps assignment, array manipulation). As there is no logic written solely for the sdk, and this is just logical grouping of some parameters, I would say that writing tests for that is overkill.

Also do you have any examples of this being used in the code? I couldn't find any references in hanging pr's, and without seeing the purpose and usage, it is hard for me to comment on the quality of this code

Fair enough, I will delete the test file.

They will be use as described in OpenFeature Specifications, in the Features.h to overload the getTypeValue :
virtual bool GetBooleanValue(std::string_view flag_key, bool default_value) = 0; virtual bool GetBooleanValue(std::string_view flag_key, bool default_value, const EvaluationContext& ctx) = 0; virtual bool GetBooleanValue(std::string_view flag_key, bool default_value, const EvaluationContext& ctx, EvaluationOptions options) = 0;