Conversation
|
Caution Review failedThe pull request is closed. 개요이 풀 리퀘스트는 음성 분석 모델을 VoiceAnalyze에서 VoiceComposite로 변경하고, 새로운 알림 시스템을 도입하며, 날짜 기반 필터링을 지원하고, 사용자 감정 추적을 위한 새로운 API 엔드포인트를 추가합니다. WalkthroughVoiceComposite로 모델 참조를 교체하고 Notification 모델/마이그레이션 및 저장소를 추가했으며, 날짜 필터링을 도입하고 사용자·보호자용 상위 감정 API와 알림 생성(저장 후 FCM 전송) 로직을 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant API as API 엔드포인트
participant VoiceSvc as VoiceService
participant JobRepo as Job(Notifier)
participant Repo as NotificationRepo
participant DB as DB
participant FCM as FCM 서버
Client->>API: 음성 업로드 요청
API->>VoiceSvc: upload_user_voice (비동기 WAV 변환)
VoiceSvc->>VoiceSvc: ffmpeg/librosa 변환 및 검증
VoiceSvc->>DB: Voice, VoiceComposite 저장
VoiceSvc->>JobRepo: 알림 전송 작업 트리거
JobRepo->>Repo: create_notification(voice_id, name, top_emotion)
Repo->>DB: Notification 레코드 생성 (commit)
JobRepo->>FCM: FCM 푸시 전송 (비동기)
FCM-->>JobRepo: 전송 결과(성공/실패)
JobRepo-->>API: 작업 결과 반환
API-->>Client: 응답
sequenceDiagram
participant Client as 클라이언트
participant API as API 엔드포인트
participant TopSvc as top_emotion_service
participant DB as DB
Client->>API: GET /users/top_emotion?date=YYYY-MM-DD
API->>TopSvc: get_top_emotion_for_date(user_id, date)
TopSvc->>DB: Voice JOIN VoiceComposite 쿼리 (date window)
TopSvc->>TopSvc: top_emotion 집계 (unknown 제외, tie-break by earliest)
TopSvc-->>API: {date, top_emotion}
API-->>Client: 응답
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/db_service.py (1)
93-135: datetime 중복 import를 제거하세요.Line 3에서 이미
datetime를 import했는데 Line 101에서 다시 import하고 있습니다.def get_care_voices(self, care_username: str, date: Optional[str] = None) -> List[Voice]: """보호자(care)의 연결 사용자 음성 중 voice_analyze가 존재하는 항목만 최신순 조회 Args: care_username: 보호자 username date: 날짜 필터 (YYYY-MM-DD, Optional). None이면 전체 조회 """ from sqlalchemy.orm import joinedload - from datetime import datetime # 1) 보호자 조회
🧹 Nitpick comments (7)
app/repositories/notification_repo.py (1)
1-7:top_emotion타입 힌트를 Optional로 맞춰주세요
Line [6]에서 기본값을None으로 주고 있지만 타입 힌트가str이라 정적 검사에서 오류가 발생할 수 있습니다.Optional[str]로 선언하고typing.Optional을 import하면 일관성이 맞습니다.-"""Notification Repository""" -from sqlalchemy.orm import Session -from ..models import Notification, Voice, VoiceComposite +"""Notification Repository""" +from typing import Optional +from sqlalchemy.orm import Session +from ..models import Notification, Voice, VoiceComposite @@ -def create_notification(session: Session, voice_id: int, name: str, top_emotion: str = None) -> Notification: +def create_notification(session: Session, voice_id: int, name: str, top_emotion: Optional[str] = None) -> Notification:app/services/top_emotion_service.py (2)
42-46: 사용하지 않는 루프 변수명을 수정하세요.루프 변수
v가 사용되지 않습니다. 언더스코어 접두사를 사용하여 의도를 명확히 하세요.- for v, vc in q: - em = vc.top_emotion if vc else None + for _v, vc in q: + em = vc.top_emotion if vc else None
82-86: 예외 처리를 구체적으로 개선하세요.일반적인
Exception을 잡아서None을 반환하면 예기치 않은 버그를 숨길 수 있습니다. 구체적인 예외만 처리하거나 최소한 로깅을 추가하는 것을 고려하세요.except ValueError: # 날짜 형식 오류 return None - except Exception: + except Exception as e: + import logging + logging.error(f"Unexpected error in get_top_emotion_for_date: {e}") return Noneapp/main.py (1)
510-530: 날짜 검증 예외 처리를 개선하세요.날짜 형식 오류 시 HTTPException을 발생시킬 때 원본 예외를 체이닝하면 디버깅이 더 쉬워집니다.
if date: try: datetime.strptime(date, "%Y-%m-%d") except ValueError: - raise HTTPException(status_code=400, detail="Invalid date format. Use YYYY-MM-DD") + raise HTTPException(status_code=400, detail="Invalid date format. Use YYYY-MM-DD") from Noneapp/voice_service.py (3)
37-95: ffmpeg 기반 변환 구현이 잘 되었으나 개선사항이 있습니다.ffmpeg를 stdin/stdout 파이프로 사용하여 임시 파일을 제거한 최적화가 훌륭합니다. 하지만 몇 가지 개선할 점이 있습니다:
- 하드코딩된 ffmpeg 명령어를 사용하므로 S603 보안 경고는 거짓 양성입니다.
- WAV 헤더 검증 로직이 좋습니다.
- 적절한 타임아웃(30초) 설정됨.
다음과 같이 상수로 분리하면 유지보수가 더 쉬워집니다:
# 파일 상단에 상수 정의 FFMPEG_TIMEOUT_SECONDS = 30 MIN_AUDIO_FILE_SIZE = 100 MIN_WAV_OUTPUT_SIZE = 3200 # 0.1초 분량 (16kHz * 2 bytes * 0.1s)
245-312: 독립적인 DB 세션 사용이 올바릅니다.비동기 백그라운드 작업에서 독립적인 세션을 생성하고 finally 블록에서 정리하는 패턴이 적절합니다. 하지만 예외 처리를 개선하면 디버깅이 더 쉬워집니다.
except Exception as e: - print(f"STT → NLP 처리 중 오류 발생: {e}") + import logging + logging.error(f"STT → NLP 처리 중 오류 발생 (voice_id={voice_id}): {e}", exc_info=True) db.rollback()
314-444: 음성 감정 분석 백그라운드 처리의 세션 관리가 적절합니다.독립적인 DB 세션을 사용하고 있으며, 에러 처리 및 롤백 로직이 올바릅니다. 마찬가지로 로깅 개선을 고려하세요.
except Exception as e: - print(f"Audio emotion background error: {e}", flush=True) + import logging + logging.error(f"Audio emotion background error (voice_id={voice_id}): {e}", exc_info=True) db.rollback()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/care_service.py(4 hunks)app/db_service.py(2 hunks)app/dto.py(2 hunks)app/main.py(6 hunks)app/models.py(1 hunks)app/repositories/job_repo.py(3 hunks)app/repositories/notification_repo.py(1 hunks)app/services/top_emotion_service.py(1 hunks)app/voice_service.py(14 hunks)database_schema.sql(1 hunks)migrations/versions/202511010002_add_notification.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
app/repositories/job_repo.py (2)
app/models.py (1)
VoiceComposite(146-180)app/repositories/notification_repo.py (1)
create_notification(6-27)
app/repositories/notification_repo.py (1)
app/models.py (3)
Notification(222-239)Voice(32-58)VoiceComposite(146-180)
app/db_service.py (1)
app/models.py (2)
Voice(32-58)VoiceAnalyze(85-110)
app/services/top_emotion_service.py (1)
app/models.py (3)
Voice(32-58)VoiceComposite(146-180)User(8-29)
app/voice_service.py (4)
app/models.py (3)
VoiceAnalyze(85-110)Voice(32-58)VoiceComposite(146-180)app/db_service.py (4)
get_db_service(328-330)create_voice_content(142-160)create_voice_analyze(192-212)get_care_voices(93-135)app/repositories/job_repo.py (3)
mark_text_done(98-101)try_aggregate(110-160)mark_audio_done(104-107)app/performance_logger.py (1)
get_performance_logger(109-115)
app/main.py (6)
app/dto.py (3)
NotificationListResponse(83-85)TopEmotionResponse(89-92)CareTopEmotionResponse(95-99)app/database.py (1)
get_db(38-44)app/services/top_emotion_service.py (1)
get_top_emotion_for_date(10-86)app/auth_service.py (2)
get_auth_service(297-299)get_user_by_username(144-146)app/voice_service.py (2)
get_voice_service(804-806)get_care_voice_list(517-537)app/models.py (3)
Notification(222-239)Voice(32-58)User(8-29)
app/care_service.py (1)
app/models.py (3)
User(8-29)Voice(32-58)VoiceComposite(146-180)
🪛 Ruff (0.14.2)
app/repositories/job_repo.py
48-48: Do not catch blind exception: Exception
(BLE001)
50-50: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
50-50: Use explicit conversion flag
Replace with conversion flag
(RUF010)
85-85: Use explicit conversion flag
Replace with conversion flag
(RUF010)
app/repositories/notification_repo.py
6-6: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
app/db_service.py
101-101: Redefinition of unused datetime from line 3
Remove definition: datetime
(F811)
app/services/top_emotion_service.py
42-42: Loop control variable v not used within loop body
Rename unused v to _v
(B007)
80-80: Consider moving this statement to an else block
(TRY300)
85-85: Do not catch blind exception: Exception
(BLE001)
app/voice_service.py
66-66: subprocess call: check for execution of untrusted input
(S603)
86-86: f-string without any placeholders
Remove extraneous f prefix
(F541)
92-92: Do not catch blind exception: Exception
(BLE001)
108-108: Avoid specifying long messages outside the exception class
(TRY003)
120-120: Avoid specifying long messages outside the exception class
(TRY003)
308-308: Do not catch blind exception: Exception
(BLE001)
440-440: Do not catch blind exception: Exception
(BLE001)
app/main.py
385-385: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
514-514: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
526-526: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
554-554: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
592-592: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (5)
app/models.py (1)
222-239: 새 Notification 모델 구조가 적절합니다.알림 기록을 위한 새 모델이 잘 설계되었습니다:
- 적절한 인덱스 구성 (voice_id, created_at)
- CASCADE 삭제로 참조 무결성 유지
- Voice와의 관계 설정 올바름
app/care_service.py (1)
2-2: VoiceComposite로의 마이그레이션이 올바르게 수행되었습니다.VoiceAnalyze에서 VoiceComposite로 전환이 일관되게 이루어졌으며, null 감정 필터링을 추가하여 데이터 품질을 개선했습니다.
Also applies to: 31-39, 74-86
app/main.py (1)
384-405: 새로운 엔드포인트들이 잘 구현되었습니다.사용자 및 보호자 대표 감정 조회, 알림 목록 조회 엔드포인트가 기존 패턴을 따르며 적절히 구현되었습니다:
- 사용자 검증 로직 포함
- 적절한 에러 처리
- 일관된 응답 형식
Also applies to: 553-588, 591-617
app/voice_service.py (2)
172-174: 비동기 처리 개선이 적절합니다.CPU 집약적인 WAV 변환 작업을
asyncio.to_thread로 처리하여 이벤트 루프 블로킹을 방지한 것이 좋습니다.Also applies to: 636-638
517-537: VoiceComposite로의 마이그레이션이 일관되게 수행되었습니다.VoiceAnalyze에서 VoiceComposite로의 전환이 모든 쿼리에서 올바르게 이루어졌습니다:
- 조인 조건 업데이트
- top_emotion 필드 참조 변경
- null 필터링 추가
Also applies to: 725-734, 756-768
1 participant
🔎 Description
🔗 Related Issue
x
🏷️ What type of PR is this?
📋 Changes Made
🧪 Testing
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선