Skip to content

Conversation

@calthejuggler
Copy link
Contributor

@calthejuggler calthejuggler commented Jan 14, 2026

This PR fixes a bug where context.set_attributes was not working correctly, and improves Ruby 3.4 compatibility.

Bug Fix

Previously we were using ||= optional object assignment, but because attributes is initialised as an empty object, this would essentially do nothing.

The fix is to switch it to merge!, and I have added some tests to ensure it.

Compatibility & CI Fixes

  • Add base64 gem dependency with version constraint (~> 0.2) for Ruby 3.4+ where it's no longer a default gem (required by Faraday 2.7.x which uses base64 internally but doesn't declare it as a dependency)
  • Update rexml to fix CI dependency resolution
  • Use Ruby 3.0 compatible syntax in client_config.rb

Version: 1.2.1

Summary by CodeRabbit

  • Bug Fixes

    • Context configuration now preserves and merges attributes across multiple calls, preventing loss of previously set values.
  • Improvements

    • Attribute-setting returns the updated object for fluent chaining.
    • Minor internal call consistency refinements.
  • Tests

    • Added tests for attribute merging, repeated calls and pre-ready attribute behaviour.
  • Chores

    • Added a runtime base64 dependency and expanded CI to include Ruby 2.7.

Version: 1.2.1

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

Walkthrough

The PR bumps Absmartly::VERSION from "1.2.0" to "1.2.1"; changes ContextConfig#set_attributes to merge incoming attributes into existing @attributes with merge! and return self; updates specs to assert merging behaviour and repeated calls to set_attributes and to validate attribute handling on Context; adds a runtime dependency "base64" to the gemspec; alters argument forwarding style in ClientConfig.create (no semantic change); and adds Ruby 2.7 to the CI matrix.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped the version up, a tiny leap,
I stitched old attributes in a cosy heap,
"merge!" I hummed and handed back self,
a base64 gem tucked on the shelf,
tests woke earlier with Ruby 2.7 to peep.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main change: fixing attribute merging in the set_attributes method, which is the primary objective of this changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e96a379 and 3cf8adf.

📒 Files selected for processing (1)
  • spec/context_spec.rb
🧰 Additional context used
🧬 Code graph analysis (1)
spec/context_spec.rb (2)
lib/context_config.rb (2)
  • set_attribute (36-39)
  • set_attributes (31-34)
lib/context.rb (3)
  • set_attribute (140-144)
  • set_attributes (146-150)
  • ready? (60-62)
🔇 Additional comments (2)
spec/context_spec.rb (2)

437-447: LGTM!

The test correctly validates that both set_attribute and set_attributes populate the @attributes array with properly constructed Attribute instances, including the correct timestamp. The use of include matcher appropriately verifies membership without enforcing order.


449-459: LGTM!

Good coverage of the pre-ready state. This test follows the established pattern (similar to "set override before ready") and correctly verifies that attributes can be set before the context becomes ready, which is essential for the SDK's typical initialisation flow.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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

@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #30

coderabbitai bot added a commit that referenced this pull request Jan 14, 2026
Docstrings generation was requested by @calthejuggler.

* #29 (comment)

The following files were modified:

* `lib/context_config.rb`
@calthejuggler calthejuggler force-pushed the bugfix/set-attributes-fix branch from 48255dd to 2731c62 Compare January 14, 2026 08:31
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@absmartly.gemspec`:
- Line 35: The gemspec adds an unnecessary runtime dependency "base64" even
though the code only uses Ruby's standard library method
Digest::MD5.base64digest (see lib/hashing.rb); remove the spec.add_dependency
"base64" entry unless you intentionally need the external gem for Ruby 3.4+
compatibility, in which case add a version constraint (e.g., "~> 0.3") and a
brief comment explaining it's only for future Ruby versions.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2731c62 and 130f786.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • absmartly.gemspec

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/workflows/main.yml:
- Line 8: The matrix currently lists "ruby-version: [ '2.7', '3.0', '3.1',
'3.2', '3.3', '3.4' ]" which can cause intermittent failures for 2.7; update the
workflow to handle 2.7 specially by either adding a matrix include for '2.7'
that pins runs-on to a stable runner (e.g., set runs-on: ubuntu-20.04 for that
include), or mark the 2.7 job as non-blocking by adding continue-on-error: true
for the 2.7 entry, or remove '2.7' from the ruby-version matrix if support is no
longer required; apply the change where the matrix and job definitions reference
ruby-version and runs-on to ensure only the 2.7 job gets the special handling.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8195778 and e96a379.

📒 Files selected for processing (1)
  • .github/workflows/main.yml

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@calthejuggler calthejuggler merged commit 5860fa7 into main Jan 14, 2026
7 checks passed
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.

3 participants