feat: chatbot request rulebook for langgraph - #84
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocker 이미지에 규칙집 데이터 포함, FastAPI 시작 시 규칙집 인덱스 비동기 빌드 추가, PDF 기반 BM25 규칙집 인덱스 및 LangGraph 비동기 검색·생성 파이프라인 구현, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Router as "Router (/chatbot)"
participant ModeDecision as "decide_mode"
participant RuleBookGraph as "RuleBook Graph"
participant RuleBookIndex as "RuleBook Index"
participant OpenAI as "OpenAI API"
Client->>Router: POST /chatbot (user_text)
Router->>ModeDecision: decide_mode(user_text)
alt mode == "rule_book"
ModeDecision-->>Router: "rule_book"
Router->>RuleBookGraph: run_rule_book(user_text)
RuleBookGraph->>RuleBookIndex: retrieve(query)
RuleBookIndex-->>RuleBookGraph: chunks
RuleBookGraph->>OpenAI: generate(chunks, system_prompt)
OpenAI-->>RuleBookGraph: answer
RuleBookGraph-->>Router: {"engine":"rule_book","text":answer}
else other modes
ModeDecision-->>Router: other
Router->>OpenAI: standard processing
end
Router-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 🚥 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 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: 2
🧹 Nitpick comments (5)
app_oss_main.py (1)
8-11:asyncio.get_event_loop()는 Python 3.10+ 이상에서 deprecated 패턴입니다.
asyncio.to_thread()를 사용하면 더 간결하고 권장되는 방식입니다.♻️ 권장 수정안
`@asynccontextmanager` async def lifespan(app: FastAPI): - await asyncio.get_event_loop().run_in_executor(None, build_index) + await asyncio.to_thread(build_index) yield🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app_oss_main.py` around lines 8 - 11, Replace the deprecated asyncio.get_event_loop().run_in_executor(None, build_index) call inside the lifespan asynccontextmanager with the modern asyncio.to_thread approach: call and await asyncio.to_thread(build_index) so build_index runs in a thread safely on Python 3.10+; update the lifespan function where build_index is invoked to use asyncio.to_thread(build_index) and keep the existing yield behavior.LLM/rule_book/graph.py (1)
24-27: private 속성_built에 직접 접근하고 있습니다.
RuleBookIndex클래스에is_ready()같은 public 메서드를 추가하면 캡슐화가 개선됩니다.♻️ 캡슐화 개선 제안
LLM/rule_book/index.py에 추가:def is_ready(self) -> bool: return self._built
LLM/rule_book/graph.py에서:async def retrieve(state: RuleState) -> RuleState: index = get_index() - if not index._built: + if not index.is_ready(): return {**state, "chunks": [], "error": "인덱스가 아직 준비되지 않았습니다."}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/graph.py` around lines 24 - 27, The code in retrieve() reads the private attribute _built on the RuleBookIndex returned by get_index(); instead add a public readiness check by implementing is_ready(self) -> bool on RuleBookIndex (returning self._built) and then change retrieve() to call get_index().is_ready() to decide whether to return the empty-chunks error, preserving behavior but avoiding direct access to _built.LLM/rule_book/index.py (2)
23-26: PDF 리소스 관리를 위해 context manager 사용을 권장합니다.
fitz.open()과doc.close()사이에 예외가 발생하면 리소스 누수가 발생할 수 있습니다.♻️ Context manager 사용
def _chunk_pdf(path: Path) -> List[Dict]: - doc = fitz.open(str(path)) - full_text = "\n".join(page.get_text() for page in doc) - doc.close() + with fitz.open(str(path)) as doc: + full_text = "\n".join(page.get_text() for page in doc) source = path.stem🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/index.py` around lines 23 - 26, The _chunk_pdf function opens a PDF with fitz.open and manually calls doc.close which can leak resources if an exception occurs; change to using a context manager (with fitz.open(str(path)) as doc:) around the block that builds full_text and processes pages so the document is always closed, move any page iteration and text extraction inside that with block, and ensure the function still returns the List[Dict] as before (refer to _chunk_pdf, fitz.open, and doc.close to locate the change).
75-78: 예외 처리가 적절합니다.PDF 파싱에서는 다양한 예외가 발생할 수 있으므로
Exception을 catch하는 것이 합리적입니다. 다만 프로덕션에서는🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/index.py` around lines 75 - 78, Replace the print call in the except block that wraps the _chunk_pdf(pdf_path) call with a proper logger call: use the module or project logger (e.g., a logger from logging.getLogger or an existing logger variable) to log an error including pdf_path.name and the exception, and pass exc_info=True so the stack trace is recorded; update the except block surrounding _chunk_pdf and all_chunks to use logger.error(...) instead of print(...) while preserving the current exception handling flow.LLM/OSS/Open_AI_OSS.py (1)
1-1:asyncioimport가 사용되지 않습니다.이 파일에서
asyncio가 직접 사용되지 않으므로 불필요한 import입니다.♻️ 불필요한 import 제거
-import os, sys, re, asyncio +import os, sys, re🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/Open_AI_OSS.py` at line 1, Remove the unused import 'asyncio' from the top-level import statement in Open_AI_OSS.py (the line "import os, sys, re, asyncio"); keep the other imports (os, sys, re) intact so there are no unused imports left.
🤖 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/rule_book/graph.py`:
- Around line 9-13: The OSS_MODEL environment variable is used when constructing
API calls (e.g., model=OSS_MODEL) but may be None; add validation when loading
OSS_MODEL in this module (where AsyncOpenAI is created and OSS_MODEL is defined)
to either provide a sensible default or raise a clear error: check OSS_MODEL =
os.getenv("OSS_MODEL") and if falsy raise ValueError("OSS_MODEL environment
variable is not set") or assign a documented default string, so subsequent uses
like model=OSS_MODEL and any functions/classes that call AsyncOpenAI won't
receive None.
In `@LLM/rule_book/index.py`:
- Around line 62-67: The RuleBookIndex.build() is not thread-safe which can
cause race conditions when called concurrently or when search() runs during
build; wrap the build logic with a threading.Lock (e.g., self._build_lock) and
use a double-checked pattern: if not self._built, acquire the lock, re-check
self._built, set a flag (or set self._built=True only after successful
completion), perform the BM25 creation (self.bm25 and self.chunks) inside the
locked section, and release the lock; also make search() check self._built (or
acquire the same lock for a short read-check) and either wait, block, or return
an explicit error until the build finishes. Ensure you add the lock in
RuleBookIndex.__init__ (self._build_lock = threading.Lock()) and reference
RuleBookIndex.build(), RuleBookIndex.search(), self._built, self.bm25, and
self.chunks in the changes.
---
Nitpick comments:
In `@app_oss_main.py`:
- Around line 8-11: Replace the deprecated
asyncio.get_event_loop().run_in_executor(None, build_index) call inside the
lifespan asynccontextmanager with the modern asyncio.to_thread approach: call
and await asyncio.to_thread(build_index) so build_index runs in a thread safely
on Python 3.10+; update the lifespan function where build_index is invoked to
use asyncio.to_thread(build_index) and keep the existing yield behavior.
In `@LLM/OSS/Open_AI_OSS.py`:
- Line 1: Remove the unused import 'asyncio' from the top-level import statement
in Open_AI_OSS.py (the line "import os, sys, re, asyncio"); keep the other
imports (os, sys, re) intact so there are no unused imports left.
In `@LLM/rule_book/graph.py`:
- Around line 24-27: The code in retrieve() reads the private attribute _built
on the RuleBookIndex returned by get_index(); instead add a public readiness
check by implementing is_ready(self) -> bool on RuleBookIndex (returning
self._built) and then change retrieve() to call get_index().is_ready() to decide
whether to return the empty-chunks error, preserving behavior but avoiding
direct access to _built.
In `@LLM/rule_book/index.py`:
- Around line 23-26: The _chunk_pdf function opens a PDF with fitz.open and
manually calls doc.close which can leak resources if an exception occurs; change
to using a context manager (with fitz.open(str(path)) as doc:) around the block
that builds full_text and processes pages so the document is always closed, move
any page iteration and text extraction inside that with block, and ensure the
function still returns the List[Dict] as before (refer to _chunk_pdf, fitz.open,
and doc.close to locate the change).
- Around line 75-78: Replace the print call in the except block that wraps the
_chunk_pdf(pdf_path) call with a proper logger call: use the module or project
logger (e.g., a logger from logging.getLogger or an existing logger variable) to
log an error including pdf_path.name and the exception, and pass exc_info=True
so the stack trace is recorded; update the except block surrounding _chunk_pdf
and all_chunks to use logger.error(...) instead of print(...) while preserving
the current exception handling flow.
🪄 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: de29194b-3b60-4acc-a852-5c4738df3a4c
📒 Files selected for processing (6)
Dockerfiles_ossLLM/OSS/Open_AI_OSS.pyLLM/rule_book/__init__.pyLLM/rule_book/graph.pyLLM/rule_book/index.pyapp_oss_main.py
Comment on lines
+9
to
+13
| _async_client = AsyncOpenAI( | ||
| base_url=os.getenv("OSS_BASE_URL"), | ||
| api_key=os.getenv("OSS_API_KEY"), | ||
| ) | ||
| OSS_MODEL = os.getenv("OSS_MODEL") |
There was a problem hiding this comment.
OSS_MODEL이 None일 수 있어 API 호출 시 오류가 발생할 수 있습니다.
환경 변수가 설정되지 않은 경우 OSS_MODEL이 None이 되어 Line 59의 model=OSS_MODEL 호출에서 오류가 발생합니다.
🛡️ 기본값 또는 검증 추가
_async_client = AsyncOpenAI(
base_url=os.getenv("OSS_BASE_URL"),
api_key=os.getenv("OSS_API_KEY"),
)
-OSS_MODEL = os.getenv("OSS_MODEL")
+OSS_MODEL = os.getenv("OSS_MODEL", "gpt-3.5-turbo") # 또는 필수 검증 추가또는 모듈 로드 시 검증:
OSS_MODEL = os.getenv("OSS_MODEL")
if not OSS_MODEL:
raise ValueError("OSS_MODEL 환경 변수가 설정되지 않았습니다.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/rule_book/graph.py` around lines 9 - 13, The OSS_MODEL environment
variable is used when constructing API calls (e.g., model=OSS_MODEL) but may be
None; add validation when loading OSS_MODEL in this module (where AsyncOpenAI is
created and OSS_MODEL is defined) to either provide a sensible default or raise
a clear error: check OSS_MODEL = os.getenv("OSS_MODEL") and if falsy raise
ValueError("OSS_MODEL environment variable is not set") or assign a documented
default string, so subsequent uses like model=OSS_MODEL and any
functions/classes that call AsyncOpenAI won't receive None.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
LLM/rule_book/index.py (1)
71-74:⚠️ Potential issue | 🔴 Critical
build()락 범위가 짧아 동시 빌드 race가 그대로 남아 있습니다.현재는
_built확인 직후 락이 해제되어, 동시 호출 시 여러 스레드가 빌드를 수행할 수 있습니다.self.chunks,self.bm25,self._built갱신까지 같은 락 범위로 보호해 원자적으로 publish하세요.🔒 제안 수정안
def build(self) -> None: - with self._lock: - if self._built: - return - rule_dir = _rule_book_dir() - if not rule_dir.exists(): - raise FileNotFoundError(f"규정집 디렉토리 없음: {rule_dir}") - - all_chunks: List[Dict] = [] - for pdf_path in sorted(rule_dir.glob("*.pdf")): - try: - all_chunks.extend(_chunk_pdf(pdf_path)) - except Exception as e: - print(f"[RuleBook] {pdf_path.name} 파싱 실패: {e}") - - if not all_chunks: - raise ValueError("규정집 청크가 0개 — PDF 파싱 실패") - - self.chunks = all_chunks - tokenized = [_tokenize(c["text"]) for c in all_chunks] - self.bm25 = BM25Okapi(tokenized) - self._built = True - print(f"[RuleBook] 인덱스 빌드 완료: {len(self.chunks)}개 청크") + with self._lock: + if self._built: + return + rule_dir = _rule_book_dir() + if not rule_dir.exists(): + raise FileNotFoundError(f"규정집 디렉토리 없음: {rule_dir}") + + all_chunks: List[Dict] = [] + for pdf_path in sorted(rule_dir.glob("*.pdf")): + try: + all_chunks.extend(_chunk_pdf(pdf_path)) + except Exception as e: + print(f"[RuleBook] {pdf_path.name} 파싱 실패: {e}") + + if not all_chunks: + raise ValueError("규정집 청크가 0개 — PDF 파싱 실패") + + self.chunks = all_chunks + tokenized = [_tokenize(c["text"]) for c in all_chunks] + self.bm25 = BM25Okapi(tokenized) + self._built = True + print(f"[RuleBook] 인덱스 빌드 완료: {len(self.chunks)}개 청크")Also applies to: 88-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/index.py` around lines 71 - 74, The build() method currently releases self._lock immediately after checking self._built, allowing concurrent threads to enter the build path; change the locking so the entire build process — from checking/invoking _rule_book_dir()/reading/processing files through assigning self.chunks, self.bm25 and setting self._built — occurs while holding self._lock to make the publish atomic; update both build code paths (the initial block around _rule_book_dir() and the other similar block that updates self.chunks/self.bm25/_built) to use the same extended lock scope so concurrent callers cannot run parallel builds.
🧹 Nitpick comments (1)
LLM/rule_book/index.py (1)
80-84: 파싱 실패 처리에서 예외를 너무 넓게 삼키고 있어 원인 추적이 어렵습니다.현재는
Exception을 잡고 메시지만 출력합니다. traceback 포함 구조화 로그로 남기거나, 예상 가능한 예외 타입으로 좁혀 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/index.py` around lines 80 - 84, The current try/except around calling _chunk_pdf(pdf_path) swallows all Exceptions and only prints a simple message, making root-cause debugging hard; change the handler in the block that extends all_chunks to catch specific expected exceptions (e.g., FileNotFoundError, ValueError, PDF parsing library errors) or re-raise unexpected ones, and replace the print with structured logging that includes the full traceback (use logger.exception or capture and log traceback.format_exc()) while keeping pdf_path.name in the message so you still record which file failed.
🤖 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/rule_book/index.py`:
- Around line 94-101: The search method should validate the top_k input to avoid
unintended slicing when top_k <= 0; inside search (function name: search) before
tokenization and after the _built/bm25 check, add a guard that either raises a
ValueError (e.g., "top_k must be a positive integer") or returns an empty list
for invalid top_k values; reference the parameters top_k and local variables
tokens/scores/top_indices when updating logic so downstream code (tokens =
_tokenize(query); scores = self.bm25.get_scores(tokens); top_indices =
sorted(... )[:top_k]) never receives a non-positive slice size.
- Around line 25-27: The PDF file handle opened by fitz.open() (variable doc)
may not be closed if page.get_text() raises an exception; wrap the open-and-read
logic in a try/finally (or use fitz.open as a context manager) so that
doc.close() is always called: open doc with fitz.open(str(path)), assign
full_text by iterating pages inside the try block (e.g., full_text =
"\n".join(page.get_text() for page in doc)), and call doc.close() in the finally
block to guarantee cleanup even on exceptions.
---
Duplicate comments:
In `@LLM/rule_book/index.py`:
- Around line 71-74: The build() method currently releases self._lock
immediately after checking self._built, allowing concurrent threads to enter the
build path; change the locking so the entire build process — from
checking/invoking _rule_book_dir()/reading/processing files through assigning
self.chunks, self.bm25 and setting self._built — occurs while holding self._lock
to make the publish atomic; update both build code paths (the initial block
around _rule_book_dir() and the other similar block that updates
self.chunks/self.bm25/_built) to use the same extended lock scope so concurrent
callers cannot run parallel builds.
---
Nitpick comments:
In `@LLM/rule_book/index.py`:
- Around line 80-84: The current try/except around calling _chunk_pdf(pdf_path)
swallows all Exceptions and only prints a simple message, making root-cause
debugging hard; change the handler in the block that extends all_chunks to catch
specific expected exceptions (e.g., FileNotFoundError, ValueError, PDF parsing
library errors) or re-raise unexpected ones, and replace the print with
structured logging that includes the full traceback (use logger.exception or
capture and log traceback.format_exc()) while keeping pdf_path.name in the
message so you still record which file failed.
🪄 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: ecc03f3a-2103-4ff3-af93-6f7482798f3f
📒 Files selected for processing (1)
LLM/rule_book/index.py
Comment on lines
+25
to
+27
| doc = fitz.open(str(path)) | ||
| full_text = "\n".join(page.get_text() for page in doc) | ||
| doc.close() |
There was a problem hiding this comment.
PDF 파싱 도중 예외 발생 시 파일 핸들 정리가 보장되지 않습니다.
텍스트 추출 중 예외가 나면 doc.close()가 실행되지 않는 경로가 있습니다. try/finally로 close를 보장하세요.
🧹 제안 수정안
def _chunk_pdf(path: Path) -> List[Dict]:
- doc = fitz.open(str(path))
- full_text = "\n".join(page.get_text() for page in doc)
- doc.close()
+ doc = fitz.open(str(path))
+ try:
+ full_text = "\n".join(page.get_text() for page in doc)
+ finally:
+ doc.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/rule_book/index.py` around lines 25 - 27, The PDF file handle opened by
fitz.open() (variable doc) may not be closed if page.get_text() raises an
exception; wrap the open-and-read logic in a try/finally (or use fitz.open as a
context manager) so that doc.close() is always called: open doc with
fitz.open(str(path)), assign full_text by iterating pages inside the try block
(e.g., full_text = "\n".join(page.get_text() for page in doc)), and call
doc.close() in the finally block to guarantee cleanup even on exceptions.
Comment on lines
+94
to
+101
| def search(self, query: str, top_k: int = 5) -> List[Dict]: | ||
| if not self._built or self.bm25 is None: | ||
| return [] | ||
| tokens = _tokenize(query) | ||
| if not tokens: | ||
| return [] | ||
| scores = self.bm25.get_scores(tokens) | ||
| top_indices = sorted(range(len(scores)), key=lambda i: scores[i], reverse=True)[:top_k] |
There was a problem hiding this comment.
top_k 입력값 검증이 필요합니다.
top_k <= 0일 때 의도치 않은 슬라이싱 결과가 나올 수 있습니다. 조기 반환 또는 ValueError 처리로 방어해 주세요.
🛡️ 제안 수정안
def search(self, query: str, top_k: int = 5) -> List[Dict]:
if not self._built or self.bm25 is None:
return []
+ if top_k <= 0:
+ return []
tokens = _tokenize(query)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/rule_book/index.py` around lines 94 - 101, The search method should
validate the top_k input to avoid unintended slicing when top_k <= 0; inside
search (function name: search) before tokenization and after the _built/bm25
check, add a guard that either raises a ValueError (e.g., "top_k must be a
positive integer") or returns an empty list for invalid top_k values; reference
the parameters top_k and local variables tokens/scores/top_indices when updating
logic so downstream code (tokens = _tokenize(query); scores =
self.bm25.get_scores(tokens); top_indices = sorted(... )[:top_k]) never receives
a non-positive slice size.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
LLM/rule_book/graph.py (1)
15-15:⚠️ Potential issue | 🟠 Major
OSS_MODEL미설정 시 모델 호출이 실패할 수 있습니다.환경변수 검증(또는 명시적 기본값)이 없어
model=None으로 호출될 수 있습니다. 모듈 로드 시점에 명확히 실패시키거나 기본값을 강제해주세요.수정 예시
OSS_MODEL = os.getenv("OSS_MODEL") +if not OSS_MODEL: + raise ValueError("OSS_MODEL 환경 변수가 설정되지 않았습니다.")Also applies to: 60-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/graph.py` at line 15, OSS_MODEL is read directly from os.getenv and can be None causing downstream model calls to fail; update the module to either enforce a clear default or fail fast at import: replace OSS_MODEL = os.getenv("OSS_MODEL") with either OSS_MODEL = os.getenv("OSS_MODEL", "<your-default-model>") or add an explicit validation block that raises a ValueError/RuntimeError if OSS_MODEL is falsy (and do the same for the other occurrences referenced around lines 60-61). Also ensure any functions that use OSS_MODEL (search for OSS_MODEL, model_name, or similar in this file) can rely on the validated value or propagate the error to caller.
🤖 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/rule_book/graph.py`:
- Around line 33-39: The code currently propagates raw exception text into state
and then into the user-facing answer (see the exception handler that returns
{**state, "chunks": [], "error": str(e)} and the generate(state: RuleState)
function that reads state["error"]); instead, stop storing or exposing the raw
exception string to users: in the except block replace storing str(e) with a
non-sensitive flag or a short internal error code (e.g., "error":
"internal_fetch_failed") and write the full exception detail to an internal
logger; then update generate(state: RuleState) to return a fixed, generic
user-facing message like "규정집 검색 중 오류가 발생했습니다." when an error flag is present,
never interpolating state["error"] into the answer, and ensure the original
exception is only in logs for debugging.
- Line 17: The TOP_K constant initialization using
int(os.getenv("RULE_BOOK_TOP_K", "5")) can raise ValueError at import time;
change it to safely parse the env var (use os.getenv("RULE_BOOK_TOP_K") value),
try converting inside a try/except (or use str.isdigit()+int), and on any parse
error or missing value fall back to the default 5 and enforce a minimum (e.g.,
max(parsed_value, 1)). Update the TOP_K assignment so it never raises during
import and clamps invalid or too-small values.
In `@LLM/rule_book/logger.py`:
- Around line 33-40: The current record populates raw user input and generated
answer (variables query and answer) and full chunk metadata
(chunks/chunks_sources), which risks storing PII; create and call a sanitizer
before building record (e.g., functions mask_pii(text), truncate_text(text,
max_len=1000), and sanitize_chunk_source(source_dict)) and replace query with
mask_pii(truncate_text(query,...)), answer with
mask_pii(truncate_text(answer,...)), and build chunks_sources from sanitized
metadata only (no raw chunk text—use hashed IDs or source filenames and truncate
any fields); keep error and elapsed_ms as-is but ensure error messages are
scrubbed via mask_pii if they may include user data.
- Around line 42-45: The except block currently swallows all exceptions from
_write_log(record); change it to catch Exception as e and emit a fallback
warning/error using the module logger (e.g., logging.getLogger(__name__).warning
or a project-wide logger) including the exception message and exc_info=True, but
do not re-raise so the service response remains unaffected; update the block
around the await _write_log(record) call to log the failure details while
preserving the existing behavior of not crashing.
---
Duplicate comments:
In `@LLM/rule_book/graph.py`:
- Line 15: OSS_MODEL is read directly from os.getenv and can be None causing
downstream model calls to fail; update the module to either enforce a clear
default or fail fast at import: replace OSS_MODEL = os.getenv("OSS_MODEL") with
either OSS_MODEL = os.getenv("OSS_MODEL", "<your-default-model>") or add an
explicit validation block that raises a ValueError/RuntimeError if OSS_MODEL is
falsy (and do the same for the other occurrences referenced around lines 60-61).
Also ensure any functions that use OSS_MODEL (search for OSS_MODEL, model_name,
or similar in this file) can rely on the validated value or propagate the error
to caller.
🪄 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: 33fb6e52-72a8-41f3-a122-ec408edee051
📒 Files selected for processing (2)
LLM/rule_book/graph.pyLLM/rule_book/logger.py
| ) | ||
| OSS_MODEL = os.getenv("OSS_MODEL") | ||
|
|
||
| TOP_K = int(os.getenv("RULE_BOOK_TOP_K", "5")) |
There was a problem hiding this comment.
RULE_BOOK_TOP_K는 안전 파싱과 하한값 보정이 필요합니다.
현재는 잘못된 환경값이 들어오면 모듈 import 단계에서 ValueError로 서비스가 기동 실패할 수 있습니다.
수정 예시
-TOP_K = int(os.getenv("RULE_BOOK_TOP_K", "5"))
+try:
+ TOP_K = max(1, int(os.getenv("RULE_BOOK_TOP_K", "5")))
+except ValueError:
+ TOP_K = 5📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TOP_K = int(os.getenv("RULE_BOOK_TOP_K", "5")) | |
| try: | |
| TOP_K = max(1, int(os.getenv("RULE_BOOK_TOP_K", "5"))) | |
| except ValueError: | |
| TOP_K = 5 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/rule_book/graph.py` at line 17, The TOP_K constant initialization using
int(os.getenv("RULE_BOOK_TOP_K", "5")) can raise ValueError at import time;
change it to safely parse the env var (use os.getenv("RULE_BOOK_TOP_K") value),
try converting inside a try/except (or use str.isdigit()+int), and on any parse
error or missing value fall back to the default 5 and enforce a minimum (e.g.,
max(parsed_value, 1)). Update the TOP_K assignment so it never raises during
import and clamps invalid or too-small values.
Comment on lines
+33
to
+40
| record = { | ||
| "timestamp": datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), | ||
| "query": query, | ||
| "chunks_count": len(chunks), | ||
| "chunks_sources": [f"{c.get('source', '')} {c.get('article', '')}".strip() for c in chunks], | ||
| "answer": answer, | ||
| "error": error, | ||
| "elapsed_ms": elapsed_ms, |
There was a problem hiding this comment.
원문 질의/응답 저장은 개인정보 컴플라이언스 리스크가 큽니다.
사용자 입력과 생성 답변을 그대로 남기면 학번/이름/연락처 등 민감정보가 로그에 축적될 수 있습니다. 최소수집(마스킹·길이 제한·보존기간) 기준을 적용하는 게 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/rule_book/logger.py` around lines 33 - 40, The current record populates
raw user input and generated answer (variables query and answer) and full chunk
metadata (chunks/chunks_sources), which risks storing PII; create and call a
sanitizer before building record (e.g., functions mask_pii(text),
truncate_text(text, max_len=1000), and sanitize_chunk_source(source_dict)) and
replace query with mask_pii(truncate_text(query,...)), answer with
mask_pii(truncate_text(answer,...)), and build chunks_sources from sanitized
metadata only (no raw chunk text—use hashed IDs or source filenames and truncate
any fields); keep error and elapsed_ms as-is but ensure error messages are
scrubbed via mask_pii if they may include user data.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
LLM/rule_book/graph.py (1)
18-20:⚠️ Potential issue | 🟠 Major환경 변수 검증 부재로 기동 실패/모델 호출 실패가 발생할 수 있습니다.
Line 18의
OSS_MODEL미검증과 Line 20의TOP_K직접 파싱은 각각 런타임 모델 호출 오류와 import 시ValueError를 유발할 수 있습니다. 모듈 로드 시 안전 파싱/검증을 추가해 주세요.✅ 안전한 초기화 예시
OSS_MODEL = os.getenv("OSS_MODEL") +if not OSS_MODEL: + raise ValueError("OSS_MODEL environment variable is not set") -TOP_K = int(os.getenv("RULE_BOOK_TOP_K", "5")) +try: + TOP_K = max(1, int(os.getenv("RULE_BOOK_TOP_K", "5"))) +except (TypeError, ValueError): + TOP_K = 5Also applies to: 64-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/graph.py` around lines 18 - 20, OSS_MODEL is read without validation and TOP_K is parsed directly which can raise ValueError at import time; update initialization for OSS_MODEL and TOP_K (and the similar vars around the second occurrence) to perform safe validation and parsing: check OSS_MODEL is non-empty and matches expected model names (or set a sensible default or raise a clear error), parse TOP_K using a safe int parser with a fallback/default and guard against non-numeric values (log or raise a descriptive error), and apply the same pattern to the other occurrence around lines 64-65; modify the module-level assignments (OSS_MODEL, TOP_K and the duplicated vars) to use a small helper or try/except so import-time exceptions are avoided and errors are logged with context.LLM/rule_book/logger.py (2)
34-40:⚠️ Potential issue | 🟠 Major원문 질의/응답을 그대로 저장하지 마세요 (PII 컴플라이언스 리스크).
Line 36, Line 39, Line 40에서 사용자 입력/응답/오류를 원문으로 저장하고 있어 개인정보가 로그에 축적될 수 있습니다. 마스킹 + 길이 제한 + 최소 메타데이터 저장으로 줄이는 게 안전합니다.
🔒 최소수집 기준 반영 예시
+MAX_TEXT_LEN = 1000 + +def _sanitize_text(text: str | None) -> str | None: + if text is None: + return None + t = text[:MAX_TEXT_LEN] + # 프로젝트 기준에 맞는 마스킹 로직으로 교체 + return t + +def _sanitize_source(chunk: dict) -> str: + source = str(chunk.get("source", ""))[:200] + article = str(chunk.get("article", ""))[:100] + return f"{source} {article}".strip() + async def log_rule_book(query: str, chunks: list, answer: str, error: str | None, elapsed_ms: int) -> None: record = { "timestamp": datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), - "query": query, + "query": _sanitize_text(query), "chunks_count": len(chunks), - "chunks_sources": [f"{c.get('source', '')} {c.get('article', '')}".strip() for c in chunks], - "answer": answer, - "error": error, + "chunks_sources": [_sanitize_source(c) for c in chunks], + "answer": _sanitize_text(answer), + "error": _sanitize_text(error), "elapsed_ms": elapsed_ms, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/logger.py` around lines 34 - 40, The record currently stores raw PII in fields 'query', 'chunks_sources' (via chunks), 'answer', and 'error'; replace direct storage with a redaction step: create and call a helper like apply_mask_and_truncate(value, max_len=200) to mask sensitive content and truncate to a safe length, store only the masked snippet plus minimal metadata (e.g., original_length, truncated boolean, and a redacted boolean) instead of the full text, and for 'chunks_sources' map each chunk to its sanitized source string (masking article text and keeping only non-sensitive identifiers) rather than embedding raw chunk content; update usages around the record construction to use these sanitized outputs.
43-46:⚠️ Potential issue | 🟠 Major로깅 실패 경고 메시지 포맷이 깨져 예외 정보가 유실됩니다.
Line 46은 문자열 안에
e, exc_info=True를 포함했을 뿐 실제 인자를 넘기지 않습니다. 예외 객체와 traceback을 실제 인자로 전달해야 운영 추적이 됩니다.🛠️ 수정 예시
try: await _write_log(record) - except Exception: - _logger.warning("rule_book 로그 기록 실패: %s, e, exc_info=True") + except Exception as e: + _logger.warning("rule_book 로그 기록 실패: %s", e, exc_info=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/logger.py` around lines 43 - 46, The warning message in the except block after calling _write_log(record) currently embeds "e, exc_info=True" inside the format string, losing the real exception and traceback; update the except clause to capture the exception (e.g., except Exception as e:) and call _logger.warning with a proper message and the exception/tracing parameters (pass the exception as an argument or use exc_info=True) so the actual exception and stacktrace from _write_log are logged; adjust the call site referencing _write_log and _logger.warning accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@LLM/rule_book/graph.py`:
- Around line 18-20: OSS_MODEL is read without validation and TOP_K is parsed
directly which can raise ValueError at import time; update initialization for
OSS_MODEL and TOP_K (and the similar vars around the second occurrence) to
perform safe validation and parsing: check OSS_MODEL is non-empty and matches
expected model names (or set a sensible default or raise a clear error), parse
TOP_K using a safe int parser with a fallback/default and guard against
non-numeric values (log or raise a descriptive error), and apply the same
pattern to the other occurrence around lines 64-65; modify the module-level
assignments (OSS_MODEL, TOP_K and the duplicated vars) to use a small helper or
try/except so import-time exceptions are avoided and errors are logged with
context.
In `@LLM/rule_book/logger.py`:
- Around line 34-40: The record currently stores raw PII in fields 'query',
'chunks_sources' (via chunks), 'answer', and 'error'; replace direct storage
with a redaction step: create and call a helper like
apply_mask_and_truncate(value, max_len=200) to mask sensitive content and
truncate to a safe length, store only the masked snippet plus minimal metadata
(e.g., original_length, truncated boolean, and a redacted boolean) instead of
the full text, and for 'chunks_sources' map each chunk to its sanitized source
string (masking article text and keeping only non-sensitive identifiers) rather
than embedding raw chunk content; update usages around the record construction
to use these sanitized outputs.
- Around line 43-46: The warning message in the except block after calling
_write_log(record) currently embeds "e, exc_info=True" inside the format string,
losing the real exception and traceback; update the except clause to capture the
exception (e.g., except Exception as e:) and call _logger.warning with a proper
message and the exception/tracing parameters (pass the exception as an argument
or use exc_info=True) so the actual exception and stacktrace from _write_log are
logged; adjust the call site referencing _write_log and _logger.warning
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b14d27b1-154b-4f55-a8d0-ed2caaf98cd1
📒 Files selected for processing (5)
.github/ISSUE_TEMPLATE/버그-이슈.md.github/ISSUE_TEMPLATE/신규-기능.md.github/pull_request_template.mdLLM/rule_book/graph.pyLLM/rule_book/logger.py
✅ Files skipped from review due to trivial changes (3)
- .github/pull_request_template.md
- .github/ISSUE_TEMPLATE/버그-이슈.md
- .github/ISSUE_TEMPLATE/신규-기능.md
There was a problem hiding this comment.
♻️ Duplicate comments (2)
LLM/rule_book/graph.py (2)
19-19:⚠️ Potential issue | 🟠 Major
OSS_MODEL미검증으로 런타임 실패 가능성이 있습니다.Line 19에서 값 검증이 없어, 비어 있는 상태로 Line 67 호출까지 진행되면 요청 실패로 이어질 수 있습니다. 시작 시점에 명시적으로 검증해 주세요.
수정 예시
OSS_MODEL = os.getenv("OSS_MODEL") +if not OSS_MODEL: + raise ValueError("OSS_MODEL 환경 변수가 설정되지 않았습니다.")Also applies to: 67-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/graph.py` at line 19, The OSS_MODEL environment variable (OSS_MODEL) is read without validation and can be empty causing runtime failures when used later (e.g., at its usage site). Add a startup validation where OSS_MODEL is loaded: check it is non-empty and matches allowed values (or provide a safe default), and if invalid throw/raise a clear error (or log and exit) so the application fails fast; update the code that references OSS_MODEL to rely on the validated value. Ensure the check is colocated with the OSS_MODEL declaration and mentions OSS_MODEL by name so reviewers can find it.
21-21:⚠️ Potential issue | 🟠 Major
RULE_BOOK_TOP_K파싱은 안전하게 처리하는 편이 좋습니다.Line 21은 잘못된 환경값(예:
"abc")에서 import 단계ValueError로 서비스 기동이 실패할 수 있고, 0 이하 값도 그대로 통과됩니다. 안전 파싱 + 하한 보정을 권장합니다.수정 예시
-TOP_K = int(os.getenv("RULE_BOOK_TOP_K", "5")) +try: + TOP_K = max(1, int(os.getenv("RULE_BOOK_TOP_K", "5"))) +except (TypeError, ValueError): + TOP_K = 5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/rule_book/graph.py` at line 21, The TOP_K constant is parsed unsafely from os.getenv("RULE_BOOK_TOP_K") so invalid strings (e.g., "abc") raise ValueError at import and non-positive numbers pass through; update parsing around RULE_BOOK_TOP_K/TOP_K to validate and sanitize: read the env with os.getenv, attempt safe int conversion inside try/except (or use a helper like parse_int_with_default), fallback to a sensible default (e.g., 5) on parse failure, and enforce a minimum (e.g., max(parsed, 1)) before assigning TOP_K so import-time errors and non-positive values are prevented.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@LLM/rule_book/graph.py`:
- Line 19: The OSS_MODEL environment variable (OSS_MODEL) is read without
validation and can be empty causing runtime failures when used later (e.g., at
its usage site). Add a startup validation where OSS_MODEL is loaded: check it is
non-empty and matches allowed values (or provide a safe default), and if invalid
throw/raise a clear error (or log and exit) so the application fails fast;
update the code that references OSS_MODEL to rely on the validated value. Ensure
the check is colocated with the OSS_MODEL declaration and mentions OSS_MODEL by
name so reviewers can find it.
- Line 21: The TOP_K constant is parsed unsafely from
os.getenv("RULE_BOOK_TOP_K") so invalid strings (e.g., "abc") raise ValueError
at import and non-positive numbers pass through; update parsing around
RULE_BOOK_TOP_K/TOP_K to validate and sanitize: read the env with os.getenv,
attempt safe int conversion inside try/except (or use a helper like
parse_int_with_default), fallback to a sensible default (e.g., 5) on parse
failure, and enforce a minimum (e.g., max(parsed, 1)) before assigning TOP_K so
import-time errors and non-positive values are prevented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f9218525-98fc-4aac-aa78-7d7d8cf70a8e
📒 Files selected for processing (1)
LLM/rule_book/graph.py
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.
관련 이슈
Close #83
🎯 배경
🔍 주요 내용
반영한 규정집
동양미래대학교 윤리 강령 ver.2 (2026. 1. 1.)
졸업 유예 운영규칙 ver.0(2024. 7. 1.)
학사학위 전공심화과정 학생 선발 규칙 ver.1(2025. 11. 19.)
학생활동준칙_ver.7_20240301
학생회 선거 규약 ver.7
학생회 회칙_ver.12_20240301
학칙 시행세칙 ver.38(2025. 5. 1.)
학칙 ver.42(2025. 9. 1.)
변경 요약(1~3줄)
학칙·규정집 기반 Q&A 챗봇을 LangGraph 비동기 파이프라인으로 추가했습니다. PDF→BM25 인덱싱, 앱 시작 시 인덱스 빌드, 비동기 엔드포인트·로깅, Docker에 규정 데이터 포함 등을 도입했습니다.
주요 변경점
주의/리스크
다음 액션