Merged
Conversation
…and observer cleanup
…-deps, and update docs
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthrough이 PR은 README의 한·영 사용 예시 추가 및 한국어 문구 수정, 환경에서 GEMINI_API_KEYS/GEMINI_API_KEY 병합·정규화 로직 추가, 파일명 폴백에서 ".txt" 제거, MessageList의 per-id ref 정리 로직 이동(언마운트 지연 정리), 사이드바 호버 노출 제거를 포함합니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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)
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 (3)
README.md (1)
60-65: 한/영 문서의 사용 예시 섹션을 동기화해 주세요.현재 사용 예시가 한국어 섹션에만 추가되어 있어 영어 사용자 기준으로 기능 이해가 불균형합니다. 동일한 3개 예시를 영어 섹션에도 맞춰 추가하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 60 - 65, The English usage examples are missing the three Korean example entries ("행복에 대한 질문", "윤리적 딜레마 질문", "사회적 질문"); add equivalent English bullets to the README's English "Usage examples" section by translating each example title and its result description (searching philosophy books for happiness; showing source cards and streaming mixed-perspective answer for ethical dilemmas; using RAG to fetch political/social philosophy metadata and provide direct multifaceted answers) so both language sections contain the same three examples and mirror content/formatting.frontend/components/sidebar/ActivePhilosophers.tsx (1)
40-46: 비상호작용 카드의 hover 강조는 줄이는 편이 좋습니다.이제 클릭이 불가능한 요소인데 hover 강조가 남아 있어 상호작용 가능하다는 인상을 줄 수 있습니다. 시각 효과를 더 중립적으로 조정하면 UX 혼선을 줄일 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/sidebar/ActivePhilosophers.tsx` around lines 40 - 46, The hover gradient on the card gives a false affordance for non-interactive items; update the classes in ActivePhilosophers.tsx so the overlay div (the element with bg-gradient-to-r) only gains hover opacity for interactive cards or uses a much subtler effect for non-interactive ones. Concretely, modify the container className/template and the gradient div to conditionally apply group-hover:opacity-100 (or use group-hover:opacity-20) based on the card's interactivity (e.g., using an isInteractive or existing isActive flag) so non-clickable cards do not show the strong hover highlight.backend/app/core/env_utils.py (1)
38-39: 주석 설명이 실제 동작과 약간 다릅니다.현재 코드는 “fallback” 조건 없이 단일
GEMINI_API_KEY를 항상 병합합니다. 주석을 실제 동작(추가 병합) 기준으로 바꾸면 유지보수 시 혼동을 줄일 수 있습니다.✏️ 제안 수정
- # Fallback to single GEMINI_API_KEY when parsing produced no key or file doesn't exist + # Also merge single GEMINI_API_KEY from environment (if present)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/env_utils.py` around lines 38 - 39, Update the misleading comment above the line that assigns k = os.getenv("GEMINI_API_KEY") in env_utils.py to accurately state that the single GEMINI_API_KEY is always merged (added) into the key list, not used only as a fallback; reference the variable k and the os.getenv("GEMINI_API_KEY") call so maintainers know the code unconditionally includes that environment value during the merge operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/scripts/generate_book_mapping.py`:
- Line 118: The code is stripping at the first dot again even though file_name
is already a stem, causing titles like "Vol. II by ..." to be truncated; stop
re-splitting and preserve the passed-in stem by assigning name_without_ext
directly from file_name (remove the os.path.splitext call) so
generate_book_mapping.py uses the original file_name stem as-is; update any
downstream code that expects name_without_ext to continue using that variable
unchanged.
In `@frontend/components/chat/MessageList.tsx`:
- Around line 120-123: The problem is deleting the ref callback entry in
observeElement when the incoming ref is null (observeElement,
refCallbackById.current.delete(id)), which breaks ref stability and causes
repeated attach/detach; fix it by removing the deletion from the null branch so
refCallbackById keeps the stable callback object, and instead only create/update
the entry when getMessageRef produces a new callback (ensure getMessageRef
initializes refCallbackById.current.set(id, callback) when first creating the
ref) and only delete the map entry on an explicit unobserve/cleanup path (not on
the null ref call).
---
Nitpick comments:
In `@backend/app/core/env_utils.py`:
- Around line 38-39: Update the misleading comment above the line that assigns k
= os.getenv("GEMINI_API_KEY") in env_utils.py to accurately state that the
single GEMINI_API_KEY is always merged (added) into the key list, not used only
as a fallback; reference the variable k and the os.getenv("GEMINI_API_KEY") call
so maintainers know the code unconditionally includes that environment value
during the merge operation.
In `@frontend/components/sidebar/ActivePhilosophers.tsx`:
- Around line 40-46: The hover gradient on the card gives a false affordance for
non-interactive items; update the classes in ActivePhilosophers.tsx so the
overlay div (the element with bg-gradient-to-r) only gains hover opacity for
interactive cards or uses a much subtler effect for non-interactive ones.
Concretely, modify the container className/template and the gradient div to
conditionally apply group-hover:opacity-100 (or use group-hover:opacity-20)
based on the card's interactivity (e.g., using an isInteractive or existing
isActive flag) so non-clickable cards do not show the strong hover highlight.
In `@README.md`:
- Around line 60-65: The English usage examples are missing the three Korean
example entries ("행복에 대한 질문", "윤리적 딜레마 질문", "사회적 질문"); add equivalent English
bullets to the README's English "Usage examples" section by translating each
example title and its result description (searching philosophy books for
happiness; showing source cards and streaming mixed-perspective answer for
ethical dilemmas; using RAG to fetch political/social philosophy metadata and
provide direct multifaceted answers) so both language sections contain the same
three examples and mirror content/formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7c5f435-a2d3-4fed-88b4-8c2fd68943fc
📒 Files selected for processing (6)
README.mdbackend/app/core/env_utils.pybackend/scripts/generate_book_mapping.pyfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsxfrontend/components/sidebar/Sidebar.tsx
| # If all keys exhausted or other error, fallback | ||
| print(f"LLM Failed for {file_name}, falling back to Kyobo Search...") | ||
| name_without_ext = file_name | ||
| name_without_ext = os.path.splitext(file_name)[0] |
There was a problem hiding this comment.
확장자 제거 방식이 점(.) 포함 제목을 잘못 잘라낼 수 있습니다.
file_name은 이미 Line [157]에서 file_path.stem으로 전달되어 확장자가 없는 값입니다. 이 상태에서 os.path.splitext(file_name)[0]를 적용하면 Vol. II by ... 같은 이름이 의도치 않게 절단되어 fallback 검색 품질이 크게 떨어질 수 있습니다.
🔧 제안 수정
- name_without_ext = os.path.splitext(file_name)[0]
+ # file_name is already stem in current call path; only strip explicit .txt when present
+ name_without_ext = file_name[:-4] if file_name.lower().endswith(".txt") else file_name🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/scripts/generate_book_mapping.py` at line 118, The code is stripping
at the first dot again even though file_name is already a stem, causing titles
like "Vol. II by ..." to be truncated; stop re-splitting and preserve the
passed-in stem by assigning name_without_ext directly from file_name (remove the
os.path.splitext call) so generate_book_mapping.py uses the original file_name
stem as-is; update any downstream code that expects name_without_ext to continue
using that variable unchanged.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
backend/app/core/env_utils.py (1)
29-41: CSV 키 파싱 시 토큰 따옴표도 제거해 두는 편이 안전합니다.일부 배포 설정에서
GEMINI_API_KEYS값이 따옴표를 포함해 들어오면(예:'k1','k2') 현재 로직은 따옴표가 포함된 키를 저장할 수 있습니다. 토큰 정규화를 한 번 더 해두면 인증 실패 가능성을 줄일 수 있습니다.🔧 제안 수정
env_keys_str = os.getenv("GEMINI_API_KEYS") if env_keys_str: for k in env_keys_str.split(','): - k = k.strip() + k = k.strip().strip('"').strip("'") if k and k not in api_keys: api_keys.append(k)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/env_utils.py` around lines 29 - 41, The CSV parsing for GEMINI_API_KEYS may retain surrounding quotes (e.g., "'k1'") leading to invalid tokens; in the loop that processes env_keys_str and in the single-key branch that reads GEMINI_API_KEY, after trimming whitespace (env_keys_str.split -> k = k.strip() and k = os.getenv("GEMINI_API_KEY")), normalize tokens by also stripping surrounding single and double quotes (e.g., k = k.strip().strip('"\'') or equivalent) before checking and appending to api_keys so stored keys never include stray quotes; apply this normalization to both the per-item loop (env_keys_str parsing) and the single-key handling.README.md (1)
63-63: 예시 문구를 조금만 다듬으면 가독성이 더 좋아집니다.현재 표현(예:
선탑재,ideal and equal state)이 약간 어색해서, 자연스러운 용어로 통일하면 문서 품질이 더 올라갑니다.✍️ 문구 다듬기 예시
- - 결과: 우측 화면에 도덕이나 윤리와 관련된 출처 카드(알라딘 도서 표지 및 메타데이터 선탑재)가 표기되며, 여러 관점을 혼합한 구조적인 답변이 스트리밍 됩니다. + - 결과: 우측 화면에 도덕·윤리 관련 출처 카드(알라딘 도서 표지 및 메타데이터 사전 로드)가 표시되며, 여러 관점을 혼합한 구조적 답변이 스트리밍됩니다. -3. **Social Question**: "What should an ideal and equal state look like?" +3. **Social Question**: "What should an ideal and egalitarian state look like?"Also applies to: 171-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 63, The sentence at line containing "결과: 우측 화면에 도덕이나 윤리와 관련된 출처 카드(알라딘 도서 표지 및 메타데이터 선탑재)가 표기되며, 여러 관점을 혼합한 구조적인 답변이 스트리밍 됩니다." should be reworded for clarity and consistent terminology: replace the awkward term "선탑재" with a natural Korean phrase like "사전 로드" or "미리 포함" and ensure any English phrase such as "ideal and equal state" elsewhere (noted at lines ~171-173) is either translated or replaced with a clearer, consistent term; update both occurrences to use the same polished wording and adjust surrounding grammar so the sentence reads smoothly and idiomatically.frontend/components/sidebar/ActivePhilosophers.tsx (1)
46-46: 비클릭 UI라면 hover 강조를 줄여도 좋겠습니다.현재
group-hover시각효과가 상호작용 가능한 요소처럼 보이게 할 수 있어, 정적 카드 성격에 맞게 완화하면 UX가 더 명확해집니다.🎨 선택적 스타일 정리 예시
-<div className={`absolute inset-0 bg-gradient-to-r ${isActive ? "from-primary/10" : "from-primary/5"} to-transparent opacity-0 group-hover:opacity-20 transition-opacity`}></div> +<div className={`absolute inset-0 bg-gradient-to-r ${isActive ? "from-primary/10" : "from-primary/5"} to-transparent opacity-0 transition-opacity`}></div> -<CheckCircle className={`ml-auto w-5 h-5 shrink-0 transition-colors ${isActive ? "text-primary" : "text-primary/20 group-hover:text-primary/50"}`} /> +<CheckCircle className={`ml-auto w-5 h-5 shrink-0 transition-colors ${isActive ? "text-primary" : "text-primary/20"}`} />Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/sidebar/ActivePhilosophers.tsx` at line 46, This non-interactive card currently uses a strong hover highlight via the Tailwind class "group-hover:opacity-20"; dial it back to convey a static, non-clickable card by reducing or removing the hover opacity (e.g., replace "group-hover:opacity-20" with a subtler value like "group-hover:opacity-10" or remove the group-hover modifier entirely) and consider lowering the default hover/base opacity (from "opacity-0" to something like "opacity-5" or keep at 0) so the visual emphasis is softened; apply the same change to the other identical occurrence that uses "group-hover:opacity-20" (the second div at the referenced location).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/core/env_utils.py`:
- Around line 29-41: The CSV parsing for GEMINI_API_KEYS may retain surrounding
quotes (e.g., "'k1'") leading to invalid tokens; in the loop that processes
env_keys_str and in the single-key branch that reads GEMINI_API_KEY, after
trimming whitespace (env_keys_str.split -> k = k.strip() and k =
os.getenv("GEMINI_API_KEY")), normalize tokens by also stripping surrounding
single and double quotes (e.g., k = k.strip().strip('"\'') or equivalent) before
checking and appending to api_keys so stored keys never include stray quotes;
apply this normalization to both the per-item loop (env_keys_str parsing) and
the single-key handling.
In `@frontend/components/sidebar/ActivePhilosophers.tsx`:
- Line 46: This non-interactive card currently uses a strong hover highlight via
the Tailwind class "group-hover:opacity-20"; dial it back to convey a static,
non-clickable card by reducing or removing the hover opacity (e.g., replace
"group-hover:opacity-20" with a subtler value like "group-hover:opacity-10" or
remove the group-hover modifier entirely) and consider lowering the default
hover/base opacity (from "opacity-0" to something like "opacity-5" or keep at 0)
so the visual emphasis is softened; apply the same change to the other identical
occurrence that uses "group-hover:opacity-20" (the second div at the referenced
location).
In `@README.md`:
- Line 63: The sentence at line containing "결과: 우측 화면에 도덕이나 윤리와 관련된 출처 카드(알라딘 도서
표지 및 메타데이터 선탑재)가 표기되며, 여러 관점을 혼합한 구조적인 답변이 스트리밍 됩니다." should be reworded for
clarity and consistent terminology: replace the awkward term "선탑재" with a
natural Korean phrase like "사전 로드" or "미리 포함" and ensure any English phrase such
as "ideal and equal state" elsewhere (noted at lines ~171-173) is either
translated or replaced with a clearer, consistent term; update both occurrences
to use the same polished wording and adjust surrounding grammar so the sentence
reads smoothly and idiomatically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ff99c50-a4a7-4164-bb81-5c2a560cfc4c
⛔ Files ignored due to path filters (1)
frontend/app/icon.pngis excluded by!**/*.png
📒 Files selected for processing (5)
README.mdbackend/app/core/env_utils.pybackend/scripts/generate_book_mapping.pyfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/components/chat/MessageList.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/components/chat/MessageList.tsx (1)
129-133:⚠️ Potential issue | 🟠 Major
nextCb의 null-cleanup 가드가 현재 경로에서는 실효성이 낮습니다.Line 122에서 이미
refCallbackById.current.delete(id)가 실행되어, Line 131 조건(refCallbackById.current.get(id) === nextCb)이 성립하기 어렵습니다. 결과적으로 이번 변경만으로는 ref 안정성 이슈가 해소되지 않습니다.🔧 제안 수정안
const observeElement = useCallback((id: string, el: HTMLDivElement | null) => { @@ } else { elementById.current.delete(id); visibleMessages.current.delete(id); - refCallbackById.current.delete(id); } }, []);다음 스크립트로 삭제 지점이 1곳(guarded cleanup)만 남는지 확인해 주세요.
#!/bin/bash set -euo pipefail # refCallbackById 삭제 호출 위치 확인 rg -n 'refCallbackById\.current\.delete\(id\)' frontend/components/chat/MessageList.tsx # 관련 컨텍스트 확인 sed -n '107,140p' frontend/components/chat/MessageList.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chat/MessageList.tsx` around lines 129 - 133, nextCb's null-cleanup guard is ineffective because refCallbackById.current.delete(id) is already called earlier, so ensure there's only one deletion site: find both uses of refCallbackById.current.delete(id) around the observeElement/nextCb flow (references: nextCb, observeElement, refCallbackById, id) and remove the premature delete outside nextCb so the guarded cleanup inside nextCb is the single, authoritative deletion point; alternatively, if you prefer the early-delete approach, remove the conditional inside nextCb and keep the earlier delete—choose one consistent deletion location and update surrounding comments to reflect the chosen strategy.
🧹 Nitpick comments (1)
backend/app/core/env_utils.py (1)
29-41: 현재 구현과 함수 설명(docstring)이 어긋납니다.Line 29-43은 파일 키 존재 여부와 무관하게 환경변수 키를 병합하는데, 설명은 “파일에 없을 때 fallback”으로 읽힙니다. 동작에 맞게 docstring을 갱신해 주세요.
✏️ 제안 수정안
def parse_gemini_api_keys(env_path: Path) -> list[str]: """ Reads active GEMINI_API_KEY assignments from the given .env file. - Only extracts active assignments and strips inline comments and quotes. - Also falls back to os.environ if no keys are found in the file. + Extracts active assignments and strips inline comments and quotes. + Also merges GEMINI_API_KEYS (comma-separated) and GEMINI_API_KEY + from os.environ with de-duplication, preserving first-seen order. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/env_utils.py` around lines 29 - 41, The docstring for the API key loading logic is inaccurate: the code (using env_keys_str, GEMINI_API_KEYS, GEMINI_API_KEY and appending into api_keys) always merges environment variables into api_keys regardless of whether keys came from a file; update the function/module docstring to state that environment variables are merged additively into api_keys (not used only as a fallback) and describe handling of comma-separated GEMINI_API_KEYS and single GEMINI_API_KEY, including trimming of quotes/whitespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/components/chat/MessageList.tsx`:
- Around line 129-133: nextCb's null-cleanup guard is ineffective because
refCallbackById.current.delete(id) is already called earlier, so ensure there's
only one deletion site: find both uses of refCallbackById.current.delete(id)
around the observeElement/nextCb flow (references: nextCb, observeElement,
refCallbackById, id) and remove the premature delete outside nextCb so the
guarded cleanup inside nextCb is the single, authoritative deletion point;
alternatively, if you prefer the early-delete approach, remove the conditional
inside nextCb and keep the earlier delete—choose one consistent deletion
location and update surrounding comments to reflect the chosen strategy.
---
Nitpick comments:
In `@backend/app/core/env_utils.py`:
- Around line 29-41: The docstring for the API key loading logic is inaccurate:
the code (using env_keys_str, GEMINI_API_KEYS, GEMINI_API_KEY and appending into
api_keys) always merges environment variables into api_keys regardless of
whether keys came from a file; update the function/module docstring to state
that environment variables are merged additively into api_keys (not used only as
a fallback) and describe handling of comma-separated GEMINI_API_KEYS and single
GEMINI_API_KEY, including trimming of quotes/whitespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7f722b0-b4eb-44c7-962a-3697ac02a73a
📒 Files selected for processing (4)
README.mdbackend/app/core/env_utils.pyfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/components/sidebar/ActivePhilosophers.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/components/chat/MessageList.tsx (1)
128-133:⚠️ Potential issue | 🟡 MinorStrictMode 사이클에서 ref 콜백 안정성이 다시 흔들릴 수 있습니다.
Line 130-132에서
el === null시 엔트리를 즉시 삭제하면, React StrictMode(dev)의setup → cleanup(null) → setup패턴에서 맵이 비어진 상태가 남아 다음 렌더에서 새 ref 콜백이 생성될 수 있습니다. 결과적으로 불필요한 detach/attach 및 observe/unobserve churn이 다시 생깁니다.제안 수정안
const nextCb = (el: HTMLDivElement | null) => { + // StrictMode의 cleanup(null) 이후 setup(el)에서 매핑 복구 + if (el !== null && refCallbackById.current.get(id) !== nextCb) { + refCallbackById.current.set(id, nextCb); + } observeElement(id, el); if (el === null && refCallbackById.current.get(id) === nextCb) { refCallbackById.current.delete(id); } };React 19.2에서 StrictMode 사용 시 callback ref 호출 순서가 setup → cleanup(null) → setup인지, 그리고 이 패턴에서 ref callback identity 안정성을 유지하기 위한 권장 방식(예: reattach 시 매핑 복구)이 무엇인지 확인해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chat/MessageList.tsx` around lines 128 - 133, The current ref callback nextCb removes the map entry immediately when el === null, which breaks callback identity across React StrictMode setup→cleanup(null)→setup cycles; change the behavior in nextCb (used with observeElement and refCallbackById) to delay the deletion so a re-setup can restore the same mapping: instead of calling refCallbackById.current.delete(id) synchronously, schedule a microtask (e.g., Promise.resolve().then(...)) to check again that refCallbackById.current.get(id) === nextCb before deleting, ensuring observeElement/refCallbackById mapping survives the transient null cleanup and prevents churn.
🧹 Nitpick comments (1)
backend/app/core/env_utils.py (1)
30-43: 키 정규화 로직은 헬퍼로 추출하는 편이 안전합니다.Line 35, Line 42(기존 Line 26 포함)에서 동일 정규화 체인이 반복됩니다. 한 곳으로 모으면 규칙 변경 시 누락을 줄일 수 있습니다.
리팩터링 예시 diff
def parse_gemini_api_keys(env_path: Path) -> list[str]: + def _normalize_key(value: str) -> str: + return value.strip().strip('"').strip("'") + """ @@ for m in matches: # Remove inline comments and strip quotes m = re.split(r'\s+#', m, 1)[0] - key = m.strip().strip('"').strip("'") + key = _normalize_key(m) if key and key not in api_keys: api_keys.append(key) @@ env_keys_str = os.getenv("GEMINI_API_KEYS") if env_keys_str: for k in env_keys_str.split(','): - k = k.strip().strip('"').strip("'") + k = _normalize_key(k) if k and k not in api_keys: api_keys.append(k) @@ k = os.getenv("GEMINI_API_KEY") if k: - k = k.strip().strip('"').strip("'") + k = _normalize_key(k) if k and k not in api_keys: api_keys.append(k)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/env_utils.py` around lines 30 - 43, Extract the repeated key-normalization chain into a single helper (e.g., normalize_key or _normalize_api_key) and replace the inline normalization in the GEMINI_API_KEYS parsing loop (env_keys_str and its inner variable k) and the single GEMINI_API_KEY handling (variable k from os.getenv("GEMINI_API_KEY")) to call that helper; ensure the helper trims whitespace and surrounding single/double quotes and returns None or empty string for invalid results so you can simply check and append to api_keys only when the normalized value is truthy and not already present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/components/chat/MessageList.tsx`:
- Around line 128-133: The current ref callback nextCb removes the map entry
immediately when el === null, which breaks callback identity across React
StrictMode setup→cleanup(null)→setup cycles; change the behavior in nextCb (used
with observeElement and refCallbackById) to delay the deletion so a re-setup can
restore the same mapping: instead of calling refCallbackById.current.delete(id)
synchronously, schedule a microtask (e.g., Promise.resolve().then(...)) to check
again that refCallbackById.current.get(id) === nextCb before deleting, ensuring
observeElement/refCallbackById mapping survives the transient null cleanup and
prevents churn.
---
Nitpick comments:
In `@backend/app/core/env_utils.py`:
- Around line 30-43: Extract the repeated key-normalization chain into a single
helper (e.g., normalize_key or _normalize_api_key) and replace the inline
normalization in the GEMINI_API_KEYS parsing loop (env_keys_str and its inner
variable k) and the single GEMINI_API_KEY handling (variable k from
os.getenv("GEMINI_API_KEY")) to call that helper; ensure the helper trims
whitespace and surrounding single/double quotes and returns None or empty string
for invalid results so you can simply check and append to api_keys only when the
normalized value is truthy and not already present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e279e616-6b5b-4ce7-8d2b-22847d53e6ea
📒 Files selected for processing (2)
backend/app/core/env_utils.pyfrontend/components/chat/MessageList.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/core/env_utils.py (1)
17-18:⚠️ Potential issue | 🟡 Minor디렉터리 경로에서
open()예외가 날 수 있습니다.Line 17에서
exists()만 검사하면 디렉터리도 통과합니다. Line 18에서IsADirectoryError를 피하려면is_file()로 제한하는 편이 안전합니다.🔧 Proposed fix
- if env_path.exists(): + if env_path.is_file(): with open(env_path, 'r', encoding='utf-8') as f:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/env_utils.py` around lines 17 - 18, The code checks env_path.exists() before opening, which allows directories and can raise IsADirectoryError; change the guard to ensure it's a regular file by using env_path.is_file() (i.e., replace the exists() check with env_path.is_file()) or alternatively add an explicit IsADirectoryError-safe try/except around the open call for the env_path variable so the open(...) on env_path is only attempted for real files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/core/env_utils.py`:
- Around line 6-14: The module-level docstring string currently sits after the
inner helper `_normalize_key` which prevents it from being recognized as the
`parse_gemini_api_keys` function docstring; move that triple-quoted string so it
immediately follows the `def parse_gemini_api_keys(...):` declaration (i.e.,
place the string as the first statement inside `parse_gemini_api_keys`) and
leave `_normalize_key` defined where it is, ensuring
`ast.get_docstring(parse_gemini_api_keys)` returns the expected value.
---
Outside diff comments:
In `@backend/app/core/env_utils.py`:
- Around line 17-18: The code checks env_path.exists() before opening, which
allows directories and can raise IsADirectoryError; change the guard to ensure
it's a regular file by using env_path.is_file() (i.e., replace the exists()
check with env_path.is_file()) or alternatively add an explicit
IsADirectoryError-safe try/except around the open call for the env_path variable
so the open(...) on env_path is only attempted for real files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc411ef0-14c7-4bc1-a4f4-d981546baf2c
📒 Files selected for processing (2)
backend/app/core/env_utils.pyfrontend/components/chat/MessageList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/components/chat/MessageList.tsx
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.
Summary by CodeRabbit
문서
개선사항
버그 수정