Repository navigation
Conversation
Add an opt-in capability for consumers that implement dependent flag evaluation, and verify its wire encoding alongside the existing rules capability. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Evaluate variant-key dependencies in authored order with depth-safe memoization and preserve successful-path exposure eligibility for SDKs. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results📦
|
BenchmarksComparisonBenchmark execution time: 2026-11-06 19:11:59 Comparing candidate commit ad8cc4c in PR branch Found 0 performance improvements and 6 performance regressions! Performance is the same for 8 metrics, 0 unstable metrics.
|
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | execution_time | 31.679ms | 31.815ms ± 0.055ms | 31.813ms ± 0.033ms | 31.841ms | 31.893ms | 31.975ms | 32.025ms | 0.67% | 0.944 | 3.283 | 0.17% | 0.008ms | 1 | 50 |
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | throughput | 78063.532op/s | 78579.686op/s ± 134.721op/s | 78583.922op/s ± 82.589op/s | 78676.577op/s | 78759.475op/s | 78846.615op/s | 78917.295op/s | 0.42% | -0.921 | 3.220 | 0.17% | 19.052op/s | 1 | 50 |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | execution_time | 202.657ms | 203.564ms ± 1.042ms | 203.250ms ± 0.151ms | 203.398ms | 206.276ms | 206.378ms | 206.394ms | 1.55% | 2.143 | 3.016 | 0.50% | 0.208ms | 1 | 25 |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | throughput | 4845.103op/s | 4912.585op/s ± 24.889op/s | 4920.039op/s ± 3.653op/s | 4923.692op/s | 4931.212op/s | 4933.909op/s | 4934.451op/s | 0.29% | -2.136 | 2.999 | 0.50% | 4.978op/s | 1 | 25 |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | execution_time | 687.295µs | 697.997µs ± 4.215µs | 697.440µs ± 2.136µs | 700.636µs | 705.605µs | 705.975µs | 706.473µs | 1.30% | -0.161 | 0.261 | 0.60% | 0.422µs | 1 | 100 |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | throughput | 141548.252op/s | 143272.296op/s ± 866.182op/s | 143381.421op/s ± 440.510op/s | 143741.124op/s | 145067.300op/s | 145411.452op/s | 145497.895op/s | 1.48% | 0.201 | 0.298 | 0.60% | 86.618op/s | 1 | 100 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | execution_time | [31.800ms; 31.830ms] or [-0.048%; +0.048%] | None | None | None |
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | throughput | [78542.344op/s; 78617.028op/s] or [-0.048%; +0.048%] | None | None | None |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | execution_time | [203.155ms; 203.972ms] or [-0.201%; +0.201%] | None | None | None |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | throughput | [4902.829op/s; 4922.342op/s] or [-0.199%; +0.199%] | None | None | None |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | execution_time | [697.171µs; 698.823µs] or [-0.118%; +0.118%] | None | None | None |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | throughput | [143102.528op/s; 143442.065op/s] or [-0.118%; +0.118%] | None | None | None |
Group 2
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| dependent_flags/one_prerequisite | execution_time | 723.014ns | 726.495ns ± 1.151ns | 726.302ns ± 0.259ns | 726.574ns | 728.640ns | 731.151ns | 735.215ns | 1.23% | 3.466 | 20.143 | 0.16% | 0.081ns | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| dependent_flags/one_prerequisite | execution_time | [726.335ns; 726.654ns] or [-0.022%; +0.022%] | None | None | None |
Group 3
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| dependent_flags/memoized_16_references | execution_time | 3.011µs | 3.025µs ± 0.004µs | 3.025µs ± 0.001µs | 3.026µs | 3.029µs | 3.037µs | 3.069µs | 1.45% | 6.088 | 61.785 | 0.14% | 0.000µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| dependent_flags/memoized_16_references | execution_time | [3.025µs; 3.026µs] or [-0.019%; +0.019%] | None | None | None |
Group 4
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | execution_time | 34.832ms | 35.007ms ± 0.078ms | 35.004ms ± 0.057ms | 35.064ms | 35.129ms | 35.166ms | 35.171ms | 0.48% | 0.086 | -0.498 | 0.22% | 0.011ms | 1 | 50 |
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | throughput | 71081.307op/s | 71413.860op/s ± 158.411op/s | 71421.022op/s ± 117.306op/s | 71529.751op/s | 71659.481op/s | 71723.778op/s | 71773.270op/s | 0.49% | -0.077 | -0.497 | 0.22% | 22.403op/s | 1 | 50 |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | execution_time | 194.877ms | 195.515ms ± 0.318ms | 195.591ms ± 0.186ms | 195.674ms | 195.990ms | 196.064ms | 196.082ms | 0.25% | -0.233 | -0.591 | 0.16% | 0.064ms | 1 | 25 |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | throughput | 5099.896op/s | 5114.720op/s ± 8.318op/s | 5112.716op/s ± 4.877op/s | 5119.974op/s | 5128.530op/s | 5130.880op/s | 5131.435op/s | 0.37% | 0.240 | -0.589 | 0.16% | 1.664op/s | 1 | 25 |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | execution_time | 842.821µs | 844.850µs ± 1.289µs | 844.481µs ± 0.929µs | 846.026µs | 846.942µs | 847.274µs | 847.282µs | 0.33% | 0.354 | -1.169 | 0.15% | 0.129µs | 1 | 100 |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | throughput | 118024.396op/s | 118364.461op/s ± 180.465op/s | 118415.932op/s ± 130.246op/s | 118504.529op/s | 118620.668op/s | 118648.818op/s | 118649.097op/s | 0.20% | -0.351 | -1.170 | 0.15% | 18.047op/s | 1 | 100 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | execution_time | [34.986ms; 35.029ms] or [-0.061%; +0.061%] | None | None | None |
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | throughput | [71369.952op/s; 71457.769op/s] or [-0.061%; +0.061%] | None | None | None |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | execution_time | [195.390ms; 195.639ms] or [-0.064%; +0.064%] | None | None | None |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | throughput | [5111.459op/s; 5117.981op/s] or [-0.064%; +0.064%] | None | None | None |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | execution_time | [844.598µs; 845.103µs] or [-0.030%; +0.030%] | None | None | None |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | throughput | [118329.090op/s; 118399.831op/s] or [-0.030%; +0.030%] | None | None | None |
Group 5
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| sdk_test_data/rules-based | execution_time | 304.961µs | 310.539µs ± 11.664µs | 307.568µs ± 1.131µs | 309.601µs | 321.662µs | 358.835µs | 426.880µs | 38.79% | 6.731 | 55.540 | 3.75% | 0.825µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| sdk_test_data/rules-based | execution_time | [308.922µs; 312.155µs] or [-0.521%; +0.521%] | None | None | None |
Group 6
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| dependent_flags/legacy | execution_time | 110.371ns | 110.947ns ± 1.254ns | 110.851ns ± 0.049ns | 110.913ns | 110.997ns | 111.088ns | 128.536ns | 15.95% | 13.903 | 192.531 | 1.13% | 0.089ns | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| dependent_flags/legacy | execution_time | [110.774ns; 111.121ns] or [-0.157%; +0.157%] | None | None | None |
Group 7
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ad8cc4c | 1793991688 | helen/ffe-dependent-flags-evaluator |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| single_flag_killswitch/rules-based | execution_time | 199.023ns | 202.221ns ± 1.994ns | 201.771ns ± 1.317ns | 203.559ns | 205.823ns | 206.979ns | 208.765ns | 3.47% | 0.606 | 0.159 | 0.98% | 0.141ns | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| single_flag_killswitch/rules-based | execution_time | [201.944ns; 202.497ns] or [-0.137%; +0.137%] | None | None | None |
Baseline
Baseline benchmark details
Group 1
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ac6320b | 1793981605 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | execution_time | 31.675ms | 31.740ms ± 0.040ms | 31.734ms ± 0.027ms | 31.766ms | 31.810ms | 31.829ms | 31.841ms | 0.34% | 0.539 | -0.529 | 0.13% | 0.006ms | 1 | 50 |
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | throughput | 78514.296op/s | 78765.216op/s ± 100.106op/s | 78778.840op/s ± 68.290op/s | 78838.459op/s | 78902.724op/s | 78915.841op/s | 78925.543op/s | 0.19% | -0.535 | -0.536 | 0.13% | 14.157op/s | 1 | 50 |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | execution_time | 191.220ms | 191.889ms ± 0.843ms | 191.706ms ± 0.189ms | 191.873ms | 193.923ms | 194.657ms | 194.763ms | 1.59% | 2.611 | 5.923 | 0.43% | 0.169ms | 1 | 25 |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | throughput | 5134.435op/s | 5211.430op/s ± 22.650op/s | 5216.325op/s ± 5.150op/s | 5222.921op/s | 5228.255op/s | 5229.285op/s | 5229.569op/s | 0.25% | -2.596 | 5.868 | 0.43% | 4.530op/s | 1 | 25 |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | execution_time | 690.677µs | 704.882µs ± 5.124µs | 707.322µs ± 1.576µs | 708.457µs | 709.715µs | 710.024µs | 710.231µs | 0.41% | -1.140 | 0.008 | 0.72% | 0.512µs | 1 | 100 |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | throughput | 140799.292op/s | 141875.155op/s ± 1040.006op/s | 141378.262op/s ± 314.265op/s | 142528.415op/s | 143900.063op/s | 144450.972op/s | 144785.479op/s | 2.41% | 1.155 | 0.052 | 0.73% | 104.001op/s | 1 | 100 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | execution_time | [31.729ms; 31.751ms] or [-0.035%; +0.035%] | None | None | None |
| flagevaluation_evp/coalescer/scale/2500flags_500users_20fields | throughput | [78737.469op/s; 78792.964op/s] or [-0.035%; +0.035%] | None | None | None |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | execution_time | [191.559ms; 192.220ms] or [-0.172%; +0.172%] | None | None | None |
| flagevaluation_evp/coalescer/stress/10flags_1000users_250fields | throughput | [5202.551op/s; 5220.309op/s] or [-0.170%; +0.170%] | None | None | None |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | execution_time | [703.878µs; 705.887µs] or [-0.142%; +0.142%] | None | None | None |
| flagevaluation_evp/coalescer/typical/100flags_50users_10fields | throughput | [141671.317op/s; 142078.992op/s] or [-0.144%; +0.144%] | None | None | None |
Group 2
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ac6320b | 1793981605 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | execution_time | 35.166ms | 35.453ms ± 0.106ms | 35.463ms ± 0.066ms | 35.509ms | 35.599ms | 35.656ms | 35.709ms | 0.69% | -0.299 | 0.305 | 0.30% | 0.015ms | 1 | 50 |
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | throughput | 70010.509op/s | 70517.244op/s ± 210.479op/s | 70496.784op/s ± 130.706op/s | 70658.310op/s | 70870.420op/s | 71039.983op/s | 71090.705op/s | 0.84% | 0.318 | 0.321 | 0.30% | 29.766op/s | 1 | 50 |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | execution_time | 182.947ms | 183.650ms ± 0.438ms | 183.607ms ± 0.240ms | 183.851ms | 184.420ms | 184.760ms | 184.839ms | 0.67% | 0.688 | 0.909 | 0.23% | 0.088ms | 1 | 25 |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | throughput | 5410.122op/s | 5445.158op/s ± 12.972op/s | 5446.420op/s ± 7.100op/s | 5450.901op/s | 5465.280op/s | 5465.868op/s | 5466.053op/s | 0.36% | -0.671 | 0.880 | 0.23% | 2.594op/s | 1 | 25 |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | execution_time | 857.989µs | 860.137µs ± 1.292µs | 860.022µs ± 1.026µs | 861.064µs | 862.072µs | 863.200µs | 863.779µs | 0.44% | 0.323 | -0.540 | 0.15% | 0.129µs | 1 | 100 |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | throughput | 115770.382op/s | 116260.804op/s ± 174.516op/s | 116276.045op/s ± 138.566op/s | 116406.920op/s | 116512.959op/s | 116549.810op/s | 116551.599op/s | 0.24% | -0.317 | -0.550 | 0.15% | 17.452op/s | 1 | 100 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | execution_time | [35.423ms; 35.482ms] or [-0.083%; +0.083%] | None | None | None |
| flagevaluation_evp/payloads/scale/2500flags_500users_20fields | throughput | [70458.903op/s; 70575.585op/s] or [-0.083%; +0.083%] | None | None | None |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | execution_time | [183.479ms; 183.822ms] or [-0.094%; +0.094%] | None | None | None |
| flagevaluation_evp/payloads/stress/10flags_1000users_250fields | throughput | [5440.073op/s; 5450.243op/s] or [-0.093%; +0.093%] | None | None | None |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | execution_time | [859.884µs; 860.390µs] or [-0.029%; +0.029%] | None | None | None |
| flagevaluation_evp/payloads/typical/100flags_50users_10fields | throughput | [116226.599op/s; 116295.008op/s] or [-0.029%; +0.029%] | None | None | None |
Group 3
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ac6320b | 1793981605 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| sdk_test_data/rules-based | execution_time | 266.806µs | 272.640µs ± 11.977µs | 269.655µs ± 1.335µs | 271.713µs | 286.294µs | 320.071µs | 392.301µs | 45.48% | 6.684 | 55.115 | 4.38% | 0.847µs | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| sdk_test_data/rules-based | execution_time | [270.980µs; 274.300µs] or [-0.609%; +0.609%] | None | None | None |
Group 4
| cpu_model | git_commit_sha | git_commit_date | git_branch |
|---|---|---|---|
| Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz | ac6320b | 1793981605 | main |
| scenario | metric | min | mean ± sd | median ± mad | p75 | p95 | p99 | max | peak_to_median_ratio | skewness | kurtosis | cv | sem | runs | sample_size |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| single_flag_killswitch/rules-based | execution_time | 163.309ns | 167.448ns ± 3.208ns | 166.681ns ± 2.162ns | 168.935ns | 174.292ns | 175.512ns | 180.208ns | 8.12% | 1.183 | 1.505 | 1.91% | 0.227ns | 1 | 200 |
| scenario | metric | 95% CI mean | Shapiro-Wilk pvalue | Ljung-Box pvalue (lag=1) | Dip test pvalue |
|---|---|---|---|---|---|
| single_flag_killswitch/rules-based | execution_time | [167.003ns; 167.892ns] or [-0.265%; +0.265%] | None | None | None |
Route flags without prerequisite conditions through the allocation-only evaluator, sharing validation and assignment construction with dependent evaluation. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Apply main’s removal of the obsolete Clippy allowance before merging main, preserving the dependent-flags benchmark. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Apply main’s removal of the obsolete Clippy allowance before merging main, preserving the dependent-flags benchmark. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cf7834903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
FFE reviewer selectionReviewers: @typotter, @sameerank Why: Dependent Flags — The shared Rust FFE engine now evaluates prerequisite (flagEvaluation) conditions. Reviewers come from the matched pod. 🤖 Automated selection. Selection detailsInitiative: project / dependent-flags Why this match: The shared Rust FFE engine now evaluates prerequisite (flagEvaluation) conditions. It compares prerequisite variant keys with ONE_OF/NOT_ONE_OF and short-circuits. It handles missing, disabled, cyclic and over-depth prerequisites, enforces maxDependencyDepth with memoized dependency height, and parses prerequisite conditions in the UFC models and compiled config. This is the dependent-flags behavior that SDKs consume, and benchmarks and tests for dependent flags are included.
Additional review: openfeature-architecture — The change adds to the public evaluation contract that SDK providers consume. It exports a new eval_flag_with_trace API that returns EvaluationResult/EvaluationRecord with per-flag results and an exposure_eligible marker. It also adds public EvaluationError variants (DependencyEvaluationFailed, DependencyCycle, DependencyDepthExceeded) that providers must map to OpenFeature error and reason metadata. Coverage: @typotter. Evidence: libdd-ffe/src/rules_based/eval/eval_assignment.rs, libdd-ffe/src/rules_based/error.rs, libdd-ffe/src/rules_based/eval/mod.rs, libdd-ffe/src/rules_based/mod.rs The PR author was excluded. Existing reviewers and teams were preserved. Debug or fix a problemWorkflow run · Bits Code session
Evidence and run details✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head. Generated asynchronously by the FFE reviewer-routing workflow. Changed paths: Evaluated head: Registry revision: Pinned skill source: Instructions: workflow-injected snapshot (no plugin loaded) Instruction snapshot SHA-256: |
typotter
left a comment
There was a problem hiding this comment.
🤖 Review from Codex working with Tyler Potter:
Requesting changes for the inline finding.
Validate dependency limits only when an authored condition reaches a prerequisite. Preserve independent evaluations and skipped branches, align condition validation and empty-list membership with JS, and record prerequisites rejected by depth. Cover lazy validation, null limits, trace ordering, and per-flag isolation. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
| Some(value) => { | ||
| let depth = value | ||
| .as_f64() | ||
| .filter(|depth| depth.fract() == 0.0 && (0.0..=255.0).contains(depth)) |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] A 255-edge dependency chain overflows small thread stacks. Please lower the accepted maximum, for example to 32.
The evaluation recurses on the caller's stack, several frames per dependency edge. I evaluated a 255-edge chain with maxDependencyDepth: 255 on threads with fixed stack sizes:
| Build | 128 KiB | 256 KiB | 512 KiB | 1 MiB |
|---|---|---|---|---|
| release | overflow | overflow | pass | pass |
| debug | overflow | overflow | overflow | overflow |
Each "overflow" run reported has overflowed its stack; Rust aborts the process on a stack overflow. Host threads with small stacks exist, so a valid configuration should not be able to crash the process.
Suggested change: accept 0..=32 here (and in the README). That cuts the maximum recursion depth by about eight times; I did not measure a 32-edge chain. Make the same change in the other evaluators, the fixtures and the design document, so all SDKs accept the same range.
|
|
||
| /// One requested evaluation and the prerequisites it actually evaluated. | ||
| #[derive(Debug)] | ||
| pub struct EvaluationResult { |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Two small API points for the new trace types.
EvaluationResultandEvaluationRecordare public structs with public fields and no#[non_exhaustive]. Adding a field later is a semver-major change forlibdd-ffe. feat(ffe): support dependent flag evaluations dd-trace-php#4421 already buildsEvaluationResultwith a struct literal. Suggest#[non_exhaustive]on both, plus a constructor such asEvaluationResult::from_error(error).eval_flag_with_tracedoes not emit thelog::trace!records, so its direct callers (the C FFI in feat(ffe): expose prerequisite evaluations through C FFI #2791, and the Python and PHP bindings) lose those diagnostics.eval_flag_with_optionsalready callseval_flag_with_traceand logs afterwards, so suggest moving the logging intoeval_flag_with_trace(covering its early returns) and removing it fromeval_flag_with_options, so each evaluation logs once.
| /// Records exclude the requested root flag and occur once per evaluated prerequisite. | ||
| /// An unsuccessful root has no eligible prerequisite exposures. SDKs must additionally | ||
| /// honor each assignment's `do_log`, consent, and exposure deduplication policy. | ||
| pub fn eval_flag_with_trace( |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Heads up: This PR conflicts with main, and the recorded checks at 983b4b0 contain no GitHub Actions test results. Several consumers also pin an older revision of this branch.
- The conflict is in this file:
mainnow has feat(ffe)!: parse split features and expose them over the FFI #2773, which addsfeatures: split.features.clone()to the assignment literal. Keep this PR's structure and add that field toAllocation::assignment. - At
983b4b0, the GitHub checks show only skipped rustfmt runs, the pending merge gate, and GitLab pipelines. I rancargo test -p libdd-ffe --features exposure-events,evaluation-metrics,flagevaluation-evplocally on983b4b0: 137 tests passed, plus 10 doc tests. - feat(ffe): expose prerequisite evaluations through C FFI #2791 is based on
1cf7834, not on this head. feat(openfeature): support dependent flags on Python delivery paths dd-trace-py#21346 pins1cf7834. The evaluator files in feat(ffe): support dependent flag evaluations dd-trace-php#4421's submodule pin are identical to1cf7834. Between1cf7834and this head, handling of an invalidmaxDependencyDepth(for example256no longer rejects the whole configuration) and of empty value lists changed. - Rebasing on
mainalso brings in fix: preserve session tokens on trace stats requests #2794 (session tokens on trace stats requests). DataDog/dd-trace-pymainalready ships that fix through v46.1.1, so consumers should not pin a revision without it.
Suggested order: rebase this PR and #2791 on main, merge both, and release. Then update the Python and PHP native pins to that release, and update Ruby to the published gem that contains both PRs.
| .any(|value| value.as_ref() == dependency.assignment.variation_key.as_str()) | ||
| == *expected_membership; | ||
| if matched { | ||
| exposures.insert(key.clone()); |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P1] Proposed exposure rule change, to follow the worked example in the dependent-flags design: a prerequisite whose own comparison rejects the rule should remain eligible for an exposure.
This block adds a prerequisite to exposures only when its condition matches. The design's example uses a rule that requires both a user attribute (country) and a prerequisite flag's value. For a user who matches the attribute but whose prerequisite evaluates to the other variant, the root falls back, and the example lists both the root and the prerequisite for exposure handling (subject to the usual logging rules and deduplication). The design's numbered rule about rejected rules can be read more broadly, so the design is ambiguous; we would like to follow the example.
Suggested evaluator change: when a FlagEvaluation condition is the condition that rejects the rule, keep that key (and dependency.exposures) in a flag-level "influencing" set, and mark those records exposure_eligible when the root succeeds. Rules rejected by another condition, and root failures, keep today's behavior.
The shared fixture cases dependent-one-of-miss and dependent-not-one-of-hit in ffe-system-test-data need the same change, and so do the other evaluators.
| // Copyright 2026-Present Datadog, Inc. https://www.datadoghq.com/ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| use super::eval_assignment::*; |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] This PR does not run the shared dependent-flags cases from ffe-system-test-data, so no checked-in shared case checks this implementation's dependency results or telemetry.
- The fixture pin here (
libdd-ffe-test-suite/ffe-system-test-dataat0781ce2) predatesevaluation-suites/dependent-flags/. [FFL-1695] test: add variant-key dependent flag conformance fixtures ffe-system-test-data#45 added that suite, and [FFL-1795] Add inapplicable dependent exposure fixture ffe-system-test-data#57, [FFL-1795] Validate rejected dependency exposure not logged ffe-system-test-data#58 and test: cover unassigned prerequisite failure ffe-system-test-data#60 extended it. feat: add evaluation case applicability controls ffe-system-test-data#56 added the case applicability controls. The reviewed fixture revision isb57eac6onmain. libdd-ffe-test-suite/tests/canonical_fixtures.rsreads only the rootufc-config.jsonandevaluation-cases/, and it callsget_assignment, so it cannot see trace records.
Suggested change: bump the fixtures to b57eac6 and add a runner for evaluation-suites/dependent-flags (or, more generally, every suite directory). Assert result and, where present, expectations.exposures and expectations.evaluationEvents (including _count and noUnmatchedEvents). Assert the case count (14 today), so the runner cannot pass with zero cases. I wrote a runner for that one suite and ran it: with the fixtures at b57eac6, all 14 cases pass at 983b4b0, and the core canonical test still passes. If every record is marked exposure_eligible, the runner fails.
Runner for the dependent-flags suite (tested)
diff --git a/libdd-ffe-test-suite/tests/canonical_fixtures.rs b/libdd-ffe-test-suite/tests/canonical_fixtures.rs
index 8d7c8b69d..d15044704 100644
--- a/libdd-ffe-test-suite/tests/canonical_fixtures.rs
+++ b/libdd-ffe-test-suite/tests/canonical_fixtures.rs
@@ -244,3 +244,150 @@ fn error_code_from_error(err: &EvaluationError) -> Option<&'static str> {
_ => Some("GENERAL"),
}
}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct SuiteCase {
+ flag: String,
+ variation_type: FlagType,
+ default_value: serde_json::Value,
+ targeting_key: Option<Str>,
+ #[serde(default)]
+ attributes: HashMap<Str, serde_json::Value>,
+ result: TestResult,
+ expectations: Option<serde_json::Value>,
+}
+
+/// Removes one actual event that matches every field the matcher states.
+fn take_match(actual: &mut Vec<serde_json::Value>, matcher: &serde_json::Value) -> bool {
+ let fields: Vec<_> = matcher
+ .as_object()
+ .unwrap()
+ .iter()
+ .filter(|(key, _)| !key.starts_with('_'))
+ .collect();
+ let Some(index) = actual.iter().position(|event| {
+ fields
+ .iter()
+ .all(|(key, value)| event.get(key.as_str()) == Some(value))
+ }) else {
+ return false;
+ };
+ actual.remove(index);
+ true
+}
+
+fn assert_events(
+ kind: &str,
+ flag: &str,
+ mut actual: Vec<serde_json::Value>,
+ expectations: &serde_json::Value,
+) {
+ for matcher in expectations[kind].as_array().cloned().unwrap_or_default() {
+ let count = matcher
+ .get("_count")
+ .and_then(|count| count.as_u64())
+ .unwrap_or(1);
+ for _ in 0..count {
+ assert!(
+ take_match(&mut actual, &matcher),
+ "{flag}: missing {kind} {matcher}; left {actual:?}"
+ );
+ }
+ }
+ if expectations["noUnmatchedEvents"] == serde_json::Value::Bool(true) {
+ assert!(actual.is_empty(), "{flag}: unexpected {kind} {actual:?}");
+ }
+}
+
+#[test]
+#[cfg_attr(miri, ignore)]
+fn evaluates_dependent_flags_suite() {
+ let suite = fixture_root().join("evaluation-suites/dependent-flags");
+ let config = Configuration::from_server_response(
+ UniversalFlagConfig::from_json(fs::read(suite.join("ufc-config.json")).unwrap()).unwrap(),
+ );
+ let cases: Vec<SuiteCase> =
+ serde_json::from_slice(&fs::read(suite.join("cases/dependent-flags.json")).unwrap())
+ .unwrap();
+ assert_eq!(cases.len(), 14);
+ for case in cases {
+ let attributes = case
+ .attributes
+ .into_iter()
+ .map(|(key, value)| (key, serde_json::from_value::<Attribute>(value).unwrap()))
+ .collect();
+ let subject = EvaluationContext::new(case.targeting_key, Arc::new(attributes));
+ let trace = config.eval_flag_with_trace(
+ &case.flag,
+ &subject,
+ case.variation_type.into(),
+ Utc::now(),
+ EvaluationOptions::default(),
+ );
+ let (value, reason, code) = match &trace.result {
+ Ok(assignment) => (
+ assignment.value.variation_value(),
+ reason_from_assignment(assignment.reason),
+ None,
+ ),
+ Err(err) => (
+ case.default_value.clone(),
+ reason_from_error(err),
+ error_code_from_error(err),
+ ),
+ };
+ assert_eq!(value, case.result.value, "{}", case.flag);
+ if let Some(expected) = case.result.reason.as_deref() {
+ assert_eq!(reason, expected, "{}", case.flag);
+ }
+ if let Some(expected) = case.result.error_code.as_deref() {
+ assert_eq!(code, Some(expected), "{}", case.flag);
+ }
+ let Some(expectations) = case.expectations else {
+ continue;
+ };
+ // This runner reports an unassigned or disabled prerequisite record as GENERAL, as the no-assignment case expects.
+ let mut events: Vec<serde_json::Value> = trace
+ .evaluations
+ .iter()
+ .map(|record| {
+ let code = match &record.result {
+ Ok(_) => None,
+ Err(EvaluationError::FlagDisabled | EvaluationError::DefaultAllocationNull) => {
+ Some("GENERAL")
+ }
+ Err(err) => error_code_from_error(err),
+ };
+ serde_json::json!({"flag": record.flag_key.as_str(), "errorCode": code})
+ })
+ .collect();
+ events.push(serde_json::json!({"flag": case.flag, "errorCode": code}));
+ assert_events("evaluationEvents", &case.flag, events, &expectations);
+
+ let mut exposures: Vec<serde_json::Value> = trace
+ .evaluations
+ .iter()
+ .filter_map(|record| match &record.result {
+ Ok(assignment) if record.exposure_eligible && assignment.do_log => {
+ Some(serde_json::json!({
+ "flag": record.flag_key.as_str(),
+ "allocation": assignment.allocation_key.as_str(),
+ "variant": assignment.variation_key.as_str(),
+ }))
+ }
+ _ => None,
+ })
+ .collect();
+ if let Ok(assignment) = &trace.result {
+ if assignment.do_log {
+ exposures.push(serde_json::json!({
+ "flag": case.flag,
+ "allocation": assignment.allocation_key.as_str(),
+ "variant": assignment.variation_key.as_str(),
+ }));
+ }
+ }
+ assert_events("exposures", &case.flag, exposures, &expectations);
+ }
+}| if depth + result.height > limit { | ||
| return Err(EvaluationError::DependencyDepthExceeded); | ||
| } | ||
| return Ok(result.clone()); |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] No test reuses a memoized prerequisite that has its own successful prerequisites, so a regression in this line passes all existing dependent-flag tests.
I changed this line to drop the nested exposures on a memo hit: return Ok(EvaluatedFlag { exposures: HashSet::new(), ..result.clone() });. All 27 dependent-flag tests still passed. The existing successful memo-reuse tests reuse leaf prerequisites, and two shared fixture cases (reuse across allocations, and a transitive success followed by a root rejection) have no exact Rust equivalent.
Suggested tests (tested)
All three pass on 983b4b0. memoized_transitive_prerequisite_keeps_nested_exposures_when_reused fails with the mutation above.
diff --git a/libdd-ffe/src/rules_based/eval/dependent_flags_tests.rs b/libdd-ffe/src/rules_based/eval/dependent_flags_tests.rs
index 23fc808ad..bebd00390 100644
--- a/libdd-ffe/src/rules_based/eval/dependent_flags_tests.rs
+++ b/libdd-ffe/src/rules_based/eval/dependent_flags_tests.rs
@@ -687,3 +687,52 @@ fn independent_flags_ignore_unrelated_dependency_graphs() {
assert_eq!(result.result.unwrap().variation_key.as_str(), "on");
assert!(result.evaluations.is_empty());
}
+
+#[test]
+fn memoized_transitive_prerequisite_keeps_nested_exposures_when_reused() {
+ let mut root = flag(vec![]);
+ root["allocations"][0]["rules"] = json!([
+ {"conditions": [dependency("parent"), fails()]},
+ {"conditions": [dependency("parent")]}
+ ]);
+ let r = eval(wire(
+ json!({"root": root, "parent": flag(vec![dependency("leaf")]), "leaf": flag(vec![])}),
+ ));
+ assert_eq!(r.result.unwrap().variation_key.as_str(), "on");
+ assert_eq!(
+ r.evaluations
+ .iter()
+ .map(|e| e.flag_key.as_str())
+ .collect::<Vec<_>>(),
+ ["leaf", "parent"]
+ );
+ assert!(r.evaluations.iter().all(|e| e.exposure_eligible));
+}
+
+#[test]
+fn transitive_success_then_root_rule_rejection_exposes_nothing() {
+ let r = eval(wire(
+ json!({"root": flag(vec![dependency("parent"), fails()]),
+ "parent": flag(vec![dependency("leaf")]), "leaf": flag(vec![])}),
+ ));
+ assert_eq!(r.result.unwrap().variation_key.as_str(), "off");
+ assert_eq!(r.evaluations.len(), 2);
+ assert!(
+ r.evaluations
+ .iter()
+ .all(|e| e.result.is_ok() && !e.exposure_eligible)
+ );
+}
+
+#[test]
+fn prerequisite_first_reached_in_rejected_allocation_is_exposed_once_by_later_allocation() {
+ let mut root = flag(vec![dependency("parent"), fails()]);
+ root["allocations"]
+ .as_array_mut()
+ .unwrap()
+ .insert(1, allocation(vec![dependency("parent")], "on"));
+ let r = eval(wire(json!({"root": root, "parent": flag(vec![])})));
+ assert_eq!(r.result.unwrap().variation_key.as_str(), "on");
+ assert_eq!(r.evaluations.len(), 1);
+ assert!(r.evaluations[0].exposure_eligible);
+}| marked `exposure_eligible` belong to the root's successful rule/split path; a failed root has | ||
| no eligible prerequisite exposures. SDKs must | ||
| also apply each assignment's `do_log`, consent, and ordinary exposure deduplication policy. | ||
| The trace does not emit telemetry itself. SDKs should advertise dependent-flag support only |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Reminder: release order for the dependent-flags stack.
- Rebase this PR and feat(ffe): expose prerequisite evaluations through C FFI #2791 on
main, merge, and release libdatadog. - Update the native pins in feat(openfeature): support dependent flags on Python delivery paths dd-trace-py#21346 and feat(ffe): support dependent flag evaluations dd-trace-php#4421 to that release, and update feat(open_feature): support dependent flag evaluations dd-trace-rb#6613 to the published libdatadog gem that contains both PRs.
- Server-side capability gating must be live before any SDK advertises
dependent-flag-variant-key-v1.
|
|
||
| #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub(crate) struct FlagEvaluationWire { |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P1] An unknown key inside flagEvaluation makes the flag that contains it fail. #[serde(deny_unknown_fields)] on FlagEvaluationWire rejects any key other than key, so a future field such as {"key": "beta", "property": "variant"} makes the flag fail and return the caller default. dd-trace-py and dd-trace-rb inherit this through libdd-ffe. Other flags still work, unless their evaluation reaches the failing flag as a prerequisite.
We want UFC parsing to tolerate newly added fields. We made the same request for the edge evaluator in ddoghq/dd-source#155394. The dependent-flags design leaves evaluating other prerequisite properties, such as enablement state and error codes, open as future extensions.
Bench result (local merged SDK builds, with the ffe-dogfooding staging UFC plus synthetic flags): the scenario added unknown keys inside each flagEvaluation, on each dependent condition, and in evaluatorParams. Go, Node.js, Java, .NET, Python and Ruby returned errors for every tested dependent flag, because each one rejects the extra flagEvaluation key. PHP, whose libdatadog branch has no deny_unknown_fields on FlagEvaluationWire, returned the expected values.
Suggested change: read key, ignore other keys, and keep rejecting a missing or empty key (remove deny_unknown_fields from FlagEvaluationWire, and change the extra-key case in invalid_dependency_conditions_are_isolated_to_the_containing_flag to expect acceptance). If strict parsing is intended instead, the server must keep new fields away from older clients, for example through a new capability version. Either way, please add a shared case with an extra flagEvaluation key to the ffe-system-test-data dependent-flags suite, so that all SDKs agree.
There was a problem hiding this comment.
going to make this change. system test to ensure this behavior added in ffe-system-test-data #71: and system-tests #8378
Run the merged canonical dependent-flag suite through the trace API and verify selected-path exposure eligibility alongside evaluation records and errors. Document SDK finalization requirements. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
What does this PR do?
Evaluates
flagEvaluationconditions using prerequisite variant keys withONE_OFandNOT_ONE_OF, preserving authored order, short-circuiting, and per-root memoization. A mismatch rejects the rule; a reached missing, disabled, malformed, unassigned, cyclic, or over-depth prerequisite aborts the root and preserves the failure cause.Configuration::eval_flag_with_tracereturns each reached prerequisite once, including errors and nodes rejected by the depth guard. Evaluation records survive rejected paths. Only prerequisites contributing to the selected rule and split are markedexposure_eligible; a failed root has no eligible prerequisite exposures. The selected assignment retains its UFCdo_logbit, including fallback allocations.SDK consumers must additionally honor
do_log, privacy, final OpenFeature hook success, and ordinary exposure deduplication. Exposure publication and cache admission belong after successful finalization; evaluation records remain reportable on error. The engine does not publish telemetry itself. Python consumes this API in dd-trace-py #21346.Dependency depth counts edges, defaults to 10, and accepts explicit limits from 0 through 255. Limit validation is deferred until a dependency condition executes, so invalid/null settings do not break independent or short-circuited evaluations. Memoized results retain the deepest visited branch, including rejected paths, to prevent reuse from bypassing the limit. Dependency-free flags retain their allocation-only fast path.
Shared contract coverage
Updates the canonical fixture submodule to merged revision
9afc3f75c3fef9643a9c1b5e2b1e5a9adac48177and activates all 14 dependent-flag cases inlibdd-ffe-test-suite. The runner asserts results, specified metadata, prerequisite/root evaluation records, and selected-path exposure eligibility, including exact-event expectations and platform selection controls. It runs in the existing crate test CI without requiring a separate activation flag.Validation
Integration and release
Stacked on #2745 for the native RC capability enum. This PR does not automatically advertise capabilities. Consumers must integrate and test both evaluation and exposure reporting before enabling dependent-flag delivery; a native library release remains separate.