Skip to content

fix: Reasons for environment default flags - #347

Merged
bakirFS merged 6 commits into
mainfrom
fix/reason-environment-default
Oct 8, 2026
Merged

bakirFS merged 6 commits into
mainfrom
fix/reason-environment-default

Conversation

@bakirFS

@bakirFS bakirFS commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Closes #341

When a flag falls back to its environment default, the engine always returned the DEFAULT reason.

Now:

  • STATIC if no segment overrides the feature
  • DEFAULT if the feature has segment overrides, but none matched

@bakirFS
bakirFS requested a review from a team as a code owner September 30, 2026 09:36
@bakirFS
bakirFS requested review from emyller and removed request for a team September 30, 2026 09:36
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 30 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 56eb3671-c541-4da4-a434-d0e55b7b124f
📥 Commits

Reviewing files that changed from the base of the PR and between edc1d08 and 0ae1af5.

📒 Files selected for processing (2)
  • .gitmodules
  • tests/engine_tests/engine-test-data

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2879153d-8478-4b74-8c50-6ce4c0693c8f
📥 Commits

Reviewing files that changed from the base of the PR and between fdf0448 and edc1d08.

📒 Files selected for processing (3)
  • .gitmodules
  • tests/engine_tests/engine-test-data
  • tests/unit/test_engine.py
💤 Files with no reviewable changes (1)
  • tests/unit/test_engine.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Feature evaluation assigns DEFAULT when segment overrides target a feature, and STATIC when they do not. Dependency resolution applies the same distinction when no segment override matches. Unit tests update expected reasons. The engine test-data submodule branch and commit reference also change.

Priority: ⬇️ Low

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

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to edc1d

Environment-default flags now report STATIC when no segment overrides target the feature, and DEFAULT when overrides exist but none match. No merge-blocking risk was found in the supplied changes.

Architecture Summary

Architecture risk: 🔵 Low · up to fdf04

The change affects 2 systems.

Changed systems: tests, flag_engine

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — tests (service) was modified; 2 changed files map to changed impact.
  • observed — flag_engine (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in flag_engine/segments/evaluator.py: evaluate_features now collects feature names targeted by segment overrides before evaluating features.
  • observed — Modified behavior in flag_engine/segments/evaluator.py: Fallback reasons now come from get_fallback_reason rather than always being DEFAULT; features targeted by any segment get DEFAULT, while untargeted features get STATIC. The new helper gathers override feature names across all segments and applies that rule.
  • observed — Modified behavior in flag_engine/segments/evaluator.py: Adds get_targeted_feature_names to collect override names from segment contexts and get_fallback_reason to return DEFAULT for targeted features and STATIC otherwise.
  • observed — Modified behavior in flag_engine/segments/evaluator.py: When dependency resolution finds no matching segment override, the fallback reason now uses the feature’s segment-key index: DEFAULT if it appears in that index, otherwise STATIC, replacing the unconditional DEFAULT.

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 51341f3c-fda6-4137-8c93-7e7e4d96bded

📥 Commits

Reviewing files that changed from the base of the PR and between 31dc381 and 1549b48.

📒 Files selected for processing (2)
  • flag_engine/segments/evaluator.py
  • tests/unit/test_engine.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread tests/unit/test_engine.py Outdated
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

File Coverage Missing
All files 100% ✅

Minimum allowed coverage is 100%

Generated by 🐒 cobertura-action against 0ae1af5

@codspeed

codspeed Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 1 untouched benchmark


Comparing fix/reason-environment-default (0ae1af5) with main (31dc381)

Open in CodSpeed

@khvn26 khvn26 changed the title fix: reason environment default fix: Reasons for environment default Oct 6, 2026
@khvn26 khvn26 changed the title fix: Reasons for environment default fix: Reasons for environment default flags Oct 6, 2026

@khvn26 khvn26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple changes needed.

Comment thread tests/unit/test_engine.py Outdated
Comment thread tests/engine_tests/engine-test-data
Comment thread flag_engine/segments/evaluator.py
bakirFS and others added 2 commits October 7, 2026 13:06
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bakirFS
bakirFS requested a review from khvn26 October 7, 2026 11:29

@khvn26 khvn26 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@bakirFS
bakirFS merged commit 534cc99 into main Oct 8, 2026
9 checks passed
@bakirFS
bakirFS deleted the fix/reason-environment-default branch October 8, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect reason used for environment default flags

4 participants