refactor: postprocess folder add and output process seoarate - #96
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 8 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 Walkthrough개요후처리 기능을 새로운 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client as 호출자
participant Engine as run_postprocess 엔진
participant Reg as MODE_PIPELINES
participant Ctx as PostProcessContext
participant Rules as CONTACT_RESPONSE_RULES
participant Msg as MESSAGES 템플릿
Client->>Engine: run_postprocess(mode, user_text, sub_answer)
Engine->>Reg: MODE_PIPELINES[mode] 조회
Engine->>Ctx: PostProcessContext 구축
Engine->>Rules: 조건별 규칙 정렬 및 매칭
alt 규칙 매칭 성공
Rules->>Msg: template_key 기반 템플릿 조회
Msg->>Engine: 포맷 문자열 반환
Engine->>Engine: {label}, {phone}, {url} 등으로 치환
else 규칙 미매칭
Engine->>Engine: first_line 또는 기본 메시지 사용
end
Engine->>Client: (formatted_text, url) 반환
예상 코드 리뷰 소요 시간🎯 4 (복잡함) | ⏱️ ~45분 관련 PR
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 1
🧹 Nitpick comments (5)
LLM/OSS/formatter.py (2)
587-588: 하드코딩된 키워드를 테이블로 분리 고려
_policy_candidate_score와_dorm_candidate_score에 하드코딩된 키워드들이 있습니다. 향후synonym_table.py와 유사하게 별도 테이블로 분리하면 일관성이 높아질 수 있습니다.# 예: Lines 587-588의 하드코딩된 키워드 ("휴학", "복학", "휴·복학", "휴복학", "학적", "학사안내") # 예: Lines 606-607의 하드코딩된 키워드 ("학생생활관", "생활관", "기숙사", "입사", "생활관비", "생활관 안내")Also applies to: 606-607
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/formatter.py` around lines 587 - 588, The two scoring blocks in formatter.py (_policy_candidate_score and _dorm_candidate_score) use hardcoded keyword tuples (e.g., ("휴학","복학","휴·복학","휴복학","학적","학사안내") and ("학생생활관","생활관","기숙사","입사","생활관비","생활관 안내")); extract these into a shared lookup (e.g., POLICY_KEYWORDS and DORM_KEYWORDS) or move them into a new synonyms/keyword table module similar to synonym_table.py, import and reference those constants from _policy_candidate_score and _dorm_candidate_score, and update the any(...) checks to use the new constants so keyword lists are centralized and reusable.
623-627: 조건 검사 순서 최적화 가능
sub_answer검사를TITLE_URL_PAT.finditer호출 전에 수행하면 불필요한 정규식 파싱을 피할 수 있습니다.♻️ 제안된 수정
def _best_title_url_candidate( user_text: str, sub_answer: str, default_text: str, scorer, prefer_good_url: bool = False, require_positive_score: bool = False, ) -> tuple[Optional[str], Optional[str], str]: - items = list(TITLE_URL_PAT.finditer(sub_answer or "")) if not sub_answer: return None, None, default_text + items = list(TITLE_URL_PAT.finditer(sub_answer)) if not items: return None, None, _first_line_or_default(sub_answer, default_text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/formatter.py` around lines 623 - 627, The code currently runs TITLE_URL_PAT.finditer(sub_answer or "") before checking if sub_answer is falsy, causing unnecessary regex work; change the order so you first check if sub_answer is falsy (returning None, None, default_text) and only then call TITLE_URL_PAT.finditer(sub_answer) to populate items; preserve the subsequent logic that returns _first_line_or_default(sub_answer, default_text) when items is empty and keep variable names TITLE_URL_PAT, sub_answer, items, _first_line_or_default, and default_text unchanged.LLM/OSS/postprocess/context.py (1)
13-13:hint필드 타입 힌트 개선 고려
dict[str, object]대신dict[str, Any]를 사용하거나,hint의 구조가 고정되어 있다면TypedDict를 정의하여 타입 안전성을 높일 수 있습니다. 현재 코드에서hint는canon,aliases,path_base키를 사용하는 것으로 보입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/postprocess/context.py` at line 13, The type for the variable/parameter named "hint" currently declared as Optional[dict[str, object]] should be made more precise: either change dict[str, object] to dict[str, Any] (importing Any) or define a TypedDict (e.g., HintDict with keys "canon", "aliases", "path_base") and use Optional[HintDict]; update the hint declaration in LLM/OSS/postprocess/context.py and add the necessary typing imports (Any or TypedDict) so callers and static checkers see the exact structure.LLM/OSS/postprocess/engine.py (2)
25-29: 가독성 개선을 위한 간소화 권장Line 26의 조건식이 복잡하고
(sub_answer or "").strip()이 두 번 평가됩니다. 변수로 분리하면 가독성이 향상됩니다.♻️ 가독성 개선 제안
def _first_line(sub_answer: str) -> str: - first = (sub_answer or "").strip().splitlines()[0].lstrip("- ").strip() if (sub_answer or "").strip() else "" + stripped = (sub_answer or "").strip() + first = stripped.splitlines()[0].lstrip("- ").strip() if stripped else "" if not first: return MESSAGES["not_found"] return first if first.endswith(("다.", "요.")) else first + "."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/postprocess/engine.py` around lines 25 - 29, The condition in _first_line duplicates (sub_answer or "").strip(); refactor by first assigning stripped = (sub_answer or "").strip(), then if not stripped return MESSAGES["not_found"], else compute first = stripped.splitlines()[0].lstrip("- ").strip() and finally return first if first.endswith(("다.", "요.")) else first + "."; update _first_line to use these variable names to improve readability and avoid re-evaluating the expression.
71-98: 코드 중복 - 공통 로직 추출 권장
_run_topic,_run_policy,_run_dorm세 함수가 거의 동일한 구조를 가지고 있습니다. 추출 함수와 메시지 키만 다릅니다. 공통 헬퍼 함수로 통합하면 유지보수성이 향상됩니다.♻️ 공통 헬퍼 함수 추출 제안
+def _run_page_processor( + user_text: str, + sub_answer: str, + extractor, + default_key: str, + page_key: str, +) -> tuple[str, Optional[str]]: + title, url, first_line = extractor(user_text, sub_answer) + if not sub_answer: + return _format_message(default_key, user_text=user_text), settings.org_homepage_url + if title and url: + url = formatter.ensure_layout_unknown(url) + return _format_message(page_key, user_text=user_text, title=title, url=url), url + return first_line, None + + def _run_topic(user_text: str, sub_answer: str) -> tuple[str, Optional[str]]: - title, url, first_line = formatter.extract_topic_candidate(user_text, sub_answer) - if not sub_answer: - return _format_message("topic_default", user_text=user_text), settings.org_homepage_url - if title and url: - url = formatter.ensure_layout_unknown(url) - return _format_message("topic_page", user_text=user_text, title=title, url=url), url - return first_line, None + return _run_page_processor( + user_text, sub_answer, + formatter.extract_topic_candidate, "topic_default", "topic_page" + ) def _run_policy(user_text: str, sub_answer: str) -> tuple[str, Optional[str]]: - title, url, first_line = formatter.extract_policy_candidate(user_text, sub_answer) - if not sub_answer: - return _format_message("policy_default", user_text=user_text), settings.org_homepage_url - if title and url: - url = formatter.ensure_layout_unknown(url) - return _format_message("official_page", user_text=user_text, title=title, url=url), url - return first_line, None + return _run_page_processor( + user_text, sub_answer, + formatter.extract_policy_candidate, "policy_default", "official_page" + ) def _run_dorm(user_text: str, sub_answer: str) -> tuple[str, Optional[str]]: - title, url, first_line = formatter.extract_dorm_candidate(user_text, sub_answer) - if not sub_answer: - return _format_message("dorm_default", user_text=user_text), settings.org_homepage_url - if title and url: - url = formatter.ensure_layout_unknown(url) - return _format_message("official_page", user_text=user_text, title=title, url=url), url - return first_line, None + return _run_page_processor( + user_text, sub_answer, + formatter.extract_dorm_candidate, "dorm_default", "official_page" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/postprocess/engine.py` around lines 71 - 98, The three functions _run_topic, _run_policy, and _run_dorm duplicate the same control flow; extract a shared helper (e.g., _run_candidate) that accepts the extractor function (formatter.extract_topic_candidate / formatter.extract_policy_candidate / formatter.extract_dorm_candidate) and the default/message keys ("topic_default", "policy_default" or "dorm_default" and "topic_page"/"official_page") as parameters, calls the extractor to get (title, url, first_line), handles the not sub_answer case by returning _format_message(...), uses formatter.ensure_layout_unknown(url) when title and url are present, and otherwise returns first_line and None; then replace each of _run_topic/_run_policy/_run_dorm with a one-line call to that helper, preserving the tuple[str, Optional[str]] return shape and usage of settings.org_homepage_url and _format_message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@LLM/OSS/postprocess/registry.py`:
- Around line 2-7: The "fast" mode in postprocess/registry.py defines flags
use_schedule_first and use_dept_clarification that are not used; either remove
these unused keys from the "fast" dict in registry.py, or wire them into the
runtime by having engine.py read the mode config and pass them into the
processing pipeline (e.g., when selecting the processor for a request).
Concretely: update the registry entry for "fast" (remove use_schedule_first and
use_dept_clarification) OR update engine.py (functions like the mode lookup or
request handler such as process_request / Engine.process) to call the registry
getter (e.g., get_mode_config) and propagate those boolean flags into the
sub_answer processor invocation so the processor receives and acts on
use_schedule_first and use_dept_clarification.
---
Nitpick comments:
In `@LLM/OSS/formatter.py`:
- Around line 587-588: The two scoring blocks in formatter.py
(_policy_candidate_score and _dorm_candidate_score) use hardcoded keyword tuples
(e.g., ("휴학","복학","휴·복학","휴복학","학적","학사안내") and
("학생생활관","생활관","기숙사","입사","생활관비","생활관 안내")); extract these into a shared lookup
(e.g., POLICY_KEYWORDS and DORM_KEYWORDS) or move them into a new
synonyms/keyword table module similar to synonym_table.py, import and reference
those constants from _policy_candidate_score and _dorm_candidate_score, and
update the any(...) checks to use the new constants so keyword lists are
centralized and reusable.
- Around line 623-627: The code currently runs TITLE_URL_PAT.finditer(sub_answer
or "") before checking if sub_answer is falsy, causing unnecessary regex work;
change the order so you first check if sub_answer is falsy (returning None,
None, default_text) and only then call TITLE_URL_PAT.finditer(sub_answer) to
populate items; preserve the subsequent logic that returns
_first_line_or_default(sub_answer, default_text) when items is empty and keep
variable names TITLE_URL_PAT, sub_answer, items, _first_line_or_default, and
default_text unchanged.
In `@LLM/OSS/postprocess/context.py`:
- Line 13: The type for the variable/parameter named "hint" currently declared
as Optional[dict[str, object]] should be made more precise: either change
dict[str, object] to dict[str, Any] (importing Any) or define a TypedDict (e.g.,
HintDict with keys "canon", "aliases", "path_base") and use Optional[HintDict];
update the hint declaration in LLM/OSS/postprocess/context.py and add the
necessary typing imports (Any or TypedDict) so callers and static checkers see
the exact structure.
In `@LLM/OSS/postprocess/engine.py`:
- Around line 25-29: The condition in _first_line duplicates (sub_answer or
"").strip(); refactor by first assigning stripped = (sub_answer or "").strip(),
then if not stripped return MESSAGES["not_found"], else compute first =
stripped.splitlines()[0].lstrip("- ").strip() and finally return first if
first.endswith(("다.", "요.")) else first + "."; update _first_line to use these
variable names to improve readability and avoid re-evaluating the expression.
- Around line 71-98: The three functions _run_topic, _run_policy, and _run_dorm
duplicate the same control flow; extract a shared helper (e.g., _run_candidate)
that accepts the extractor function (formatter.extract_topic_candidate /
formatter.extract_policy_candidate / formatter.extract_dorm_candidate) and the
default/message keys ("topic_default", "policy_default" or "dorm_default" and
"topic_page"/"official_page") as parameters, calls the extractor to get (title,
url, first_line), handles the not sub_answer case by returning
_format_message(...), uses formatter.ensure_layout_unknown(url) when title and
url are present, and otherwise returns first_line and None; then replace each of
_run_topic/_run_policy/_run_dorm with a one-line call to that helper, preserving
the tuple[str, Optional[str]] return shape and usage of
settings.org_homepage_url and _format_message.
🪄 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: 31487d12-f702-4e2c-bea9-3b206e87be08
📒 Files selected for processing (10)
.gitignoreLLM/OSS/formatter.pyLLM/OSS/postprocess/__init__.pyLLM/OSS/postprocess/context.pyLLM/OSS/postprocess/engine.pyLLM/OSS/postprocess/message_table.pyLLM/OSS/postprocess/registry.pyLLM/OSS/postprocess/rules_table.pyLLM/OSS/postprocess/synonym_table.pyLLM/OSS/service.py
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.
관련 이슈
Close #91
🎯 배경
🔍 주요 내용
postpreocess폴더 생성registry파이프라인 매핑rules_table응답 규칙 테이블message_table문구 테이블synonym_table동의어 테이블engine실행one_sentence->run_process변경formatter알고리즘 로직 유직, 새로운 엔진에서 쓸 수 있는 추출 helper 추가.gitignoreharnes engineering file 예외처리