feat: add TTL-based response cache for chatbot - #88
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughchat 함수에 입력 정규화(공백 정리)를 기반으로 키를 만들어 인메모리 TTL 캐싱을 도입했습니다. 캐시는 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Chat as ChatFunction
participant Cache
participant Mode as ModeHandler/LLM
User->>Chat: 요청(모드, 텍스트)
Chat->>Cache: 키 생성 및 조회 (with Lock)
Cache-->>Chat: 캐시 히트? (있음/없음)
alt 캐시 히트
Chat-->>User: 캐시된 응답 복사 반환
else 캐시 미스
Chat->>Mode: 모드별 처리 / LLM 호출
Mode-->>Chat: 응답
Chat->>Cache: 응답 저장 (with Lock) <!-- 저장 시 TTL 적용 -->
Chat-->>User: 응답 반환
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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: 1
🧹 Nitpick comments (1)
LLM/OSS/Open_AI_OSS.py (1)
22-25: 캐시 TTL/maxsize를 환경변수로 외부화하는 것을 권장합니다.현재
maxsize와ttl이 하드코딩되어 있어 운영 중 트래픽/메모리 특성에 맞춰 튜닝하려면 재배포가 필요합니다. 규정집 갱신 이벤트가 있을 때 TTL을 짧게 바꾸거나, 메모리 압박 시maxsize를 줄이는 시나리오에서 환경변수 기반이면 운영 편의성이 크게 좋아집니다.♻️ 제안 예시
-# rule_book: 24시간 / 그 외 모드: 1시간 -_CACHE_RULE_BOOK: TTLCache = TTLCache(maxsize=200, ttl=86400) -_CACHE_GENERAL: TTLCache = TTLCache(maxsize=500, ttl=3600) -_cache_lock = threading.Lock() +# rule_book: 24시간 / 그 외 모드: 1시간 (환경변수로 오버라이드 가능) +_RB_MAXSIZE = int(os.getenv("CHAT_CACHE_RULE_BOOK_MAXSIZE", "200")) +_RB_TTL = int(os.getenv("CHAT_CACHE_RULE_BOOK_TTL", "86400")) +_GEN_MAXSIZE = int(os.getenv("CHAT_CACHE_GENERAL_MAXSIZE", "500")) +_GEN_TTL = int(os.getenv("CHAT_CACHE_GENERAL_TTL", "3600")) +_CACHE_RULE_BOOK: TTLCache = TTLCache(maxsize=_RB_MAXSIZE, ttl=_RB_TTL) +_CACHE_GENERAL: TTLCache = TTLCache(maxsize=_GEN_MAXSIZE, ttl=_GEN_TTL) +_cache_lock = threading.Lock()🤖 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 22 - 25, The hardcoded TTLCache parameters (_CACHE_RULE_BOOK and _CACHE_GENERAL) should be made configurable via environment variables: read and parse integer env vars (e.g., RULE_BOOK_CACHE_TTL, RULE_BOOK_CACHE_MAXSIZE, GENERAL_CACHE_TTL, GENERAL_CACHE_MAXSIZE) with sensible defaults (86400/200 and 3600/500) before creating TTLCache instances; validate/fallback on parse errors and then instantiate TTLCache(maxsize=..., ttl=...) for _CACHE_RULE_BOOK and _CACHE_GENERAL respectively (leave _cache_lock as-is), and update any inline comment to note the env var names.
🤖 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 833-849: The cached dict is returned by reference which risks
cache pollution; change the logic around
_CACHE_SKIP/_cache_key/_cached/_cache_and_return so you (1) promote _CACHE_SKIP
to a module-level constant, (2) normalize user_text before building _cache_key
(e.g., strip and collapse whitespace) to improve hit rate, and (3) return a
shallow copy of cached dicts instead of the stored object and also store a
shallow copy when setting the cache (use the same copy semantics for both
_CACHE_RULE_BOOK and _CACHE_GENERAL under _cache_lock) so callers cannot mutate
the cached entry.
---
Nitpick comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 22-25: The hardcoded TTLCache parameters (_CACHE_RULE_BOOK and
_CACHE_GENERAL) should be made configurable via environment variables: read and
parse integer env vars (e.g., RULE_BOOK_CACHE_TTL, RULE_BOOK_CACHE_MAXSIZE,
GENERAL_CACHE_TTL, GENERAL_CACHE_MAXSIZE) with sensible defaults (86400/200 and
3600/500) before creating TTLCache instances; validate/fallback on parse errors
and then instantiate TTLCache(maxsize=..., ttl=...) for _CACHE_RULE_BOOK and
_CACHE_GENERAL respectively (leave _cache_lock as-is), and update any inline
comment to note the env var names.
🪄 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: d9b5331f-f885-4f82-9136-f675435d64d6
📒 Files selected for processing (2)
LLM/OSS/Open_AI_OSS.pyTHIRD_PARTY_LICENSES.md
Comment on lines
+833
to
+849
| # oss 모드는 대화 히스토리 의존 → 캐시 제외 | ||
| # greet/whoami/relation 은 고정 문자열 → 캐시 불필요 | ||
| _CACHE_SKIP = {"oss", "greet", "whoami", "relation", "guard"} | ||
| if mode not in _CACHE_SKIP: | ||
| _cache = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL | ||
| _cache_key = f"{mode}:{user_text}" | ||
| with _cache_lock: | ||
| _cached = _cache.get(_cache_key) | ||
| if _cached is not None: | ||
| return _cached | ||
|
|
||
| def _cache_and_return(resp: dict) -> dict: | ||
| if mode not in _CACHE_SKIP: | ||
| _c = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL | ||
| with _cache_lock: | ||
| _c[_cache_key] = resp | ||
| return resp |
There was a problem hiding this comment.
캐시 히트 시 dict 참조를 그대로 반환하면 캐시 오염 위험이 있습니다.
Line 842 의 return _cached 는 캐시에 저장된 동일 객체를 그대로 돌려줍니다. 호출 체인 어딘가(미들웨어, 추후 리팩터에서 resp["url"] = ... 같은 후처리가 추가되는 경우 등)에서 응답 dict 를 변형하면, 이후 같은 키로 히트한 모든 요청이 변형된 값을 받게 되어 재현하기 어려운 버그의 원인이 됩니다. 방어적으로 얕은 복사본을 반환하는 편이 안전합니다.
부가로, _CACHE_SKIP 은 요청마다 재생성할 필요가 없으므로 모듈 상수로 올리는 것을 권장합니다. 또한 _cache_key = f"{mode}:{user_text}" 는 공백·트레일링 기호 차이로 키가 달라져 히트율이 떨어지므로, 간단한 정규화(예: user_text.strip() + 다중 공백 축약)를 추가하면 효과가 큽니다.
🛡️ 제안 diff
-# rule_book: 24시간 / 그 외 모드: 1시간
-_CACHE_RULE_BOOK: TTLCache = TTLCache(maxsize=200, ttl=86400)
-_CACHE_GENERAL: TTLCache = TTLCache(maxsize=500, ttl=3600)
-_cache_lock = threading.Lock()
+# rule_book: 24시간 / 그 외 모드: 1시간
+_CACHE_RULE_BOOK: TTLCache = TTLCache(maxsize=200, ttl=86400)
+_CACHE_GENERAL: TTLCache = TTLCache(maxsize=500, ttl=3600)
+_cache_lock = threading.Lock()
+_CACHE_SKIP = frozenset({"oss", "greet", "whoami", "relation", "guard"})- # oss 모드는 대화 히스토리 의존 → 캐시 제외
- # greet/whoami/relation 은 고정 문자열 → 캐시 불필요
- _CACHE_SKIP = {"oss", "greet", "whoami", "relation", "guard"}
- if mode not in _CACHE_SKIP:
- _cache = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL
- _cache_key = f"{mode}:{user_text}"
- with _cache_lock:
- _cached = _cache.get(_cache_key)
- if _cached is not None:
- return _cached
-
- def _cache_and_return(resp: dict) -> dict:
- if mode not in _CACHE_SKIP:
- _c = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL
- with _cache_lock:
- _c[_cache_key] = resp
- return resp
+ # oss 모드는 대화 히스토리 의존 → 캐시 제외
+ # greet/whoami/relation 은 고정 문자열 → 캐시 불필요
+ _cache_key = f"{mode}:{re.sub(r'\s+', ' ', (user_text or '').strip())}"
+ if mode not in _CACHE_SKIP:
+ _cache = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL
+ with _cache_lock:
+ _cached = _cache.get(_cache_key)
+ if _cached is not None:
+ return dict(_cached) # 참조 공유로 인한 캐시 오염 방지
+
+ def _cache_and_return(resp: dict) -> dict:
+ if mode not in _CACHE_SKIP:
+ _c = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL
+ with _cache_lock:
+ _c[_cache_key] = dict(resp)
+ 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 833 - 849, The cached dict is returned
by reference which risks cache pollution; change the logic around
_CACHE_SKIP/_cache_key/_cached/_cache_and_return so you (1) promote _CACHE_SKIP
to a module-level constant, (2) normalize user_text before building _cache_key
(e.g., strip and collapse whitespace) to improve hit rate, and (3) return a
shallow copy of cached dicts instead of the stored object and also store a
shallow copy when setting the cache (use the same copy semantics for both
_CACHE_RULE_BOOK and _CACHE_GENERAL under _cache_lock) so callers cannot mutate
the cached entry.
…code append, cache list shallow copy
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
LLM/OSS/Open_AI_OSS.py (1)
845-850:⚠️ Potential issue | 🟡 Minor캐시에 저장할 때도 응답 dict를 복사해 주세요.
Line 849에서 저장한
resp와 반환한resp가 같은 객체라서, 반환 후 객체가 변형되면 캐시 엔트리도 오염될 수 있습니다.🛡️ 제안 diff
_c = _CACHE_RULE_BOOK if mode == "rule_book" else _CACHE_GENERAL with _cache_lock: - _c[_cache_key] = resp + _c[_cache_key] = dict(resp) 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 845 - 850, The function _cache_and_return currently stores the same resp object into the cache so later mutations to the returned dict will corrupt the cached entry; fix it by storing a deep copy of resp under _c[_cache_key] (e.g., use copy.deepcopy(resp)) while still returning the original resp, and ensure copy is imported (add import copy at top) and the assignment happens inside the existing with _cache_lock block that uses _CACHE_RULE_BOOK/_CACHE_GENERAL and _cache_key (skip caching when mode in _CACHE_SKIP).
🤖 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 836-837: The cache key currently stores the entire normalized
user_text in _cache_key which can leak large/sensitive inputs and cause stale
results for relative-date queries; change this to compute a fixed-size hash
(e.g., SHA256) of _normalized and combine it with a date-scope token (e.g.,
current ISO date, ISO week, or month depending on required granularity) and the
mode so the key uses symbols _normalized, _cache_key, user_text, and mode;
ensure the hash is used instead of raw text and the date-scope is generated at
runtime so relative-time queries expire at the desired boundary.
- Line 875: Replace the single-line if statements that assign url into the resp
dict with a proper block form; specifically find occurrences of the pattern if
url: resp["url"] = url (variables resp and url) and change them to an indented
block form (if url: newline with an indented resp["url"] = url) at each
occurrence (the occurrences reported around the if url usage in this file).
---
Duplicate comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 845-850: The function _cache_and_return currently stores the same
resp object into the cache so later mutations to the returned dict will corrupt
the cached entry; fix it by storing a deep copy of resp under _c[_cache_key]
(e.g., use copy.deepcopy(resp)) while still returning the original resp, and
ensure copy is imported (add import copy at top) and the assignment happens
inside the existing with _cache_lock block that uses
_CACHE_RULE_BOOK/_CACHE_GENERAL and _cache_key (skip caching when mode in
_CACHE_SKIP).
🪄 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: 5f7a0737-a8d1-4862-8d09-64b2f6384310
📒 Files selected for processing (1)
LLM/OSS/Open_AI_OSS.py
3 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 #87
🎯 배경
🔍 주요 내용
_cache_and_return()변경 요약(1~3줄)
응답에 TTL 기반 인메모리 캐시를 추가해 LLM 호출을 줄였습니다. 규정집(rule_book) 응답은 24시간, 그 외 응답은 1시간 캐시하며 OSS 호출 등 특정 모드는 캐시에서 제외됩니다.
주요 변경점
주의/리스크
다음 액션