Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough사용자 첫 쿼리로부터 한국어 채팅방 제목을 생성하는 POST /api/v1/chat/title 엔드포인트 추가, Gemini 다중 API 키 지원 및 키 로테이션/폴백 로직 도입, 프론트엔드에 chatTitle·activeMetadata 상태와 가시성 기반 메타데이터 추적 기능 추가, 대규모 책 매핑 데이터·DB 메타데이터 업데이트 스크립트와 SQL 변경 포함. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Frontend as 프론트엔드\n(page.tsx)
participant ChatAPI as 채팅 API\n(/api/v1/chat/title)
participant LLMService as LLM 서비스\n(generate_chat_title_async)
participant GeminiAPI as Gemini API
User->>Frontend: 첫 메시지 입력
Frontend->>ChatAPI: POST /api/v1/chat/title (query)
ChatAPI->>LLMService: generate_chat_title_async(query)
LLMService->>GeminiAPI: 요청 (한국어 타이틀 프롬프트, 키 로테이션/폴백)
GeminiAPI-->>LLMService: 생성된 제목 응답
LLMService-->>ChatAPI: 정제된 제목(최대20자)
ChatAPI-->>Frontend: {"title": "..."}
Frontend->>ChatMain: chatTitle 상태 업데이트
ChatMain-->>User: 제목 표시
sequenceDiagram
participant MessageList as MessageList
participant Observer as IntersectionObserver
participant DOM as DOM 요소\n(ai-message-card)
participant Sidebar as Sidebar
participant Callback as onVisibleMessageChange
MessageList->>Observer: 마운트 시 옵저버 등록
Observer->>DOM: ai-message-card 요소 모니터링
DOM-->>Observer: 교차 비율 변화 이벤트
Observer->>Observer: 가장 보이는 메시지 결정
Observer->>Callback: 해당 메시지 메타데이터 전달
Callback-->>Sidebar: activeMetadata 갱신
Sidebar-->>Sidebar: 활성 철학자 정렬/강조 갱신
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/components/chat/MessageList.tsx (1)
15-33:⚠️ Potential issue | 🔴 CriticalReact Hook 규칙을 위반합니다. 조건부 return을 Hooks 선언 이후로 이동하세요.
Line 15의 조기 return이 Line 29-32의 Hook 호출(
useRef,useEffect)보다 앞에 위치합니다.messages.length === 0일 때 Hook이 호출되지 않아 렌더링마다 Hook 호출 순서가 달라지므로 React Hook 규칙 위반입니다.🔧 수정 제안
export function MessageList({ messages, onOpenCitation, onVisibleMessageChange }: Props) { + const observer = useRef<IntersectionObserver | null>(null); + const visibleMessages = useRef<Map<string, number>>(new Map()); + + useEffect(() => { + if (!onVisibleMessageChange) return; + ... + }, [messages, onVisibleMessageChange]); + if (messages.length === 0) { return ( <div className="w-full h-full flex flex-col items-center justify-center text-center p-8"> ... </div> ); } - - const observer = useRef<IntersectionObserver | null>(null); - const visibleMessages = useRef<Map<string, number>>(new Map()); - - useEffect(() => { - if (!onVisibleMessageChange) return; - ... - }, [messages, onVisibleMessageChange]); return ( ...🤖 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 15 - 33, The early return that checks messages.length === 0 is placed before Hook calls and breaks React Hook order; move the conditional rendering so that the Hooks (useRef for observer and visibleMessages and useEffect that references onVisibleMessageChange) are always invoked unconditionally — e.g., declare const observer = useRef..., const visibleMessages = useRef..., and the useEffect(...) before any return, then perform the messages.length === 0 UI return after those Hook declarations so Hook order remains stable.
🧹 Nitpick comments (1)
frontend/components/sidebar/Sidebar.tsx (1)
56-56: 클릭 핸들러가 TODO/no-op이라 사용자 액션이 무시됩니다.Line 56의
onPhilosopherClick이 빈 구현이라 인터랙션이 살아있어도 동작이 없습니다.원하시면
scholar기준 필터 상태까지 포함한 최소 구현안을 바로 제안해드릴게요. 이 TODO를 이슈로 분리해드릴까요?🤖 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 56, The onPhilosopherClick prop passed to ActivePhilosophers is a no-op so clicks are ignored; implement it to update the Sidebar's filter state by filtering allMetadata by the clicked philosopher's scholar field and setting displayMetadata (or calling the existing setter like setDisplayMetadata / updateDisplayMetadata) so the UI reflects the scholar filter; locate the ActivePhilosophers usage in the Sidebar component and replace the empty handler with a function that accepts the clicked philosopher identifier, filters allMetadata where metadata.scholar === clickedScholar (or matches accordingly), and updates the displayMetadata state (also make sure to clear the filter when a "clear" value is received).
🤖 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 140-147: Validate and short-circuit empty/whitespace input before
calling generate_chat_title_async by checking title_request.query (strip and
return a default/400 response if empty), and add a timeout wrapper around the
external call to generate_chat_title_async (or use asyncio.wait_for) so the LLM
call does not hang; also ensure exceptions from the timeout are caught and
converted to a clear error/log message. Use the identifiers
generate_chat_title_async and title_request.query to locate where to add the
input validation and timeout handling, and update the existing except block to
handle timeout-specific exceptions separately for proper logging/response.
In `@backend/data/books_mapping.json`:
- Line 20: Many entries in books_mapping.json store URLs with HTML-escaped
ampersands (the "link" key contains "&"), which will break query parameters;
update the data generation or ingestion path to HTML-decode entity references
before persisting or serving URLs (e.g., replace "&" with "&" or use an HTML
entity decoder) so all "link" values are stored as valid URLs; locate
occurrences by searching for the "link" key and the "&" token and change the
generator/serializer to perform decoding.
In `@backend/scripts/generate_book_mapping.py`:
- Around line 100-115: The except block around the LLM call is incrementing
current_key_idx for all exceptions (including JSON parsing and other non-auth
errors); update the handler so only key-related errors (determined by
should_rotate computed from error_str) advance current_key_idx. Specifically, in
the except block referencing current_key_idx, should_rotate, error_str and
file_name, move the current_key_idx += 1 inside the if should_rotate branch (and
do not rotate on parse/other errors), and instead log the parse failure (or
rethrow/continue) without consuming the API key.
In `@backend/scripts/update_db_metadata.py`:
- Around line 85-90: The batch can crash if metadata isn't a dict because
metadata.get(...) is used; in update_db_metadata.py around the doc handling
(variables doc_id, metadata, db_title) add a per-row guard that checks
isinstance(metadata, dict) (or use a try/except TypeError) before calling
metadata.get('book_info', {}).get('title', ''), and when the guard fails log or
warn with the doc_id and skip that row so the batch continues.
In `@backend/update_metadata.sql`:
- Around line 4-7: The global session timeout is being set with "SET
statement_timeout = '120s';" which will persist beyond this migration; change it
to use a transaction-local setting by replacing that line with a
transaction-scoped command (i.e., use "SET LOCAL statement_timeout = '120s';"
immediately after BEGIN) so the timeout applies only for the current transaction
and does not affect subsequent migrations or sessions.
In `@frontend/components/chat/ChatMain.tsx`:
- Line 16: The prop type onVisibleMessageChange uses DocumentMetadata but that
type isn't imported, causing TypeScript errors; add an import for
DocumentMetadata from '../../types/chat' at the top of ChatMain.tsx (where other
imports live) so the onVisibleMessageChange?: (meta: DocumentMetadata[]) =>
void; signature resolves correctly.
In `@frontend/components/sidebar/Sidebar.tsx`:
- Around line 17-18: The current allMetadata = aiMessages.flatMap(m =>
m.metadata || []) merely concatenates metadata and does not deduplicate as the
“unique” comment suggests; change the logic after computing allMetadata (or
replace it) to dedupe by a stable key (e.g., metadata.id or another unique
property) using a Set or Map so only one entry per key remains; update
references in Sidebar.tsx that use allMetadata to rely on the deduped array and
ensure you pick the correct unique identifier field from each metadata object.
---
Outside diff comments:
In `@frontend/components/chat/MessageList.tsx`:
- Around line 15-33: The early return that checks messages.length === 0 is
placed before Hook calls and breaks React Hook order; move the conditional
rendering so that the Hooks (useRef for observer and visibleMessages and
useEffect that references onVisibleMessageChange) are always invoked
unconditionally — e.g., declare const observer = useRef..., const
visibleMessages = useRef..., and the useEffect(...) before any return, then
perform the messages.length === 0 UI return after those Hook declarations so
Hook order remains stable.
---
Nitpick comments:
In `@frontend/components/sidebar/Sidebar.tsx`:
- Line 56: The onPhilosopherClick prop passed to ActivePhilosophers is a no-op
so clicks are ignored; implement it to update the Sidebar's filter state by
filtering allMetadata by the clicked philosopher's scholar field and setting
displayMetadata (or calling the existing setter like setDisplayMetadata /
updateDisplayMetadata) so the UI reflects the scholar filter; locate the
ActivePhilosophers usage in the Sidebar component and replace the empty handler
with a function that accepts the clicked philosopher identifier, filters
allMetadata where metadata.scholar === clickedScholar (or matches accordingly),
and updates the displayMetadata state (also make sure to clear the filter when a
"clear" value is received).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.gitignorebackend/app/api/routes/chat.pybackend/app/core/config.pybackend/app/core/env_utils.pybackend/app/services/llm.pybackend/data/books_mapping.jsonbackend/scripts/check_models.pybackend/scripts/generate_book_mapping.pybackend/scripts/generate_sql_updates.pybackend/scripts/update_db_metadata.pybackend/update_metadata.sqlfrontend/app/page.tsxfrontend/components/chat/ChatMain.tsxfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/ActivePhilosophers.tsxfrontend/components/sidebar/Sidebar.tsxfrontend/types/chat.ts
Comment on lines
+100
to
+115
| except Exception as e: | ||
| error_str = str(e).lower() | ||
| should_rotate = any( | ||
| token in error_str | ||
| for token in ( | ||
| "429", "quota", "exhausted", "toomanyrequests", | ||
| "401", "403", "invalid api key", "permission", "unauthenticated", | ||
| ) | ||
| ) | ||
| if should_rotate: | ||
| print(f"API Key {current_key_idx} exhausted/invalid. Switching to next key...") | ||
| current_key_idx += 1 | ||
| else: | ||
| print(f"Failed to parse LLM translation for {file_name}: {e}") | ||
| current_key_idx += 1 | ||
|
|
There was a problem hiding this comment.
비키 관련 예외에서도 키 인덱스를 소모해 조기 폴백이 발생합니다.
Line 113-114는 JSON 파싱 실패 같은 비키 이슈에서도 전역 키 인덱스를 증가시켜, 정상 키까지 빠르게 소진시킬 수 있습니다.
🔧 수정 제안
except Exception as e:
error_str = str(e).lower()
should_rotate = any(
token in error_str
for token in (
"429", "quota", "exhausted", "toomanyrequests",
"401", "403", "invalid api key", "permission", "unauthenticated",
)
)
if should_rotate:
print(f"API Key {current_key_idx} exhausted/invalid. Switching to next key...")
current_key_idx += 1
else:
print(f"Failed to parse LLM translation for {file_name}: {e}")
- current_key_idx += 1
+ return await kyobo_fallback(file_name, "")🧰 Tools
🪛 Ruff (0.15.2)
[warning] 100-100: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/scripts/generate_book_mapping.py` around lines 100 - 115, The except
block around the LLM call is incrementing current_key_idx for all exceptions
(including JSON parsing and other non-auth errors); update the handler so only
key-related errors (determined by should_rotate computed from error_str) advance
current_key_idx. Specifically, in the except block referencing current_key_idx,
should_rotate, error_str and file_name, move the current_key_idx += 1 inside the
if should_rotate branch (and do not rotate on parse/other errors), and instead
log the parse failure (or rethrow/continue) without consuming the API key.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/update_metadata.sql (1)
17-17:⚠️ Potential issue | 🟠 Major
link값에&가 저장되어 URL 파라미터가 깨집니다.Line 17 패턴이 파일 전반에 반복됩니다. 메타데이터에 HTML 엔티티가 그대로 들어가 프론트에서 링크가 오작동할 수 있습니다.
수정 제안
-"link": "https://www.aladin.co.kr/shop/wproduct.aspx?ItemId=4359030&partner=openAPI&start=api" +"link": "https://www.aladin.co.kr/shop/wproduct.aspx?ItemId=4359030&partner=openAPI&start=api"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/update_metadata.sql` at line 17, The stored JSON metadata contains HTML-escaped entities (e.g., "&") in the "link" field causing broken URLs; locate the SET metadata = metadata || ... assignment(s) that set the "link" key in update_metadata.sql and replace/transform the value so it contains a proper URL (unescape HTML entities to "&" or percent-encode query parameters) before casting to jsonb; apply this change to every repeated occurrence of the metadata assignment so the "link" field stores a valid URL rather than HTML entities.
🧹 Nitpick comments (3)
backend/app/api/routes/chat.py (1)
25-27:query필드에 최대 길이 제한 추가를 권장합니다.현재 모델에 길이 제한이 없어 과도하게 긴 쿼리가 전달될 수 있습니다. Pydantic 레벨에서 조기 검증하면 불필요한 처리를 방지할 수 있습니다.
♻️ 수정 제안
class TitleRequest(BaseModel): - query: str + query: str = Field(..., max_length=500)🤖 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 25 - 27, The TitleRequest model's query field lacks a max length constraint; add Pydantic validation to limit input size (e.g., 1024 chars) to prevent excessively long queries. Update the TitleRequest class by changing the type of query to a constrained string (constr(max_length=1024)) or use Field(..., max_length=1024) and import the needed symbol from pydantic so validation runs at model parse time.backend/update_metadata.sql (1)
1-1: 운영 환경이면 인덱스 생성 락 영향을 점검하세요.Line 1의 일반
CREATE INDEX는 대용량/트래픽 환경에서 쓰기 지연을 유발할 수 있습니다. 온라인 마이그레이션이면CREATE INDEX CONCURRENTLY또는 점검 시간대 실행을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/update_metadata.sql` at line 1, The CREATE INDEX statement for idx_documents_book_title on documents uses a blocking operation; change it to an online-safe operation by running CREATE INDEX CONCURRENTLY idx_documents_book_title ON documents ((metadata->'book_info'->>'title')) or perform the index creation during a maintenance window/low-traffic period to avoid write locks; ensure any deployment tooling supports CONCURRENTLY (no transaction blocks) and reference idx_documents_book_title, table documents, and the expression metadata->'book_info'->>'title' when updating the migration.backend/scripts/update_db_metadata.py (1)
126-126: 백오프가 선형이며 주석(Exponential)과 불일치합니다.Line 126은 선형 증가라 재시도 충돌 완화가 약합니다. 주석 의도대로 지수 백오프로 맞추는 편이 안전합니다.
수정 제안
- sleep(0.5 * (attempt + 1)) # Exponential backoff + sleep(0.5 * (2 ** attempt)) # Exponential backoff🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/scripts/update_db_metadata.py` at line 126, The current retry pause uses linear backoff via sleep(0.5 * (attempt + 1)) which contradicts the "Exponential backoff" comment; change the sleep call to exponential backoff using the attempt variable (e.g. sleep(min(max_delay, 0.5 * (2 ** attempt)))) and optionally add small jitter for safety, replacing the current sleep(0.5 * (attempt + 1)) expression and ensuring max_delay is defined or set inline.
🤖 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`:
- Around line 117-118: The fallback call currently drops author info by calling
kyobo_fallback(file_name, "") when LLM fails; change it to pass the
extracted/parsed author (e.g., the variable that holds the author parsed from
file_name or the same value used to populate translated_author) into
kyobo_fallback so kyobo_fallback(file_name, translated_author) (or the
appropriate author variable name) is used instead of an empty string; update the
return at the print("LLM Failed...") site and ensure the referenced variable
(e.g., translated_author or parsed_author) is defined/available in that scope
before returning.
In `@frontend/components/chat/MessageList.tsx`:
- Around line 45-54: When mostVisibleId exists but the matched message has no
metadata the code currently does nothing and leaves the previous metadata in
place; update the branch in MessageList.tsx so that after finding msg =
messages.find(m => m.id === mostVisibleId) you check for msg.metadata and if
absent fall back to the existing aiMessages lookup (messages.filter(m => m.role
=== "ai" && m.metadata && m.metadata.length > 0) and call onVisibleMessageChange
with the last AI message metadata) and if that also yields nothing call
onVisibleMessageChange with an empty/clearing value (e.g., [] or null depending
on the prop type) so the previous metadata is cleared; ensure you reference
mostVisibleId, messages, msg, aiMessages and the onVisibleMessageChange callback
in the fix.
- Line 62: Change the forEach callback to use a block body so it doesn't
implicitly return a value: replace the expression-bodied arrow used with
elements.forEach(el => observer.current?.observe(el)) by a block-bodied callback
that calls observer.current?.observe(el); (i.e., elements.forEach(el => {
observer.current?.observe(el); })), ensuring the forEach callback does not
return anything to satisfy the useIterableCallbackReturn lint rule in
MessageList.tsx.
---
Duplicate comments:
In `@backend/update_metadata.sql`:
- Line 17: The stored JSON metadata contains HTML-escaped entities (e.g.,
"&") in the "link" field causing broken URLs; locate the SET metadata =
metadata || ... assignment(s) that set the "link" key in update_metadata.sql and
replace/transform the value so it contains a proper URL (unescape HTML entities
to "&" or percent-encode query parameters) before casting to jsonb; apply this
change to every repeated occurrence of the metadata assignment so the "link"
field stores a valid URL rather than HTML entities.
---
Nitpick comments:
In `@backend/app/api/routes/chat.py`:
- Around line 25-27: The TitleRequest model's query field lacks a max length
constraint; add Pydantic validation to limit input size (e.g., 1024 chars) to
prevent excessively long queries. Update the TitleRequest class by changing the
type of query to a constrained string (constr(max_length=1024)) or use
Field(..., max_length=1024) and import the needed symbol from pydantic so
validation runs at model parse time.
In `@backend/scripts/update_db_metadata.py`:
- Line 126: The current retry pause uses linear backoff via sleep(0.5 * (attempt
+ 1)) which contradicts the "Exponential backoff" comment; change the sleep call
to exponential backoff using the attempt variable (e.g. sleep(min(max_delay, 0.5
* (2 ** attempt)))) and optionally add small jitter for safety, replacing the
current sleep(0.5 * (attempt + 1)) expression and ensuring max_delay is defined
or set inline.
In `@backend/update_metadata.sql`:
- Line 1: The CREATE INDEX statement for idx_documents_book_title on documents
uses a blocking operation; change it to an online-safe operation by running
CREATE INDEX CONCURRENTLY idx_documents_book_title ON documents
((metadata->'book_info'->>'title')) or perform the index creation during a
maintenance window/low-traffic period to avoid write locks; ensure any
deployment tooling supports CONCURRENTLY (no transaction blocks) and reference
idx_documents_book_title, table documents, and the expression
metadata->'book_info'->>'title' when updating the migration.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/app/api/routes/chat.pybackend/data/books_mapping.jsonbackend/scripts/generate_book_mapping.pybackend/scripts/update_db_metadata.pybackend/update_metadata.sqlfrontend/components/chat/ChatMain.tsxfrontend/components/chat/MessageList.tsxfrontend/components/sidebar/Sidebar.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/components/chat/MessageList.tsx (1)
45-59:⚠️ Potential issue | 🟠 Major메타데이터가 없는 경우 상태를 비우지 않아 이전 값이 남습니다.
mostVisibleId가 있거나 fallback 분기에 들어가도, 유효한 metadata를 찾지 못하면onVisibleMessageChange가 호출되지 않아 이전 메타데이터가 유지됩니다. 빈 배열로 명시적으로 초기화해 주세요.수정 제안
if (mostVisibleId) { const msg = messages.find(m => m.id === mostVisibleId); if (msg && msg.metadata && msg.metadata.length > 0) { onVisibleMessageChange(msg.metadata); } else { const aiMessages = messages.filter(m => m.role === "ai" && m.metadata && m.metadata.length > 0); if (aiMessages.length > 0) { onVisibleMessageChange(aiMessages[aiMessages.length - 1].metadata!); + } else { + onVisibleMessageChange([]); } } } else { const aiMessages = messages.filter(m => m.role === "ai" && m.metadata && m.metadata.length > 0); if (aiMessages.length > 0) { onVisibleMessageChange(aiMessages[aiMessages.length - 1].metadata!); + } else { + onVisibleMessageChange([]); } }🤖 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 45 - 59, When neither the mostVisibleId branch nor the AI fallback finds any message with metadata, the previous metadata remains because onVisibleMessageChange is not called; update the logic around mostVisibleId, the messages.find(...) result, and the aiMessages fallback to explicitly call onVisibleMessageChange([]) whenever no valid metadata array is found so the visible metadata state is cleared (ensure you handle both the case where msg exists but has no metadata and the case where aiMessages.length === 0).
🤖 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 151-153: In the except asyncio.TimeoutError block that currently
calls logger.warning(f"Timeout generating chat title for query: {query}"),
remove the raw user query from logs and replace it with a privacy-safe message
(e.g., include no user content or only a non-sensitive identifier); update the
logger.warning call in that handler to use a generic message like "Timeout
generating chat title" or include a sanitized/hashed ID if available, ensuring
the variable query is not logged anywhere in that exception path.
- Around line 147-150: After cleaning the model-generated title (the title
variable) ensure you fall back to a default non-empty title when the cleaned
result is empty; i.e., after title = title.replace(...).strip() check if title
== "" and if so set a sensible default like "New chat" or "Untitled" before
applying the length truncation and returning {"title": title}; update the logic
around the existing title trimming so the default is also truncated if longer
than 20 characters.
---
Duplicate comments:
In `@frontend/components/chat/MessageList.tsx`:
- Around line 45-59: When neither the mostVisibleId branch nor the AI fallback
finds any message with metadata, the previous metadata remains because
onVisibleMessageChange is not called; update the logic around mostVisibleId, the
messages.find(...) result, and the aiMessages fallback to explicitly call
onVisibleMessageChange([]) whenever no valid metadata array is found so the
visible metadata state is cleared (ensure you handle both the case where msg
exists but has no metadata and the case where aiMessages.length === 0).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/app/api/routes/chat.pybackend/scripts/generate_book_mapping.pybackend/scripts/generate_sql_updates.pybackend/scripts/update_db_metadata.pybackend/update_metadata.sqlfrontend/app/page.tsxfrontend/components/chat/ChatMain.tsxfrontend/components/chat/MessageList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/scripts/generate_book_mapping.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/api/routes/chat.py (1)
142-158: 기본 제목 문자열을 상수로 추출해 중복을 줄이세요.Line 142, 149, 155, 158에서 동일 문자열이 반복됩니다. 상수화하면 변경/현지화 시 누락 위험이 줄어듭니다.
리팩터링 예시
logger = logging.getLogger(__name__) +DEFAULT_CHAT_TITLE = "새로운 대화" @@ query = title_request.query.strip() if not query: - return {"title": "새로운 대화"} + return {"title": DEFAULT_CHAT_TITLE} @@ title = title.replace('"', '').replace("'", "").strip() if not title: - return {"title": "새로운 대화"} + return {"title": DEFAULT_CHAT_TITLE} @@ except asyncio.TimeoutError: logger.warning("Timeout generating chat title") - return {"title": "새로운 대화"} + return {"title": DEFAULT_CHAT_TITLE} except Exception: logger.exception("Failed to generate chat title") - return {"title": "새로운 대화"} + return {"title": DEFAULT_CHAT_TITLE}🤖 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 142 - 158, Extract the repeated default title string "새로운 대화" into a single module-level constant (e.g., DEFAULT_CHAT_TITLE) and replace all hard-coded occurrences in this route handler where the function calls/generates the title (references to generate_chat_title_async, the asyncio.TimeoutError and generic Exception handlers, and the early return) to use that constant; ensure the constant is used for every return that currently returns the literal so future changes or localization only need updating in one place.
🤖 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 150-151: The current truncation logic for variable title slices to
20 chars then appends "..." which makes the final length up to 23; change the
truncation to account for the ellipsis length by cutting title to (MAX_LEN -
len(ellipsis)) characters before appending the ellipsis (e.g., compute ellipsis
= "..." and slice title to max_len - len(ellipsis) when len(title) > max_len) so
the final title length is at most 20; update the logic around the title variable
where the length check and truncation occur.
In `@frontend/components/chat/MessageList.tsx`:
- Around line 16-76: The visibleMessages ref (visibleMessages.current) is not
cleared when the effect mounts or unmounts, causing stale IDs to persist and
leading getMostVisible logic in the IntersectionObserver callback (within the
useEffect that sets observer.current) to pick non-existent message IDs; fix by
clearing visibleMessages.current at the start of the effect and again in the
cleanup (before disconnecting observer.current) so the Map is reset whenever
messages/onVisibleMessageChange change or the component unmounts, ensuring the
mostVisibleId computation and subsequent onVisibleMessageChange calls use only
current elements.
---
Nitpick comments:
In `@backend/app/api/routes/chat.py`:
- Around line 142-158: Extract the repeated default title string "새로운 대화" into a
single module-level constant (e.g., DEFAULT_CHAT_TITLE) and replace all
hard-coded occurrences in this route handler where the function calls/generates
the title (references to generate_chat_title_async, the asyncio.TimeoutError and
generic Exception handlers, and the early return) to use that constant; ensure
the constant is used for every return that currently returns the literal so
future changes or localization only need updating in one place.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/components/chat/MessageList.tsx (1)
18-76:⚠️ Potential issue | 🟠 Major
visibleMessages를 effect 생명주기마다 초기화해야 합니다.Line 18-76에서
visibleMessages.current를 재사용하지만 초기화가 없어, 제거된 메시지 id가 남아mostVisibleId계산이 틀어질 수 있습니다. effect 시작 시와 cleanup 시clear()를 호출해 주세요.수정 제안
useEffect(() => { if (!onVisibleMessageChange) return; + visibleMessages.current.clear(); observer.current = new IntersectionObserver((entries) => { let changed = false; entries.forEach(entry => { @@ const elements = document.querySelectorAll(".ai-message-card"); elements.forEach(el => { observer.current?.observe(el); }); return () => { + visibleMessages.current.clear(); observer.current?.disconnect(); }; }, [messages, onVisibleMessageChange]);🤖 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 18 - 76, The effect using visibleMessages (in MessageList.tsx inside the useEffect that sets up observer.current) fails to reset visibleMessages.current between runs causing stale ids to skew mostVisibleId; fix it by calling visibleMessages.current.clear() at the start of the effect (before observing elements) and again in the cleanup (before/after observer.current?.disconnect()) so the map is emptied whenever the effect re-runs or unmounts; update the useEffect body that references visibleMessages, observer, and onVisibleMessageChange to perform these clears.
🧹 Nitpick comments (1)
frontend/components/chat/MessageList.tsx (1)
45-63: fallback 로직 중복을 헬퍼로 추출하면 유지보수가 쉬워집니다.Line 50-63에서 동일한
aiMessagesfallback 계산이 반복됩니다. 공통 함수로 추출하면 분기 수정 시 누락 위험을 줄일 수 있습니다.리팩터 예시
+ const emitLatestMetadataOrEmpty = () => { + const aiMessages = messages.filter( + m => m.role === "ai" && m.metadata && m.metadata.length > 0 + ); + if (aiMessages.length > 0) { + onVisibleMessageChange(aiMessages[aiMessages.length - 1].metadata!); + } else { + onVisibleMessageChange([]); + } + }; + if (mostVisibleId) { const msg = messages.find(m => m.id === mostVisibleId); if (msg && msg.metadata && msg.metadata.length > 0) { onVisibleMessageChange(msg.metadata); } else { - const aiMessages = messages.filter(m => m.role === "ai" && m.metadata && m.metadata.length > 0); - if (aiMessages.length > 0) { - onVisibleMessageChange(aiMessages[aiMessages.length - 1].metadata!); - } else { - onVisibleMessageChange([]); - } + emitLatestMetadataOrEmpty(); } } else { - const aiMessages = messages.filter(m => m.role === "ai" && m.metadata && m.metadata.length > 0); - if (aiMessages.length > 0) { - onVisibleMessageChange(aiMessages[aiMessages.length - 1].metadata!); - } else { - onVisibleMessageChange([]); - } + emitLatestMetadataOrEmpty(); }🤖 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 45 - 63, Extract the repeated fallback logic that finds the last AI message with metadata into a helper (e.g., getLastAiMetadata(messages): string[] | []), then replace the duplicated blocks in the mostVisibleId branch and the else branch to call that helper and pass its result to onVisibleMessageChange; update the code paths using mostVisibleId, messages, and onVisibleMessageChange to call getLastAiMetadata(messages) where aiMessages calculation or fallback [] was used so the logic is centralized and easier to maintain.
🤖 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 18-76: The effect using visibleMessages (in MessageList.tsx inside
the useEffect that sets up observer.current) fails to reset
visibleMessages.current between runs causing stale ids to skew mostVisibleId;
fix it by calling visibleMessages.current.clear() at the start of the effect
(before observing elements) and again in the cleanup (before/after
observer.current?.disconnect()) so the map is emptied whenever the effect
re-runs or unmounts; update the useEffect body that references visibleMessages,
observer, and onVisibleMessageChange to perform these clears.
---
Nitpick comments:
In `@frontend/components/chat/MessageList.tsx`:
- Around line 45-63: Extract the repeated fallback logic that finds the last AI
message with metadata into a helper (e.g., getLastAiMetadata(messages): string[]
| []), then replace the duplicated blocks in the mostVisibleId branch and the
else branch to call that helper and pass its result to onVisibleMessageChange;
update the code paths using mostVisibleId, messages, and onVisibleMessageChange
to call getLastAiMetadata(messages) where aiMessages calculation or fallback []
was used so the logic is centralized and easier to maintain.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/components/chat/FloatingInput.tsxfrontend/components/chat/MessageList.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/components/chat/FloatingInput.tsx
…and observer cleanup
This was referenced Mar 3, 2026
Merged
Merged
Merged
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
릴리스 노트
새로운 기능
버그 수정
개선