feat: Ruby 3.3 compatibility and cross-SDK improvements #37
Conversation
WalkthroughThe PR updates Ruby from 3.0.6 to 3.3.10, adds attribute-sequence tracking via Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
joalves
force-pushed
the
feat/ruby-3.3-compatibility
branch
from
1107860 to
9f29c56
Compare
- Add custom_field_value method for accessing custom field values - Fix type coercion for custom field values (json, number, boolean types)
joalves
force-pushed
the
feat/ruby-3.3-compatibility
branch
from
48aa997 to
ad0aea2
Compare
- Upgrade Ruby version from 3.0.6 to 3.3.10 - Update gem dependencies for compatibility - Add peek alias to peek_treatment method
joalves
force-pushed
the
feat/ruby-3.3-compatibility
branch
from
ad0aea2 to
3c01a7a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/context.rb (1)
555-560: Handle boolean custom fields provided as actual booleans or differently-cased strings.
custom_value == "true"will coercetrue(boolean) and"TRUE"tofalse, which can silently flip values if upstream JSON booleans or case variants appear. Consider normalising.🔧 Proposed fix
- elsif custom_field_value.type.start_with?("boolean") - value.value = custom_value == "true" + elsif custom_field_value.type.start_with?("boolean") + value.value = (custom_value == true) || custom_value.to_s.casecmp?("true")
- Add object key containment test for in_operator - Add peek_treatment audience re-evaluation tests - Add treatment audience re-evaluation tests for strict/non-strict modes - Add tests for audience cache invalidation behavior
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/context.rb`:
- Around line 558-559: The boolean branch using
custom_field_value.type.start_with?("boolean") currently sets value.value =
custom_value == "true", which fails when custom_value is already a Ruby boolean;
update the assignment to correctly handle actual booleans and string booleans by
detecting the incoming type (e.g., TrueClass/FalseClass) and returning it
directly, and for string values normalize (downcase) and compare to "true";
ensure falsy/other types are coerced to false as appropriate so value.value
becomes a proper boolean in all cases.
🧹 Nitpick comments (2)
spec/spec_helper.rb (1)
3-6: Remove the duplicaterequire "ostruct".It’s redundant (require is idempotent) and adds noise to the test setup.
♻️ Proposed tidy-up
require "absmartly" require "helpers" -require "ostruct"lib/context.rb (1)
382-388: Consider extracting attribute hash construction to a helper.The same attribute-to-hash logic now appears in multiple places; a small helper would avoid divergence.
♻️ Suggested refactor
+ def attributes_hash + `@attributes.each_with_object`({}) do |attr, hash| + hash[attr.name] = attr.value + end + end + def audience_matches(experiment, assignment) if !experiment.audience.nil? && experiment.audience.size > 0 if `@attrs_seq` > (assignment.attrs_seq || 0) - attrs = `@attributes.inject`({}) do |hash, attr| - hash[attr.name] = attr.value - hash - end + attrs = attributes_hash match = `@audience_matcher.evaluate`(experiment.audience, attrs) new_audience_mismatch = match && !match.result @@ if !experiment.data.audience.nil? && experiment.data.audience.size > 0 - attrs = `@attributes.inject`({}) do |hash, attr| - hash[attr.name] = attr.value - hash - end + attrs = attributes_hash match = `@audience_matcher.evaluate`(experiment.data.audience, attrs)
| elsif custom_field_value.type.start_with?("boolean") | ||
| value.value = custom_value.to_bool | ||
| value.value = custom_value == "true" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the actual code at the specified lines in lib/context.rb
cat -n lib/context.rb | sed -n '550,570p'Repository: absmartly/ruby-sdk
Length of output: 986
🏁 Script executed:
# Search for custom_field_value usage patterns and payloads
rg -n "custom_field_value|custom_value" lib/context.rb -B 5 -A 5 | head -100Repository: absmartly/ruby-sdk
Length of output: 1900
🏁 Script executed:
# Look for any test files that might show payload examples
fd -t f '\.(test|spec)\.rb$|fixtures' -x rg -l "custom_field_value|custom.*boolean" {}Repository: absmartly/ruby-sdk
Length of output: 44
🏁 Script executed:
# Search for schema or API documentation about custom fields
rg -n "boolean.*custom|custom.*boolean" -g '*.{rb,md,json,yml,yaml}' -B 2 -A 2Repository: absmartly/ruby-sdk
Length of output: 399
🏁 Script executed:
# Search for test files and fixtures that might show payload structure
find . -type f \( -name '*test*.rb' -o -name '*spec*.rb' -o -name '*fixture*' \) | head -20Repository: absmartly/ruby-sdk
Length of output: 846
🏁 Script executed:
# Look for JSON parsing or deserialization of custom_field_values
rg -n "custom_field" -A 10 -B 2 | grep -E "parse|json|deserialize|from|struct" -i | head -30Repository: absmartly/ruby-sdk
Length of output: 2672
🏁 Script executed:
# Search for example payloads in README or documentation
fd -t f '\.(md|txt|json)$' -x rg -l "custom_field" {}Repository: absmartly/ruby-sdk
Length of output: 44
🏁 Script executed:
# Look for the definition/model of custom_field_value
rg -n "class.*CustomField|attr.*custom_field" -g '*.rb' | head -20Repository: absmartly/ruby-sdk
Length of output: 167
🏁 Script executed:
# Read the CustomFieldValue class definition
cat -n lib/json/custom_field_value.rbRepository: absmartly/ruby-sdk
Length of output: 882
🏁 Script executed:
# Search for boolean custom_field examples in tests
rg -n "boolean" spec/ -B 3 -A 3Repository: absmartly/ruby-sdk
Length of output: 15920
🏁 Script executed:
# Search for CustomFieldValue initialization with boolean type
rg -n "CustomFieldValue.*boolean|boolean.*CustomFieldValue" spec/Repository: absmartly/ruby-sdk
Length of output: 44
🏁 Script executed:
# Look at the full context spec to see how boolean is used
rg -n "custom_field.*boolean|boolean.*custom_field" -A 5 -B 5 spec/context_spec.rbRepository: absmartly/ruby-sdk
Length of output: 44
🏁 Script executed:
# Search for DefaultContextDataDeserializer implementation to see how JSON is parsed
rg -n "class DefaultContextDataDeserializer|custom_field" lib/ -A 20 | head -80Repository: absmartly/ruby-sdk
Length of output: 3513
🏁 Script executed:
# Look for JSON parsing or deserialize_custom_field methods
rg -n "deserialize|parse.*custom_field|json.*custom" lib/ -iRepository: absmartly/ruby-sdk
Length of output: 3195
🏁 Script executed:
# Check if there's any README or API documentation showing custom_field_value structure
fd -t f '\.(md|txt|json)$' | xargs grep -l "custom_field\|customField" 2>/dev/nullRepository: absmartly/ruby-sdk
Length of output: 98
🏁 Script executed:
# Search for how values are actually assigned - look for JSON.parse calls
rg -n "JSON.parse|JSON.generate|\.to_h|from_json" lib/ -B 2 -A 2 | head -50Repository: absmartly/ruby-sdk
Length of output: 1388
🏁 Script executed:
# Check the actual fixture payload to see how custom_field_values are structured
cat -n spec/fixtures/resources/context.json | head -150Repository: absmartly/ruby-sdk
Length of output: 3519
🏁 Script executed:
# Search for any integration tests that might show actual API payload
rg -n "custom_field" spec/ --type jsonRepository: absmartly/ruby-sdk
Length of output: 44
Handle boolean custom field values that arrive as actual booleans.
When the API sends a boolean value (e.g., true or false) in JSON, JSON.parse preserves the type, converting it to a Ruby boolean rather than a string. The current comparison custom_value == "true" will fail: true == "true" evaluates to false, breaking the logic for actual boolean values.
Suggested fix
- value.value = custom_value == "true"
+ value.value = (custom_value == true) || (custom_value.to_s.downcase == "true")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elsif custom_field_value.type.start_with?("boolean") | |
| value.value = custom_value.to_bool | |
| value.value = custom_value == "true" | |
| elsif custom_field_value.type.start_with?("boolean") | |
| value.value = (custom_value == true) || (custom_value.to_s.downcase == "true") |
🤖 Prompt for AI Agents
In `@lib/context.rb` around lines 558 - 559, The boolean branch using
custom_field_value.type.start_with?("boolean") currently sets value.value =
custom_value == "true", which fails when custom_value is already a Ruby boolean;
update the assignment to correctly handle actual booleans and string booleans by
detecting the incoming type (e.g., TrueClass/FalseClass) and returning it
directly, and for string values normalize (downcase) and compare to "true";
ensure falsy/other types are coerced to false as appropriate so value.value
becomes a proper boolean in all cases.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
Code review fixesFixed in commit fb2e0f4:
Lines 3 to 5 in fb2e0f4 Not a bug (false positive):
https://github.com/absmartly/javascript-sdk/blob/main/src/context.ts#L468-L472 🤖 Generated with Claude Code |
Summary
ostructrequire for Ruby 3.3+ compatibilitycustom_field_valuemethod for accessing custom field valuespeekalias topeek_treatmentmethodTest plan
Summary by CodeRabbit
Chores
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.