From f99b3517d9c781815589050117ce0ae5b87ea1e6 Mon Sep 17 00:00:00 2001 From: chiguyong Date: Tue, 16 Jun 2026 00:24:14 +0800 Subject: [PATCH] fix(review): apply code review fixes from ce-code-review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P1: Use _resolve_execution_mode() instead of hardcoding SKILL_REACT in semantic_low_complexity, semantic_high, and merged_llm paths - P1: QualityGate escalation uses name-based check (c.name) instead of identity check (c is) for robustness - P2: Remove tautological complexity >= 0.3 in short_text_llm_hint - P2: Add empty query guard in SemanticRouter.route() - P2: Upgrade debug → warning log level for low-complexity fallback errors - P2: Validate skill_hint against _SKILL_NAME_RE in _classify_merged - P2: Rename has_high_signal → has_non_low_signal for clarity --- src/agentkit/chat/semantic_router.py | 3 +++ src/agentkit/chat/skill_routing.py | 20 +++++++++++--------- src/agentkit/quality/gate.py | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/agentkit/chat/semantic_router.py b/src/agentkit/chat/semantic_router.py index 2888282..1e4ea8b 100644 --- a/src/agentkit/chat/semantic_router.py +++ b/src/agentkit/chat/semantic_router.py @@ -178,6 +178,9 @@ class SemanticRouter: if self._index.size == 0: return SemanticRouteResult(confidence="low", skill_name=None, similarity=0.0) + if not query or not query.strip(): + return SemanticRouteResult(confidence="low", skill_name=None, similarity=0.0) + try: # Get query embedding (with cache) query_embedding = self._query_cache.get(query) diff --git a/src/agentkit/chat/skill_routing.py b/src/agentkit/chat/skill_routing.py index 8eb729e..9e798b9 100644 --- a/src/agentkit/chat/skill_routing.py +++ b/src/agentkit/chat/skill_routing.py @@ -643,10 +643,10 @@ class HeuristicClassifier: self._MEDIUM_EXACT_RE.findall(content) ) - has_high_signal = high_hits > 0 or medium_hits > 0 + has_non_low_signal = high_hits > 0 or medium_hits > 0 # 低复杂度信号仅在无高/中复杂度信号时生效 - if has_low_signal and not has_high_signal: + if has_low_signal and not has_non_low_signal: score = 0.05 # 问候/闲聊直接给极低分 length = len(content) if length > 200: @@ -853,8 +853,11 @@ class CostAwareRouter: merged_complexity = max(0.0, min(1.0, merged_complexity)) skill_hint = data.get("skill_hint") - # If skill_hint provided and valid, route directly to that skill + # Validate skill_hint against name pattern before lookup if skill_hint and skill_registry: + if not _SKILL_NAME_RE.match(str(skill_hint).strip().lower()): + logger.warning(f"Invalid skill_hint from LLM: {skill_hint!r}") + skill_hint = None try: matched_skill = skill_registry.get(skill_hint) result = SkillRoutingResult( @@ -866,7 +869,7 @@ class CostAwareRouter: match_method="merged_llm", match_confidence=0.7, complexity=merged_complexity, - execution_mode=ExecutionMode.SKILL_REACT, + execution_mode=_resolve_execution_mode(matched_skill.config), ) # Merge tools agent_tools = ( @@ -1355,14 +1358,14 @@ class CostAwareRouter: result.match_confidence = semantic_result.similarity result.complexity = complexity if result.matched: - result.execution_mode = ExecutionMode.SKILL_REACT + result.execution_mode = _resolve_execution_mode(result.skill_config) result.execution_trace = trace if transparency != "SILENT" else [] result.transparency_level = transparency span.set_attribute("route.layer", "semantic_low_complexity") span.set_attribute("route.target", result.skill_name or "default") return result except Exception as e: - logger.debug(f"Semantic routing for low-complexity query failed: {e}") + logger.warning(f"Semantic routing for low-complexity query failed: {e}") # Try IntentRouter keyword match before falling back to direct chat # Low-complexity queries like "翻译这段话" should still match skills @@ -1396,7 +1399,7 @@ class CostAwareRouter: span.set_attribute("route.target", result.skill_name or "default") return result except Exception as e: - logger.debug(f"Intent routing for low-complexity query failed: {e}") + logger.warning(f"Intent routing for low-complexity query failed: {e}") # No semantic or intent match → direct chat result = SkillRoutingResult( @@ -1457,7 +1460,7 @@ class CostAwareRouter: result.match_confidence = semantic_result.similarity result.complexity = complexity if result.matched: - result.execution_mode = ExecutionMode.SKILL_REACT + result.execution_mode = _resolve_execution_mode(result.skill_config) result.execution_trace = trace if transparency != "SILENT" else [] result.transparency_level = transparency span.set_attribute("route.layer", "semantic_high") @@ -1490,7 +1493,6 @@ class CostAwareRouter: if ( skill_hint is None and len(clean_content) < 20 - and complexity >= 0.3 and self._merged_llm_classify and self._llm_gateway is not None ): diff --git a/src/agentkit/quality/gate.py b/src/agentkit/quality/gate.py index 9af94a3..93684ee 100644 --- a/src/agentkit/quality/gate.py +++ b/src/agentkit/quality/gate.py @@ -126,12 +126,12 @@ class QualityGate: and skill_match_check.message and "Warning" in skill_match_check.message ): - other_failed = any(not c.passed for c in checks if c is not skill_match_check) + other_failed = any(not c.passed for c in checks if c.name != "skill_match") if other_failed: # 升级：将 skill_match 的 passed 也设为 False checks = [ QualityCheck(name=c.name, passed=False, message=c.message) - if c is skill_match_check + if c.name == "skill_match" else c for c in checks ]