From bdc26846bda570d4a15cf1801fe5d88efca7cb86 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 16 Mar 2026 22:36:24 +0000
Subject: [PATCH] refactor: address code review feedback - simplify dispatch,
 add warning log

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
---
 app/tasks/classify_document.py    |  1 +
 app/utils/classification_rules.py | 24 +++++++++---------------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/app/tasks/classify_document.py b/app/tasks/classify_document.py
index 846270a1..d87f9f13 100644
--- a/app/tasks/classify_document.py
+++ b/app/tasks/classify_document.py
@@ -103,6 +103,7 @@ def classify_document_task(
                 try:
                     existing_metadata = json.loads(file_record.ai_metadata)
                 except (json.JSONDecodeError, TypeError):
+                    logger.warning("Failed to parse ai_metadata for file %s, starting fresh", file_id)
                     existing_metadata = {}
 
             # Load custom rules
diff --git a/app/utils/classification_rules.py b/app/utils/classification_rules.py
index d8f440ab..760f03e4 100644
--- a/app/utils/classification_rules.py
+++ b/app/utils/classification_rules.py
@@ -257,10 +257,10 @@ def _match_metadata(rule: ClassificationRule, metadata: dict[str, Any] | None) -
     return str(actual).lower() == expected_value.strip().lower()
 
 
-_MATCHERS = {
-    RULE_TYPE_FILENAME: _match_filename,
-    RULE_TYPE_CONTENT: _match_content,
-    RULE_TYPE_METADATA: _match_metadata,
+_MATCHERS: dict[str, tuple] = {
+    RULE_TYPE_FILENAME: (_match_filename, "filename"),
+    RULE_TYPE_CONTENT: (_match_content, "text"),
+    RULE_TYPE_METADATA: (_match_metadata, "metadata"),
 }
 
 
@@ -271,19 +271,13 @@ def _evaluate_rule(
     metadata: dict[str, Any] | None,
 ) -> MatchedRule | None:
     """Evaluate a single rule against the document.  Return a :class:`MatchedRule` on match."""
-    matcher = _MATCHERS.get(rule.rule_type)
-    if matcher is None:
+    entry = _MATCHERS.get(rule.rule_type)
+    if entry is None:
         return None
 
-    # Dispatch to the appropriate matcher based on rule type
-    if rule.rule_type == RULE_TYPE_FILENAME:
-        matched = matcher(rule, filename)
-    elif rule.rule_type == RULE_TYPE_CONTENT:
-        matched = matcher(rule, text)
-    elif rule.rule_type == RULE_TYPE_METADATA:
-        matched = matcher(rule, metadata)
-    else:
-        matched = False
+    matcher, arg_key = entry
+    arg_map = {"filename": filename, "text": text, "metadata": metadata}
+    matched = matcher(rule, arg_map[arg_key])
 
     if matched:
         return MatchedRule(