refactor: introduce chatbot tool routing abstraction - #119
Merged
Conversation
📝 Walkthrough워크스루OSS 쿼리 처리 로직을 도구 기반 라우팅으로 재구성했습니다. 새로운 변경 사항도구 기반 라우팅 중앙화
🎯 3 (중간) | ⏱️ ~20분관련된 가능성 있는 PR
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@LLM/OSS/service.py`:
- Around line 334-358: The OSS branch currently returns early (including
caching/logging) after handling empty-OSS fallbacks or static canned replies,
preventing downstream run_final_fallback_tools() and the fused fallback path
from executing; modify the mode == "oss" block so: 1) keep the fast-path
short-circuit via run_oss_fast_path_tools() and cache_and_return(), 2) call
call_oss_async() and scrub_non_contact() as now, 3) if output is empty run
run_empty_oss_fallback_tools() and if it.handled set output =
fallback.to_response() (but do NOT return immediately), otherwise set output to
the greeting or "잘 이해하지 못했어요..." text and continue (do not log/return here), and
4) remove the early _log_chatbot submit + return at the end of the oss block so
execution falls through to the shared final fallback/fusion logic
(run_final_fallback_tools(), fused fallback and final logging) while still using
the same identifiers (_log_chatbot, run_empty_oss_fallback_tools,
run_final_fallback_tools, call_oss_async, scrub_non_contact, cache_and_return).
- Around line 330-332: The current early return after run_mode_tools(mode,
user_text) checks tool_result.handled, but per stack contract this router must
only return immediately when the ToolResult is resolved; change the condition to
check tool_result.resolved (or the ToolResult.is_resolved/ resolved property)
before calling cache_and_return(tool_result.to_response()), so that
handled-but-unresolved ToolResult instances do not short-circuit OSS calls or
the final fallback; leave run_mode_tools, cache_and_return and to_response usage
intact and only gate the immediate return on resolved.
In `@LLM/OSS/tools.py`:
- Around line 183-189: The postprocess URL returned by run_postprocess is being
discarded (text, _ = run_postprocess(...)) which loses reference info; change
that to capture the URL (text, url = run_postprocess(mode, user_text,
sub_answer)) and include it in the returned ToolResult (e.g., add url=url or
source_url=url in the ToolResult construction that returns
name=f"{mode}_oss_empty_fallback", text=text, engine="oss", confidence=...), so
the fallback response preserves the postprocessing reference.
- Around line 66-129: The helper functions _direct_answer_tool, _schedule_tool,
_clarification_tool, _postprocess_tool, and _confident_search_tool currently
call external helpers (metadata_direct_answer, schedule_search,
dept_clarification_message, call_submodel/run_postprocess,
confident_search_answer) without exception handling; wrap each external call in
a try/except that catches broad exceptions, logs the error, and returns
EMPTY_TOOL_RESULT on failure so a single tool error won't abort routing. Ensure
you handle both the initial call (e.g., metadata_direct_answer, schedule_search,
dept_clarification_message, confident_search_answer) and the
submodel/postprocess sequence (call_submodel then run_postprocess) in
_postprocess_tool, returning EMPTY_TOOL_RESULT if either step raises.
- Around line 45-59: call_submodel currently silences all exceptions in both
build_answer and schedule_search which hides failures; update call_submodel to
catch specific exceptions (e.g., network/IO or model-specific errors) instead of
bare except Exception, and log the full exception details when build_answer or
schedule_search fails (use the module logger or processLogger) while preserving
fallback behavior: reference build_answer/result and schedule_search/schedule
and ensure settings.json_only_mode logic remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2c22addd-b975-4afb-ab23-ce4411a50fe6
📒 Files selected for processing (3)
LLM/OSS/service.pyLLM/OSS/tools.pydocs/PLANS.md
Yu-JeSeung
added
run-rag-check
9 tasks
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
1 participant
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.
관련 이슈
Open #118
🎯 배경
service.py에 집중되어 있어, 비용 우선 tool routing 구조를 명확히 드러내기 어려웠습니다.🔍 주요 내용
LLM/OSS/tools.py를 추가해ToolResult와 deterministic chatbot tool routing helper를 분리했습니다.service.py의fast/policy/dorm/grad/topic/oss반복 분기를run_mode_tools(),run_oss_fast_path_tools()중심으로 정리했습니다.docs/PLANS.md에 새 챗봇 tool routing 모듈을 반영했습니다.변경 요약
챗봇의 도구 라우팅 로직을 추상화하여
service.py의 복잡한 모드별 분기를 체계적으로 정리했습니다. 새로운tools.py모듈에서ToolResult기반 결정적 도구 실행 흐름을 구성하고,service.py는 이를 활용해 더 간결하게 리팩토링했습니다.주요 변경점
name,text,engine,confidence,llm_required등을 관리run_mode_tools(),run_oss_fast_path_tools(),run_empty_oss_fallback_tools(),run_final_fallback_tools()추가로 단계별 라우팅 구현fast/policy/dorm/grad/topic/oss분기를 도구 함수로 통합해chat_with_oss()가독성 개선docs/PLANS.md에tools.py를 "비용 우선 결정적 도구 라우팅"으로 등록주의/리스크
fast,policy,dorm,grad,topic,oss)별 엔드투엔드 테스트 확인 필요다음 액션
oss모드) 통합 테스트 실행 및 응답 품질 확인