Merged
Conversation
… retry/error display fix
…or-handling # Conflicts: # frontend/app/globals.css # frontend/app/layout.tsx # frontend/app/page.tsx # frontend/components/chat/ChatMain.tsx # frontend/components/chat/FloatingInput.tsx # frontend/components/chat/MessageList.tsx # frontend/components/sidebar/ActivePhilosophers.tsx # frontend/components/sidebar/ContextSources.tsx # frontend/components/sidebar/Sidebar.tsx # frontend/package.json # frontend/tsconfig.json
…, keys, subset and mock structure
|
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백엔드: HuggingFace 임베딩으로 전환, GEMINI_API_KEY 필수화, Supabase 클라이언트 lazy singleton으로 변경, chat 라우트에서 번역/임베딩/DB 호출을 asyncio.to_thread로 비동기화하고 SSE 스트리밍을 get_response_stream_async로 처리. 프론트엔드: UUID 도입, IME 조합 처리, 사이드바·참고자료·철학자 컴포넌트에 props 기반 동적 렌더링 추가. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as Chat API
participant Worker as WorkerThread
participant DB as Supabase DB
participant LLM as Gemini LLM
Client->>API: POST /chat (query + context)
API->>Worker: asyncio.to_thread(translate -> English)
Worker-->>API: translated_text
API->>Worker: asyncio.to_thread(generate_embedding)
Worker-->>API: embedding_vector
API->>Worker: asyncio.to_thread(get_client RPC -> search)
Worker->>DB: RPC: similarity_search
DB-->>Worker: documents
Worker-->>API: search_results
API->>LLM: get_response_stream_async(context, query)
loop stream chunks
LLM-->>API: chunk
API-->>Client: SSE event (chunk)
end
API-->>Client: SSE event (done)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/api/routes/chat.py (1)
24-45:⚠️ Potential issue | 🟠 Majorasync 경로에서 동기 I/O를 직접 호출해 이벤트 루프를 블로킹합니다.
번역/임베딩/DB RPC/LLM 스트림이 모두 동기 호출이라 트래픽 시 전체 응답성이 급격히 떨어질 수 있습니다.
asyncio.to_thread또는 비동기 클라이언트 경로로 분리해 주세요.🐛 제안 수정안
- english_query = get_english_translation(query) + english_query = await asyncio.to_thread(get_english_translation, query) - query_vector = embedding_service.generate_embedding(english_query) + query_vector = await asyncio.to_thread( + embedding_service.generate_embedding, + english_query + ) - response = supabase_client.rpc( - 'match_documents', - {'query_embedding': query_vector, 'match_count': 3} - ).execute() + response = await asyncio.to_thread( + lambda: supabase_client.rpc( + "match_documents", + {"query_embedding": query_vector, "match_count": 3} + ).execute() + ) - llm_stream = get_response_stream(context=combined_context, query=english_query) - - for chunk in llm_stream: + # 가능하면 서비스 레이어에서 async stream API를 노출하세요. + async for chunk in get_response_stream_async(context=combined_context, query=english_query): if await request.is_disconnected(): breakAlso applies to: 79-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/chat.py` around lines 24 - 45, The code in the async generator is calling blocking sync functions (get_english_translation, embedding_service.generate_embedding, and supabase_client.rpc) directly and will block the event loop under load; wrap those blocking calls with asyncio.to_thread (or replace with truly async clients) so they run off the event loop, e.g., call await asyncio.to_thread(get_english_translation, query) and await asyncio.to_thread(embedding_service.generate_embedding, english_query) (or swap to an async embedding client), and invoke the Supabase RPC via an async client or via await asyncio.to_thread(supabase_client.rpc, 'match_documents', {...}) to avoid blocking; apply the same pattern to the calls referenced later around the LLM stream (lines ~79-83).
🧹 Nitpick comments (14)
frontend/next.config.ts (1)
4-11:remotePatterns허용 범위를port/pathname까지 명시해 더 좁히는 것을 권장합니다.현재 설정(Line 4-11)은 도메인만 제한되어 있어, 모든 포트와 경로의 이미지를 허용합니다.
port: ""(기본 포트만 허용)와pathname(실제 사용 경로로 제한)을 명시하면 보안 범위를 더욱 줄일 수 있습니다.권장 수정안
images: { remotePatterns: [ { protocol: "https", hostname: "image.aladin.co.kr", + port: "", + pathname: "/**", // 실제 사용 경로 prefix가 있으면 더 구체적으로 제한 권장 (예: "/images/**") } ] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/next.config.ts` around lines 4 - 11, The images.remotePatterns entry is too permissive because it only restricts hostname; update the images configuration (the remotePatterns array) to include explicit port and pathname fields to narrow allowed images—set port to an empty string ("") to allow only default ports and add a pathname that matches the actual image path pattern used by your app (e.g., the specific folder or glob) so only those URLs from image.aladin.co.kr are accepted.backend/app/services/database.py (1)
16-17: 모듈 로드 시 전역 클라이언트 초기화는 임포트 실패를 유발할 수 있습니다.
get_supabase_client()가 모듈 수준에서 즉시 호출되므로, 설정이 누락된 경우 애플리케이션 시작 시 불명확한 임포트 에러가 발생할 수 있습니다. 지연 초기화(lazy initialization) 패턴이나 의존성 주입을 고려해 보세요.💡 지연 초기화 패턴 예시
-# Initialize a global client to be reused -supabase_client = get_supabase_client() +# Lazy initialization for Supabase client +_supabase_client: Client | None = None + +def get_client() -> Client: + global _supabase_client + if _supabase_client is None: + _supabase_client = get_supabase_client() + return _supabase_client🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/database.py` around lines 16 - 17, The module currently calls get_supabase_client() at import time and assigns it to the global supabase_client, which can raise unclear import-time failures when configuration is missing; change this to lazy initialization by replacing the eager call with a None placeholder and add an accessor (e.g., get_global_supabase_client()) that checks if supabase_client is None and only then calls get_supabase_client() to construct and cache the client (optionally guard with a simple lock for thread-safety), or refactor functions that use supabase_client to accept the client via dependency injection instead of relying on the module-level supabase_client.backend/app/services/llm.py (1)
16-16:max_retries=0설정은 일시적 네트워크 오류에 취약할 수 있습니다.재시도 없이 API 호출이 실패하면 일시적인 네트워크 문제에도 즉시 에러가 발생합니다. 프로덕션 환경에서는 적절한 재시도 횟수(예: 2-3회)를 고려해 보세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm.py` at line 16, The current LLM client instantiation sets max_retries=0 which makes API calls fail on transient network errors; update the instantiation in backend/app/services/llm.py to use a small retry count (e.g., max_retries=2 or 3) instead of 0, and ensure any retry/backoff strategy used by the client or wrapper around the function that calls the LLM (look for the parameter name max_retries in the client creation or function that configures the LLM) is applied so transient network errors are retried a few times before surfacing an error.frontend/.stylelintrc.json (1)
1-13: 기본 Stylelint 설정 확장을 고려해 보세요.현재
scss/at-rule-no-unknown규칙만 설정되어 있습니다.stylelint-config-standard-scss와 같은 기본 설정을 확장하면 더 포괄적인 린팅이 가능합니다. 또한, Tailwind CSS를 사용하는 경우@apply,@screen,@layer등의 at-rule도ignoreAtRules에 추가가 필요할 수 있습니다.💡 권장 설정 예시
{ + "extends": ["stylelint-config-standard-scss"], "rules": { "scss/at-rule-no-unknown": [ true, { "ignoreAtRules": [ "theme", - "import" + "import", + "apply", + "layer", + "screen" ] } ] } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/.stylelintrc.json` around lines 1 - 13, Extend the Stylelint config to use a standard SCSS preset and add common Tailwind at-rules to the ignore list: add an "extends": ["stylelint-config-standard-scss"] entry at the top-level of the JSON and keep the existing "scss/at-rule-no-unknown" rule but expand its "ignoreAtRules" array to include "apply", "screen", "layer", "variants", "responsive", and "tailwind"; optionally add "plugins": ["stylelint-scss"] if not already present to ensure SCSS rules are available.backend/requirements.txt (1)
13-13: sentence-transformers에 버전 제약 조건 추가를 권장합니다.langchain-core, langchain-classic, langchain-google-genai과 같은 핵심 의존성들은 버전 범위가 명시되어 있지만
sentence-transformers는 버전이 고정되어 있지 않습니다. 프로덕션 환경에서의 호환성과 재현성을 위해 명시적인 버전 제약을 추가하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/requirements.txt` at line 13, Pin the sentence-transformers package by replacing the bare "sentence-transformers" entry with an explicit version or constrained range (e.g., exact version or a compatible >=/<= range) in the requirements list; ensure the chosen version of sentence-transformers is compatible with the existing langchain-core/langchain-classic/langchain-google-genai versions used in the repo, then update dependency lock/CI (pip-compile, pip freeze or tests) to verify compatibility.backend/tests/unit/test_llm.py (3)
18-24: mock 변수들에 대한 설명.정적 분석 도구에서
mock_llm과mock_parser가 사용되지 않는다고 표시하지만, 이들은with patch(...)컨텍스트에서 모듈 수준 객체를 패치하기 위해 필요합니다. 명시적으로 참조되지 않더라도 패치 자체가 테스트에 필수적입니다. 린터 경고를 해소하려면 언더스코어 접두사를 사용할 수 있습니다.♻️ 린터 경고 해소를 위한 수정
with patch("app.services.llm.translation_prompt") as mock_prompt, \ - patch("app.services.llm.llm") as mock_llm, \ - patch("app.services.llm.StrOutputParser") as mock_parser: + patch("app.services.llm.llm"), \ + patch("app.services.llm.StrOutputParser"):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` around lines 18 - 24, The linter flags unused mocks mock_llm and mock_parser even though they are required to patch module-level objects; rename them with underscore prefixes (e.g., _mock_llm and _mock_parser) in the with patch(...) line so they remain patched but suppressed by the linter, leaving mock_prompt and mock_chain behavior (translation_prompt, llm, StrOutputParser) unchanged.
1-2: 사용되지 않는 import 제거.
asyncio와os가 import되었지만 파일 내에서 사용되지 않습니다.♻️ 수정 제안
-import asyncio -import os import sys import pytest from pathlib import Path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` around lines 1 - 2, Remove the unused imports asyncio and os from the top of backend/tests/unit/test_llm.py: locate the import statements for asyncio and os and delete them so only actually used imports remain (e.g., keep other imports in that file intact).
29-30: try/except 래핑이 불필요합니다.pytest는 이미 실패한 assertion에 대해 상세한 오류 메시지를 제공합니다. 추가적인 try/except 래핑은 오버헤드만 증가시키고 스택 트레이스를 복잡하게 만듭니다.
♻️ 간소화된 테스트 구조
def test_translation(): print("Testing translation...") - try: - with patch("app.services.llm.translation_prompt") as mock_prompt, \ - patch("app.services.llm.llm") as mock_llm, \ - patch("app.services.llm.StrOutputParser") as mock_parser: - - mock_chain = MagicMock() - mock_chain.invoke.return_value = "Translated Text" - mock_prompt.__or__.return_value.__or__.return_value = mock_chain - - translated = get_english_translation("미덕이란 무엇인가?") - print("Translation:", translated) - assert translated == "Translated Text", "Translation output mocked mismatch" - except Exception as e: - raise AssertionError(f"Translation error: {e}") from e + with patch("app.services.llm.translation_prompt") as mock_prompt, \ + patch("app.services.llm.llm"), \ + patch("app.services.llm.StrOutputParser"): + + mock_chain = MagicMock() + mock_chain.invoke.return_value = "Translated Text" + mock_prompt.__or__.return_value.__or__.return_value = mock_chain + + translated = get_english_translation("미덕이란 무엇인가?") + assert translated == "Translated Text", "Translation output mocked mismatch"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` around lines 29 - 30, Remove the unnecessary try/except wrapper that catches Exception and re-raises AssertionError in the test (the block using "except Exception as e: raise AssertionError(f\"Translation error: {e}\") from e"); instead, let the original assertions run unwrapped so pytest can report failures and full tracebacks directly—delete the try/except and any related error re-raise around the assertion in the test function.frontend/components/chat/MessageList.tsx (3)
1-1: 사용되지 않는 import.
User아이콘이 import되었지만 컴포넌트에서 사용되지 않습니다.♻️ 수정 제안
-import { Sparkles, SquareArrowOutUpRight, ThumbsUp, Copy, RotateCcw, ChevronRight, User } from "lucide-react"; +import { Sparkles, SquareArrowOutUpRight, ThumbsUp, Copy, RotateCcw, ChevronRight } from "lucide-react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chat/MessageList.tsx` at line 1, The import list in MessageList.tsx includes an unused symbol `User` from "lucide-react"; remove `User` from the named import to eliminate the unused-import warning (update the import statement that currently lists Sparkles, SquareArrowOutUpRight, ThumbsUp, Copy, RotateCcw, ChevronRight, User to exclude User).
90-96: Citation 버튼에 onClick 핸들러 누락.참고 문헌 열기 버튼에
aria-label은 있지만onClick핸들러가 없어서 실제로 동작하지 않습니다. 이 기능이 아직 구현되지 않았다면 TODO 주석을 추가하거나, 구현 예정이라면 이슈를 생성하는 것을 권장합니다.이 기능을 구현하거나 추적을 위한 이슈를 생성해 드릴까요?
🤖 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 90 - 96, The citation button in MessageList.tsx is missing an onClick handler so it does nothing; add an onClick prop to the <button> that calls a handler (e.g., openCitation or handleOpenCitation) which opens the citation panel/modal or dispatches the existing action that shows references, or wire it to a prop function (e.g., onOpenCitation) passed into the MessageList/MessageItem component so the parent can control opening; if you don't implement behavior now, add a clear TODO comment on the button referencing a newly created issue ID so the missing implementation is tracked.
73-74: non-null assertion 사용 시 안전성 고려.
msg.metadata!.find(...)!패턴은Set에서 가져온 title이 항상find에서 찾아질 것이라고 가정합니다. 현재 로직상 안전하지만, 향후 리팩토링 시 안전성을 위해 optional chaining과 fallback을 고려할 수 있습니다.♻️ 더 안전한 접근 방식
- {msg.metadata && msg.metadata.length > 0 && Array.from(new Set(msg.metadata.map(m => m.book_info.title))).map((title, idx) => { - const meta = msg.metadata!.find(m => m.book_info.title === title)!; + {msg.metadata && msg.metadata.length > 0 && Array.from(new Set(msg.metadata.map(m => m.book_info.title))).map((title, idx) => { + const meta = msg.metadata?.find(m => m.book_info.title === title); + if (!meta) return null;🤖 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 73 - 74, Replace the unsafe non-null assertions around msg.metadata when deriving meta: instead of using msg.metadata!.find(...)! assume the find may return undefined and handle it with optional chaining and a fallback or by skipping that title; update the mapping logic in MessageList.tsx where you compute meta from msg.metadata and title so it uses msg.metadata?.find(...) and either continues only if meta exists or supplies a safe default object (e.g., a book_info fallback using the title) to avoid runtime errors if the item isn’t found.frontend/components/sidebar/ActivePhilosophers.tsx (1)
24-43: 버튼에 onClick 핸들러 누락.철학자 버튼들이 인터랙티브한 스타일링(hover, focus-visible)을 갖추고 있지만
onClick핸들러가 없습니다. 클릭 시 예상되는 동작(예: 해당 철학자 필터링, 상세 정보 표시)이 있다면 구현이 필요합니다.♻️ onClick 핸들러 추가 예시
interface Props { metadata: DocumentMetadata[]; + onPhilosopherClick?: (scholar: string) => void; } -export function ActivePhilosophers({ metadata }: Props) { +export function ActivePhilosophers({ metadata, onPhilosopherClick }: Props) { ... <button key={meta.scholar} type="button" + onClick={() => onPhilosopherClick?.(meta.scholar)} className="w-full text-left group relative ..." >🤖 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 24 - 43, Add an onClick handler to the philosopher button in ActivePhilosophers.tsx so the button actually performs the expected action (e.g., filter or show details). Locate the button element that uses meta.scholar/meta.school and wire it to the existing selection callback or state updater (e.g., call props.onSelectPhilosopher(meta.scholar) or dispatch(selectPhilosopher(meta.scholar))); ensure the handler uses the unique identifier meta.scholar, keeps the current styling and accessibility (preserve focus-visible classes), and update any parent prop types or handlers (onSelectPhilosopher) if missing so the component receives the callback.frontend/components/sidebar/ContextSources.tsx (1)
26-45: 동작 없는<button>은 접근성 의미를 왜곡합니다.현재 버튼이 실제 액션 없이 렌더링되어 상호작용 가능한 요소로 오인될 수 있습니다. 클릭 동작이 없다면 비버튼 요소로 바꾸거나, 실제 상세 보기 액션을 연결해 주세요.
♻️ 제안 수정안
- <button - type="button" - className="w-full text-left group flex gap-4 cursor-pointer hover:bg-white/5 p-2 rounded-xl transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary/60" - > + <div + className="w-full text-left group flex gap-4 hover:bg-white/5 p-2 rounded-xl transition-colors" + > ... - </button> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/sidebar/ContextSources.tsx` around lines 26 - 45, The rendered interactive-looking element is a plain <button> with no action which misleads assistive tech; either replace the <button> in ContextSources.tsx with a non-interactive container (e.g., <div> or <article>) keeping the same className and children, or wire up a real action by adding an onClick handler (e.g., openDetail or handleSelectSource) plus keyboard support (onKeyDown handling Enter/Space) and appropriate ARIA (aria-label/role if needed) so the element's semantics match its behavior; update the JSX where the <button> wrapper around the cover, title and meta fields is declared.frontend/components/sidebar/Sidebar.tsx (1)
47-53: 준비 중 액션은alert()대신 비활성 패턴이 더 안전합니다.현재는 클릭마다 차단형 모달이 떠서 사용 흐름을 끊습니다. 기능 준비 전에는
disabled/aria-disabled+ 툴팁 안내가 더 자연스럽습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/sidebar/Sidebar.tsx` around lines 47 - 53, Replace the blocking alert() handlers on the sidebar action buttons with a proper disabled pattern: remove the onClick={() => alert("준비 중입니다.")} from the buttons (the ones rendering Settings, History and Profile icons) and add disabled and/or aria-disabled attributes, a non-interactive tabIndex (e.g., tabIndex={-1}) and a visual "disabled" CSS state (reduced opacity and remove hover/bg focus ring classes) while keeping the informative title/tooltip text ("준비 중입니다.") so screen readers still announce it; ensure you update the button props and className used by the Settings, History and Profile button elements in Sidebar.tsx accordingly.
🤖 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/api/routes/chat.py`:
- Around line 47-50: In the except block that catches database errors (the one
using logger.exception and yielding an error event), stop including the raw
exception text (str(e)) in the response to the client; instead yield a generic
error message like "An internal error occurred while searching the database" and
keep the full details in server logs by leaving logger.exception(...) as-is (or
call logger.exception("Database search failed", exc_info=True) if needed).
Locate the except block referencing logger.exception, the exception variable e,
and the yield {"event": "error", ...} and replace the client-facing payload to
remove str(e) while preserving detailed logging.
In `@backend/app/core/config.py`:
- Around line 13-16: The env file path resolution in model_config
(SettingsConfigDict) uses Path(__file__).resolve().parents[3], which points to
the project root instead of the backend directory described in the README;
update the Path expression to use parents[2] so SettingsConfigDict(env_file=...)
points to the backend/.env location (adjust the env_file argument in the
model_config assignment accordingly).
In `@backend/app/services/llm.py`:
- Line 14: Remove the hardcoded "dummy_key_for_testing" fallback and make API
key handling explicit: use settings.GEMINI_API_KEY consistently (both where
genai.configure is called and where google_api_key is passed), and if it is
missing raise an explicit error or abort early; if you need a dummy key for
tests only, gate that behavior behind a clear test-only flag (e.g.,
settings.ALLOW_DUMMY_API_KEY or a TEST environment check) so production never
falls back silently. Ensure the change touches the google_api_key argument and
the genai.configure call so they read the same validated value and fail fast
when no real key is provided.
In `@frontend/components/chat/ChatMain.tsx`:
- Line 39: The session start time is being recomputed on every render because
new Date() is called directly in the JSX; update ChatMain to capture the mount
time once (e.g., in ChatMain component use a useRef or useState initializer like
sessionStartRef = useRef(new Date()) or [sessionStart] = useState(() => new
Date())) and then replace the inline new Date() in the <p> (the line rendering
"세션 시작: ...") with the stored value (sessionStartRef.current or sessionStart)
formatted via toLocaleTimeString with the existing options so the displayed "세션
시작" remains constant after initial mount.
In `@frontend/components/chat/FloatingInput.tsx`:
- Around line 43-47: The onKeyDown handler in FloatingInput.tsx must be
IME-safe: add composition tracking and a short post-composition guard so Enter
during or immediately after IME composition doesn't trigger handleSend.
Implement onCompositionStart and onCompositionEnd handlers (attach to the same
input/textarea) that set a ref/boolean (e.g., isComposingRef) and
onCompositionEnd also record lastCompositionEndTimestamp (e.g.,
lastCompositionEndRef = Date.now()). Then update the existing onKeyDown logic to
only call handleSend when e.key === "Enter" && !e.shiftKey &&
!isComposingRef.current && (Date.now() - lastCompositionEndRef.current) > 200ms
(adjust threshold as needed). Ensure these new handlers and refs are defined in
the FloatingInput component and used instead of relying only on
event.isComposing.
In `@README.md`:
- Around line 33-39: Update the README's environment variable section to include
the Supabase variables required by backend/app/core/config.py: add SUPABASE_URL
and SUPABASE_SERVICE_KEY to the .env example and mention they must be set in the
backend/.env (alongside GEMINI_API_KEY); ensure the example shows both keys
(e.g. SUPABASE_URL="..." and SUPABASE_SERVICE_KEY="...") and a brief note that
these are required by the config module.
---
Outside diff comments:
In `@backend/app/api/routes/chat.py`:
- Around line 24-45: The code in the async generator is calling blocking sync
functions (get_english_translation, embedding_service.generate_embedding, and
supabase_client.rpc) directly and will block the event loop under load; wrap
those blocking calls with asyncio.to_thread (or replace with truly async
clients) so they run off the event loop, e.g., call await
asyncio.to_thread(get_english_translation, query) and await
asyncio.to_thread(embedding_service.generate_embedding, english_query) (or swap
to an async embedding client), and invoke the Supabase RPC via an async client
or via await asyncio.to_thread(supabase_client.rpc, 'match_documents', {...}) to
avoid blocking; apply the same pattern to the calls referenced later around the
LLM stream (lines ~79-83).
---
Nitpick comments:
In `@backend/app/services/database.py`:
- Around line 16-17: The module currently calls get_supabase_client() at import
time and assigns it to the global supabase_client, which can raise unclear
import-time failures when configuration is missing; change this to lazy
initialization by replacing the eager call with a None placeholder and add an
accessor (e.g., get_global_supabase_client()) that checks if supabase_client is
None and only then calls get_supabase_client() to construct and cache the client
(optionally guard with a simple lock for thread-safety), or refactor functions
that use supabase_client to accept the client via dependency injection instead
of relying on the module-level supabase_client.
In `@backend/app/services/llm.py`:
- Line 16: The current LLM client instantiation sets max_retries=0 which makes
API calls fail on transient network errors; update the instantiation in
backend/app/services/llm.py to use a small retry count (e.g., max_retries=2 or
3) instead of 0, and ensure any retry/backoff strategy used by the client or
wrapper around the function that calls the LLM (look for the parameter name
max_retries in the client creation or function that configures the LLM) is
applied so transient network errors are retried a few times before surfacing an
error.
In `@backend/requirements.txt`:
- Line 13: Pin the sentence-transformers package by replacing the bare
"sentence-transformers" entry with an explicit version or constrained range
(e.g., exact version or a compatible >=/<= range) in the requirements list;
ensure the chosen version of sentence-transformers is compatible with the
existing langchain-core/langchain-classic/langchain-google-genai versions used
in the repo, then update dependency lock/CI (pip-compile, pip freeze or tests)
to verify compatibility.
In `@backend/tests/unit/test_llm.py`:
- Around line 18-24: The linter flags unused mocks mock_llm and mock_parser even
though they are required to patch module-level objects; rename them with
underscore prefixes (e.g., _mock_llm and _mock_parser) in the with patch(...)
line so they remain patched but suppressed by the linter, leaving mock_prompt
and mock_chain behavior (translation_prompt, llm, StrOutputParser) unchanged.
- Around line 1-2: Remove the unused imports asyncio and os from the top of
backend/tests/unit/test_llm.py: locate the import statements for asyncio and os
and delete them so only actually used imports remain (e.g., keep other imports
in that file intact).
- Around line 29-30: Remove the unnecessary try/except wrapper that catches
Exception and re-raises AssertionError in the test (the block using "except
Exception as e: raise AssertionError(f\"Translation error: {e}\") from e");
instead, let the original assertions run unwrapped so pytest can report failures
and full tracebacks directly—delete the try/except and any related error
re-raise around the assertion in the test function.
In `@frontend/.stylelintrc.json`:
- Around line 1-13: Extend the Stylelint config to use a standard SCSS preset
and add common Tailwind at-rules to the ignore list: add an "extends":
["stylelint-config-standard-scss"] entry at the top-level of the JSON and keep
the existing "scss/at-rule-no-unknown" rule but expand its "ignoreAtRules" array
to include "apply", "screen", "layer", "variants", "responsive", and "tailwind";
optionally add "plugins": ["stylelint-scss"] if not already present to ensure
SCSS rules are available.
In `@frontend/components/chat/MessageList.tsx`:
- Line 1: The import list in MessageList.tsx includes an unused symbol `User`
from "lucide-react"; remove `User` from the named import to eliminate the
unused-import warning (update the import statement that currently lists
Sparkles, SquareArrowOutUpRight, ThumbsUp, Copy, RotateCcw, ChevronRight, User
to exclude User).
- Around line 90-96: The citation button in MessageList.tsx is missing an
onClick handler so it does nothing; add an onClick prop to the <button> that
calls a handler (e.g., openCitation or handleOpenCitation) which opens the
citation panel/modal or dispatches the existing action that shows references, or
wire it to a prop function (e.g., onOpenCitation) passed into the
MessageList/MessageItem component so the parent can control opening; if you
don't implement behavior now, add a clear TODO comment on the button referencing
a newly created issue ID so the missing implementation is tracked.
- Around line 73-74: Replace the unsafe non-null assertions around msg.metadata
when deriving meta: instead of using msg.metadata!.find(...)! assume the find
may return undefined and handle it with optional chaining and a fallback or by
skipping that title; update the mapping logic in MessageList.tsx where you
compute meta from msg.metadata and title so it uses msg.metadata?.find(...) and
either continues only if meta exists or supplies a safe default object (e.g., a
book_info fallback using the title) to avoid runtime errors if the item isn’t
found.
In `@frontend/components/sidebar/ActivePhilosophers.tsx`:
- Around line 24-43: Add an onClick handler to the philosopher button in
ActivePhilosophers.tsx so the button actually performs the expected action
(e.g., filter or show details). Locate the button element that uses
meta.scholar/meta.school and wire it to the existing selection callback or state
updater (e.g., call props.onSelectPhilosopher(meta.scholar) or
dispatch(selectPhilosopher(meta.scholar))); ensure the handler uses the unique
identifier meta.scholar, keeps the current styling and accessibility (preserve
focus-visible classes), and update any parent prop types or handlers
(onSelectPhilosopher) if missing so the component receives the callback.
In `@frontend/components/sidebar/ContextSources.tsx`:
- Around line 26-45: The rendered interactive-looking element is a plain
<button> with no action which misleads assistive tech; either replace the
<button> in ContextSources.tsx with a non-interactive container (e.g., <div> or
<article>) keeping the same className and children, or wire up a real action by
adding an onClick handler (e.g., openDetail or handleSelectSource) plus keyboard
support (onKeyDown handling Enter/Space) and appropriate ARIA (aria-label/role
if needed) so the element's semantics match its behavior; update the JSX where
the <button> wrapper around the cover, title and meta fields is declared.
In `@frontend/components/sidebar/Sidebar.tsx`:
- Around line 47-53: Replace the blocking alert() handlers on the sidebar action
buttons with a proper disabled pattern: remove the onClick={() => alert("준비
중입니다.")} from the buttons (the ones rendering Settings, History and Profile
icons) and add disabled and/or aria-disabled attributes, a non-interactive
tabIndex (e.g., tabIndex={-1}) and a visual "disabled" CSS state (reduced
opacity and remove hover/bg focus ring classes) while keeping the informative
title/tooltip text ("준비 중입니다.") so screen readers still announce it; ensure you
update the button props and className used by the Settings, History and Profile
button elements in Sidebar.tsx accordingly.
In `@frontend/next.config.ts`:
- Around line 4-11: The images.remotePatterns entry is too permissive because it
only restricts hostname; update the images configuration (the remotePatterns
array) to include explicit port and pathname fields to narrow allowed images—set
port to an empty string ("") to allow only default ports and add a pathname that
matches the actual image path pattern used by your app (e.g., the specific
folder or glob) so only those URLs from image.aladin.co.kr are accepted.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/design/screen.pngis excluded by!**/*.png
📒 Files selected for processing (138)
README.mdapp/services/embedding.pybackend/.coveragebackend/app/api/routes/chat.pybackend/app/core/config.pybackend/app/main.pybackend/app/models/schema.sqlbackend/app/services/database.pybackend/app/services/embedding.pybackend/app/services/llm.pybackend/check_progress.pybackend/data/A Budget of Paradoxes Volume I by Augustus De Morgan.txtbackend/data/A Pickle for the Knowing Ones by Timothy Dexter.txtbackend/data/A Treatise of Human Nature by David Hume.txtbackend/data/A Vindication of the Rights of Woman by Mary Wollstonecraft.txtbackend/data/Also sprach Zarathustra Ein Buch für Alle und Keinen German by Friedrich Wilhelm Nietzsche.txtbackend/data/An Enquiry Concerning Human Understanding by David Hume.txtbackend/data/An Essay Concerning Humane Understanding Volume 1 by John Locke.txtbackend/data/Apology Crito and Phaedo of Socrates by Plato.txtbackend/data/Apology by Plato.txtbackend/data/As a man thinketh by James Allen.txtbackend/data/Bacons Essays and Wisdom of the Ancients by Francis Bacon.txtbackend/data/Beyond Good and Evil by Friedrich Wilhelm Nietzsche.txtbackend/data/Ciceros Tusculan Disputations by Marcus Tullius Cicero.txtbackend/data/De Officiis Latin by Marcus Tullius Cicero.txtbackend/data/Democracy and Education An Introduction to the Philosophy of Education by John Dewey.txtbackend/data/Democracy in America Volume 2 by Alexis de Tocqueville.txtbackend/data/Demonology and Devil-lore by Moncure Daniel Conway.txtbackend/data/Discourse on the Method of Rightly Conducting Ones Reason and of Seeking Truth in the Sciences by René Descartes.txtbackend/data/Ecce Homo by Friedrich Wilhelm Nietzsche.txtbackend/data/Essays by Ralph Waldo Emerson by Ralph Waldo Emerson.txtbackend/data/Essays of Schopenhauer by Arthur Schopenhauer.txtbackend/data/Ethics by Benedictus de Spinoza.txtbackend/data/Etiquette by Emily Post.txtbackend/data/Euthyphro by Plato.txtbackend/data/Fundamental Principles of the Metaphysic of Morals by Immanuel Kant.txtbackend/data/Goethes Theory of Colours by Johann Wolfgang von Goethe.txtbackend/data/Gorgias by Plato.txtbackend/data/How We Think by John Dewey.txtbackend/data/Human All Too Human A Book for Free Spirits by Friedrich Wilhelm Nietzsche.txtbackend/data/Isis unveiled Volume 1 of 2 Science A master-key to mysteries of ancient and modern science and theology by H P Blavatsky.txtbackend/data/Laws by Plato.txtbackend/data/Leviathan by Thomas Hobbes.txtbackend/data/Meditations by Emperor of Rome Marcus Aurelius.txtbackend/data/Nature by Ralph Waldo Emerson.txtbackend/data/On Heroes Hero-Worship and the Heroic in History by Thomas Carlyle.txtbackend/data/On Liberty by John Stuart Mill.txtbackend/data/On War by Carl von Clausewitz.txtbackend/data/On the Duty of Civil Disobedience by Henry David Thoreau.txtbackend/data/On the Nature of Things by Titus Lucretius Carus.txtbackend/data/Pascals Pensées by Blaise Pascal.txtbackend/data/Perpetual Peace A Philosophical Essay by Immanuel Kant.txtbackend/data/Phaedo by Plato.txtbackend/data/Phaedrus by Plato.txtbackend/data/Plato and the Other Companions of Sokrates 3rd ed Volume 1 by George Grote.txtbackend/data/Plutarchs Morals by Plutarch.txtbackend/data/Politics A Treatise on Government by Aristotle.txtbackend/data/Pragmatism A New Name for Some Old Ways of Thinking by William James.txtbackend/data/Psyche The Cult of Souls and Belief in Immortality among the Greeks by Erwin Rohde.txtbackend/data/Psychology of the Unconscious by C G Jung.txtbackend/data/Reflections or Sentences and Moral Maxims by François duc de La Rochefoucauld.txtbackend/data/Revelations of Divine Love by of Norwich Julian.txtbackend/data/Roman Stoicism by Edward Vernon Arnold.txtbackend/data/Second Treatise of Government by John Locke.txtbackend/data/Siddhartha by Hermann Hesse.txtbackend/data/Sun Tzŭ on the Art of War The Oldest Military Treatise in the World by active 6th century BC Sunzi.txtbackend/data/Symposium by Plato.txtbackend/data/The Anatomy of Melancholy by Robert Burton.txtbackend/data/The Antichrist by Friedrich Wilhelm Nietzsche.txtbackend/data/The Birth of Tragedy or Hellenism and Pessimism by Friedrich Wilhelm Nietzsche.txtbackend/data/The Case of Wagner Nietzsche Contra Wagner and Selected Aphorisms by Friedrich Wilhelm Nietzsche.txtbackend/data/The City of God Volume I by Saint of Hippo Augustine.txtbackend/data/The City of God Volume II by Saint of Hippo Augustine.txtbackend/data/The Communist Manifesto by Karl Marx and Friedrich Engels.txtbackend/data/The Confessions of St Augustine by Saint of Hippo Augustine.txtbackend/data/The Consolation of Philosophy by Boethius.txtbackend/data/The Critique of Pure Reason by Immanuel Kant.txtbackend/data/The Enchiridion by Epictetus.txtbackend/data/The Essays of Arthur Schopenhauer Studies in Pessimism by Arthur Schopenhauer.txtbackend/data/The Essays of Arthur Schopenhauer the Wisdom of Life by Arthur Schopenhauer.txtbackend/data/The Ethics of Aristotle by Aristotle.txtbackend/data/The Genealogy of Morals by Friedrich Wilhelm Nietzsche.txtbackend/data/The Grand Inquisitor by Fyodor Dostoyevsky.txtbackend/data/The Kama Sutra of Vatsyayana by Vatsyayana.txtbackend/data/The Lives and Opinions of Eminent Philosophers by Diogenes Laertius.txtbackend/data/The Man Who Was Thursday A Nightmare by G K Chesterton.txtbackend/data/The Marriage of Heaven and Hell by William Blake.txtbackend/data/The Meditations of the Emperor Marcus Aurelius Antoninus by Emperor of Rome Marcus Aurelius.txtbackend/data/The Poetics of Aristotle by Aristotle.txtbackend/data/The Prince by Niccolò Machiavelli.txtbackend/data/The Principles of Psychology Volume 1 of 2 by William James.txtbackend/data/The Problems of Philosophy by Bertrand Russell.txtbackend/data/The Prophet by Kahlil Gibran.txtbackend/data/The Republic by Plato.txtbackend/data/The Republic of Plato by Plato.txtbackend/data/The Secret Doctrine Vol 1 of 4 by H P Blavatsky.txtbackend/data/The Secret Doctrine Vol 2 of 4 by H P Blavatsky.txtbackend/data/The Song Celestial Or Bhagavad-Gîtâ from the Mahâbhârata.txtbackend/data/The Twilight of the Idols or How to Philosophize with the Hammer The Antichrist by Friedrich Wilhelm Nietzsche.txtbackend/data/The Will to Believe and Other Essays in Popular Philosophy by William James.txtbackend/data/The World as Will and Idea Vol 1 of 3 by Arthur Schopenhauer.txtbackend/data/The history of magic including a clear and precise exposition of its procedure its rites and its mysteries by Éliphas Lévi.txtbackend/data/The social contract discourses by Jean-Jacques Rousseau.txtbackend/data/The symbolism of Freemasonry Illustrating and explaining its science and philosophy its legends myths and symbols by Albert Gallatin Mackey.txtbackend/data/Thus Spake Zarathustra A Book for All and None by Friedrich Wilhelm Nietzsche.txtbackend/data/Utilitarianism by John Stuart Mill.txtbackend/data/Utopia by Saint Thomas More.txtbackend/data/Walden and On The Duty Of Civil Disobedience by Henry David Thoreau.txtbackend/data/What Is Art by graf Leo Tolstoy.txtbackend/data/新序 Chinese by Xiang Liu.txtbackend/data/日知錄 Chinese by Yanwu Gu.txtbackend/data/韓詩外傳 Complete Chinese by active 150 BC Ying Han.txtbackend/download_books.pybackend/requirements.txtbackend/scripts/check_db.pybackend/scripts/ingest_all_data.pybackend/scripts/ingest_data.pybackend/tests/e2e/test_chat_endpoint.pybackend/tests/integration/test_supabase_match.pybackend/tests/unit/test_llm.pybackend/verify_and_clear.pyfrontend/.gitignorefrontend/.stylelintrc.jsonfrontend/app/globals.cssfrontend/app/layout.tsxfrontend/app/page.tsxfrontend/components/chat/ChatMain.tsxfrontend/components/chat/FloatingInput.tsxfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsxfrontend/components/sidebar/ContextSources.tsxfrontend/components/sidebar/Sidebar.tsxfrontend/design/code.htmlfrontend/next.config.tsfrontend/package.jsonfrontend/tsconfig.jsonfrontend/types/chat.tstests/unit/test_llm.py
💤 Files with no reviewable changes (2)
- tests/unit/test_llm.py
- app/services/embedding.py
…gurations, SSE, and frontend interactivity
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/components/chat/FloatingInput.tsx (1)
44-50:⚠️ Potential issue | 🟠 MajorIME 조합 중 Enter 처리 순서가 아직 안전하지 않습니다.
Line [48]에서 조합 상태 확인 전에
e.preventDefault()가 먼저 실행되어, 한글 IME 확정 Enter 입력을 방해할 수 있습니다. 조합 중/직후에는preventDefault와 전송을 모두 건너뛰도록 순서를 바꿔주세요.🐛 권장 수정안
import { useState, useRef } from "react"; @@ export function FloatingInput({ onSendMessage, isSubmitting }: FloatingInputProps) { const [inputValue, setInputValue] = useState(""); const isComposing = useRef(false); + const lastCompositionEndAt = useRef(0); @@ - onCompositionStart={() => isComposing.current = true} - onCompositionEnd={() => isComposing.current = false} + onCompositionStart={() => { isComposing.current = true; }} + onCompositionEnd={() => { + isComposing.current = false; + lastCompositionEndAt.current = Date.now(); + }} onKeyDown={(e) => { if (e.key === "Enter" && !e.shiftKey) { - e.preventDefault(); - if (!isComposing.current) { - handleSend(); - } + const justEndedComposition = + Date.now() - lastCompositionEndAt.current <= 200; + if (isComposing.current || e.nativeEvent.isComposing || justEndedComposition) { + return; + } + e.preventDefault(); + handleSend(); } }}#!/bin/bash # 목적: FloatingInput.tsx의 IME 가드 구현 여부를 정적 확인합니다. # 기대 결과: # 1) onKeyDown 내부에서 preventDefault 이전에 조합 가드가 존재 # 2) nativeEvent.isComposing 체크 존재 # 3) lastCompositionEnd 타임스탬프 기반 짧은 가드 존재 FILE="$(fd -t f FloatingInput.tsx | head -n 1)" echo "Checking: ${FILE}" nl -ba "${FILE}" | sed -n '35,70p' echo "---- pattern check ----" rg -n "onCompositionStart|onCompositionEnd|isComposing|nativeEvent\.isComposing|lastCompositionEnd|preventDefault\(" "${FILE}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chat/FloatingInput.tsx` around lines 44 - 50, The onKeyDown handler currently calls e.preventDefault() before checking IME state, which can block Hangul composition confirms; modify the onKeyDown logic in FloatingInput.tsx (the handler around onKeyDown, isComposing, nativeEvent.isComposing, lastCompositionEnd and handleSend) to first check IME guards (if isComposing.current || e.nativeEvent?.isComposing || Date.now() - lastCompositionEnd < SOME_MS) and return early, skipping both preventDefault and handleSend, and only call preventDefault/handleSend when those checks pass.
🧹 Nitpick comments (6)
frontend/components/sidebar/Sidebar.tsx (1)
29-29: 임시console.log핸들러는 제거하거나 실제 액션으로 대체해 주세요.현재는 클릭 시 콘솔 노이즈만 발생하므로, 기능이 준비되기 전까지는 prop 미전달이 더 깔끔합니다.
🧹 제안 패치
- <ActivePhilosophers metadata={currentMetadata} onPhilosopherClick={(scholar) => console.log('Philosopher clicked:', scholar)} /> + <ActivePhilosophers metadata={currentMetadata} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/sidebar/Sidebar.tsx` at line 29, Remove the temporary console.log click handler from the ActivePhilosophers usage: drop the onPhilosopherClick={(scholar) => console.log(...)} prop or replace it with a real handler (e.g., selectPhilosopher or navigateToPhilosopher) that invokes the proper action/dispatcher; update the parent component to provide that real callback if needed and ensure the ActivePhilosophers prop signature is respected.frontend/components/sidebar/ActivePhilosophers.tsx (1)
10-12: 중복 추출 로직을 O(n)으로 단순화하는 리팩터를 권장합니다.현재 구현은
find를 다시 순회해서 데이터가 커지면 비용이 커집니다. 같은 결과를 한 번의 순회로 만들 수 있습니다.♻️ 제안 패치
- const uniquePhilosophers = Array.from(new Set(metadata.map(m => m.scholar))) - .map(scholar => metadata.find(m => m.scholar === scholar)) - .filter((m): m is DocumentMetadata => m !== undefined); + const uniquePhilosophers = Array.from( + metadata.reduce((acc, item) => { + if (!acc.has(item.scholar)) acc.set(item.scholar, item); + return acc; + }, new Map<string, DocumentMetadata>()).values() + );🤖 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 10 - 12, The current dedup logic for uniquePhilosophers does a nested traversal (map + find) which is O(n^2); replace it with a single-pass O(n) approach by iterating metadata once (e.g., using a Map or object keyed by m.scholar) to store the first DocumentMetadata per scholar, then produce uniquePhilosophers from the Map's values; update the variable uniquePhilosophers and keep the resulting type as DocumentMetadata[] so downstream code remains unchanged.frontend/components/chat/FloatingInput.tsx (1)
15-18: 전송 값도trim결과를 사용하도록 일관성 맞추는 것을 권장합니다.현재 Line [16]은
trim()으로 검증하지만 Line [17]은 원문을 전송합니다. 검증/전송 값을 동일하게 맞추면 불필요한 공백 전송을 줄일 수 있습니다.♻️ 권장 수정안
const handleSend = () => { - if (!inputValue.trim() || isSubmitting) return; - onSendMessage(inputValue); + const message = inputValue.trim(); + if (!message || isSubmitting) return; + onSendMessage(message); setInputValue(""); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chat/FloatingInput.tsx` around lines 15 - 18, The handler handleSend currently trims inputValue only for validation but sends the original string; change it to compute a trimmed value (e.g., const trimmed = inputValue.trim()), use that for the emptiness check and pass trimmed to onSendMessage, and then call setInputValue("") as before so sent and validated values are consistent; update references to inputValue in handleSend to use the trimmed variable.backend/app/services/database.py (1)
19-23: 지연 초기화에서 잠재적 경합 조건 가능성.
asyncio.to_thread를 통해 여러 요청이 동시에get_client()를 호출하면_supabase_client가 여러 번 초기화될 수 있습니다. Supabase 클라이언트 자체는 thread-safe할 가능성이 높아 실제 문제가 될 확률은 낮지만, 더 안전한 패턴을 원한다면threading.Lock을 사용할 수 있습니다.현재 구현도 실용적으로는 문제없이 동작할 것입니다.
🔒 Thread-safe 초기화 제안 (선택사항)
+import threading + from supabase import create_client, Client from app.core.config import settings +_lock = threading.Lock() _supabase_client: Client | None = None def get_client() -> Client: global _supabase_client - if _supabase_client is None: - _supabase_client = get_supabase_client() + if _supabase_client is None: + with _lock: + if _supabase_client is None: + _supabase_client = get_supabase_client() return _supabase_client🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/database.py` around lines 19 - 23, get_client currently lazily initializes the module-level _supabase_client without synchronization, allowing a race where multiple threads can initialize it concurrently; add a module-level threading.Lock (e.g., _client_lock), and in get_client perform a double-checked locking pattern: if _supabase_client is None, acquire _client_lock, check again, if still None call get_supabase_client() and assign to _supabase_client, then release the lock; reference get_client and _supabase_client when making the change so only one thread initializes the client.README.md (1)
78-78: 아키텍처 설명의 정확성을 확인해 주세요."SentenceTransformers embedding"이라고 명시되어 있지만, 코드에서는
HuggingFaceEmbeddings를 사용하고 있습니다. 내부적으로 sentence-transformers를 사용할 수 있지만, 일관성을 위해 "HuggingFace embedding" 또는 실제 사용 중인 모델명으로 수정하는 것이 좋습니다.📝 제안 수정안
-- Backend: FastAPI, LangChain, SentenceTransformers embedding, and Supabase integration. +- Backend: FastAPI, LangChain, HuggingFace embedding, and Supabase integration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 78, Update the architecture line that currently reads "Backend: FastAPI, LangChain, SentenceTransformers embedding, and Supabase integration." to accurately reflect the embedding implementation used (e.g., change "SentenceTransformers embedding" to "HuggingFace Embeddings" or the exact model name used with HuggingFaceEmbeddings); locate the string in README.md and replace it so terminology matches the actual code symbol/class HuggingFaceEmbeddings (or the specific model identifier) for consistency.backend/app/api/routes/chat.py (1)
42-47: Lambda 대신 직접 호출 고려 (선택사항).현재 lambda를 사용한 패턴도 동작하지만, 가독성을 위해 별도 함수로 분리하거나
functools.partial을 사용할 수 있습니다.♻️ 리팩토링 제안
+def _search_documents(query_vector): + return get_client().rpc( + 'match_documents', + {'query_embedding': query_vector, 'match_count': 3} + ).execute() + # In generate_chat_events: - response = await asyncio.to_thread( - lambda: get_client().rpc( - 'match_documents', - {'query_embedding': query_vector, 'match_count': 3} - ).execute() - ) + response = await asyncio.to_thread(_search_documents, query_vector)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/chat.py` around lines 42 - 47, The inline lambda passed to asyncio.to_thread reduces readability; extract the RPC call into a named helper (e.g., def match_documents(query_vector): return get_client().rpc('match_documents', {'query_embedding': query_vector, 'match_count': 3}).execute()) and then call it with asyncio.to_thread(match_documents, query_vector) or alternately use functools.partial to bind the RPC call before passing to asyncio.to_thread; refer to asyncio.to_thread, get_client().rpc(...).execute and the new match_documents helper when updating the code.
🤖 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/FloatingInput.tsx`:
- Around line 44-50: The onKeyDown handler currently calls e.preventDefault()
before checking IME state, which can block Hangul composition confirms; modify
the onKeyDown logic in FloatingInput.tsx (the handler around onKeyDown,
isComposing, nativeEvent.isComposing, lastCompositionEnd and handleSend) to
first check IME guards (if isComposing.current || e.nativeEvent?.isComposing ||
Date.now() - lastCompositionEnd < SOME_MS) and return early, skipping both
preventDefault and handleSend, and only call preventDefault/handleSend when
those checks pass.
---
Nitpick comments:
In `@backend/app/api/routes/chat.py`:
- Around line 42-47: The inline lambda passed to asyncio.to_thread reduces
readability; extract the RPC call into a named helper (e.g., def
match_documents(query_vector): return get_client().rpc('match_documents',
{'query_embedding': query_vector, 'match_count': 3}).execute()) and then call it
with asyncio.to_thread(match_documents, query_vector) or alternately use
functools.partial to bind the RPC call before passing to asyncio.to_thread;
refer to asyncio.to_thread, get_client().rpc(...).execute and the new
match_documents helper when updating the code.
In `@backend/app/services/database.py`:
- Around line 19-23: get_client currently lazily initializes the module-level
_supabase_client without synchronization, allowing a race where multiple threads
can initialize it concurrently; add a module-level threading.Lock (e.g.,
_client_lock), and in get_client perform a double-checked locking pattern: if
_supabase_client is None, acquire _client_lock, check again, if still None call
get_supabase_client() and assign to _supabase_client, then release the lock;
reference get_client and _supabase_client when making the change so only one
thread initializes the client.
In `@frontend/components/chat/FloatingInput.tsx`:
- Around line 15-18: The handler handleSend currently trims inputValue only for
validation but sends the original string; change it to compute a trimmed value
(e.g., const trimmed = inputValue.trim()), use that for the emptiness check and
pass trimmed to onSendMessage, and then call setInputValue("") as before so sent
and validated values are consistent; update references to inputValue in
handleSend to use the trimmed variable.
In `@frontend/components/sidebar/ActivePhilosophers.tsx`:
- Around line 10-12: The current dedup logic for uniquePhilosophers does a
nested traversal (map + find) which is O(n^2); replace it with a single-pass
O(n) approach by iterating metadata once (e.g., using a Map or object keyed by
m.scholar) to store the first DocumentMetadata per scholar, then produce
uniquePhilosophers from the Map's values; update the variable uniquePhilosophers
and keep the resulting type as DocumentMetadata[] so downstream code remains
unchanged.
In `@frontend/components/sidebar/Sidebar.tsx`:
- Line 29: Remove the temporary console.log click handler from the
ActivePhilosophers usage: drop the onPhilosopherClick={(scholar) =>
console.log(...)} prop or replace it with a real handler (e.g.,
selectPhilosopher or navigateToPhilosopher) that invokes the proper
action/dispatcher; update the parent component to provide that real callback if
needed and ensure the ActivePhilosophers prop signature is respected.
In `@README.md`:
- Line 78: Update the architecture line that currently reads "Backend: FastAPI,
LangChain, SentenceTransformers embedding, and Supabase integration." to
accurately reflect the embedding implementation used (e.g., change
"SentenceTransformers embedding" to "HuggingFace Embeddings" or the exact model
name used with HuggingFaceEmbeddings); locate the string in README.md and
replace it so terminology matches the actual code symbol/class
HuggingFaceEmbeddings (or the specific model identifier) for consistency.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.mdbackend/app/api/routes/chat.pybackend/app/core/config.pybackend/app/services/database.pybackend/app/services/llm.pybackend/requirements.txtbackend/tests/unit/test_llm.pyfrontend/.stylelintrc.jsonfrontend/components/chat/ChatMain.tsxfrontend/components/chat/FloatingInput.tsxfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsxfrontend/components/sidebar/ContextSources.tsxfrontend/components/sidebar/Sidebar.tsxfrontend/next.config.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/components/sidebar/ContextSources.tsx
- frontend/components/chat/MessageList.tsx
- frontend/next.config.ts
- frontend/components/chat/ChatMain.tsx
- backend/requirements.txt
- backend/tests/unit/test_llm.py
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/page.tsx (1)
84-97:⚠️ Potential issue | 🟠 Major스트림 종료 시
TextDecoderflush 누락으로 인한 멀티바이트 문자 손실현재 코드는 96번 줄에서
decoder.decode(value, { stream: true })를 사용하여 스트림을 처리하고 있습니다.stream: true옵션을 사용하면 불완전한 멀티바이트 시퀀스(예: UTF-8의 마지막 1-3바이트)는 내부 버퍼에 보관되며 출력되지 않습니다. 스트림 종료 시(85-93번 줄의done분기)에decoder.decode()를 호출하여 내부 버퍼를 명시적으로 비워야 하는데, 현재는 이 호출이 누락되어 있습니다. 그 결과 한글, 이모지 등 마지막 멀티바이트 문자 조각이 손실될 수 있습니다.수정 제안
while (true) { const { done, value } = await reader.read(); if (done) { + // 디코더 내부 버퍼 플러시 (불완전한 멀티바이트 문자 처리) + buffer += decoder.decode(); // Process any remaining data in the buffer if (buffer) { const lines = buffer.split('\n'); for (const line of lines) { processLine(line, eventObj); } } break; } buffer += decoder.decode(value, { stream: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` around lines 84 - 97, When handling the reader.read() done branch, flush the TextDecoder's internal buffer before splitting/processing the remaining text: call decoder.decode() (without {stream: true}) and append its return to buffer so incomplete multi-byte sequences are emitted; then split buffer and call processLine(line, eventObj) as currently done. This ensures decoder.decode is invoked on stream end to avoid losing multi-byte characters.
🧹 Nitpick comments (5)
backend/app/services/embedding.py (1)
20-20: 로깅에 f-string 대신 lazy formatting 사용을 권장합니다.로깅 레벨이 비활성화된 경우에도 f-string은 항상 평가됩니다.
%s포맷을 사용하면 불필요한 문자열 생성을 피할 수 있습니다.♻️ 제안 수정안
- logger.info(f"Loading local embedding model: {MODEL_NAME} (HuggingFace)...") + logger.info("Loading local embedding model: %s (HuggingFace)...", MODEL_NAME)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/embedding.py` at line 20, Replace the f-string log call to use lazy logging formatting: change the logger.info(f"Loading local embedding model: {MODEL_NAME} (HuggingFace)...") usage in embedding.py to pass a format string and MODEL_NAME as an argument (e.g., "Loading local embedding model: %s (HuggingFace)..." , MODEL_NAME) so the message is only formatted when the log level is enabled; update any similar logger.* calls in the same module to use this %s-style lazy formatting.backend/tests/unit/test_llm.py (2)
35-35: Generator는 한 번만 소비됩니다.Generator expression
(chunk for chunk in [...])은 한 번 소비되면 빈 상태가 됩니다. 현재 테스트에서는list(stream)으로 한 번만 호출하므로 문제없지만, 테스트 재사용성을 위해 list를 직접 사용하는 것이 더 명확합니다.♻️ 제안 수정안
- mock_chain.stream.return_value = (chunk for chunk in ["안녕하세요", " ", "철학자", "입니다."]) + mock_chain.stream.return_value = iter(["안녕하세요", " ", "철학자", "입니다."])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` at line 35, Replace the single-use generator assigned to mock_chain.stream.return_value with a reusable iterable (use a list) so the stream can be iterated multiple times during tests; update the assignment to set mock_chain.stream.return_value to the list ["안녕하세요", " ", "철학자", "입니다."] (referencing mock_chain.stream.return_value in the test_llm test).
16-22: Mock 체이닝 패턴이 구현에 강하게 결합되어 있습니다.
mock_prompt.__or__.return_value.__or__.return_value패턴은 LangChain의|연산자 체이닝 구현에 의존합니다. 향후 LangChain 내부 구현이 변경되면 테스트가 깨질 수 있습니다.보다 안정적인 접근법으로, 전체 chain 객체를 mock하거나 통합 테스트에서 실제 동작을 검증하는 것을 고려하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` around lines 16 - 22, The test is tightly coupled to LangChain's internal '|' chaining by setting mock_prompt.__or__.return_value.__or__.return_value; instead, have the patched translation_prompt produce a chain object directly (e.g., set translation_prompt.return_value or configure the factory that builds the chain to return mock_chain) and ensure mock_chain.invoke returns the desired result; update the patches around translation_prompt (and/or the chain constructor) so tests inject a single mock chain (mock_chain) rather than mocking __or__ chaining to avoid fragility if LangChain changes.backend/app/services/database.py (1)
16-16:import threading을 파일 상단으로 이동하세요.
threadingimport가 함수 정의 이후(line 16)에 위치해 있습니다. Python 컨벤션(PEP 8)에 따라 모든 import는 파일 상단에 그룹화되어야 합니다.♻️ 제안 수정안
from supabase import create_client, Client from app.core.config import settings +import threading def get_supabase_client() -> Client:그리고 line 16의
import threading을 제거하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/database.py` at line 16, Move the stray "import threading" into the module's top import block and remove the duplicate import that currently appears after the function definitions; ensure the top-of-file imports include threading alongside the other standard/library imports so there are no in-function imports left.frontend/components/chat/MessageList.tsx (1)
1-1: 전역@next/next/no-img-element비활성화는 범위를 줄이는 것이 좋습니다.파일 전체에서 규칙을 꺼두면 이후
<img>사용이 계속 누락될 수 있습니다. 이 카드 영역만 국소 예외 처리하거나next/image로 전환하는 쪽이 안전합니다.Also applies to: 80-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/chat/MessageList.tsx` at line 1, The global ESLint disable comment for `@next/next/no-img-element` should be removed and narrowed: either replace the file-level "/* eslint-disable `@next/next/no-img-element` */" with a localized exception (use // eslint-disable-next-line `@next/next/no-img-element` immediately above the specific <img> usage in the MessageList component or wrap only the small JSX block with /* eslint-disable *///* eslint-enable */), or better yet convert the offending <img> elements inside the MessageList JSX to Next.js' Image component; update references inside the MessageList component where <img> is used to follow one of these approaches so the rule remains enforced elsewhere.
🤖 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/tests/unit/test_llm.py`:
- Line 12: Add an async unit test for get_response_stream_async in the same test
module: import get_response_stream_async, decorate the test with
pytest.mark.asyncio (or use asyncio.run in the test runner), monkeypatch/mock
the underlying streaming client or generator used by get_response_stream_async
to yield a few predictable chunks, then await get_response_stream_async(...) to
collect the streamed pieces and assert they match the expected sequence and
final result; ensure the test mirrors the existing get_response_stream test
structure so it verifies both chunked yields and completion behavior.
In `@frontend/app/page.tsx`:
- Around line 58-63: The code assigns metaJson.philosophers directly into
message.metadata which can break rendering if the payload isn't an array; update
the handler that runs when currentEvent === "metadata" (using currentData,
metaJson, setMessages, aiMsgId) to parse JSON then guard metaJson.philosophers
with Array.isArray and fall back to an empty array ([]) before calling
setMessages so message.metadata is always an array.
In `@frontend/components/chat/MessageList.tsx`:
- Around line 95-98: The citation button currently only logs in the onClick
handler (console.log("Citation clicked:", meta)) which mismatches its
aria-label; either wire it to a safe prop handler like onOpenCitation(meta)
passed from the parent (add an optional prop onOpenCitation and call it from the
onClick) or disable/hide the button until such a handler exists (set disabled or
omit rendering when onOpenCitation is undefined) so the UI matches the
aria-label and avoids a no-op; update the onClick in MessageList (the click
handler referencing meta) accordingly.
---
Outside diff comments:
In `@frontend/app/page.tsx`:
- Around line 84-97: When handling the reader.read() done branch, flush the
TextDecoder's internal buffer before splitting/processing the remaining text:
call decoder.decode() (without {stream: true}) and append its return to buffer
so incomplete multi-byte sequences are emitted; then split buffer and call
processLine(line, eventObj) as currently done. This ensures decoder.decode is
invoked on stream end to avoid losing multi-byte characters.
---
Nitpick comments:
In `@backend/app/services/database.py`:
- Line 16: Move the stray "import threading" into the module's top import block
and remove the duplicate import that currently appears after the function
definitions; ensure the top-of-file imports include threading alongside the
other standard/library imports so there are no in-function imports left.
In `@backend/app/services/embedding.py`:
- Line 20: Replace the f-string log call to use lazy logging formatting: change
the logger.info(f"Loading local embedding model: {MODEL_NAME} (HuggingFace)...")
usage in embedding.py to pass a format string and MODEL_NAME as an argument
(e.g., "Loading local embedding model: %s (HuggingFace)..." , MODEL_NAME) so the
message is only formatted when the log level is enabled; update any similar
logger.* calls in the same module to use this %s-style lazy formatting.
In `@backend/tests/unit/test_llm.py`:
- Line 35: Replace the single-use generator assigned to
mock_chain.stream.return_value with a reusable iterable (use a list) so the
stream can be iterated multiple times during tests; update the assignment to set
mock_chain.stream.return_value to the list ["안녕하세요", " ", "철학자", "입니다."]
(referencing mock_chain.stream.return_value in the test_llm test).
- Around line 16-22: The test is tightly coupled to LangChain's internal '|'
chaining by setting mock_prompt.__or__.return_value.__or__.return_value;
instead, have the patched translation_prompt produce a chain object directly
(e.g., set translation_prompt.return_value or configure the factory that builds
the chain to return mock_chain) and ensure mock_chain.invoke returns the desired
result; update the patches around translation_prompt (and/or the chain
constructor) so tests inject a single mock chain (mock_chain) rather than
mocking __or__ chaining to avoid fragility if LangChain changes.
In `@frontend/components/chat/MessageList.tsx`:
- Line 1: The global ESLint disable comment for `@next/next/no-img-element` should
be removed and narrowed: either replace the file-level "/* eslint-disable
`@next/next/no-img-element` */" with a localized exception (use //
eslint-disable-next-line `@next/next/no-img-element` immediately above the
specific <img> usage in the MessageList component or wrap only the small JSX
block with /* eslint-disable *///* eslint-enable */), or better yet convert the
offending <img> elements inside the MessageList JSX to Next.js' Image component;
update references inside the MessageList component where <img> is used to follow
one of these approaches so the rule remains enforced elsewhere.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.mdbackend/app/api/routes/chat.pybackend/app/services/database.pybackend/app/services/embedding.pybackend/tests/unit/test_llm.pyfrontend/app/layout.tsxfrontend/app/page.tsxfrontend/components/chat/ChatMain.tsxfrontend/components/chat/FloatingInput.tsxfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- README.md
- frontend/components/chat/FloatingInput.tsx
- frontend/components/sidebar/ActivePhilosophers.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (4)
backend/tests/unit/test_llm.py (1)
57-59: 사용하지 않는args와kwargs매개변수에 언더스코어 접두사 추가 권장정적 분석 도구에서 지적된 대로,
mock_astream함수의args와kwargs매개변수가 사용되지 않습니다. Python 컨벤션에 따라 언더스코어 접두사를 추가하면 의도적으로 무시되는 매개변수임을 명시할 수 있습니다.♻️ 제안된 수정
- async def mock_astream(*args, **kwargs): + async def mock_astream(*_args, **_kwargs): for chunk in ["안녕하세요", " ", "철학자", "입니다."]: yield chunk🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` around lines 57 - 59, The mock_astream async generator declares unused parameters named args and kwargs; rename them to _args and _kwargs in the mock_astream signature to follow Python convention for intentionally ignored parameters (update the async def mock_astream(*args, **kwargs) to async def mock_astream(*_args, **_kwargs) so linters/static analysis no longer flag them).frontend/app/page.tsx (1)
45-45: 응답 상태 코드 포함 권장디버깅 시 유용하도록 에러 메시지에 HTTP 상태 코드를 포함하면 좋습니다.
♻️ 제안된 수정
- if (!res.ok) throw new Error("Failed to fetch"); + if (!res.ok) throw new Error(`Failed to fetch: ${res.status} ${res.statusText}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/page.tsx` at line 45, The current check throws a generic Error when the fetch response is not ok (the line using res.ok), which lacks HTTP status details; update the error thrown in that branch to include the response status and statusText (and optionally response body) so the thrown Error conveys e.g. status and statusText for easier debugging, by replacing the generic throw in the res.ok check with a descriptive message that interpolates res.status and res.statusText (and/or await res.text() if you want body details) in the Error constructed.backend/app/services/database.py (2)
18-28: 스레드 안전한 지연 초기화 싱글톤 패턴이 잘 구현되었습니다.Double-checked locking 패턴이 올바르게 구현되어 있으며, Python의 GIL로 인해 스레드 안전성이 보장됩니다. 첫 번째 검사(line 24)로 빠른 경로를 제공하고, 락 내부 두 번째 검사(line 26)로 경쟁 조건을 방지합니다.
선택적으로,
get_supabase_client()가 이제get_client()의 내부 팩토리로만 사용되므로, 이를 private으로 변경(_get_supabase_client)하여 의도를 명확히 하는 것을 고려해보실 수 있습니다.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/database.py` around lines 18 - 28, Rename the external factory function get_supabase_client() to a private helper (e.g., _get_supabase_client) since it is only used inside the lazy-initialization singleton get_client(); update all references inside this module (specifically the call in get_client()) to the new name and ensure the original public name is removed or redirected to maintain intent and encapsulation.
13-14: 환경 설정 검증 로직은 좋습니다. 예외 메시지 리팩토링을 고려해보세요.기본값 제거 및 명시적 검증은 보안 측면에서 좋은 개선입니다. 정적 분석 도구(Ruff TRY003)에서 지적한 대로, 긴 예외 메시지를 상수로 추출하거나 커스텀 예외 클래스를 사용하는 것을 고려해보실 수 있습니다.
♻️ 예외 메시지 상수 추출 제안
+_CONFIG_ERROR_MSG = "SUPABASE_URL and SUPABASE_SERVICE_KEY must be configured" + def get_supabase_client() -> Client: """ Returns a configured Supabase client using the URL and Service Key. The Service Key is used to bypass RLS for administrative backend tasks like upserting documents or fetching metadata securely. """ supabase_url = settings.SUPABASE_URL supabase_key = settings.SUPABASE_SERVICE_KEY if not supabase_url or not supabase_key: - raise RuntimeError("SUPABASE_URL and SUPABASE_SERVICE_KEY must be configured") + raise RuntimeError(_CONFIG_ERROR_MSG) return create_client(supabase_url, supabase_key)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/database.py` around lines 13 - 14, Replace the inline RuntimeError message with a named constant or a custom exception to satisfy Ruff TRY003: define a constant like SUPABASE_CONFIG_ERROR (or a custom exception class MissingConfigError) and use it when raising instead of the literal string; update the check that inspects supabase_url and supabase_key (the raise in the module-level validation) to raise the new constant/exception so the message is centralized and reusable.
🤖 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/services/database.py`:
- Around line 18-28: Rename the external factory function get_supabase_client()
to a private helper (e.g., _get_supabase_client) since it is only used inside
the lazy-initialization singleton get_client(); update all references inside
this module (specifically the call in get_client()) to the new name and ensure
the original public name is removed or redirected to maintain intent and
encapsulation.
- Around line 13-14: Replace the inline RuntimeError message with a named
constant or a custom exception to satisfy Ruff TRY003: define a constant like
SUPABASE_CONFIG_ERROR (or a custom exception class MissingConfigError) and use
it when raising instead of the literal string; update the check that inspects
supabase_url and supabase_key (the raise in the module-level validation) to
raise the new constant/exception so the message is centralized and reusable.
In `@backend/tests/unit/test_llm.py`:
- Around line 57-59: The mock_astream async generator declares unused parameters
named args and kwargs; rename them to _args and _kwargs in the mock_astream
signature to follow Python convention for intentionally ignored parameters
(update the async def mock_astream(*args, **kwargs) to async def
mock_astream(*_args, **_kwargs) so linters/static analysis no longer flag them).
In `@frontend/app/page.tsx`:
- Line 45: The current check throws a generic Error when the fetch response is
not ok (the line using res.ok), which lacks HTTP status details; update the
error thrown in that branch to include the response status and statusText (and
optionally response body) so the thrown Error conveys e.g. status and statusText
for easier debugging, by replacing the generic throw in the res.ok check with a
descriptive message that interpolates res.status and res.statusText (and/or
await res.text() if you want body details) in the Error constructed.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/app/services/database.pybackend/app/services/embedding.pybackend/pytest_log.txtbackend/pytest_log_utf8.txtbackend/tests/unit/test_llm.pyfrontend/app/page.tsxfrontend/components/chat/MessageList.tsx
✅ Files skipped from review due to trivial changes (1)
- backend/pytest_log_utf8.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/components/chat/MessageList.tsx
- backend/app/services/embedding.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/tests/unit/test_llm.py (1)
11-14: 모듈 레벨 환경 변수 설정은 테스트 격리에 영향을 줄 수 있습니다.환경 변수를 모듈 레벨에서 설정하면 같은 프로세스에서 실행되는 다른 테스트에 영향을 줄 수 있습니다. pytest fixture를 사용하면 테스트 격리를 개선하고 teardown 시 원래 값을 복원할 수 있습니다.
♻️ pytest fixture 사용 제안
conftest.py또는 테스트 파일에 추가:import pytest import os `@pytest.fixture`(autouse=True) def setup_test_env(monkeypatch): monkeypatch.setenv("GEMINI_API_KEY", "dummy_test_key") monkeypatch.setenv("SUPABASE_URL", "http://localhost:8000") monkeypatch.setenv("SUPABASE_SERVICE_KEY", "dummy_test_key")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/unit/test_llm.py` around lines 11 - 14, The test file backend/tests/unit/test_llm.py currently sets environment variables at module level which can leak between tests; remove the top-level os.environ assignments and replace them with a pytest fixture (e.g., setup_test_env) that uses monkeypatch.setenv to set GEMINI_API_KEY, SUPABASE_URL, and SUPABASE_SERVICE_KEY for each test (autouse=True or explicitly used), ensuring the fixture lives in conftest.py or the test file so values are restored after each test; update any imports or references to rely on the fixture rather than module-level os.environ mutations.
🤖 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/tests/unit/test_llm.py`:
- Around line 49-66: The test uses the `@pytest.mark.asyncio` decorator
(test_streaming_async) and async generator get_response_stream_async, so add the
pytest-asyncio package to the project's test dependencies (e.g., the
requirements used for running tests) so the decorator is available at test
runtime; update the dependency list to include "pytest-asyncio" and ensure the
test environment installs it before running the tests.
In `@frontend/app/page.tsx`:
- Around line 51-79: The processLine handler must stop the streaming read loop
when an "error" event is received to avoid subsequent "content" chunks being
appended to the same AI message; update processLine (the function handling
events for aiMsgId and eventObj) so that on currentEvent === "error" it calls
reader.cancel() (or otherwise cancels the reader) and returns a signal (e.g.,
boolean) to the caller so the outer read loop can break immediately; adjust the
caller of processLine to check that return value and break/stop reading when
true, leaving the existing setMessages update in place.
---
Nitpick comments:
In `@backend/tests/unit/test_llm.py`:
- Around line 11-14: The test file backend/tests/unit/test_llm.py currently sets
environment variables at module level which can leak between tests; remove the
top-level os.environ assignments and replace them with a pytest fixture (e.g.,
setup_test_env) that uses monkeypatch.setenv to set GEMINI_API_KEY,
SUPABASE_URL, and SUPABASE_SERVICE_KEY for each test (autouse=True or explicitly
used), ensuring the fixture lives in conftest.py or the test file so values are
restored after each test; update any imports or references to rely on the
fixture rather than module-level os.environ mutations.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/services/database.pybackend/tests/unit/test_llm.pyfrontend/app/page.tsx
Comment on lines
+49
to
+66
| @pytest.mark.asyncio | ||
| async def test_streaming_async(): | ||
| print("Testing streaming async...") | ||
| with patch("app.services.llm.get_rag_prompt") as mock_prompt, \ | ||
| patch("app.services.llm.llm") as _mock_llm, \ | ||
| patch("app.services.llm.StrOutputParser") as _mock_parser: | ||
|
|
||
| mock_chain = MagicMock() | ||
| async def mock_astream(*_args, **_kwargs): | ||
| for chunk in ["안녕하세요", " ", "철학자", "입니다."]: | ||
| yield chunk | ||
| mock_chain.astream = mock_astream | ||
| mock_chain.__or__.return_value = mock_chain | ||
| mock_prompt.return_value.__or__.return_value = mock_chain | ||
|
|
||
| stream = get_response_stream_async(context="Virtue is excellence.", query="What is virtue?") | ||
| results = [chunk async for chunk in stream] | ||
| assert results == ["안녕하세요", " ", "철학자", "입니다."], "Async stream chunks mocked mismatch" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for pytest-asyncio in test dependencies
echo "=== Checking for pytest-asyncio in dependencies ==="
# Search in common dependency files
for file in pyproject.toml setup.py setup.cfg requirements*.txt; do
if [ -f "$file" ]; then
echo "--- Checking $file ---"
cat -n "$file" | grep -i "pytest-asyncio\|pytest_asyncio" || echo "Not found in $file"
fi
done
echo ""
echo "=== Listing all test/dev dependency files ==="
find . -maxdepth 2 -type f \( -name "pyproject.toml" -o -name "setup.py" -o -name "setup.cfg" -o -name "*requirements*.txt" \) 2>/dev/nullRepository: SanghunYun95/philo-rag
Length of output: 191
🏁 Script executed:
#!/bin/bash
# Read the requirements.txt file to see all dependencies
echo "=== Contents of backend/requirements.txt ==="
cat backend/requirements.txt
echo ""
echo "=== Searching for pytest or asyncio related packages ==="
cat backend/requirements.txt | grep -i "pytest\|asyncio"Repository: SanghunYun95/philo-rag
Length of output: 436
🏁 Script executed:
#!/bin/bash
# Check if there are other requirements files or test configuration
echo "=== Finding all requirements files ==="
find . -name "*requirements*.txt" -o -name "*requirements*.in" 2>/dev/null | head -20
echo ""
echo "=== Checking for pytest configuration ==="
find . -name "pytest.ini" -o -name "pyproject.toml" -o -name "setup.cfg" -o -name "conftest.py" 2>/dev/null | head -20
echo ""
echo "=== Checking backend directory structure ==="
ls -la backend/ | head -20Repository: SanghunYun95/philo-rag
Length of output: 1025
🏁 Script executed:
#!/bin/bash
# Check the pytest log for information about pytest-asyncio
echo "=== Checking pytest log ==="
cat backend/pytest_log.txt | head -100
echo ""
echo "=== Searching for asyncio or pytest-asyncio mentions in logs ==="
grep -i "asyncio\|pytest.mark.asyncio" backend/pytest_log.txt || echo "No mentions found"Repository: SanghunYun95/philo-rag
Length of output: 3921
🏁 Script executed:
#!/bin/bash
# Check the UTF-8 version of the log for clearer reading
echo "=== Checking pytest_log_utf8.txt ==="
cat backend/pytest_log_utf8.txt | grep -A 5 "plugins:" || cat backend/pytest_log_utf8.txt | head -50Repository: SanghunYun95/philo-rag
Length of output: 461
🏁 Script executed:
#!/bin/bash
# The log shows asyncio-1.3.0 is installed. Let's verify where it comes from
echo "=== Checking if pytest-asyncio comes from dependencies ==="
grep -r "pytest-asyncio\|pytest_asyncio" . --include="*.txt" --include="*.toml" --include="*.py" 2>/dev/null | grep -v ".coverage" | grep -v "pytest_log" || echo "Not found in source files"
echo ""
echo "=== Checking for dev/test dependencies in the repo root ==="
ls -la | grep -i "require\|setup\|pyproject"Repository: SanghunYun95/philo-rag
Length of output: 211
pytest-asyncio 패키지를 의존성에 추가해야 합니다.
@pytest.mark.asyncio 데코레이터를 사용하는 비동기 스트리밍 테스트가 잘 구현되었으나, backend/requirements.txt에 pytest-asyncio 패키지가 명시적으로 나열되어 있지 않습니다. 테스트가 정상적으로 실행되려면 pytest-asyncio를 의존성에 추가하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/tests/unit/test_llm.py` around lines 49 - 66, The test uses the
`@pytest.mark.asyncio` decorator (test_streaming_async) and async generator
get_response_stream_async, so add the pytest-asyncio package to the project's
test dependencies (e.g., the requirements used for running tests) so the
decorator is available at test runtime; update the dependency list to include
"pytest-asyncio" and ensure the test environment installs it before running the
tests.
Comment on lines
+51
to
+79
| const processLine = (line: string, eventObj: { current: string }) => { | ||
| if (line.startsWith("event: ")) { | ||
| eventObj.current = line.substring(7).trim(); | ||
| } else if (line.startsWith("data: ")) { | ||
| const currentData = line.substring(6); | ||
| const currentEvent = eventObj.current; | ||
|
|
||
| if (currentEvent === "metadata" && currentData.trim() !== "") { | ||
| try { | ||
| const metaJson = JSON.parse(currentData); | ||
| const philosophersArray = Array.isArray(metaJson.philosophers) ? metaJson.philosophers : []; | ||
| setMessages((prev) => | ||
| prev.map(msg => msg.id === aiMsgId ? { ...msg, metadata: philosophersArray } : msg) | ||
| ); | ||
| } catch { console.error("Could not parse metadata event:", currentData) } | ||
| } else if (currentEvent === "content") { | ||
| // un-escape \\n to real newlines | ||
| const char = currentData.replace(/\\n/g, '\n'); | ||
| setMessages((prev) => | ||
| prev.map(msg => msg.id === aiMsgId ? { ...msg, content: msg.content + char } : msg) | ||
| ); | ||
| } else if (currentEvent === "error") { | ||
| console.error("Chat error:", currentData); | ||
| setMessages((prev) => | ||
| prev.map(msg => msg.id === aiMsgId ? { ...msg, content: currentData, isStreaming: false } : msg) | ||
| ); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
오류 이벤트 수신 후 스트림 루프를 즉시 중단해 주세요.
Line 72에서 error 이벤트를 처리해도 읽기 루프가 계속 진행되어, 이후 content 데이터가 같은 AI 메시지에 추가될 수 있습니다. 에러를 수신하면 루프를 종료(필요 시 reader.cancel())하도록 바꾸는 게 안전합니다.
수정 예시
- const processLine = (line: string, eventObj: { current: string }) => {
+ const processLine = (line: string, eventObj: { current: string }): boolean => {
if (line.startsWith("event: ")) {
eventObj.current = line.substring(7).trim();
} else if (line.startsWith("data: ")) {
const currentData = line.substring(6);
const currentEvent = eventObj.current;
@@
} else if (currentEvent === "error") {
console.error("Chat error:", currentData);
setMessages((prev) =>
prev.map(msg => msg.id === aiMsgId ? { ...msg, content: currentData, isStreaming: false } : msg)
);
+ return true;
}
}
+ return false;
};
@@
- while (true) {
+ let shouldStop = false;
+ while (true) {
const { done, value } = await reader.read();
if (done) {
@@
if (buffer) {
const lines = buffer.split('\n');
for (const line of lines) {
- processLine(line, eventObj);
+ if (processLine(line, eventObj)) {
+ shouldStop = true;
+ break;
+ }
}
}
break;
}
@@
for (const line of lines) {
- processLine(line, eventObj);
+ if (processLine(line, eventObj)) {
+ shouldStop = true;
+ break;
+ }
}
+ if (shouldStop) {
+ await reader.cancel();
+ break;
+ }
}Also applies to: 84-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/page.tsx` around lines 51 - 79, The processLine handler must
stop the streaming read loop when an "error" event is received to avoid
subsequent "content" chunks being appended to the same AI message; update
processLine (the function handling events for aiMsgId and eventObj) so that on
currentEvent === "error" it calls reader.cancel() (or otherwise cancels the
reader) and returns a signal (e.g., boolean) to the caller so the outer read
loop can break immediately; adjust the caller of processLine to check that
return value and break/stop reading when true, leaving the existing setMessages
update in place.
This was referenced Mar 1, 2026
Merged
Merged
Merged
This was referenced Mar 28, 2026
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
릴리스 노트
새로운 기능
개선 사항
테스트
스타일