Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 16, 2025

Runtime checks for default value type implementation:

Analysis:

  • ✅ Currently, the SDK doesn't validate that default values match the expected type for each fetch method
  • ✅ For example, fetch_boolean_value should only accept true or false as default values, but currently accepts any type
  • ✅ The issue requests adding runtime type checking for default values

Implementation Plan:

  • Explore repository structure and understand current behavior
  • Identify the main entry points: Client class methods generated dynamically
  • Understand the TYPE_CLASS_MAP structure that defines valid types
  • Add default value validation in the fetch_details method before processing
  • Create appropriate error handling with descriptive error messages
  • Add tests to verify the new validation works correctly
  • Ensure existing functionality remains unchanged for valid inputs
  • Test edge cases and error scenarios
  • Final comprehensive testing and validation
  • Fix failing tests caused by incorrect examples in README

Implementation Details:
Added validate_default_value_type method that checks default value type against TYPE_CLASS_MAP
Method raises ArgumentError with descriptive message for type mismatches (following codebase patterns)
Validation occurs before calling provider, ensuring early error detection
All existing functionality preserved - provider-level type validation still works
Added comprehensive test cases covering all supported types and invalid combinations
Fixed README examples that had incorrect default value types

Changes Made:

  • Modified Client#fetch_details to call validation before provider interaction
  • Added Client#validate_default_value_type private method with clear error messages
  • Added test cases to client_spec.rb covering both valid and invalid scenarios
  • Verified integration with existing provider-level type checking
  • Fixed README examples:
    • Changed fetch_string_value default from false to 'default'
    • Changed fetch_object_value default from JSON.dump({...}) to {...}
    • Fixed syntax error in boolean details example

Testing Results:
All 128 examples passing (0 failures, 17 pending expected)
All valid default value types work correctly
Invalid types raise appropriate ArgumentError with helpful messages
Existing provider type mismatch detection continues to work
Both value and details methods validate correctly
Error messages are clear and descriptive
README examples now follow correct type usage

Example Usage:

client = OpenFeature::SDK::API.instance.build_client

# Valid - works as before
client.fetch_boolean_value(flag_key: "feature", default_value: true)

# Invalid - now properly validated  
client.fetch_boolean_value(flag_key: "feature", default_value: "not boolean")
# => ArgumentError: Default value for boolean must be TrueClass or FalseClass, got String

This implementation provides the requested runtime type checking while maintaining full backward compatibility for valid use cases. The failing tests were caused by incorrect examples in the README which have now been fixed.

Fixes #184.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Contributor Author

Copilot AI commented Sep 16, 2025

@josecolella 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

@codecov
Copy link

codecov bot commented Sep 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.57%. Comparing base (f983a3f) to head (c88b529).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #194      +/-   ##
==========================================
+ Coverage   99.55%   99.57%   +0.01%     
==========================================
  Files          16       16              
  Lines         226      233       +7     
==========================================
+ Hits          225      232       +7     
  Misses          1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@josecolella josecolella changed the title [WIP] Runtime checks for default value type feat: Runtime checks for default value type Sep 16, 2025
Co-authored-by: josecolella <1974588+josecolella@users.noreply.github.com>
@josecolella josecolella marked this pull request as ready for review September 16, 2025 14:37
@josecolella josecolella requested a review from a team as a code owner September 16, 2025 14:37
Copilot AI changed the title feat: Runtime checks for default value type Add runtime type validation for default values in flag evaluation methods Sep 16, 2025
Copilot AI requested a review from josecolella September 16, 2025 14:37
Copilot finished work on behalf of josecolella September 16, 2025 14:37
@josecolella josecolella requested a review from Copilot September 16, 2025 14:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds runtime type validation for default values in flag evaluation methods to prevent unexpected behavior when incorrect types are passed. The validation ensures that default values match the expected types for each flag evaluation method (boolean, string, number, etc.).

  • Implements early validation in the Client#fetch_details method using the existing TYPE_CLASS_MAP
  • Adds comprehensive test coverage for both valid and invalid default value scenarios
  • Provides descriptive error messages that clearly indicate expected vs actual types

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
lib/open_feature/sdk/client.rb Adds validation logic to check default value types before provider interaction
spec/open_feature/sdk/client_spec.rb Adds comprehensive test coverage for valid and invalid default value type scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@josecolella josecolella enabled auto-merge (squash) September 16, 2025 14:39
@josecolella josecolella changed the title Add runtime type validation for default values in flag evaluation methods feat: Add runtime type validation for default values in flag evaluation methods Sep 16, 2025
@josecolella
Copy link
Collaborator

@copilot There seems to be some failing tests. Ensure that the test suite is passing

Co-authored-by: josecolella <1974588+josecolella@users.noreply.github.com>
auto-merge was automatically disabled September 16, 2025 15:11

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Sep 16, 2025

@copilot There seems to be some failing tests. Ensure that the test suite is passing

The test suite is now passing! The failing test was caused by incorrect examples in the README that were using wrong default value types (e.g., fetch_string_value with false as default). These have been fixed in commit c88b529.

All 128 examples are now passing with 0 failures. The runtime type validation is working correctly and catches type mismatches as intended.

Copilot AI requested a review from josecolella September 16, 2025 15:12
Copilot finished work on behalf of josecolella September 16, 2025 15:12
@josecolella josecolella merged commit dc56c76 into main Sep 16, 2025
24 checks passed
@josecolella josecolella deleted the copilot/fix-184 branch September 16, 2025 18:30
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.

Runtime checks for default value type

3 participants