Skip to content

[score boosting] Error on unexpected type#6187

Merged
coszio merged 4 commits intodevfrom
error-on-unexpected-type
Mar 18, 2025
Merged

[score boosting] Error on unexpected type#6187
coszio merged 4 commits intodevfrom
error-on-unexpected-type

Conversation

@coszio
Copy link
Copy Markdown
Contributor

@coszio coszio commented Mar 17, 2025

Changes the behavior to not use a silent 0.0 if the payload nor the defaults have a valid type.

This will error if the payload is not an expected value, even if the defined default is.

@coszio coszio added this to the score boosting milestone Mar 17, 2025
@coszio coszio requested review from generall and timvisee March 17, 2025 23:55
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 2025 •
edited
Loading

📝 Walkthrough

Walkthrough

This pull request implements several changes across three files. First, the OperationError enum in the segment module is updated to derive PartialEq, and its VariableTypeError variant now includes an additional field, description, with an updated error message format. Second, in the formula scorer module, a new trait FriendlyName is introduced with implementations for ScoreType and GeoPoint. The eval_expression method is refactored to utilize a new helper method, get_parsed_payload_value, which simplifies error handling and retrieval of payload values. Additionally, the handling of ParsedExpression::GeoDistance is modified to use this helper method, and the test cases are updated to reflect the new return type, OperationResult<ScoreType>, ensuring consistency across the codebase. Lastly, the test_formula function in the test suite has been modified to include a new key in the query structure and streamline assertion statements.

Possibly related PRs

  • [score boosting] Only accept finite numbers #6092: Focuses on handling non-finite numbers during formula evaluations by returning an OperationError::NonFiniteNumber, which is directly related to the changes in error handling present in this pull request.

Suggested reviewers

  • timvisee

Tip

⚡🧪 Multi-step agentic review comment chat (experimental)
  • We're introducing multi-step agentic chat in review comments. This experimental feature enhances review discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments.
    - To enable this feature, set early_access to true under in the settings.
📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b5e147 and 185d935.

📒 Files selected for processing (2)
  • lib/segment/src/index/query_optimization/rescore_formula/formula_scorer.rs (6 hunks)
  • tests/openapi/test_query_formula.py (3 hunks)
🔇 Additional comments (9)
tests/openapi/test_query_formula.py (3)

64-64: Explicit default value added to align with new error handling

The addition of "defaults": {"price": 0.0} in the query structure ensures that the test has explicitly defined default values rather than relying on implicit defaults. This aligns with the PR objective of raising errors for unexpected types instead of silently using defaults.

80-82: Improved assertion formatting

The updated formatting makes the assertion more readable while maintaining the same validation logic.

103-105: Enhanced assertion error message

The updated formatting with more descriptive error messages makes debugging test failures easier by showing both the expected and actual scores, along with the point data.

lib/segment/src/index/query_optimization/rescore_formula/formula_scorer.rs (6)

35-49: Well-designed trait for improved error messages

The new FriendlyName trait is a good addition that provides human-readable type names for error messages. This makes debugging and error reporting more user-friendly, especially when dealing with type mismatches.

116-123: Refactored payload handling for better error reporting

The refactoring of the payload variable handling now uses the new get_parsed_payload_value helper method, which properly handles error cases and returns meaningful error messages. This approach aligns with the PR objective of raising errors for unexpected types instead of silently using default values.

130-138: Improved GeoDistance handling with consistent error reporting

The GeoDistance case now uses the same helper method as other payload values, providing consistent error handling and reporting. The code is more concise and the error messages are more informative.

243-280: Well-structured helper methods for payload value extraction and parsing

These new helper methods are well-designed:

  1. get_payload_value - Cleanly extracts a value from the payload
  2. get_parsed_payload_value - Handles the complete flow of fetching, defaulting, and parsing values with comprehensive error reporting

The method properly considers:

  • Payload values
  • Default values when payload is missing
  • Type conversion errors
  • Missing values in both payload and defaults

This is a significant improvement in error handling and meets the PR objective by raising explicit errors with detailed information when unexpected types are encountered.

400-416: Updated test cases to reflect new error handling behavior

The test cases have been updated to use Ok for successful cases and Err with appropriate error details for failure cases. This validates that the new error handling works as expected and provides appropriate error messages.

420-423: Updated test method signature for the new return type

The test method signature has been updated to use OperationResult<ScoreType> instead of just ScoreType, and the assertion has been updated to compare the entire result including potential errors. This ensures that error cases are properly tested.

Also applies to: 442-442

✨ Finishing Touches
  • 📝 Generate Docstrings
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions Bot mentioned this pull request Mar 18, 2025
Open
@coszio coszio marked this pull request as draft March 18, 2025 12:41
@coszio coszio marked this pull request as ready for review March 18, 2025 13:18
@coszio coszio merged commit 4149e6d into dev Mar 18, 2025
@coszio coszio deleted the error-on-unexpected-type branch March 18, 2025 15:21
timvisee pushed a commit that referenced this pull request Mar 21, 2025
@coszio @timvisee
[score boosting] Error on unexpected type (#6187)
0184e2d 
* helper for getting payload value

* Error instead of silent default

* fix clippy

* fix openapi test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timvisee timvisee timvisee approved these changes

@generall generall Awaiting requested review from generall

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

score boosting

Development

Successfully merging this pull request may close these issues.

2 participants

@coszio @timvisee