fix(intelligence): replace unsafe number-grabbing fallback in _parse_importance_response - #1075
Merged
lightzt99 merged 1 commit intoJun 23, 2026
Conversation
…importance_response (oceanbase#1074) The second-level fallback in _parse_importance_response() used re.findall(r'\d+\.?\d*') to blindly grab the first number from the entire LLM response text. This could misinterpret dimension counts (6), percentages (85), or unrelated numbers as the importance_score, causing incorrect memory classification and review scheduling. Changes: - L1: Use parse_json_from_text() for robust JSON extraction; support both importance_score and overall_score fields; synthesize from criteria_scores via weighted sum when no total is present - L2: Only extract numbers anchored to recognized field names (importance_score, overall_score, score); reject values outside [0.0, 1.0] instead of clamping - L3: Return None instead of fixed 0.5; caller falls back to _rule_based_evaluation (consistent with LLM exception path) - Add 17 unit tests covering parsing and fallback behavior Closes oceanbase#1074
Collaborator
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closed #1074
The second-level fallback in
_parse_importance_response()blindly grabbed the first number from the entire LLM response text viare.findall(r'\d+\.?\d*'). This could misinterpret dimension counts (6→ clamped to1.0), percentages (85%→1.0), or unrelated config references as theimportance_score, causing incorrect memory classification and review scheduling downstream.Changes
find("{")/rfind("}")withparse_json_from_text()(existing utility); support bothimportance_scoreandoverall_scorefield names; when onlycriteria_scoresis present, compute weighted sum usingcriteria_weightsimportance_score,overall_score,score); reject values outside[0.0, 1.0]instead of clampingNoneinstead of fixed0.5; caller_llm_based_evaluationfalls back to_rule_based_evaluation(consistent with LLM exception path)Why
importance_scoredirectly determines:working/short_term/long_term)initial_retention * importance_score)interval * (1 - importance_score * 0.3))A silently wrong score cascades through the entire memory lifecycle and is nearly impossible to diagnose from external behavior.
Test plan
tests/unit/intelligence/test_importance_evaluator.py)