feat : result data(unique url) redefine - #76
Merged
Conversation
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughLLM/OSS/Open_AI_OSS.py에 전역 플래그 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant OSS as OSS Router
participant Sub as Submodel
participant Scheduler as Scheduler/Augmentor
User->>OSS: 요청 (텍스트)
OSS->>OSS: decide_mode(compact text)
alt JSON_ONLY_MODE=true
OSS->>Sub: call_submodel(base prompt)
Sub-->>OSS: base submodel output
OSS-->>User: base output (JSON_ONLY_MODE short-circuit)
else JSON_ONLY_MODE=false
OSS->>Sub: call_submodel(base prompt)
Sub-->>OSS: base submodel output
OSS->>Scheduler: schedule/augment with sub output
Scheduler-->>OSS: augmented results
OSS->>OSS: one_sentence_grad/one_sentence_from_sub_answer (extract URL via _extract_url_from_text)
OSS-->>User: composed sentence + source URL
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 2
🧹 Nitpick comments (2)
LLM/sub_model/build_index.py (1)
93-100: 중복 로그 출력Line 96의
after_rows와 Line 100의len(df)는 Line 95 할당 직후이므로 항상 동일한 값입니다. 둘 중 하나를 제거하면 로그가 더 명확해집니다.♻️ 중복 로그 제거
before_rows = len(df) before_unique_url = int(df["url"].nunique(dropna=True)) df = _merge_by_url(df).reset_index(drop=True) -after_rows = len(df) print(f" - 원본 행 수: {before_rows} / 고유 URL 수: {before_unique_url}") -print(f" - URL 병합 후 문서 수: {after_rows}") - -print(f" - 중복 제거(병합) 후: {len(df)}개 문서") +print(f" - 중복 제거(병합) 후: {len(df)}개 문서")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/sub_model/build_index.py` around lines 93 - 100, The two debug prints after calling _merge_by_url (the one printing after_rows and the one printing len(df) labeled "중복 제거(병합) 후") are redundant because after_rows is assigned from len(df) immediately after the merge; remove the duplicate print (the " - 중복 제거(병합) 후: {len(df)}개 문서" line) or consolidate the messages into a single clear log so only one line reports the post-merge document count, keeping variables before_rows, before_unique_url, _merge_by_url and after_rows unchanged.LLM/OSS/Open_AI_OSS.py (1)
892-899: OSS 폴백 경로에서 URL 미반환
one_sentence_topic과one_sentence_from_sub_answer모두(text, url)튜플을 반환하지만, 여기서url은_로 버려지고 Line 900에서 URL 없이 응답합니다. 다른 모드(fast,policy,topic등)에서는url을 응답에 포함하고 있어 일관성이 떨어집니다.의도적인 것이라면 무시해도 되지만, URL이 있으면 사용자에게 도움이 될 수 있습니다.
♻️ URL 반환 추가 제안
if sub: if looks_like_topic(user_text): - text, _ = one_sentence_topic(user_text, sub) + text, url = one_sentence_topic(user_text, sub) else: - text, _ = one_sentence_from_sub_answer(user_text, sub) + text, url = one_sentence_from_sub_answer(user_text, sub) out = text + if url: + return {"engine": "oss", "text": out, "url": url} else: out = "잘 이해하지 못했어요. 다시 질문해주세요."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/Open_AI_OSS.py` around lines 892 - 899, The OSS fallback path is discarding the URL returned by one_sentence_topic and one_sentence_from_sub_answer (currently assigned to _), causing out to lack the URL while other modes include it; change the code in the branch that handles sub and user_text to capture the returned url (e.g., text, url = one_sentence_topic(user_text, sub) and text, url = one_sentence_from_sub_answer(user_text, sub)) and append or include the url in out only when url is truthy so the response matches the other modes (preserve existing text variable and only modify how out is constructed).
🤖 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/Open_AI_OSS.py`:
- Around line 766-789: one_sentence_grad currently matches the keyword "졸업이수 학점"
literally which can miss variants like "졸업이수학점"; update the matching to
normalize internal whitespace or use a regex that ignores spaces so both "졸업이수
학점" and "졸업이수학점" match. Specifically, in one_sentence_grad adjust the keyword
check (the loop over lines and the any(...) condition) to either compare against
a whitespace-normalized version of ln (e.g., remove all internal spaces before
testing) or use re.search(r"졸업\s*이수\s*학점", ln) for that keyword so the function
reliably picks lines regardless of spacing variations; keep other keywords
unchanged.
In `@LLM/sub_model/build_index.py`:
- Around line 71-91: The bug is that _merge_by_url groups all rows with an
empty-string url into one merged record; change _merge_by_url so it does not
merge rows where url == "" by detecting an empty url in the group loop (the
group from df_in.groupby("url", ...)) and, when url == "", iterate the group's
rows and append each row individually to rows (preserving its title, source,
content, fulltext computed per-row) instead of computing
merged_title/merged_content/merged_fulltext for the whole group; keep the
existing merge behavior for non-empty urls.
---
Nitpick comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 892-899: The OSS fallback path is discarding the URL returned by
one_sentence_topic and one_sentence_from_sub_answer (currently assigned to _),
causing out to lack the URL while other modes include it; change the code in the
branch that handles sub and user_text to capture the returned url (e.g., text,
url = one_sentence_topic(user_text, sub) and text, url =
one_sentence_from_sub_answer(user_text, sub)) and append or include the url in
out only when url is truthy so the response matches the other modes (preserve
existing text variable and only modify how out is constructed).
In `@LLM/sub_model/build_index.py`:
- Around line 93-100: The two debug prints after calling _merge_by_url (the one
printing after_rows and the one printing len(df) labeled "중복 제거(병합) 후") are
redundant because after_rows is assigned from len(df) immediately after the
merge; remove the duplicate print (the " - 중복 제거(병합) 후: {len(df)}개 문서" line)
or consolidate the messages into a single clear log so only one line reports the
post-merge document count, keeping variables before_rows, before_unique_url,
_merge_by_url and after_rows unchanged.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
LLM/sub_model/build_index.py (1)
71-115: 빈 URL 병합 버그 수정 확인 — LGTM이전 리뷰에서 지적된 빈 URL 문서 병합 버그가 올바르게 수정되었습니다.
df_has/df_no분리(line 75, 101)로 빈 URL 행은 병합 없이 개별 문서로 유지됩니다.다만 line 76의
dropna=False는 line 73에서 이미fillna("").astype(str).str.strip()으로 정규화가 끝난df_has에 사용되므로 실질적으로 무효입니다.♻️ dropna=False 제거 제안
- for url, g in df_has.groupby("url", dropna=False, sort=False): + for url, g in df_has.groupby("url", sort=False):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/sub_model/build_index.py` around lines 71 - 115, In _merge_by_url, the groupby call on df_has uses groupby("url", dropna=False, sort=False) even though df["url"] was already normalized with fillna("").astype(str).str.strip(), so dropna=False is redundant; remove the dropna=False argument from the df_has.groupby call (i.e., change df_has.groupby("url", dropna=False, sort=False) to df_has.groupby("url", sort=False")) to keep behavior identical and clean up the code.LLM/OSS/Open_AI_OSS.py (1)
766-789:one_sentence_grad개선 — LGTM이전 리뷰에서 지적된
"졸업이수 학점"공백 매칭 문제가 line 781의 키워드 목록에"졸업이수학점"변형을 추가함으로써 해결되었습니다. 빈sub_answer처리, 마크다운 볼드 제거, URL 추출 로직 모두 올바르게 구현되었습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/Open_AI_OSS.py` around lines 766 - 789, one_sentence_grad previously failed to match the "졸업이수 학점" variant; you fixed it by adding the compact form "졸업이수학점" to the keyword list in the preferred-selection logic (see one_sentence_grad, lines, preferred, and the keyword tuple on the line that included "졸업학점", "졸업이수 학점", "전공최저", "총 졸업학점", "졸업이수학점"). Ensure this covers other spacing variants by either adding a regex match with optional whitespace (e.g., "졸업\\s*이수\\s*학점") or adding unit tests for 2/3년제 queries and both spaced/unspaced variants so the preferred-selection branch returns expected summary_line and src_url.
🧹 Nitpick comments (2)
LLM/OSS/Open_AI_OSS.py (1)
755-764:_extract_url_from_text— 다수 URL 존재 시 품질 기반 선택 없음
URL_PAT.findall(text)가 여러 URL을 반환할 때urls[0](첫 번째 URL)을 그대로 반환합니다._parse_bullets_and_pick처럼GOOD_URL_RE/BAD_URL_RE기반 점수를 적용하면 더 적합한 URL을 선택할 수 있습니다.현재
one_sentence_grad의sub_answer포맷은 예측 가능하여 실제 문제가 될 가능성이 낮지만, 이 함수가 다른 곳에서 재사용될 경우 문제가 될 수 있습니다.♻️ URL 점수 기반 선택 제안
urls = URL_PAT.findall(text) if urls: - return ensure_layout_unknown(_clean_url(urls[0])) + good = [u for u in urls if GOOD_URL_RE.search(u) and not BAD_URL_RE.search(u)] + picked = _clean_url(good[0] if good else next((u for u in urls if not BAD_URL_RE.search(u)), urls[0])) + return ensure_layout_unknown(picked)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/Open_AI_OSS.py` around lines 755 - 764, When multiple URLs are returned by URL_PAT.findall in _extract_url_from_text, don't just return urls[0]; instead score and pick the best URL using the same quality logic as _parse_bullets_and_pick (evaluate each candidate against GOOD_URL_RE and BAD_URL_RE or call the existing _parse_bullets_and_pick helper), then pass the chosen URL through _clean_url and ensure_layout_unknown before returning; keep using SRC_URL_PAT/SRC_URL_DOT_PAT short-circuiting first, and only apply scoring when URL_PAT.findall yields more than one candidate.LLM/sub_model/build_index.py (1)
122-124: 병합 후 문서 수를 두 번 출력 — 중복 로그line 119에서
df = _merge_by_url(df)이 실행된 후 line 120에서after_rows = len(df)를 캡처합니다. line 122(after_rows)와 line 124(len(df))는 동일한 값을 서로 다른 메시지로 출력합니다.♻️ 중복 print 제거 제안
print(f" - 원본 행 수: {before_rows} / 고유 URL 수: {before_unique_url}") print(f" - URL 병합 후 문서 수: {after_rows}") - -print(f" - 중복 제거(병합) 후: {len(df)}개 문서")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/sub_model/build_index.py` around lines 122 - 124, The two print statements after calling _merge_by_url(df) are duplicating the same document count (after_rows and len(df)); remove the redundant log by deleting one of the prints (preferably the second "중복 제거(병합) 후: {len(df)}개 문서") or, if you intended two different messages, use the captured after_rows variable consistently; adjust references to after_rows, _merge_by_url, and df accordingly so only a single, non-duplicative message is emitted.
🤖 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/Open_AI_OSS.py`:
- Around line 892-899: The OSS fallback branch currently drops the URL returned
by one_sentence_topic/one_sentence_from_sub_answer and only returns
{"engine":"oss","text": out}; modify the return assembly in the block that
handles sub (after computing out) to include the fallback URL when present:
build a resp dict with keys "engine" and "text" (using out) and, if fallback_url
is truthy, add resp["url"]=fallback_url before returning; reference the
variables/functions one_sentence_topic, one_sentence_from_sub_answer, sub, out,
and fallback_url so the fix is applied where the OSS fallback return is created.
- Around line 929-934: Ruff flags multiple statements on one line; split
combined statements into separate lines so there are no semicolons or multiple
statements per line: move the tuple assignment contact_kws =
("연락처","전화","번호","상담","문의") to its own line, ensure each if uses its own line
(the first if checking "학사일정"/looks_like_schedule(text) and the subsequent ifs
that reference COUNCIL_KWS, contact_kws, and CONTACT_INTENT_RE) and place each
return "fast" on its own line within those if blocks (refer to the uses of
looks_like_schedule, COUNCIL_KWS, contact_kws, and CONTACT_INTENT_RE to locate
the statements).
In `@LLM/sub_model/build_index.py`:
- Around line 117-118: The unique-URL count uses df["url"].nunique(dropna=True)
but earlier code did df["url"].fillna(""), so empty strings "" are counted as a
unique URL; update the computation to exclude empty strings before calling
nunique (e.g., replace empty strings with NaN or filter them out) so
before_unique_url is computed from df["url"] without "" values (reference df and
the "url" column and the before_unique_url variable).
---
Duplicate comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 766-789: one_sentence_grad previously failed to match the "졸업이수
학점" variant; you fixed it by adding the compact form "졸업이수학점" to the keyword
list in the preferred-selection logic (see one_sentence_grad, lines, preferred,
and the keyword tuple on the line that included "졸업학점", "졸업이수 학점", "전공최저", "총
졸업학점", "졸업이수학점"). Ensure this covers other spacing variants by either adding a
regex match with optional whitespace (e.g., "졸업\\s*이수\\s*학점") or adding unit
tests for 2/3년제 queries and both spaced/unspaced variants so the
preferred-selection branch returns expected summary_line and src_url.
In `@LLM/sub_model/build_index.py`:
- Around line 71-115: In _merge_by_url, the groupby call on df_has uses
groupby("url", dropna=False, sort=False) even though df["url"] was already
normalized with fillna("").astype(str).str.strip(), so dropna=False is
redundant; remove the dropna=False argument from the df_has.groupby call (i.e.,
change df_has.groupby("url", dropna=False, sort=False) to df_has.groupby("url",
sort=False")) to keep behavior identical and clean up the code.
---
Nitpick comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 755-764: When multiple URLs are returned by URL_PAT.findall in
_extract_url_from_text, don't just return urls[0]; instead score and pick the
best URL using the same quality logic as _parse_bullets_and_pick (evaluate each
candidate against GOOD_URL_RE and BAD_URL_RE or call the existing
_parse_bullets_and_pick helper), then pass the chosen URL through _clean_url and
ensure_layout_unknown before returning; keep using SRC_URL_PAT/SRC_URL_DOT_PAT
short-circuiting first, and only apply scoring when URL_PAT.findall yields more
than one candidate.
In `@LLM/sub_model/build_index.py`:
- Around line 122-124: The two print statements after calling _merge_by_url(df)
are duplicating the same document count (after_rows and len(df)); remove the
redundant log by deleting one of the prints (preferably the second "중복 제거(병합) 후:
{len(df)}개 문서") or, if you intended two different messages, use the captured
after_rows variable consistently; adjust references to after_rows,
_merge_by_url, and df accordingly so only a single, non-duplicative message is
emitted.
Comment on lines
+892
to
+899
| if sub: | ||
| if looks_like_topic(user_text): | ||
| text, _ = one_sentence_topic(user_text, sub) | ||
| else: | ||
| text, _ = one_sentence_from_sub_answer(user_text, sub) | ||
| out = text | ||
| else: | ||
| out = "잘 이해하지 못했어요. 다시 질문해주세요." |
There was a problem hiding this comment.
OSS 폴백 경로에서 URL이 응답에 포함되지 않음
text, _ = one_sentence_topic(...) 및 text, _ = one_sentence_from_sub_answer(...)에서 URL을 _로 버리고 line 900에서 {"engine":"oss", "text": out}만 반환합니다. fast, policy, dorm, grad, topic 모드는 모두 URL이 있을 경우 응답에 포함하므로, 이 경로만 URL이 누락되어 일관성이 깨집니다.
🐛 URL 포함 수정 제안
- if sub:
- if looks_like_topic(user_text):
- text, _ = one_sentence_topic(user_text, sub)
- else:
- text, _ = one_sentence_from_sub_answer(user_text, sub)
- out = text
- else:
- out = "잘 이해하지 못했어요. 다시 질문해주세요."
+ fallback_url = None
+ if sub:
+ if looks_like_topic(user_text):
+ out, fallback_url = one_sentence_topic(user_text, sub)
+ else:
+ out, fallback_url = one_sentence_from_sub_answer(user_text, sub)
+ else:
+ out = "잘 이해하지 못했어요. 다시 질문해주세요."
return {"engine":"oss", "text": out}그리고 return 직전에:
resp = {"engine": "oss", "text": out}
if fallback_url:
resp["url"] = fallback_url
return resp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/OSS/Open_AI_OSS.py` around lines 892 - 899, The OSS fallback branch
currently drops the URL returned by
one_sentence_topic/one_sentence_from_sub_answer and only returns
{"engine":"oss","text": out}; modify the return assembly in the block that
handles sub (after computing out) to include the fallback URL when present:
build a resp dict with keys "engine" and "text" (using out) and, if fallback_url
is truthy, add resp["url"]=fallback_url before returning; reference the
variables/functions one_sentence_topic, one_sentence_from_sub_answer, sub, out,
and fallback_url so the fix is applied where the OSS fallback return is created.
Comment on lines
+929
to
+934
| if "학사일정" in text or "학사 일정" in text or "학사일정" in compact or looks_like_schedule(text): | ||
| return "fast" | ||
|
|
||
| contact_kws = ("연락처","전화","번호","상담","문의") | ||
| if any(k in text for k in COUNCIL_KWS): return "fast" | ||
| if any(k in text for k in contact_kws): return "fast" | ||
| if any(k in text for k in contact_kws) or CONTACT_INTENT_RE.search(text): return "fast" |
There was a problem hiding this comment.
Ruff E701 — 한 줄에 복수 문장 (콜론)
Ruff가 lines 933-934를 E701(Multiple statements on one line (colon))로 플래그합니다. line 929도 동일한 패턴에 해당합니다.
🐛 E701 수정 제안
- if any(k in text for k in COUNCIL_KWS): return "fast"
- if any(k in text for k in contact_kws) or CONTACT_INTENT_RE.search(text): return "fast"
+ if any(k in text for k in COUNCIL_KWS):
+ return "fast"
+ if any(k in text for k in contact_kws) or CONTACT_INTENT_RE.search(text):
+ return "fast"🧰 Tools
🪛 Ruff (0.15.1)
[error] 933-933: Multiple statements on one line (colon)
(E701)
[error] 934-934: Multiple statements on one line (colon)
(E701)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/OSS/Open_AI_OSS.py` around lines 929 - 934, Ruff flags multiple
statements on one line; split combined statements into separate lines so there
are no semicolons or multiple statements per line: move the tuple assignment
contact_kws = ("연락처","전화","번호","상담","문의") to its own line, ensure each if uses
its own line (the first if checking "학사일정"/looks_like_schedule(text) and the
subsequent ifs that reference COUNCIL_KWS, contact_kws, and CONTACT_INTENT_RE)
and place each return "fast" on its own line within those if blocks (refer to
the uses of looks_like_schedule, COUNCIL_KWS, contact_kws, and CONTACT_INTENT_RE
to locate the statements).
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.
📌 챗봇 서브 모델 재정의
📝 작업 내용
✅ 체크리스트
변경 요약(1~3줄)
챗봇 서브모델의 응답 처리와 인덱스 병합 로직을 재정의했습니다. URL 기반 문서 병합, 출처 URL 추출 강화, JSON 전용 모드와 학년/등급 하드코딩 제거 등으로 응답의 유연성과 견고성이 향상되었습니다.
주요 변경점
build_index.py:
_merge_by_url()추가 — 같은 URL을 가진 행들을 소스 우선순위로 병합하고 제목/내용을 정제·연결하며 빈 URL 행은 보존함. 병합 전후 로그 추가 및 문서 수 집계 개선.Open_AI_OSS.py: 환경변수 기반
JSON_ONLY_MODE추가 및 공개 변수로 노출 — 활성화 시 스케줄링 결과를 건너뛰고 기본 응답만 반환하도록 동작 변경.URL 추출/정규화:
_extract_url_from_text()도입으로 응답 텍스트에서 출처 URL을 파싱·정규화하여 우선 출처로 사용.one_sentence_grad 개선: 하드코딩된 학년/등급 제거, 서브답변을 파싱해 핵심 문장 선택 및 출처 URL 포함 반환(오탈자·공백에 강해짐).
응답 흐름·판단 개선: decide_mode에서 텍스트 컴팩트화로 "학사일정" 탐지 강화하고 연락처 의도 감지 범위 확장. OSS 경로에서 서브 존재 여부·주제 판별에 따라 다른 요약 함수 사용.
기타: URL 병합 관련 로직 리팩토링(강제 병합/삭제 동작 관련 리팩터 및 grade 변수 도입), 일부 임포트 경로 정리와 로그/포맷 소소한 수정.
주의/리스크
응답 형식 변화로 기존 클라이언트 파싱 로직이 깨질 수 있음(특히 URL 포함/미포함 경우).
URL 기반 병합 시 의도치 않은 정보 합침으로 요약 왜곡 가능 — 병합 규칙 검증 필요.
JSON_ONLY_MODE 사용 시 스케줄링·보강된 응답이 빠져 정확도 저하 우려.
다음 액션
로컬/스테이징에서 챗봇 응답 테스트 — URL 추출, 학사일정 탐지, JSON_ONLY_MODE 동작 확인.
병합 로직의 엣지케이스(빈 URL, 서로 다른 출처의 동일 URL 등) 확인 및 테스트 케이스 보강.
클라이언트(혹은 downstream) 응답 파싱 영향 여부 점검 및 필요 시 버전별 호환 안내 문서화.