Skip to content

Fix handling of booleans in conditions #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented May 23, 2025

Booleans are currently handled very poorly.

Especially when the context contains real true or false booleans.

The Amplitude UI often shows True and False as auto completions.

Since this SDK uses strval to convert a boolean to a string value, a boolean true becomes string 1. And boolean false becomes string 0.

But if the flag's condition uses string True or False, it won't match.

To solve this, we add more test cases, and refactor the matchesIs in the EvaluationEngine.

I also tried to add the following test cases:

        yield [false, ['false']];
        yield [false, ['False']];
        yield [false, ['0']];

But these fail because they are intercepted and handled as nullables:

private function matchCondition(array $target, array $condition): bool
{
$propValue = select($target, $condition['selector']);
if (!$propValue && $propValue !== '0') {
return $this->matchNull($condition['op'], $condition['values']);

While inspecting the code responsible for that, I really think this Evaluation Engine shouldn't have been made loosely typed but strictly typed instead 🤯

ruudk added 2 commits May 23, 2025 14:12
This adds test cases for all the current supported boolean comparisons.
Booleans are currently handled very poorly.

Especially when the context contains real `true` or `false` booleans.

The Amplitude UI often shows `True` and `False` as auto completions.

Since this SDK uses `strval` to convert a boolean to a string value,
a boolean `true` becomes string `1`. And boolean `false` becomes string `0`.

But if the flag's condition uses string `True` or `False`, it won't match.

To solve this, we add more test cases, and refactor the `matchesIs` in the EvaluationEngine.
Copy link

promptless bot commented May 23, 2025

📝 Documentation updates detected!

New suggestion: Update PHP SDK documentation for boolean handling in conditions

@tyiuhc
Copy link
Collaborator

tyiuhc commented May 29, 2025

@ruudk thanks for raising this issue! This bug is addressed in v1.2.3, please let us know if it is not functioning as expected.

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.

2 participants