Merged
Conversation
…and observer cleanup
…-deps, and update docs
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough채팅 API 라우트에서 서비스 임포트를 로컬 범위로 이동하고 번역 및 임베딩 생성에 대한 타임아웃을 추가했습니다. 또한 SSE 오류 메시지를 통해 명시적인 타임아웃 및 오류 처리를 구현하고, 앱 시작 시 preload 작업의 로깅을 개선했으며, 테스트 파일의 패치 경로를 업데이트했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatAPI as Chat API Handler
participant TranslationSvc as Translation Service
participant EmbeddingSvc as Embedding Service
participant LLMSvc as LLM Service
Client->>ChatAPI: POST /chat/events (user message)
rect rgba(100, 150, 200, 0.5)
Note over ChatAPI,TranslationSvc: Translation with Timeout (10s)
ChatAPI->>TranslationSvc: asyncio.wait_for(translate, timeout=10s)
alt Success
TranslationSvc-->>ChatAPI: translated text
else Timeout/Error
ChatAPI-->>Client: SSE error event
ChatAPI->>ChatAPI: log timeout/error
Note over ChatAPI: return early
end
end
rect rgba(200, 150, 100, 0.5)
Note over ChatAPI,EmbeddingSvc: Embedding with Timeout (10s)
ChatAPI->>EmbeddingSvc: asyncio.wait_for(agenerate_embedding, timeout=10s)
alt Success
EmbeddingSvc-->>ChatAPI: embedding vector
else Timeout/Error
ChatAPI-->>Client: SSE error event
ChatAPI->>ChatAPI: log timeout/error
Note over ChatAPI: return early
end
end
rect rgba(150, 200, 100, 0.5)
Note over ChatAPI,LLMSvc: LLM Response Stream
ChatAPI->>LLMSvc: get_response_stream_async(context)
LLMSvc-->>ChatAPI: response stream
ChatAPI-->>Client: SSE response events
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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.
🧹 Nitpick comments (1)
backend/app/main.py (1)
79-92:/ready실패 분기에서 예외 로그를 남겨주세요.Line 91에서 예외를 바로 503으로만 변환하면 원인 추적이 어려워집니다. 실패 분기에도 로그를 남기는 편이 운영 관측성에 유리합니다.
🔧 제안 코드
`@app.get`("/ready") async def readiness_check(): @@ try: preload_task.result() # re-raises if failed return {"status": "ready"} except Exception: + logger.exception("Preload task failed during readiness check") return JSONResponse({"status": "failed"}, status_code=503)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/main.py` around lines 79 - 92, The readiness_check handler currently swallows exceptions when preload_task.result() fails and returns a 503 without logging; update readiness_check to catch the exception, log the error with stacktrace (e.g., using logging.exception or logger.error(..., exc_info=True)) before returning JSONResponse({"status":"failed"}, status_code=503) so operators can trace the root cause; locate the readiness_check function and the use of preload_task.result() to add the logging in the except Exception block (and consider logging when preload_task.cancelled() is true as well).
🤖 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/main.py`:
- Around line 79-92: The readiness_check handler currently swallows exceptions
when preload_task.result() fails and returns a 503 without logging; update
readiness_check to catch the exception, log the error with stacktrace (e.g.,
using logging.exception or logger.error(..., exc_info=True)) before returning
JSONResponse({"status":"failed"}, status_code=503) so operators can trace the
root cause; locate the readiness_check function and the use of
preload_task.result() to add the logging in the except Exception block (and
consider logging when preload_task.cancelled() is true as well).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 944cee09-de4b-42b2-bef2-879a335cffa8
📒 Files selected for processing (7)
.gitignorebackend/app/api/routes/chat.pybackend/app/main.pybackend/pytest_log_utf8.txtbackend/tests/e2e/test_chat_endpoint.pybackend/tests/integration/test_supabase_match.pypytest.ini
💤 Files with no reviewable changes (1)
- backend/pytest_log_utf8.txt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/main.py (1)
89-94:return문을else블록으로 이동하는 것을 고려해 주세요.정적 분석 도구(Ruff TRY300)에서 지적한 대로,
try블록 내의return문을else블록으로 이동하면 예외 처리 로직이 더 명확해집니다.♻️ 제안하는 수정
try: preload_task.result() # re-raises if failed - return {"status": "ready"} except Exception: logger.exception("Preload task failed during readiness check") return JSONResponse({"status": "failed"}, status_code=503) + else: + return {"status": "ready"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/main.py` around lines 89 - 94, Move the successful-return out of the try block into an else block so the control flow is explicit: call preload_task.result() inside the try (to re-raise any exception), catch exceptions in the except block (logging with logger.exception and returning the JSONResponse with status_code=503), and place return {"status": "ready"} in an else clause that runs only when no exception was raised; locate the snippet around preload_task.result(), logger.exception(...) and the JSONResponse(...) to make this change.
🤖 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/main.py`:
- Around line 89-94: Move the successful-return out of the try block into an
else block so the control flow is explicit: call preload_task.result() inside
the try (to re-raise any exception), catch exceptions in the except block
(logging with logger.exception and returning the JSONResponse with
status_code=503), and place return {"status": "ready"} in an else clause that
runs only when no exception was raised; locate the snippet around
preload_task.result(), logger.exception(...) and the JSONResponse(...) to make
this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb612f39-90a7-46c7-b0b9-a68c0215582b
📒 Files selected for processing (1)
backend/app/main.py
…check to satisfy Ruff TRY300
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/app/main.py (2)
14-14:asyncioimport 위치가 표준 라이브러리 그룹과 일관되지 않습니다.
asyncio는 표준 라이브러리이므로contextlib,logging과 함께 상단에 위치하는 것이 PEP 8 스타일에 맞습니다.🔧 Import 정리 제안
from fastapi import FastAPI from fastapi.responses import JSONResponse from fastapi.middleware.cors import CORSMiddleware from slowapi import _rate_limit_exceeded_handler from slowapi.errors import RateLimitExceeded from app.api.routes import chat from app.core.rate_limit import limiter from contextlib import asynccontextmanager import logging +import asyncio logger = logging.getLogger(__name__) - -import asyncio🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/main.py` at line 14, Move the "import asyncio" statement into the standard-library import group alongside other stdlib imports such as "contextlib" and "logging" so imports follow PEP 8 grouping and ordering; locate the standalone "asyncio" import in main.py and place it with the other standard-library imports, keeping third-party and local imports separate and maintaining any existing alphabetical order within the stdlib group.
89-93: 반복적인 예외 로깅으로 로그 폭주 가능성이 있습니다.
readiness_check가 health check 등에 의해 주기적으로 호출될 경우, preload 실패 시 매 요청마다logger.exception이 호출되어 동일한 스택 트레이스가 반복 기록됩니다.♻️ 로깅 개선 제안
Done callback에서 이미 예외를 로깅하고 있으므로, readiness endpoint에서는 간단한 warning 레벨로 충분합니다:
try: preload_task.result() # re-raises if failed except Exception: - logger.exception("Preload task failed during readiness check") + logger.warning("Preload task failed - check startup logs for details") return JSONResponse({"status": "failed"}, status_code=503)또는 flag를 사용하여 최초 1회만 로깅하는 방식도 고려해볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/main.py` around lines 89 - 93, The readiness_check handler currently calls preload_task.result() and uses logger.exception on failure, which re-logs the full traceback repeatedly; change the except block in readiness_check to catch Exception as e and call logger.warning("Preload task failed during readiness check: %s", e) (or use a module-level boolean like preload_failure_logged to log only once) instead of logger.exception so you avoid repeated stack traces while still returning the 503 JSONResponse; keep the done-callback exception logging as-is.
🤖 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/main.py`:
- Around line 44-50: The finally block only catches asyncio.TimeoutError so if
preload_task itself raises, that exception propagates during shutdown; update
the await block around asyncio.wait_for(asyncio.shield(preload_task),
timeout=3.0) to also catch exceptions raised by the shielded task (e.g., add an
except Exception as e or a separate except for task exceptions) and log them via
logger.warning or logger.exception (referencing preload_task, asyncio.wait_for,
asyncio.shield, and logger.warning/logger.exception) so the shutdown sequence
doesn't produce an unhandled exception.
---
Nitpick comments:
In `@backend/app/main.py`:
- Line 14: Move the "import asyncio" statement into the standard-library import
group alongside other stdlib imports such as "contextlib" and "logging" so
imports follow PEP 8 grouping and ordering; locate the standalone "asyncio"
import in main.py and place it with the other standard-library imports, keeping
third-party and local imports separate and maintaining any existing alphabetical
order within the stdlib group.
- Around line 89-93: The readiness_check handler currently calls
preload_task.result() and uses logger.exception on failure, which re-logs the
full traceback repeatedly; change the except block in readiness_check to catch
Exception as e and call logger.warning("Preload task failed during readiness
check: %s", e) (or use a module-level boolean like preload_failure_logged to log
only once) instead of logger.exception so you avoid repeated stack traces while
still returning the 503 JSONResponse; keep the done-callback exception logging
as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e63f6d9-4f63-4205-b9db-982e7e88b0cd
📒 Files selected for processing (1)
backend/app/main.py
…ifespan, and move asyncio import to stdlib group
There was a problem hiding this comment.
♻️ Duplicate comments (1)
backend/app/main.py (1)
44-52:⚠️ Potential issue | 🟡 Minor종료 경로에서 예외를 완전히 무시하지 마세요.
Line 50-52의 bare except는
asyncio.wait_for()호출 중 발생할 수 있는 타이밍 관련 예외나 기타 예상 밖의 예외를 무시합니다. Done 콜백이 task의 예외는 로깅하지만(line 38), 대기 중 발생하는 예외는 별개입니다. 최소한 이들 예외도 로깅하면 종료 시점 문제 추적이 훨씬 용이합니다.제안 변경
if not preload_task.done(): try: # Use wait_for to shield and wait so we don't aggressively cancel a thread that might hang await asyncio.wait_for(asyncio.shield(preload_task), timeout=3.0) except asyncio.TimeoutError: logger.warning("Preload task did not finish before shutdown.") - except Exception: - # Exception already logged by done callback - pass + except Exception: + logger.exception("Exception occurred while waiting for preload task during shutdown.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/main.py` around lines 44 - 52, The bare "except Exception: pass" in the shutdown path swallowing errors from awaiting preload_task must be replaced with logging; change the handler after awaiting asyncio.wait_for(asyncio.shield(preload_task), ...) to "except Exception as e:" and call logger.exception or logger.error with the exception to record stack/context (e.g., message like 'Exception while waiting for preload_task during shutdown'). Keep the existing asyncio.wait_for/asyncio.shield usage and do not re-raise — just ensure preload_task, asyncio.wait_for, asyncio.shield and the logger capture the unexpected exceptions so they aren't silently ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@backend/app/main.py`:
- Around line 44-52: The bare "except Exception: pass" in the shutdown path
swallowing errors from awaiting preload_task must be replaced with logging;
change the handler after awaiting asyncio.wait_for(asyncio.shield(preload_task),
...) to "except Exception as e:" and call logger.exception or logger.error with
the exception to record stack/context (e.g., message like 'Exception while
waiting for preload_task during shutdown'). Keep the existing
asyncio.wait_for/asyncio.shield usage and do not re-raise — just ensure
preload_task, asyncio.wait_for, asyncio.shield and the logger capture the
unexpected exceptions so they aren't silently ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 414a5a77-ee86-454a-ad71-06253379f01b
📒 Files selected for processing (1)
backend/app/main.py
…ion logging feedback
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/main.py`:
- Around line 52-53: Remove the unused exception variable by changing the except
clause that currently reads "except Exception as e:" to "except Exception:" in
the shutdown/preload-wait block (the block that calls
logger.exception("Exception occurred while waiting for preload task during
shutdown.")). This keeps the existing logger.exception call (which logs the
traceback) while eliminating the unused variable and the lint F841 warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 095034e5-c495-4b0f-a2e7-2f4e3d8f9180
📒 Files selected for processing (1)
backend/app/main.py
Comment on lines
+52
to
+53
| except Exception as e: | ||
| logger.exception("Exception occurred while waiting for preload task during shutdown.") |
There was a problem hiding this comment.
사용하지 않는 예외 변수 e를 제거해주세요.
Line 52에서 e를 바인딩하지만 사용하지 않아 불필요한 lint 경고(F841)가 발생합니다.
🔧 제안 수정
- except Exception as e:
+ except Exception:
logger.exception("Exception occurred while waiting for preload task during shutdown.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| except Exception as e: | |
| logger.exception("Exception occurred while waiting for preload task during shutdown.") | |
| except Exception: | |
| logger.exception("Exception occurred while waiting for preload task during shutdown.") |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 52-52: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/main.py` around lines 52 - 53, Remove the unused exception
variable by changing the except clause that currently reads "except Exception as
e:" to "except Exception:" in the shutdown/preload-wait block (the block that
calls logger.exception("Exception occurred while waiting for preload task during
shutdown.")). This keeps the existing logger.exception call (which logs the
traceback) while eliminating the unused variable and the lint F841 warning.
This was referenced Mar 5, 2026
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
버그 수정